Merge pull request #4260 from Ericson2314/build-tools-path

Unknown build-tools are looked up on the PATH -- fix and test

Author: Ericson2314

Cabal/Distribution/Backpack/ComponentsGraph.hs

@@ -1,21 +1,21 @@
 -- | See <https://github.com/ezyang/ghc-proposals/blob/backpack/proposals/0000-backpack.rst>
-
 module Distribution.Backpack.ComponentsGraph (
     ComponentsGraph,
     dispComponentsGraph,
     toComponentsGraph,
     componentCycleMsg
 ) where
 
+import Prelude ()
+import Distribution.Compat.Prelude
+
 import Distribution.Package
 import Distribution.PackageDescription as PD hiding (Flag)
 import Distribution.Simple.BuildToolDepends
 import Distribution.Simple.LocalBuildInfo
 import Distribution.Types.ComponentRequestedSpec
 import Distribution.Types.Dependency
-import Distribution.Types.ExeDependency
 import Distribution.Types.UnqualComponentName
-import Distribution.Simple.Utils
 import Distribution.Compat.Graph (Node(..))
 import qualified Distribution.Compat.Graph as Graph
 
@@ -55,9 +55,7 @@ toComponentsGraph enabled pkg_descr =
   where
     -- The dependencies for the given component
     componentDeps component =
