Skip to content

Commit 8aa782d

Browse files
Fix Backpack CInst planning for installed signatures
Backpack packages can be indefinite. An indefinite package contains signature modules, such as Str.hsig, that describe required modules but do not provide their implementations. When another package uses a Backpack mixin, Stack must build a concrete instantiation, a CInst, where those holes are filled by real modules from implementation packages. Stack represents dependency resolution with AddDepRes. ADRFound means the package is already installed and can be used as-is. ADRToInstall means Stack must build a Task for that package before dependents can use it. That distinction matters because only ADRToInstall entries become normal build-plan tasks. The previous implementation needed Package metadata for already-installed indefinite packages so it could create CInst tasks. It got that metadata by upgrading selected ADRFound entries to ADRToInstall. That made CInst creation possible, but it also changed the meaning of the plan: clean installed signature libraries started looking like normal packages that needed to be built. That was too broad and could trigger incorrect rebuild and unregister behavior. Replace that upgrade step with a separate Backpack template-loading pass. Installed indefinite packages now remain ADRFound in the normal build plan. When Backpack instantiation needs metadata, Stack loads a source-backed Task template for the signature package and keeps it in a side map. These templates are blueprints, not ordinary build-plan tasks. They are only used to construct CInst tasks with the right package description, configure options, dependency information, and source location. This keeps normal CLib planning unchanged. Clean installed signature packages remain installed dependencies, metadata lookup no longer implies a rebuild, and non-Backpack builds stay outside the new path. Also keep CInst-only tasks out of local unregister selection. A CInst registers an instantiated unit for the same package name, but its configure step still needs the indefinite CLib registration to exist. If Stack treats a CInst-only package as the representative local rebuild task, it can unregister the indefinite unit immediately before the CInst configure step needs it. Filtering CInst keys there preserves the installed indefinite unit while still allowing the CInst action itself to run. The normal ConstructPlan dependency resolver and the Backpack template resolver serve different purposes. The normal resolver recursively decides the real build plan: which packages are found, which packages must be installed, and which dependencies are missing or present. The Backpack resolver does not decide the main build plan. It only reconstructs enough dependency information for template tasks, because clean installed indefinite packages intentionally remain ADRFound and therefore do not have normal Tasks in the plan. While loading templates, follow only Backpack mixin dependencies. This discovers inherited holes without walking ordinary dependencies as Backpack targets. That keeps the extra work scoped to Backpack plans and avoids unnecessary lookups for packages like base. Also support source-backed snapshot packages in the template loader. Some already-installed signature packages can come from a snapshot while still having source metadata available. Loading that metadata is needed when inherited holes come through a snapshot-installed signature package. Share the AddDepRes-to-MissingPresentDeps conversion between the normal ConstructPlan dependency path and the Backpack template path. This avoids parallel logic and keeps ADRFound and ADRToInstall dependencies represented consistently in both places. Add focused integration coverage for stale CInst planning when the implementation package changes, the deepest signature package is dirty, an intermediate signature changes, an intermediate signature package source file changes, the final consumer changes, a required hole is inherited through another signature package, and an inherited signature package comes from a snapshot archive. Keep the existing transitive Backpack test as the clean rebuild baseline.
1 parent 9b7984b commit 8aa782d

104 files changed

Lines changed: 1469 additions & 156 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/Stack/Build/Backpack.hs

Lines changed: 200 additions & 114 deletions
Large diffs are not rendered by default.

src/Stack/Build/ConstructPlan.hs

