From 7f1fff501faa9525ca09bde076059a65388d638a Mon Sep 17 00:00:00 2001 From: Pepe Iborra Date: Mon, 17 May 2021 13:26:28 +0100 Subject: Avoid fingerprinting the absolute path to the source file This change aims to make source files relocatable w.r.t. to the interface files produced by the compiler. This is so that we can download interface files produced by a cloud build system and then reuse them in a local ghcide session catch another case of implicit includes actually use the implicit quote includes add another missing case recomp020 test that .hi files are reused even if .hs files are moved to a new location Added recomp021 to record behaviour with non implicit includes add a note additional pointer to the note Mention #16956 in Note (cherry picked from commit 9faafb0aaff04e86a58b9e108f84618b12f2057c) (cherry picked from commit 3c630d737b81ed896b392032aadfae551c42abc7) --- compiler/GHC/Driver/Pipeline.hs | 13 +++++---- compiler/GHC/Driver/Session.hs | 34 +++++++++++++++++++++-- compiler/GHC/Iface/Recomp/Flags.hs | 6 +++- testsuite/tests/driver/recomp020/A.hs | 4 +++ testsuite/tests/driver/recomp020/Makefile | 15 ++++++++++ testsuite/tests/driver/recomp020/all.T | 4 +++ testsuite/tests/driver/recomp021/A.hs | 6 ++++ testsuite/tests/driver/recomp021/B.hs | 1 + testsuite/tests/driver/recomp021/Makefile | 16 +++++++++++ testsuite/tests/driver/recomp021/all.T | 4 +++ testsuite/tests/driver/recomp021/recomp021.stdout | 1 + 11 files changed, 96 insertions(+), 8 deletions(-) create mode 100644 testsuite/tests/driver/recomp020/A.hs create mode 100644 testsuite/tests/driver/recomp020/Makefile create mode 100644 testsuite/tests/driver/recomp020/all.T create mode 100644 testsuite/tests/driver/recomp021/A.hs create mode 100644 testsuite/tests/driver/recomp021/B.hs create mode 100644 testsuite/tests/driver/recomp021/Makefile create mode 100644 testsuite/tests/driver/recomp021/all.T create mode 100644 testsuite/tests/driver/recomp021/recomp021.stdout diff --git a/compiler/GHC/Driver/Pipeline.hs b/compiler/GHC/Driver/Pipeline.hs index f596593419..b6978ad851 100644 --- a/compiler/GHC/Driver/Pipeline.hs +++ b/compiler/GHC/Driver/Pipeline.hs @@ -312,7 +312,7 @@ compileOne' m_tc_result mHscMessage old_paths = includePaths dflags2 !prevailing_dflags = hsc_dflags hsc_env0 dflags = - dflags2 { includePaths = addQuoteInclude old_paths [current_dir] + dflags2 { includePaths = addImplicitQuoteInclude old_paths [current_dir] , log_action = log_action prevailing_dflags } -- use the prevailing log_action / log_finaliser, -- not the one cached in the summary. This is so @@ -1099,7 +1099,7 @@ runPhase (RealPhase (Hsc src_flavour)) input_fn dflags0 -- the .hs files resides) to the include path, since this is -- what gcc does, and it's probably what you want. let current_dir = takeDirectory basename - new_includes = addQuoteInclude paths [current_dir] + new_includes = addImplicitQuoteInclude paths [current_dir] paths = includePaths dflags0 dflags = dflags0 { includePaths = new_includes } @@ -1286,7 +1286,8 @@ runPhase (RealPhase cc_phase) input_fn dflags let include_paths_global = foldr (\ x xs -> ("-I" ++ x) : xs) [] (includePathsGlobal cmdline_include_paths ++ pkg_include_dirs) let include_paths_quote = foldr (\ x xs -> ("-iquote" ++ x) : xs) [] - (includePathsQuote cmdline_include_paths) + (includePathsQuote cmdline_include_paths ++ + includePathsQuoteImplicit cmdline_include_paths) let include_paths = include_paths_quote ++ include_paths_global -- pass -D or -optP to preprocessor when compiling foreign C files @@ -1423,7 +1424,8 @@ runPhase (RealPhase (As with_cpp)) input_fn dflags let global_includes = [ GHC.SysTools.Option ("-I" ++ p) | p <- includePathsGlobal cmdline_include_paths ] let local_includes = [ GHC.SysTools.Option ("-iquote" ++ p) - | p <- includePathsQuote cmdline_include_paths ] + | p <- includePathsQuote cmdline_include_paths ++ + includePathsQuoteImplicit cmdline_include_paths] let runAssembler inputFilename outputFilename = liftIO $ do withAtomicRename outputFilename $ \temp_outputFilename -> do @@ -2028,7 +2030,8 @@ doCpp dflags raw input_fn output_fn = do let include_paths_global = foldr (\ x xs -> ("-I" ++ x) : xs) [] (includePathsGlobal cmdline_include_paths ++ pkg_include_dirs) let include_paths_quote = foldr (\ x xs -> ("-iquote" ++ x) : xs) [] - (includePathsQuote cmdline_include_paths) + (includePathsQuote cmdline_include_paths ++ + includePathsQuoteImplicit cmdline_include_paths) let include_paths = include_paths_quote ++ include_paths_global let verbFlags = getVerbFlags dflags diff --git a/compiler/GHC/Driver/Session.hs b/compiler/GHC/Driver/Session.hs index c6c7fea229..53bf1dc5cb 100644 --- a/compiler/GHC/Driver/Session.hs +++ b/compiler/GHC/Driver/Session.hs @@ -231,6 +231,7 @@ module GHC.Driver.Session ( -- * Include specifications IncludeSpecs(..), addGlobalInclude, addQuoteInclude, flattenIncludes, + addImplicitQuoteInclude, -- * SDoc initSDocContext, initDefaultSDocContext, @@ -400,6 +401,8 @@ import Foreign (Ptr) data IncludeSpecs = IncludeSpecs { includePathsQuote :: [String] , includePathsGlobal :: [String] + -- | See note [Implicit include paths] + , includePathsQuoteImplicit :: [String] } deriving Show @@ -416,10 +419,37 @@ addQuoteInclude :: IncludeSpecs -> [String] -> IncludeSpecs addQuoteInclude spec paths = let f = includePathsQuote spec in spec { includePathsQuote = f ++ paths } +-- | These includes are not considered while fingerprinting the flags for iface +-- | See note [Implicit include paths] +addImplicitQuoteInclude :: IncludeSpecs -> [String] -> IncludeSpecs +addImplicitQuoteInclude spec paths = let f = includePathsQuoteImplicit spec + in spec { includePathsQuoteImplicit = f ++ paths } + + -- | Concatenate and flatten the list of global and quoted includes returning -- just a flat list of paths. flattenIncludes :: IncludeSpecs -> [String] -flattenIncludes specs = includePathsQuote specs ++ includePathsGlobal specs +flattenIncludes specs = + includePathsQuote specs ++ + includePathsQuoteImplicit specs ++ + includePathsGlobal specs + +{- Note [Implicit include paths] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + The compile driver adds the path to the folder containing the source file being + compiled to the 'IncludeSpecs', and this change gets recorded in the 'DynFlags' + that are used later to compute the interface file. Because of this, + the flags fingerprint derived from these 'DynFlags' and recorded in the + interface file will end up containing the absolute path to the source folder. + + Build systems with a remote cache like Bazel or Buck (or Shake, see #16956) + store the build artifacts produced by a build BA for reuse in subsequent builds. + + Embedding source paths in interface fingerprints will thwart these attemps and + lead to unnecessary recompilations when the source paths in BA differ from the + source paths in subsequent builds. + -} + -- | The various Safe Haskell modes data SafeHaskellMode @@ -1357,7 +1387,7 @@ defaultDynFlags mySettings llvmConfig = dumpPrefix = Nothing, dumpPrefixForce = Nothing, ldInputs = [], - includePaths = IncludeSpecs [] [], + includePaths = IncludeSpecs [] [] [], libraryPaths = [], frameworkPaths = [], cmdlineFrameworks = [], diff --git a/compiler/GHC/Iface/Recomp/Flags.hs b/compiler/GHC/Iface/Recomp/Flags.hs index 391aaf2c86..fd7101e158 100644 --- a/compiler/GHC/Iface/Recomp/Flags.hs +++ b/compiler/GHC/Iface/Recomp/Flags.hs @@ -44,8 +44,12 @@ fingerprintDynFlags dflags@DynFlags{..} this_mod nameio = lang = (fmap fromEnum language, map fromEnum $ EnumSet.toList extensionFlags) + -- avoid fingerprinting the absolute path to the directory of the source file + -- see note [Implicit include paths] + includePathsMinusImplicit = includePaths { includePathsQuoteImplicit = [] } + -- -I, -D and -U flags affect CPP - cpp = ( map normalise $ flattenIncludes includePaths + cpp = ( map normalise $ flattenIncludes includePathsMinusImplicit -- normalise: eliminate spurious differences due to "./foo" vs "foo" , picPOpts dflags , opt_P_signature dflags) diff --git a/testsuite/tests/driver/recomp020/A.hs b/testsuite/tests/driver/recomp020/A.hs new file mode 100644 index 0000000000..900eaa3929 --- /dev/null +++ b/testsuite/tests/driver/recomp020/A.hs @@ -0,0 +1,4 @@ +module A where + +a :: () +a = () diff --git a/testsuite/tests/driver/recomp020/Makefile b/testsuite/tests/driver/recomp020/Makefile new file mode 100644 index 0000000000..9e66837d24 --- /dev/null +++ b/testsuite/tests/driver/recomp020/Makefile @@ -0,0 +1,15 @@ +TOP=../../.. +include $(TOP)/mk/boilerplate.mk +include $(TOP)/mk/test.mk + +# Recompilation test for when .hi files are up to date but .hs files have been moved + + +recomp020: + mkdir src1 + mkdir src2 + cp A.hs src1/A.hs + cp A.hs src2/A.hs + '$(TEST_HC)' $(TEST_HC_OPTS) -S src1/A.hs + '$(TEST_HC)' $(TEST_HC_OPTS) -S src2/A.hs + diff src1/A.hi src2/A.hi diff --git a/testsuite/tests/driver/recomp020/all.T b/testsuite/tests/driver/recomp020/all.T new file mode 100644 index 0000000000..5085d130bd --- /dev/null +++ b/testsuite/tests/driver/recomp020/all.T @@ -0,0 +1,4 @@ +# Recompilation test for when .hi files are up to date but .o files are +# not + +test('recomp020', [extra_files(['A.hs'])], makefile_test, []) diff --git a/testsuite/tests/driver/recomp021/A.hs b/testsuite/tests/driver/recomp021/A.hs new file mode 100644 index 0000000000..3851590cf0 --- /dev/null +++ b/testsuite/tests/driver/recomp021/A.hs @@ -0,0 +1,6 @@ +{-# LANGUAGE CPP #-} + +module A(foo) where + + +#include "B.hs" diff --git a/testsuite/tests/driver/recomp021/B.hs b/testsuite/tests/driver/recomp021/B.hs new file mode 100644 index 0000000000..c0c7d42f6c --- /dev/null +++ b/testsuite/tests/driver/recomp021/B.hs @@ -0,0 +1 @@ +foo = () diff --git a/testsuite/tests/driver/recomp021/Makefile b/testsuite/tests/driver/recomp021/Makefile new file mode 100644 index 0000000000..8a8cdcf21c --- /dev/null +++ b/testsuite/tests/driver/recomp021/Makefile @@ -0,0 +1,16 @@ +TOP=../../.. +include $(TOP)/mk/boilerplate.mk +include $(TOP)/mk/test.mk + +# Recompilation test for .hs files with CPP includes + +clean: + +recomp021: clean + mkdir src1 + mkdir src2 + cp *.hs src1 + cp *.hs src2 + '$(TEST_HC)' $(TEST_HC_OPTS) -S src1/A.hs + '$(TEST_HC)' $(TEST_HC_OPTS) -S src2/A.hs + ! diff src1/A.hi src2/A.hi diff --git a/testsuite/tests/driver/recomp021/all.T b/testsuite/tests/driver/recomp021/all.T new file mode 100644 index 0000000000..a2511ed6f7 --- /dev/null +++ b/testsuite/tests/driver/recomp021/all.T @@ -0,0 +1,4 @@ +# Recompilation test for when .hi files are up to date but .o files are +# not + +test('recomp021', [extra_files(['A.hs', 'B.hs'])], makefile_test, []) diff --git a/testsuite/tests/driver/recomp021/recomp021.stdout b/testsuite/tests/driver/recomp021/recomp021.stdout new file mode 100644 index 0000000000..670403817b --- /dev/null +++ b/testsuite/tests/driver/recomp021/recomp021.stdout @@ -0,0 +1 @@ +Binary files src1/A.hi and src2/A.hi differ -- cgit v1.2.1