Compute correct install directories for instantiated Backpack compone…

…nts (#6317) * Restructure Includes3 tests so that I can also test Hackage case. Signed-off-by: Edward Z. Yang <[email protected]> * Compute correct install directories for instantiated Backpack components Previously, we would compute elabInstallDirs once when configuring a component, and then reuse the exactly same install directories for every instantiation of the package. But this is wrong, since we're installing the header/object files for each instantiation to a different directory. We refactor install directory computation into a helper function and then call it again at instantiation time to refresh the install directories. For some reason, this bug ONLY manifests for packages installed from Hackage; it seems install dirs are not respected for inplace packages. Fixes #6005 Signed-off-by: Edward Z. Yang <[email protected]> * Test fix Signed-off-by: Edward Z. Yang <[email protected]> * Copy backpack version-guard from other backpack testcases
haskell · Nov 3, 2019 · 129775a · 129775a
1 parent 70da03e
commit 129775a
Show file tree

Hide file tree

Showing 19 changed files with 130 additions and 86 deletions.
diff --git a/cabal-install/Distribution/Client/ProjectPlanning.hs b/cabal-install/Distribution/Client/ProjectPlanning.hs
@@ -649,7 +649,12 @@ rebuildInstallPlan verbosity
                 projectConfigAllPackages
                 projectConfigLocalPackages
                 (getMapMappend projectConfigSpecificPackage)
-        let instantiatedPlan = instantiateInstallPlan elaboratedPlan
+        let instantiatedPlan
+              = instantiateInstallPlan
+                  cabalStoreDirLayout
+                  defaultInstallDirs
+                  elaboratedShared
+                  elaboratedPlan
         liftIO $ debugNoWrap verbosity (InstallPlan.showInstallPlan instantiatedPlan)
         return (instantiatedPlan, elaboratedShared)
       where
@@ -1478,7 +1483,7 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
 
             -- 5. Construct the final ElaboratedConfiguredPackage
             let
-                elab = elab1 {
+                elab2 = elab1 {
                     elabModuleShape = lc_shape lc,
                     elabUnitId      = abstractUnitId (lc_uid lc),
                     elabComponentId = lc_cid lc,
@@ -1488,8 +1493,14 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
                         compOrderLibDependencies =
                           ordNub (map (abstractUnitId . ci_id)
                                       (lc_includes lc ++ lc_sig_includes lc))
-                      },
-                    elabInstallDirs = install_dirs cid
+                      }
+                   }
+                elab = elab2 {
+                    elabInstallDirs = computeInstallDirs
+                      storeDirLayout
+                      defaultInstallDirs
+                      elaboratedSharedConfig
+                      elab2
                    }
 
             -- 6. Construct the updated local maps
@@ -1545,31 +1556,6 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
                 | PkgconfigDependency pn _ <- PD.pkgconfigDepends
                                                 (Cabal.componentBuildInfo comp) ]
 
-            install_dirs cid
-              | shouldBuildInplaceOnly spkg
-              -- use the ordinary default install dirs
-              = (InstallDirs.absoluteInstallDirs
-                   pkgid
-                   (newSimpleUnitId cid)
-                   (compilerInfo compiler)
-                   InstallDirs.NoCopyDest
-                   platform
-                   defaultInstallDirs) {
-
-                  -- absoluteInstallDirs sets these as 'undefined' but we have
-                  -- to use them as "Setup.hs configure" args
-                  InstallDirs.libsubdir  = "",
-                  InstallDirs.libexecsubdir  = "",
-                  InstallDirs.datasubdir = ""
-                }
-
-              | otherwise
-              -- use special simplified install dirs
-              = storePackageInstallDirs
-                  storeDirLayout
-                  (compilerId compiler)
-                  cid
-
             inplace_bin_dir elab =
                 binDirectoryFor
                     distDirLayout
@@ -1631,14 +1617,20 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
         elab
       where
         elab0@ElaboratedConfiguredPackage{..} = elaborateSolverToCommon pkg