Lines changed: 30 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import RIO.State
3434
( State, StateT (..), execState, get, modify, modify', put )
3535
import RIO.Writer ( WriterT (..), pass, tell )
3636
import Stack.Build.Backpack
37-
( addInstantiationTasks, upgradeFoundIndefinites )
37+
( addInstantiationTasks, loadFoundIndefiniteTasks )
3838
import Stack.Build.Cache ( tryGetFlagCache )
3939
import Stack.Build.Haddock ( shouldHaddockDeps )
4040
import Stack.Build.Source ( loadLocalPackage )
@@ -56,7 +56,8 @@ import Stack.Types.Build.ConstructPlan
5656
( AddDepRes (..), CombinedMap, Ctx (..), LibraryMap, M
5757
, MissingPresentDeps (..), PackageInfo (..), PackageLoader
5858
, ToolWarning(..), UnregisterState (..), W (..)
59-
, adrHasLibrary, adrVersion, isAdrToInstall, toTask
59+
, adrHasLibrary, adrVersion, isAdrToInstall, processAdr
60+
, toTask
6061
)
6162
import Stack.Types.Build.Exception
6263
( BadDependency (..), BuildException (..)
@@ -95,14 +96,14 @@ import Stack.Types.NamedComponent
9596
)
9697
import Stack.Types.Package
9798
( ExeName (..), LocalPackage (..), Package (..)
98-
, PackageSource (..), installedMapGhcPkgId
99-
, packageIdentifier, psVersion, runMemoizedWith
99+
, PackageSource (..), packageIdentifier, psVersion
100+
, runMemoizedWith
100101
)
101102
import Stack.Types.Plan
102103
( ComponentKey (..), Plan (..), Task (..)
103104
, TaskConfigOpts (..), TaskType (..), componentKeyPkgName
104105
, fromComponentKey, installLocationIsMutable, taskIsTarget
105-
, taskLocation, taskProvides, taskTargetIsMutable
106+
, taskLocation, taskProvides
106107
)
107108
import Stack.Types.ProjectConfig ( isPCGlobalProject )
108109
import Stack.Types.Runner ( HasRunner (..), globalOptsL )
@@ -226,9 +227,9 @@ constructPlan
226227
errs = errlibs ++ errfinals
227228
if null errs
228229
then do
229-
-- Upgrade ADRFound entries to ADRToInstall for indefinite packages
230-
-- whose source is available. This ensures addInstantiationTasks can
231-
-- clone their Task to create CInst instantiation tasks.
230+
-- Load source-backed templates for clean installed indefinite packages.
231+
-- This lets addInstantiationTasks clone them for CInst tasks without
232+
-- turning their ADRFound CLib entries into build tasks.
232233
let loadPkg w x y z = applyForceCustomBuild globalCabalVersion
233234
<$> loadPackage0 w x y z
234235
-- Build a module lookup map from installed packages (dump data).
@@ -239,11 +240,15 @@ constructPlan
239240
| dp <- allDumpPkgs
240241
, isNothing dp.sublib -- Only main libraries
241242
]
242-
adrs' <- upgradeFoundIndefinites
243-
loadPkg econfig ctx.combinedMap ctx.baseConfigOpts adrs
244-
let expandedAdrs = concatMap (uncurry expandToComponentKeys) adrs'
243+
foundIndefiniteTasks <- loadFoundIndefiniteTasks
244+
loadPkg econfig sources ctx.combinedMap ctx.baseConfigOpts adrs
245+
let expandedAdrs = concatMap (uncurry expandToComponentKeys) adrs
245246
(withInstantiations, bpWarnings) =
246-
addInstantiationTasks installedModules adrs' expandedAdrs
247+
addInstantiationTasks
248+
installedModules
249+
foundIndefiniteTasks
250+
adrs
251+
expandedAdrs
247252
tasks = Map.fromList $ mapMaybe
248253
(toMaybe . second toTask)
249254
withInstantiations
@@ -416,10 +421,17 @@ mkUnregisterLocal componentTasks dirtyReason localDumpPkgs initialBuildSteps =
416421
-- directly or transitively depending on it.
417422
loop Map.empty localDumpPkgs
418423
where
419-
-- Derive a per-package task map: pick one representative task per package.
424+
-- Derive a per-package task map: pick one representative non-CInst task per
425+
-- package. A CInst task registers a Backpack instantiation, but it relies on
426+
-- the existing indefinite library registration for the same package name.
427+
-- Treating CInst-only packages as normal rebuilds unregisters that
428+
-- indefinite unit before the CInst configure step can use it.
420429
tasks :: Map PackageName Task
421430
tasks = Map.fromList
422-
[ (componentKeyPkgName ck, t) | (ck, t) <- Map.toList componentTasks ]
431+
[ (componentKeyPkgName ck, t)
432+
| (ck, t) <- Map.toList componentTasks
433+
, not (componentKeyIsCInst ck)
434+
]
423435

424436
loop ::
425437
Map GhcPkgId (PackageIdentifier, Text)
@@ -505,6 +517,10 @@ mkUnregisterLocal componentTasks dirtyReason localDumpPkgs initialBuildSteps =
505517
relevantPkgName :: PackageName
506518
relevantPkgName = maybe (pkgName ident) pkgName mParentLibId
507519

