Merge pull request #5929 from hvr/pr/bkpcabal01-regression

Add bkpcabal01 regression test-case and fix the bug

Author: ezyang

cabal-install/Distribution/Client/ProjectPlanning.hs

@@ -2231,6 +2231,41 @@ instantiateInstallPlan plan =
 
     indefiniteComponent :: UnitId -> ComponentId -> InstM ElaboratedPlanPackage
     indefiniteComponent _uid cid
+      -- Only need Configured; this phase happens before improvement, so
+      -- there shouldn't be any Installed packages here.
+      | Just (InstallPlan.Configured epkg) <- Map.lookup cid cmap
+      , ElabComponent elab_comp <- elabPkgOrComp epkg
+      = do -- We need to do a little more processing of the includes: some
+           -- of them are fully definite even without substitution.  We
+           -- want to build those too; see #5634.
+           --
+           -- This code mimics similar code in Distribution.Backpack.ReadyComponent;
+           -- however, unlike the conversion from LinkedComponent to
+           -- ReadyComponent, this transformation is done *without*
+           -- changing the type in question; and what we are simply
+           -- doing is enforcing tighter invariants on the data
+           -- structure in question.  The new invariant is that there
+           -- is no IndefFullUnitId in compLinkedLibDependencies that actually
+           -- has no holes.  We couldn't specify this invariant when
+           -- we initially created the ElaboratedPlanPackage because
+           -- we have no way of actually refiying the UnitId into a
+           -- DefiniteUnitId (that's what substUnitId does!)
+           new_deps <- forM (compLinkedLibDependencies elab_comp) $ \uid ->
+             if Set.null (openUnitIdFreeHoles uid)
+                then fmap DefiniteUnitId (substUnitId Map.empty uid)
+                else return uid
+           return $ InstallPlan.Configured epkg {
+            elabPkgOrComp = ElabComponent elab_comp {
+                compLinkedLibDependencies = new_deps,
+                -- I think this is right: any new definite unit ids we
+                -- minted in the phase above need to be built before us.
+                -- Add 'em in.  This doesn't remove any old dependencies
+                -- on the indefinite package; they're harmless.
+                compOrderLibDependencies =
+                    ordNub $ compOrderLibDependencies elab_comp ++
+                             [unDefUnitId d | DefiniteUnitId d <- new_deps]
+            }
+           }
       | Just planpkg <- Map.lookup cid cmap
       = return planpkg
       | otherwise = error ("indefiniteComponent: " ++ display cid)

cabal-install/Distribution/Client/ProjectPlanning/Types.hs

@@ -648,6 +648,12 @@ data ElaboratedComponent
     -- | The paths all our executable dependencies will be installed
     -- to once they are installed.
     compExeDependencyPaths :: [(ConfiguredId, FilePath)],
+    -- | The UnitIds of the librarys (identifying elaborated packages/
+    -- components) which must be built before this project.  This
+    -- is used purely for ordering purposes.  It can contain both
+    -- references to definite and indefinite packages; an indefinite
+    -- UnitId indicates that we must typecheck that indefinite package
+    -- before we can build this one.
     compOrderLibDependencies :: [UnitId]
    }
   deriving (Eq, Show, Generic)

cabal-testsuite/PackageTests/Backpack/bkpcabal01/Main.hs

@@ -0,0 +1,2 @@
+import Q
+main = print out

cabal-testsuite/PackageTests/Backpack/bkpcabal01/bkpcabal01.cabal