-        elab = elab0 {
+        elab1 = elab0 {
                 elabUnitId = newSimpleUnitId pkgInstalledId,
                 elabComponentId = pkgInstalledId,
                 elabLinkedInstantiatedWith = Map.empty,
-                elabInstallDirs = install_dirs,
                 elabPkgOrComp = ElabPackage $ ElaboratedPackage {..},
                 elabModuleShape = modShape
             }
+        elab = elab1 {
+                elabInstallDirs =
+                  computeInstallDirs storeDirLayout
+                                     defaultInstallDirs
+                                     elaboratedSharedConfig
+                                     elab1
+            }
 
         modShape = case find (matchElabPkg (== (CLibName LMainLibName))) comps of
                         Nothing -> emptyModuleShape
@@ -1701,31 +1693,6 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
         -- pkgStanzasEnabled is a superset of elabStanzasRequested
         pkgStanzasEnabled  = Map.keysSet (Map.filter (id :: Bool -> Bool) elabStanzasRequested)
 
-        install_dirs
-          | shouldBuildInplaceOnly pkg
-          -- use the ordinary default install dirs
-          = (InstallDirs.absoluteInstallDirs
-               pkgid
-               (newSimpleUnitId pkgInstalledId)
-               (compilerInfo compiler)
-               InstallDirs.NoCopyDest
-               platform
-               defaultInstallDirs) {
-
-              -- absoluteInstallDirs sets these as 'undefined' but we have to
-              -- use them as "Setup.hs configure" args
-              InstallDirs.libsubdir  = "",
-              InstallDirs.libexecsubdir = "",
-              InstallDirs.datasubdir = ""
-            }
-
-          | otherwise
-          -- use special simplified install dirs
-          = storePackageInstallDirs
-              storeDirLayout
-              (compilerId compiler)
-              pkgInstalledId
-
     elaborateSolverToCommon :: SolverPackage UnresolvedPkgLoc
                             -> ElaboratedConfiguredPackage
     elaborateSolverToCommon
@@ -2152,8 +2119,8 @@ getComponentId (InstallPlan.PreExisting dipkg) = IPI.installedComponentId dipkg
 getComponentId (InstallPlan.Configured elab) = elabComponentId elab
 getComponentId (InstallPlan.Installed elab) = elabComponentId elab
 
-instantiateInstallPlan :: ElaboratedInstallPlan -> ElaboratedInstallPlan
-instantiateInstallPlan plan =
+instantiateInstallPlan :: StoreDirLayout -> InstallDirs.InstallDirTemplates -> ElaboratedSharedConfig -> ElaboratedInstallPlan -> ElaboratedInstallPlan
+instantiateInstallPlan storeDirLayout defaultInstallDirs elaboratedShared plan =
     InstallPlan.new (IndependentGoals False)
                     (Graph.fromDistinctList (Map.elems ready_map))
   where
@@ -2181,12 +2148,12 @@ instantiateInstallPlan plan =
     instantiateComponent uid cid insts
       | Just planpkg <- Map.lookup cid cmap
       = case planpkg of
-          InstallPlan.Configured (elab@ElaboratedConfiguredPackage
+          InstallPlan.Configured (elab0@ElaboratedConfiguredPackage
                                     { elabPkgOrComp = ElabComponent comp }) -> do
             deps <- mapM (substUnitId insts)
                          (compLinkedLibDependencies comp)
             let getDep (Module dep_uid _) = [dep_uid]
-            return $ InstallPlan.Configured elab {
+                elab1 = elab0 {
                     elabUnitId = uid,
                     elabComponentId = cid,
                     elabInstantiatedWith = insts,
@@ -2197,7 +2164,14 @@ instantiateInstallPlan plan =
                             ordNub (map unDefUnitId
                                 (deps ++ concatMap getDep (Map.elems insts)))
                     }
-                }
+                  }
+                elab = elab1 {
+                    elabInstallDirs = computeInstallDirs storeDirLayout
+                                                         defaultInstallDirs
+                                                         elaboratedShared
+                                                         elab1
+                  }
+            return $ InstallPlan.Configured elab
           _ -> return planpkg
       | otherwise = error ("instantiateComponent: " ++ display cid)
 
@@ -3255,6 +3229,38 @@ storePackageInstallDirs' StoreDirLayout{ storePackageDirectory
     sysconfdir   = prefix </> "etc"
 
 
+
+computeInstallDirs :: StoreDirLayout
+                   -> InstallDirs.InstallDirTemplates
+                   -> ElaboratedSharedConfig
+                   -> ElaboratedConfiguredPackage
+                   -> InstallDirs.InstallDirs FilePath
+computeInstallDirs storeDirLayout defaultInstallDirs elaboratedShared elab
+  | elabBuildStyle elab == BuildInplaceOnly
+  -- use the ordinary default install dirs
+  = (InstallDirs.absoluteInstallDirs
+       (elabPkgSourceId elab)
+       (elabUnitId elab)
+       (compilerInfo (pkgConfigCompiler elaboratedShared))
+       InstallDirs.NoCopyDest
+       (pkgConfigPlatform elaboratedShared)
+       defaultInstallDirs) {
+
+      -- absoluteInstallDirs sets these as 'undefined' but we have
+      -- to use them as "Setup.hs configure" args
+      InstallDirs.libsubdir  = "",
+      InstallDirs.libexecsubdir  = "",
+      InstallDirs.datasubdir = ""
+    }
+
+  | otherwise
+  -- use special simplified install dirs
+  = storePackageInstallDirs'
+      storeDirLayout
+      (compilerId (pkgConfigCompiler elaboratedShared))
+      (elabUnitId elab)
+
+
 --TODO: [code cleanup] perhaps reorder this code
 -- based on the ElaboratedInstallPlan + ElaboratedSharedConfig,
 -- make the various Setup.hs {configure,build,copy} flags

diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/Includes3.cabal b/cabal-testsuite/PackageTests/Backpack/Includes3/Includes3.cabal
@@ -9,17 +9,17 @@ cabal-version:       1.25
 library sigs
   build-depends:       base
   signatures: Data.Map
-  hs-source-dirs:      sigs
+  hs-source-dirs:      repo/sigs-0.1.0.0
   default-language:    Haskell2010
 
 library indef
   build-depends:       base, sigs
   exposed-modules:     Foo
-  hs-source-dirs:      indef
+  hs-source-dirs:      repo/indef-0.1.0.0
   default-language:    Haskell2010
 
 executable exe
   build-depends:       base, containers, indef
   main-is:             Main.hs
-  hs-source-dirs:      exe
+  hs-source-dirs:      repo/exe-0.1.0.0
   default-language:    Haskell2010
diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/cabal-repo.out b/cabal-testsuite/PackageTests/Backpack/Includes3/cabal-repo.out
@@ -0,0 +1,29 @@
+# cabal v1-update
+Downloading the latest package list from test-local-repo
+# cabal v2-build
+Resolving dependencies...
+Build profile: -w ghc-<GHCVER> -O1
+In order, the following will be built:
+ - sigs-0.1.0.0 (lib) (requires download & build)
+ - indef-0.1.0.0 (lib) (requires download & build)
+ - indef-0.1.0.0 (lib with Data.Map=containers-<VERSION>:Data.Map) (requires download & build)
+ - exe-0.1.0.0 (exe:exe) (first run)
+Configuring library for sigs-0.1.0.0..
+Preprocessing library for sigs-0.1.0.0..
+Building library instantiated with Data.Map = <Data.Map>
+for sigs-0.1.0.0..
+Installing library in <PATH>
+Configuring library for indef-0.1.0.0..
+Preprocessing library for indef-0.1.0.0..
+Building library instantiated with Data.Map = <Data.Map>
+for indef-0.1.0.0..
+Installing library in <PATH>
+Configuring library instantiated with Data.Map = containers-<VERSION>:Data.Map
+for indef-0.1.0.0..
+Preprocessing library for indef-0.1.0.0..
+Building library instantiated with Data.Map = containers-<VERSION>:Data.Map
+for indef-0.1.0.0..
+Installing library in <PATH>
+Configuring executable 'exe' for exe-0.1.0.0..
+Preprocessing executable 'exe' for exe-0.1.0.0..
+Building executable 'exe' for exe-0.1.0.0..
diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/cabal-repo.test.hs b/cabal-testsuite/PackageTests/Backpack/Includes3/cabal-repo.test.hs
@@ -0,0 +1,8 @@
+import Test.Cabal.Prelude
+main = withShorterPathForNewBuildStore $ \storeDir ->
+  cabalTest $ do
+    skipUnless =<< ghcVersionIs (>= mkVersion [8,1])
+    skipIf =<< isWindows -- TODO: https://github.com/haskell/cabal/issues/6271
+    withProjectFile "cabal.repo.project" $ do
+      withRepo "repo" $ do
+        cabalG ["--store-dir=" ++ storeDir] "v2-build" ["exe"]
diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/cabal.external.project b/cabal-testsuite/PackageTests/Backpack/Includes3/cabal.external.project
@@ -1 +1 @@
-packages: exe indef sigs
+packages: repo/exe-0.1.0.0 repo/indef-0.1.0.0 repo/sigs-0.1.0.0
diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/cabal.repo.project b/cabal-testsuite/PackageTests/Backpack/Includes3/cabal.repo.project
@@ -0,0 +1 @@
+packages: repo/exe-0.1.0.0
diff --git a/...ckageTests/Backpack/Includes3/exe/Main.hs → ...ckpack/Includes3/repo/exe-0.1.0.0/Main.hs b/...ckageTests/Backpack/Includes3/exe/Main.hs → ...ckpack/Includes3/repo/exe-0.1.0.0/Main.hs
diff --git a/...ageTests/Backpack/Includes3/exe/exe.cabal → ...pack/Includes3/repo/exe-0.1.0.0/exe.cabal b/...ageTests/Backpack/Includes3/exe/exe.cabal → ...pack/Includes3/repo/exe-0.1.0.0/exe.cabal
diff --git a/...kageTests/Backpack/Includes3/indef/Foo.hs → ...kpack/Includes3/repo/indef-0.1.0.0/Foo.hs b/...kageTests/Backpack/Includes3/indef/Foo.hs → ...kpack/Includes3/repo/indef-0.1.0.0/Foo.hs
diff --git a/...ests/Backpack/Includes3/indef/indef.cabal → .../Includes3/repo/indef-0.1.0.0/indef.cabal b/...ests/Backpack/Includes3/indef/indef.cabal → .../Includes3/repo/indef-0.1.0.0/indef.cabal
diff --git a/...sts/Backpack/Includes3/sigs/Data/Map.hsig → ...Includes3/repo/sigs-0.1.0.0/Data/Map.hsig b/...sts/Backpack/Includes3/sigs/Data/Map.hsig → ...Includes3/repo/sigs-0.1.0.0/Data/Map.hsig
diff --git a/...eTests/Backpack/Includes3/sigs/sigs.cabal → ...ck/Includes3/repo/sigs-0.1.0.0/sigs.cabal b/...eTests/Backpack/Includes3/sigs/sigs.cabal → ...ck/Includes3/repo/sigs-0.1.0.0/sigs.cabal
diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-explicit.out b/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-explicit.out
@@ -8,7 +8,7 @@ for sigs-0.1.0.0..
 Preprocessing library for sigs-0.1.0.0..
 Running Haddock on library instantiated with Data.Map = <Data.Map>
 for sigs-0.1.0.0..
-Documentation created: ../setup-external-explicit.dist/work/sigs/dist/doc/html/sigs/index.html
+Documentation created: ../../setup-external-explicit.dist/work/repo/sigs-0.1.0.0/dist/doc/html/sigs/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -24,7 +24,7 @@ for indef-0.1.0.0..
 Preprocessing library for indef-0.1.0.0..
 Running Haddock on library instantiated with Data.Map = <Data.Map>
 for indef-0.1.0.0..
-Documentation created: ../setup-external-explicit.dist/work/indef/dist/doc/html/indef/index.html
+Documentation created: ../../setup-external-explicit.dist/work/repo/indef-0.1.0.0/dist/doc/html/indef/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register

diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-explicit.test.hs b/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-explicit.test.hs
@@ -3,5 +3,5 @@ import Test.Cabal.Prelude
 main = setupTest $ do
     skipUnless =<< ghcVersionIs (>= mkVersion [8,1])
     withPackageDb $ do
-      withDirectory "sigs" $ setup_install_with_docs ["--cid", "sigs-0.1.0.0", "lib:sigs"]
-      withDirectory "indef" $ setup_install_with_docs ["--cid", "indef-0.1.0.0", "--dependency=sigs=sigs-0.1.0.0", "lib:indef"]
+      withDirectory "repo/sigs-0.1.0.0" $ setup_install_with_docs ["--cid", "sigs-0.1.0.0", "lib:sigs"]
+      withDirectory "repo/indef-0.1.0.0" $ setup_install_with_docs ["--cid", "indef-0.1.0.0", "--dependency=sigs=sigs-0.1.0.0", "lib:indef"]
diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-fail.test.hs b/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-fail.test.hs
@@ -2,10 +2,10 @@ import Test.Cabal.Prelude
 main = setupAndCabalTest $ do
     skipUnless =<< ghcVersionIs (>= mkVersion [8,1])
     withPackageDb $ do
-      withDirectory "sigs" $ setup_install []
-      withDirectory "indef" $ setup_install []
+      withDirectory "repo/sigs-0.1.0.0" $ setup_install []
+      withDirectory "repo/indef-0.1.0.0" $ setup_install []
       -- Forgot to build the instantiated versions!
-      withDirectory "exe" $ do
+      withDirectory "repo/exe-0.1.0.0" $ do
         -- Missing package message includes a unit identifier,
         -- which wobbles when version numbers change
         r <- recordMode DoNotRecord . fails $ setup' "configure" []

diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-ok.cabal.out b/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-ok.cabal.out
@@ -9,7 +9,7 @@ for sigs-0.1.0.0..
 Preprocessing library for sigs-0.1.0.0..
 Running Haddock on library instantiated with Data.Map = <Data.Map>
 for sigs-0.1.0.0..
-Documentation created: ../setup-external-ok.cabal.dist/work/sigs/dist/doc/html/sigs/index.html
+Documentation created: ../../setup-external-ok.cabal.dist/work/repo/sigs-0.1.0.0/dist/doc/html/sigs/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -26,7 +26,7 @@ for indef-0.1.0.0..
 Preprocessing library for indef-0.1.0.0..
 Running Haddock on library instantiated with Data.Map = <Data.Map>
 for indef-0.1.0.0..
-Documentation created: ../setup-external-ok.cabal.dist/work/indef/dist/doc/html/indef/index.html
+Documentation created: ../../setup-external-ok.cabal.dist/work/repo/indef-0.1.0.0/dist/doc/html/indef/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -44,7 +44,7 @@ Preprocessing library for sigs-0.1.0.0..
 Running Haddock on library instantiated with
   Data.Map = containers-<VERSION>:Data.Map
 for sigs-0.1.0.0..
-Documentation created: ../setup-external-ok.cabal.dist/work/sigs/dist/doc/html/sigs/index.html
+Documentation created: ../../setup-external-ok.cabal.dist/work/repo/sigs-0.1.0.0/dist/doc/html/sigs/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -62,7 +62,7 @@ Preprocessing library for indef-0.1.0.0..
 Running Haddock on library instantiated with
   Data.Map = containers-<VERSION>:Data.Map
 for indef-0.1.0.0..
-Documentation created: ../setup-external-ok.cabal.dist/work/indef/dist/doc/html/indef/index.html
+Documentation created: ../../setup-external-ok.cabal.dist/work/repo/indef-0.1.0.0/dist/doc/html/indef/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register

diff --git a/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-ok.out b/cabal-testsuite/PackageTests/Backpack/Includes3/setup-external-ok.out
@@ -8,7 +8,7 @@ for sigs-0.1.0.0..
 Preprocessing library for sigs-0.1.0.0..
 Running Haddock on library instantiated with Data.Map = <Data.Map>
 for sigs-0.1.0.0..
-Documentation created: ../setup-external-ok.dist/work/sigs/dist/doc/html/sigs/index.html
+Documentation created: ../../setup-external-ok.dist/work/repo/sigs-0.1.0.0/dist/doc/html/sigs/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -24,7 +24,7 @@ for indef-0.1.0.0..
 Preprocessing library for indef-0.1.0.0..
 Running Haddock on library instantiated with Data.Map = <Data.Map>
 for indef-0.1.0.0..
-Documentation created: ../setup-external-ok.dist/work/indef/dist/doc/html/indef/index.html
+Documentation created: ../../setup-external-ok.dist/work/repo/indef-0.1.0.0/dist/doc/html/indef/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -41,7 +41,7 @@ Preprocessing library for sigs-0.1.0.0..
 Running Haddock on library instantiated with
   Data.Map = containers-<VERSION>:Data.Map
 for sigs-0.1.0.0..
-Documentation created: ../setup-external-ok.dist/work/sigs/dist/doc/html/sigs/index.html
+Documentation created: ../../setup-external-ok.dist/work/repo/sigs-0.1.0.0/dist/doc/html/sigs/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
@@ -58,7 +58,7 @@ Preprocessing library for indef-0.1.0.0..
 Running Haddock on library instantiated with
   Data.Map = containers-<VERSION>:Data.Map
 for indef-0.1.0.0..
-Documentation created: ../setup-external-ok.dist/work/indef/dist/doc/html/indef/index.html
+Documentation created: ../../setup-external-ok.dist/work/repo/indef-0.1.0.0/dist/doc/html/indef/index.html
 # Setup copy
 Installing library in <PATH>
 # Setup register
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		packages: exe indef sigs
		packages: repo/exe-0.1.0.0 repo/indef-0.1.0.0 repo/sigs-0.1.0.0