520+
componentKeyIsCInst :: ComponentKey -> Bool
521+
componentKeyIsCInst (ComponentKey _ CInst{}) = True
522+
componentKeyIsCInst _ = False
523+
508524
-- | Given a t'LocalPackage' and its test\/benchmark 'Package', adds a
509525
-- t'Task' for running its tests and benchmarks. Resolves the package's deps
510526
-- via 'addPackageDeps' (in the common case these have already been resolved
@@ -1154,31 +1170,6 @@ adrInRange pkgId name range adr = if adrVersion adr `withinRange` range
11541170
, reason <> "."
11551171
]
11561172

1157-
-- | Given a result of 'addDep', yields a triple indicating: (1) if the
1158-
-- dependency is to be installed, its package identifier; (2) if the dependency
1159-
-- is installed and a library, its package identifier and 'GhcPkgId'; and (3) if
1160-
-- the dependency is, or will be when installed, mutable or immutable.
1161-
processAdr ::
1162-
AddDepRes
1163-
-> MissingPresentDeps
1164-
processAdr adr = case adr of
1165-
ADRToInstall task ->
1166-
MissingPresentDeps
1167-
{ missingPackages = Set.singleton $ taskProvides task
1168-
, presentPackages = mempty
1169-
, isMutable = taskTargetIsMutable task
1170-
}
1171-
ADRFound loc installed ->
1172-
MissingPresentDeps
1173-
{ missingPackages = mempty
1174-
, presentPackages = presentPackagesV
1175-
, isMutable = installLocationIsMutable loc
1176-
}
1177-
where
1178-
presentPackagesV = case installed of
1179-
Library ident installedInfo -> installedMapGhcPkgId ident installedInfo
1180-
_ -> Map.empty
1181-
11821173
checkDirtiness ::
11831174
PackageSource
11841175
-> Installed

src/Stack/Types/Build/ConstructPlan.hs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ module Stack.Types.Build.ConstructPlan
2020
, toTask
2121
, adrVersion
2222
, adrHasLibrary
23+
, processAdr
2324
, isAdrToInstall
2425
, Ctx (..)
2526
, PackageLoader
@@ -29,6 +30,8 @@ module Stack.Types.Build.ConstructPlan
2930
) where
3031

3132
import Generics.Deriving.Monoid ( mappenddefault, memptydefault )
33+
import qualified Data.Map.Strict as Map
34+
import qualified Data.Set as Set
3235
import RIO.Process ( HasProcessContext (..) )
3336
import RIO.State ( StateT )
3437
import RIO.Writer ( WriterT (..) )
@@ -52,11 +55,14 @@ import Stack.Types.Installed
5255
import Stack.Types.IsMutable ( IsMutable )
5356
import Stack.Types.Package
5457
( ExeName (..), LocalPackage (..), Package (..)
55-
, PackageSource (..)
58+
, PackageSource (..), installedMapGhcPkgId
5659
)
5760
import Stack.Types.ParentMap ( ParentMap )
5861
import Stack.Types.Plan
59-
( ComponentKey, Task (..), TaskType (..), taskProvides )
62+
( ComponentKey, Task (..), TaskType (..)
63+
, installLocationIsMutable, taskProvides
64+
, taskTargetIsMutable
65+
)
6066
import Stack.Types.Platform ( HasPlatform (..) )
6167
import Stack.Types.Runner ( HasRunner (..) )
6268

@@ -152,6 +158,32 @@ adrHasLibrary (ADRToInstall task) = case task.taskType of
152158
adrHasLibrary (ADRFound _ Library{}) = True
153159
adrHasLibrary (ADRFound _ Executable{}) = False
154160