-         [ CExeName toolname
-         | (ExeDependency _ toolname _)
-            <- getAllInternalToolDependencies pkg_descr bi ]
+      (CExeName <$> getAllInternalToolDependencies pkg_descr bi)
 
       ++ [ if pkgname == packageName pkg_descr
            then CLibName

Cabal/Distribution/Backpack/ConfiguredComponent.hs

@@ -19,7 +19,6 @@ import Distribution.Compat.Prelude hiding ((<>))
 import Distribution.Backpack.Id
 
 import Distribution.Types.Dependency
-import Distribution.Types.ExeDependency
 import Distribution.Types.IncludeRenaming
 import Distribution.Types.Mixin
 import Distribution.Types.UnqualComponentName
@@ -170,8 +169,7 @@ toConfiguredComponent pkg_descr this_cid
         | otherwise
         = Map.toList external_lib_map
     exe_deps = [ cid
-               | (ExeDependency _ toolName _)
-                 <- getAllInternalToolDependencies pkg_descr bi
+               | toolName <- getAllInternalToolDependencies pkg_descr bi
                , Just cid <- [ Map.lookup toolName exe_map ] ]
 
 -- | Also computes the 'ComponentId', and sets cc_public if necessary.

Cabal/Distribution/PackageDescription/Check.hs

@@ -603,7 +603,8 @@ checkFields pkg =
     internalExeDeps =
       [ dep
       | bi <- allBuildInfo pkg
-      , dep <- getAllInternalToolDependencies pkg bi
+      , dep <- getAllToolDependencies pkg bi
+      , isInternal pkg dep
       ]
 
     depInternalLibraryWithExtraVersion =

Cabal/Distribution/Simple/Build.hs

@@ -30,7 +30,6 @@ import Prelude ()
 import Distribution.Compat.Prelude
 
 import Distribution.Types.Dependency
-import Distribution.Types.ExeDependency
 import Distribution.Types.LocalBuildInfo
 import Distribution.Types.TargetInfo
 import Distribution.Types.ComponentRequestedSpec
@@ -537,7 +536,7 @@ addInternalBuildTools pkg lbi bi progs =
   where
     internalBuildTools =
       [ simpleConfiguredProgram toolName' (FoundOnSystem toolLocation)
-      | ExeDependency _ toolName _ <- getAllInternalToolDependencies pkg bi
+      | toolName <- getAllInternalToolDependencies pkg bi
       , let toolName' = unUnqualComponentName toolName
       , let toolLocation = buildDir lbi </> toolName' </> toolName' <.> exeExtension ]
 

Cabal/Distribution/Simple/BuildToolDepends.hs

@@ -5,7 +5,9 @@
 -- the functions contained to access those fields directly.
 module Distribution.Simple.BuildToolDepends where
 
-import           Data.Maybe
+import           Prelude ()
+import           Distribution.Compat.Prelude
+
 import qualified Data.Map as Map
 
 import           Distribution.Package
@@ -86,6 +88,9 @@ isInternal pkg (ExeDependency n _ _) = n == packageName pkg
 -- restrictions that apply to `isInternal` also apply to this function.
 getAllInternalToolDependencies :: PackageDescription
                                -> BuildInfo
-                               -> [ExeDependency]
+                               -> [UnqualComponentName]
 getAllInternalToolDependencies pkg bi =
-  filter (isInternal pkg) $ getAllToolDependencies pkg bi
+  [ toolname
+  | dep@(ExeDependency _ toolname _) <- getAllToolDependencies pkg bi
+  , isInternal pkg dep
+  ]

Cabal/Distribution/Simple/Configure.hs

@@ -570,16 +570,28 @@ configure (pkg_descr0', pbi) cfg = do
       die $ "Cannot build some foreign libraries: "
          ++ intercalate "," unsupportedFLibs
 
-    -- Configure known/required programs & external build tools.
-    -- Exclude build-tool deps on "internal" exes in the same package
-    --
-    -- TODO: Factor this into a helper package.
-    let requiredBuildTools =
-          [ LegacyExeDependency (unUnqualComponentName eName) versionRange
-          | bi <- enabledBuildInfos pkg_descr enabled
-          , buildTool@(ExeDependency _ eName versionRange)
-            <- getAllToolDependencies pkg_descr bi
-          , not $ isInternal pkg_descr buildTool ]
+    -- Configure certain external build tools, see below for which ones.
+    let requiredBuildTools = do
+          bi <- enabledBuildInfos pkg_descr enabled
+          -- First, we collect any tool dep that we know is external. This is,
+          -- in practice:
+          --
+          -- 1. `build-tools` entries on the whitelist
+          --
+          -- 2. `build-tool-depends` that aren't from the current package.
+          let externBuildToolDeps =
+                [ LegacyExeDependency (unUnqualComponentName eName) versionRange
+                | buildTool@(ExeDependency _ eName versionRange)
+                  <- getAllToolDependencies pkg_descr bi
+                , not $ isInternal pkg_descr buildTool ]
+          -- Second, we collect any build-tools entry we don't know how to
+          -- desugar. We'll never have any idea how to build them, so we just
+          -- hope they are already on the PATH.
+          let unknownBuildTools =
+                [ buildTool
+                | buildTool <- buildTools bi
+                , Nothing == desugarBuildTool pkg_descr buildTool ]
+          externBuildToolDeps ++ unknownBuildTools
 
     programDb' <-
           configureAllKnownPrograms (lessVerbose verbosity) programDb

cabal-testsuite/PackageTests/BuildToolsPathForeign/A.hs

@@ -0,0 +1,5 @@
+{-# OPTIONS_GHC -F -pgmF my-foreign-preprocessor #-}
+module A where
+
+a :: String
+a = "0000"

cabal-testsuite/PackageTests/BuildToolsPathForeign/build-tools-path-foreign.cabal

@@ -0,0 +1,20 @@
+name:                build-tools-path-foreign
+version:             0.1.0.0
+synopsis:            Checks unknown build-tools are drawn from PATH
+license:             BSD3
+category:            Testing
+build-type:          Simple
+cabal-version:       >=1.10
+
+library
+  exposed-modules:     A
+  build-depends:       base
+  build-tools:         my-foreign-preprocessor
+  -- ^ Note the unknown dependency.
+  default-language:    Haskell2010
+
+executable             hello-world
+  main-is:             Hello.hs
+  build-depends:       base, build-tools-path-foreign
+  default-language:    Haskell2010
+  hs-source-dirs:      hello

cabal-testsuite/PackageTests/BuildToolsPath/hello/Hello.hs renamed to cabal-testsuite/PackageTests/BuildToolsPathForeign/hello/Hello.hs

@@ -0,0 +1,3 @@
+#!/usr/bin/env sh
+
+sed -e 's/0/1/g' < $2 > $3

cabal-testsuite/PackageTests/BuildToolsPathForeign/my-foreign-preprocessor

@@ -0,0 +1,15 @@
+import Test.Cabal.Prelude
+
+import Control.Applicative ((<$>))
+import Control.Monad.IO.Class
+import System.Environment
+
+-- Test PATH-munging
+-- TODO: Enable this test on Windows
+main = setupAndCabalTest $ do
+    skipIf =<< isWindows
+    path <- liftIO $ getEnv "PATH"
+    cwd <- testCurrentDir <$> getTestEnv
+    r <- withEnv [("PATH", Just $ cwd ++ ":" ++ path)] $ setup_build []
+    runExe' "hello-world" []
+        >>= assertOutputContains "1111"

cabal-testsuite/PackageTests/BuildToolsPathForeign/setup.test.hs

@@ -1,4 +1,4 @@
-name:                build-tools-path
+name:                build-tools-path-internal
 version:             0.1.0.0
 synopsis:            Checks build-tools are put in PATH
 license:             BSD3
@@ -20,6 +20,6 @@ library
 
 executable             hello-world
   main-is:             Hello.hs
-  build-depends:       base, build-tools-path
+  build-depends:       base, build-tools-path-internal
   default-language:    Haskell2010
   hs-source-dirs:      hello

cabal-testsuite/PackageTests/BuildToolsPath/A.hs renamed to cabal-testsuite/PackageTests/BuildToolsPathInternal/A.hs

@@ -0,0 +1,6 @@
+module Main where
+
+import A
+
+main :: IO ()
+main = putStrLn a