@@ -0,0 +1,36 @@
+cabal-version:       2.0
+name:                bkpcabal01
+version:             0.1.0.0
+description:         This test also exists in GHC's test-suite under the same name
+                     and was ported over to cabal's testsuite as it exposed a
+                     regression (see #5929)
+license:             BSD3
+author:              Edward Z. Yang
+maintainer:          [email protected]
+build-type:          Simple
+
+library impl
+  exposed-modules: H, I
+  build-depends: base
+  hs-source-dirs: impl
+  default-language:    Haskell2010
+
+library p
+  exposed-modules: P
+  signatures: H
+  hs-source-dirs: p
+  build-depends: base
+  default-language:    Haskell2010
+
+library q
+  exposed-modules: Q
+  signatures: I
+  hs-source-dirs: q
+  build-depends: p, impl, base
+  mixins: impl (H)
+  default-language:    Haskell2010
+
+executable exe
+  main-is: Main.hs
+  build-depends: base, q, impl
+  default-language:    Haskell2010

cabal-testsuite/PackageTests/Backpack/bkpcabal01/cabal.out

@@ -0,0 +1,34 @@
+# cabal v2-build
+Resolving dependencies...
+Build profile: -w ghc-<GHCVER> -O1
+In order, the following will be built:
+ - bkpcabal01-0.1.0.0 (lib:impl) (first run)
+ - bkpcabal01-0.1.0.0 (lib:p) (first run)
+ - bkpcabal01-0.1.0.0 (lib:p with H=bkpcabal01-0.1.0.0-inplace-impl:H) (first run)
+ - bkpcabal01-0.1.0.0 (lib:q) (first run)
+ - bkpcabal01-0.1.0.0 (lib:q with I=bkpcabal01-0.1.0.0-inplace-impl:I) (first run)
+ - bkpcabal01-0.1.0.0 (exe:exe) (first run)
+Configuring library 'impl' for bkpcabal01-0.1.0.0..
+Preprocessing library 'impl' for bkpcabal01-0.1.0.0..
+Building library 'impl' for bkpcabal01-0.1.0.0..
+Configuring library 'p' for bkpcabal01-0.1.0.0..
+Preprocessing library 'p' for bkpcabal01-0.1.0.0..
+Building library 'p' instantiated with H = <H>
+for bkpcabal01-0.1.0.0..
+Configuring library 'p' instantiated with H = bkpcabal01-0.1.0.0-inplace-impl:H
+for bkpcabal01-0.1.0.0..
+Preprocessing library 'p' for bkpcabal01-0.1.0.0..
+Building library 'p' instantiated with H = bkpcabal01-0.1.0.0-inplace-impl:H
+for bkpcabal01-0.1.0.0..
+Configuring library 'q' for bkpcabal01-0.1.0.0..
+Preprocessing library 'q' for bkpcabal01-0.1.0.0..
+Building library 'q' instantiated with I = <I>
+for bkpcabal01-0.1.0.0..
+Configuring library 'q' instantiated with I = bkpcabal01-0.1.0.0-inplace-impl:I
+for bkpcabal01-0.1.0.0..
+Preprocessing library 'q' for bkpcabal01-0.1.0.0..
+Building library 'q' instantiated with I = bkpcabal01-0.1.0.0-inplace-impl:I
+for bkpcabal01-0.1.0.0..
+Configuring executable 'exe' for bkpcabal01-0.1.0.0..
+Preprocessing executable 'exe' for bkpcabal01-0.1.0.0..
+Building executable 'exe' for bkpcabal01-0.1.0.0..

cabal-testsuite/PackageTests/Backpack/bkpcabal01/cabal.project

@@ -0,0 +1 @@
+packages: .

cabal-testsuite/PackageTests/Backpack/bkpcabal01/cabal.test.hs

@@ -0,0 +1,5 @@
+import Test.Cabal.Prelude
+main = cabalTest $ do
+    -- GHC 8.2.2 had a regression ("unknown package: hole"), see also #4908
+    skipUnless =<< ghcVersionIs (\v -> v >= mkVersion [8,2] && v /= mkVersion [8,2,2])
+    cabal "v2-build" ["all"]

cabal-testsuite/PackageTests/Backpack/bkpcabal01/impl/H.hs

@@ -0,0 +1,2 @@
+module H where
+x = True

cabal-testsuite/PackageTests/Backpack/bkpcabal01/impl/I.hs

@@ -0,0 +1 @@
+module I where

cabal-testsuite/PackageTests/Backpack/bkpcabal01/p/H.hsig

@@ -0,0 +1,2 @@
+signature H where
+x :: Bool

cabal-testsuite/PackageTests/Backpack/bkpcabal01/p/P.hs

@@ -0,0 +1,4 @@
+{-# LANGUAGE CPP #-}
+module P where
+import H
+y = x

cabal-testsuite/PackageTests/Backpack/bkpcabal01/q/I.hsig

@@ -0,0 +1 @@
+signature I where

cabal-testsuite/PackageTests/Backpack/bkpcabal01/q/Q.hs

@@ -0,0 +1,3 @@
+module Q where
+import P
+out = y