Skip to content

Commit

Permalink
Fix flag goal generation (and hopefully haskell#1855).
Browse files Browse the repository at this point in the history
Package flags generate (Boolean) goals. Package flags can be dependent
on one another, in situations such as this one:

  if flag(a)
    ...
  else
    if flag(b)
      ...
    else
      ...

In such a scenario, it's important to record that flag b depends on flag
a. This affects conflict set generation. If something fails due to the
choice of flag b, we should not backjump beyond flag a.

While the code handling the proper insertion of goals with their correct
dependencies was always there, it was accidentally overridden by another
piece of code that created flag goals (without dependencies) for all
flags defined in a package. The reason I add flag goals separately is
because not all paths in the decision tree may contain choices for all
flags of a package. For example, if a is chosen to be True in the
example above, b does not occur at all. But flag choices may still
affect other things that aren't visible to the solver (directory
choices, exposed modules, ...), so all flags declared should always be
chosen explicitly. So we want to keep adding all flags declared in a
package as dummies, but we have to make sure to do so *before* adding
the actual dependency tree. This way, while traversing the dependency
tree, the ones occurring in dependencies will be added again, overriding
the dummies, rather than the other way round (which we used to have
before, where the dummies were overwriting the more informative
versions).
  • Loading branch information
kosmikus committed May 19, 2014
1 parent 3e33a0f commit a51b837
Showing 1 changed file with 24 additions and 2 deletions.
26 changes: 24 additions & 2 deletions cabal-install/Distribution/Client/Dependency/Modular/Builder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ extendOpen qpn' gs s@(BS { rdeps = gs', open = o' }) = go gs' o' gs
go :: RevDepMap -> PSQ OpenGoal () -> [OpenGoal] -> BuildState
go g o [] = s { rdeps = g, open = o }
go g o (ng@(OpenGoal (Flagged _ _ _ _) _gr) : ngs) = go g (cons ng () o) ngs
-- Note: for 'Flagged' goals, we always insert, so later additions win.
-- This is important, because in general, if a goal is inserted twice,
-- the later addition will have better dependency information.
go g o (ng@(OpenGoal (Stanza _ _ ) _gr) : ngs) = go g (cons ng () o) ngs
go g o (ng@(OpenGoal (Simple (Dep qpn _)) _gr) : ngs)
| qpn == qpn' = go g o ngs
Expand All @@ -69,11 +72,30 @@ scopedExtendOpen :: QPN -> I -> QGoalReasonChain -> FlaggedDeps PN -> FlagInfo -
scopedExtendOpen qpn i gr fdeps fdefs s = extendOpen qpn gs s
where
sc = scope s
-- Qualify all package names
qfdeps = L.map (fmap (qualify sc)) fdeps -- qualify all the package names
-- Introduce all package flags
qfdefs = L.map (\ (fn, b) -> Flagged (FN (PI qpn i) fn) b [] []) $ M.toList fdefs
gs = L.map (flip OpenGoal gr) (qfdeps ++ qfdefs)
-- Combine new package and flag goals
gs = L.map (flip OpenGoal gr) (qfdefs ++ qfdeps)
-- IMPORTANT AND SUBTLE: The order of the concatenation above is
-- important. Flags occur potentially multiple times: both via the
-- flag declaration ('qfdefs') and via dependencies ('qfdeps').
-- We want the information from qfdeps if it's present, because that
-- includes dependencies between flags. We use qfdefs mainly so that
-- we are forced to make choices for flags that don't affect
-- dependencies at all.
--
-- When goals are actually extended in 'extendOpen', later additions
-- override earlier additions, so it's important that the
-- lower-quality templates without dependency information come first.

data BuildType = Goals | OneGoal OpenGoal | Instance QPN I PInfo QGoalReasonChain
-- | Datatype that encodes what to build next
data BuildType =
Goals -- ^ build a goal choice node
| OneGoal OpenGoal -- ^ build a node for this goal
| Instance QPN I PInfo QGoalReasonChain -- ^ build a tree for a concrete instance
deriving Show

build :: BuildState -> Tree (QGoalReasonChain, Scope)
build = ana go
Expand Down

0 comments on commit a51b837

Please sign in to comment.