161+
-- | Given a result of 'Stack.Build.ConstructPlan.addDep', yields a triple
162+
-- indicating: (1) if the dependency is to be installed, its package identifier;
163+
-- (2) if the dependency is installed and a library, its package identifier and
164+
-- 'GhcPkgId'; and (3) if the dependency is, or will be when installed, mutable
165+
-- or immutable.
166+
processAdr ::
167+
AddDepRes
168+
-> MissingPresentDeps
169+
processAdr adr = case adr of
170+
ADRToInstall task ->
171+
MissingPresentDeps
172+
{ missingPackages = Set.singleton $ taskProvides task
173+
, presentPackages = mempty
174+
, isMutable = taskTargetIsMutable task
175+
}
176+
ADRFound loc installed ->
177+
MissingPresentDeps
178+
{ missingPackages = mempty
179+
, presentPackages = presentPackagesV
180+
, isMutable = installLocationIsMutable loc
181+
}
182+
where
183+
presentPackagesV = case installed of
184+
Library ident installedInfo -> installedMapGhcPkgId ident installedInfo
185+
_ -> Map.empty
186+
155187
data MissingPresentDeps = MissingPresentDeps
156188
{ missingPackages :: !(Set PackageIdentifier)
157189
, presentPackages :: !(Map MungedPackageId GhcPkgId)
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
-- Regression test for stale Backpack instantiation state when the final
2+
-- Backpack consumer changes. consumer-pkg is dirty, but both indefinite
3+
-- signature packages are clean and already installed.
4+
5+
import Control.Monad ( unless )
6+
import Data.List ( isInfixOf )
7+
import System.Directory ( removeFile )
8+
import StackTest
9+
10+
main :: IO ()
11+
main = do
12+
-- Build all four packages. This exercises:
13+
-- 1. str-sig CLib (indefinite, typecheck-only)
14+
-- 2. impl-pkg CLib (concrete Str + Logger)
15+
-- 3. str-sig CInst (instantiation with impl-pkg's Str)
16+
-- 4. logger-sig CLib (indefinite, typecheck-only, inherits Str hole)
17+
-- 5. logger-sig CInst (fills BOTH Logger and Str holes)
18+
-- 6. consumer-pkg CLib + CExe
19+
stack ["build"]
20+
21+
-- Verify the consumer executable calls through the transitive chain
22+
stackCheckStdout ["exec", "consumer-demo"] $ \out ->
23+
unless ("[LOG] Hello from transitive chain" `isInfixOf` out) $
24+
error $ "Expected '[LOG] Hello from transitive chain' in output, got: "
25+
++ show out
26+
27+
replaceFile "consumer-pkg/src/Consumer.hs" $ unlines
28+
[ "module Consumer where"
29+
, ""
30+
, "import LogHelper (greetWithLog)"
31+
, ""
32+
, "hello :: String"
33+
, "hello = greetWithLog ++ \" after consumer edit\""
34+
]
35+
36+
-- Rebuild should succeed because only the final consumer changed.
37+
stackCheckStderr ["build"] $ \err ->
38+
expectBuildLine "consumer-pkg" "Compiling Consumer" err
39+
40+
-- Verify output still correct after rebuild
41+
stackCheckStdout ["exec", "consumer-demo"] $ \out ->
42+
unless ("[LOG] Hello from transitive chain after consumer edit" `isInfixOf` out) $
43+
error $
44+
"Expected edited consumer-pkg output after rebuild, got: "
45+
++ show out
46+
47+
replaceFile :: FilePath -> String -> IO ()
48+
replaceFile file contents = do
49+
removeFile file
50+
writeFile file contents
51+
52+
expectBuildLine :: String -> String -> String -> IO ()
53+
expectBuildLine package marker err =
54+
unless (any matches $ lines err) $
55+
error $
56+
"Expected build output line containing "
57+
++ show package
58+
++ " and "
59+
++ show marker
60+
++ ", got stderr: "
61+
++ show err
62+
where
63+
matches line = package `isInfixOf` line && marker `isInfixOf` line
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
consumer-pkg.cabal
2+
impl-pkg.cabal
3+
logger-sig.cabal
4+
str-sig.cabal
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module Main where
2+
3+
import Consumer (hello)
4+
5+
main :: IO ()
6+
main = putStrLn hello
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
spec-version: 0.36.0
2+
3+
name: consumer-pkg
4+
5+
dependencies:
6+
- base
7+
8+
library:
9+
source-dirs: src
10+
dependencies:
11+
- name: str-sig
12+
mixin:
13+
- requires (Str as Str)
14+
- name: logger-sig
15+
mixin:
16+
- requires (Logger as Logger)
17+
- impl-pkg
18+
19+
executables:
20+
consumer-demo:
21+
source-dirs: app
22+
main: Main.hs
23+
dependencies:
24+
- consumer-pkg
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module Consumer where
2+
3+
import LogHelper (greetWithLog)
4+
5+
hello :: String
6+
hello = greetWithLog
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
spec-version: 0.36.0
2+
3+
name: impl-pkg
4+
5+
dependencies:
6+
- base
7+
8+
library:
9+
source-dirs: src
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module Logger where
2+
3+
logMessage :: String -> String
4+
logMessage msg = "[LOG] " ++ msg

0 commit comments

Comments
 (0)