Skip to content

Commit

Permalink
Rework pkgStanzasEnabled calculation.
Browse files Browse the repository at this point in the history
Before: pkgStanzasEnabled is initialized to an empty list and
filled in on the first pruning pass.

After: pkgStanzasEnabled is initialized to the set of explicitly
requested stanzas, and then we add more enabled stanzas as we
do "pruning" passes (a bit of a misnomer).

Why is it good?  Now we always satisfy the invariant that
the set of enabled stanzas is a superset of the set of requested
stanzas; previously, the invariant was broken up until the point
you ran pruning (which, in the case of new-configure, meant that
the invariant was always broken!)  and we were tripping over this
when attempting to render the set of configure flags for plan
display.

Signed-off-by: Edward Z. Yang <[email protected]>
  • Loading branch information
ezyang committed Jan 3, 2018
1 parent 93df28f commit 1f00ad8
Showing 1 changed file with 34 additions and 18 deletions.
52 changes: 34 additions & 18 deletions cabal-install/Distribution/Client/ProjectPlanning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1563,8 +1563,14 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
} <- comps
]

-- Filled in later
pkgStanzasEnabled = Set.empty
-- NB: This is not the final setting of 'pkgStanzasEnabled'.
-- See [Sticky enabled testsuites]; we may enable some extra
-- stanzas opportunistically when it is cheap to do so.
--
-- However, we start off by enabling everything that was
-- requested, so that we can maintain an invariant that
-- pkgStanzasEnabled is a superset of elabStanzasRequested
pkgStanzasEnabled = Map.keysSet (Map.filter (id :: Bool -> Bool) elabStanzasRequested)

install_dirs
| shouldBuildInplaceOnly pkg
Expand Down Expand Up @@ -2393,6 +2399,10 @@ data TargetAction = TargetActionBuild
-- to specify which optional stanzas to enable, and which targets within each
-- package to build.
--
-- NB: Pruning happens after improvement, which is important because we
-- will prune differently depending on what is already installed (to
-- implement "sticky" test suite enabling behavior).
--
pruneInstallPlanToTargets :: TargetAction
-> Map UnitId [ComponentTarget]
-> ElaboratedInstallPlan -> ElaboratedInstallPlan
Expand Down Expand Up @@ -2479,7 +2489,7 @@ pruneInstallPlanPass1 pkgs =
roots = mapMaybe find_root pkgs'

prune elab = PrunedPackage elab' (pruneOptionalDependencies elab')
where elab' = pruneOptionalStanzas elab
where elab' = addOptionalStanzas elab

find_root (InstallPlan.Configured (PrunedPackage elab _)) =
if not (null (elabBuildTargets elab)
Expand All @@ -2491,8 +2501,8 @@ pruneInstallPlanPass1 pkgs =
else Nothing
find_root _ = Nothing

-- Decide whether or not to enable testsuites and benchmarks
--
-- Note [Sticky enabled testsuites]
-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- The testsuite and benchmark targets are somewhat special in that we need
-- to configure the packages with them enabled, and we need to do that even
-- if we only want to build one of several testsuites.
Expand All @@ -2506,18 +2516,31 @@ pruneInstallPlanPass1 pkgs =
-- would involve unnecessarily reconfiguring the package with testsuites
-- disabled. Technically this introduces a little bit of stateful
-- behaviour to make this "sticky", but it should be benign.
--
pruneOptionalStanzas :: ElaboratedConfiguredPackage -> ElaboratedConfiguredPackage
pruneOptionalStanzas elab@ElaboratedConfiguredPackage{ elabPkgOrComp = ElabPackage pkg } =

-- Decide whether or not to enable testsuites and benchmarks.
-- See [Sticky enabled testsuites]
addOptionalStanzas :: ElaboratedConfiguredPackage -> ElaboratedConfiguredPackage
addOptionalStanzas elab@ElaboratedConfiguredPackage{ elabPkgOrComp = ElabPackage pkg } =
elab {
elabPkgOrComp = ElabPackage (pkg { pkgStanzasEnabled = stanzas })
}
where
stanzas :: Set OptionalStanza
stanzas = optionalStanzasRequiredByTargets elab
<> optionalStanzasRequestedByDefault elab
-- By default, we enabled all stanzas requested by the user,
-- as per elabStanzasRequested, done in
-- 'elaborateSolverToPackage'
stanzas = pkgStanzasEnabled pkg
-- optionalStanzasRequiredByTargets has to be done at
-- prune-time because it depends on 'elabTestTargets'
-- et al, which is done by 'setRootTargets' at the
-- beginning of pruning.
<> optionalStanzasRequiredByTargets elab
-- optionalStanzasWithDepsAvailable has to be done at
-- prune-time because it depends on what packages are
-- installed, which is not known until after improvement
-- (pruning is done after improvement)
<> optionalStanzasWithDepsAvailable availablePkgs elab pkg
pruneOptionalStanzas elab = elab
addOptionalStanzas elab = elab

-- Calculate package dependencies but cut out those needed only by
-- optional stanzas that we've determined we will not enable.
Expand Down Expand Up @@ -2548,13 +2571,6 @@ pruneInstallPlanPass1 pkgs =
, stanza <- maybeToList (componentOptionalStanza cname)
]

optionalStanzasRequestedByDefault :: ElaboratedConfiguredPackage
-> Set OptionalStanza
optionalStanzasRequestedByDefault =
Map.keysSet
. Map.filter (id :: Bool -> Bool)
. elabStanzasRequested

availablePkgs =
Set.fromList
[ installedUnitId pkg
Expand Down

0 comments on commit 1f00ad8

Please sign in to comment.