Fixes spurious recompiles when sublibs are involved in copyPreCompiled

TODO: The integration test fails for this currently. This needs to be
fixed before upstreaming.

Author: qxjit

src/Stack/Build/ExecutePackage.hs

@@ -851,17 +851,21 @@ copyPreCompiled ee task pkgId (PrecompiledCache mlib subLibs exes) = do
     subLibNames = Set.toList $ buildableSubLibs $ case task.taskType of
       TTLocalMutable lp -> lp.package
       TTRemotePackage _ p _ -> p
-    toMungedPackageId :: StackUnqualCompName -> MungedPackageId
-    toMungedPackageId subLib =
-      let subLibName = LSubLibName $ toCabalName subLib
-      in  MungedPackageId (MungedPackageName pname subLibName) pversion
-    toPackageId :: MungedPackageId -> PackageIdentifier
-    toPackageId (MungedPackageId n v) =
-      PackageIdentifier (encodeCompatPackageName n) v
+    -- The sub-library names paired with the munged package names under which
+    -- they are registered in the package database.
+    subLibsMungedNames :: [(StackUnqualCompName, PackageName)]
+    subLibsMungedNames =
+      let toMungedName subLib =
+            ( subLib
+            , encodeCompatPackageName $ toCabalMungedPackageName pname subLib
+            )
+      in  map toMungedName subLibNames
     allToUnregister :: [Either PackageIdentifier GhcPkgId]
     allToUnregister = mcons
       (Left pkgId <$ mlib)
-      (map (Left . toPackageId . toMungedPackageId) subLibNames)
+      (map
+         (\(_, mungedName) -> Left $ PackageIdentifier mungedName pversion)
+         subLibsMungedNames)
     allToRegister = mcons mlib subLibs
 
   unless (null allToRegister) $
@@ -918,10 +922,20 @@ copyPreCompiled ee task pkgId (PrecompiledCache mlib subLibs exes) = do
     Just _ -> do
       mpkgid <- loadInstalledPkg pkgDbs ee.snapshotDumpPkgs pname
 
-      pure $ Just $
-        case mpkgid of
-          Nothing -> assert False $ Executable pkgId
-          Just pkgid -> simpleInstalledLib pkgId pkgid mempty
+      case mpkgid of
+        Nothing -> pure $ Just $ assert False $ Executable pkgId
+        Just pkgid -> do
+          -- Gather the GhcPkgId values of the sub-libraries registered above,
+          -- so that the 'Installed' value (and, hence, the config caches of
+          -- packages that depend on this one) includes them, as it does when
+          -- the package is built from source (see
+          -- 'fetchAndMarkInstalledPackage') and when the package is loaded
+          -- from a package database (see 'Stack.Build.Installed').
+          subLibsPkgIds <- fmap (Map.fromList . catMaybes) $
+            forM subLibsMungedNames $ \(subLib, mungedName) -> do
+              mSubLibPkgId <- loadInstalledPkg pkgDbs ee.snapshotDumpPkgs mungedName
+              pure $ fmap (\subLibPkgId -> (subLib, subLibPkgId)) mSubLibPkgId
+          pure $ Just $ simpleInstalledLib pkgId pkgid subLibsPkgIds
  where
   bindir = ee.baseConfigOpts.snapInstallRoot </> bindirSuffix
 

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/Main.hs

@@ -0,0 +1,31 @@
+import Control.Monad (unless)
+import Data.List (isInfixOf)
+import StackTest
+
+-- This tests building a project package (pkg) that depends on an immutable
+-- dependency (dep) with both a main library and a sub-library, in the case
+-- where the dependency is installed by copying it from the precompiled cache.
+-- The dependency is first built from source in the custom1 project. The
+-- custom2 project uses a different custom snapshot, so it has a distinct
+-- snapshot package database and installs the dependency from the precompiled
+-- cache. A subsequent build of the custom2 project should then do no work.
+-- Previously, the Cabal configuration cache written for the project package
+-- recorded the dependency without the package ids of its sub-libraries, while
+-- later builds compared it against a set that included them, causing a
+-- spurious "dependencies changed" rebuild.
+
+main :: IO ()
+main = do
+  -- The '--install-ghc' flag is passed here, because IntegrationSpec.runApp
+  -- sets up `config.yaml` with `system-ghc: true` and `install-ghc: false`.
+  stack ["build", "--install-ghc", "--stack-yaml", "custom1/stack.yaml", "dep"]
+  stackCheckStderr ["build", "--stack-yaml", "custom2/stack.yaml"] $ \out ->
+    unless ("using precompiled package" `isInfixOf` out) $
+      error "Didn't use precompiled package!"
+  stackCheckStderr ["build", "--stack-yaml", "custom2/stack.yaml"] $ \out ->
+    unless (null (compilingModulesLines out)) $
+      error "Stack recompiled code"
+
+-- Returns the lines where a module is compiled
+compilingModulesLines :: String -> [String]
+compilingModulesLines = filter (isInfixOf " Compiling ") . lines

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/files/custom1/custom1.yaml

@@ -0,0 +1,4 @@
+snapshot: ghc-9.10.3
+name: custom1
+packages:
+- archive: ../dep.tar.gz

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/files/custom1/stack.yaml

@@ -0,0 +1,2 @@
+snapshot: custom1.yaml
+packages: []

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/files/custom2/custom2.yaml

@@ -0,0 +1,4 @@
+snapshot: ghc-9.10.3
+name: custom2
+packages:
+- archive: ../dep.tar.gz

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/files/custom2/pkg.cabal

@@ -0,0 +1,10 @@
+cabal-version: 2.0
+name:          pkg
+version:       0.1.0.0
+build-type:    Simple
+
+library
+  hs-source-dirs:   src
+  exposed-modules:  Pkg
+  build-depends:    base, dep
+  default-language: Haskell2010

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/files/custom2/src/Pkg.hs

@@ -0,0 +1,8 @@
+module Pkg
+  ( pkgValue
+  ) where
+
+import Dep ( depValue )
+
+pkgValue :: Int
+pkgValue = depValue + 1

tests/integration/tests/NNNN-precompiled-sublibrary-no-rebuild/files/custom2/stack.yaml

@@ -0,0 +1,3 @@
+snapshot: custom2.yaml
+packages:
+- .