[hadrian] Do not add full tool pathswip/angerman/fix-nix-store

This prohuibits CC=clang to work generically and will always bake
in the clang that is found on the build machine in PATH, what ever
clang that might be. It might not even be on the final host.

3 files changed, 32 insertions, 4 deletions

diff --git a/hadrian/src/Builder.hs b/hadrian/src/Builder.hs
index aac3b26370..1e471b2a26 100644
--- a/hadrian/src/Builder.hs
+++ b/hadrian/src/Builder.hs
@@ -18,6 +18,7 @@ import Control.Exception.Extra (Partial)
 import qualified Data.ByteString.Lazy.Char8 as BSL
 import Development.Shake.Classes
 import Development.Shake.Command
+import Development.Shake.FilePath
 import GHC.Generics
 import qualified Hadrian.Builder as H
 import Hadrian.Builder hiding (Builder)
@@ -227,7 +228,12 @@ instance H.Builder Builder where
                 msgIn  = "[askBuilder] Exactly one input file expected."
             needBuilder builder
             path <- H.builderPath builder
-            need [path]
+            -- we do not depend on bare builders. E.g. we won't depend on `clang`
+            -- or `ld` or `ar`.  Unless they are provided with fully qualified paths
+            -- this is the job of the person invoking ./configure to pass e.g.
+            -- CC=$(which clang) if they want the fully qualified clang path!
+            when (path /= takeFileName path) $
+                need [path]
             Stdout stdout <- cmd' [path] ["--no-user-package-db", "field", input, "depends"]
             return stdout
         _ -> error $ "Builder " ++ show builder ++ " can not be asked!"
@@ -375,9 +381,15 @@ systemBuilderPath builder = case builder of
                 ++ quote key ++ " is not specified" ++ inCfg
             return "" -- TODO: Use a safe interface.
         else do
+            -- angerman: I find this lookupInPath rather questionable.
+            -- if we specify CC, LD, ... *without* a path, that is intentional
+            -- lookupInPath should be done by the person invoking the configure
+            -- script iif they want to have that full path, if they just want
+            -- some generic tool name (on purpose!) the build system should not
+            -- go behind their backs to add a path they likely never wanted.
             fullPath <- lookupInPath path
             case (windowsHost, hasExtension fullPath) of
-                (False, _    ) -> return fullPath
+                (False, _    ) -> return path
                 (True , True ) -> fixAbsolutePathOnWindows fullPath
                 (True , False) -> fixAbsolutePathOnWindows fullPath <&> (<.> exe)
 
diff --git a/hadrian/src/Hadrian/Builder.hs b/hadrian/src/Hadrian/Builder.hs
index fdfdb666ef..56c0333125 100644
--- a/hadrian/src/Hadrian/Builder.hs
+++ b/hadrian/src/Hadrian/Builder.hs
@@ -19,12 +19,15 @@ module Hadrian.Builder (
 
 import Data.List
 import Development.Shake
+import Development.Shake.FilePath
 
 import Hadrian.Expression hiding (inputs, outputs)
 import Hadrian.Oracles.ArgsHash
 import Hadrian.Target
 import Hadrian.Utilities
 
+import Base
+
 -- | This data structure captures all information relevant to invoking a builder.
 data BuildInfo = BuildInfo {
     -- | Command line arguments.
@@ -67,7 +70,15 @@ needBuilder :: Builder b => b -> Action ()
 needBuilder builder = do
     path <- builderPath builder
     deps <- runtimeDependencies builder
-    need (path : deps)
+    -- so `path` might be just `gcc`, in which case we won't issue a "need" on
+    -- it.  If someone really wants the full qualified path, he ought to pass
+    -- CC=$(which gcc) to the configure script.  If CC=gcc was passed, we should
+    -- respect that choice and not resolve that via $PATH into a fully qualified
+    -- path.  We can only `need` fully qualified path's though, hence we won't
+    -- `need` bare tool names.
+    when (path /= takeFileName path) $
+        need [path]
+    need deps
 
 -- | Run a builder with a specified list of command line arguments, reading a
 -- list of input files and writing a list of output files. A lightweight version
diff --git a/hadrian/src/Settings/Builders/Cabal.hs b/hadrian/src/Settings/Builders/Cabal.hs
index 5996749536..c175cd6500 100644
--- a/hadrian/src/Settings/Builders/Cabal.hs
+++ b/hadrian/src/Settings/Builders/Cabal.hs
@@ -1,5 +1,6 @@
 module Settings.Builders.Cabal (cabalBuilderArgs) where
 
+import Development.Shake.FilePath
 import Hadrian.Haskell.Cabal
 
 import Builder
@@ -181,7 +182,11 @@ with b = do
     if null path then mempty else do
         top <- expr topDirectory
         expr $ needBuilder b
-        arg  $ withBuilderKey b ++ unifyPath (top </> path)
+        -- Do not inject top, if we have a bare name. E.g. do not turn
+        -- `ar` into `$top/ar`. But let `ar` be `ar` as found on $PATH.
+        arg  $ withBuilderKey b ++ unifyPath (if path /= takeFileName path
+                                              then top </> path
+                                              else path)
 
 withStaged :: (Stage -> Builder) -> Args
 withStaged sb = with . sb =<< getStage