From a57fe48d5d6c24fab8c3e982eaf5764dd538774d Mon Sep 17 00:00:00 2001 From: Andres Loeh Date: Mon, 19 May 2014 21:53:16 +0200 Subject: [PATCH] Fix flag goal generation (and hopefully #1855). 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). (cherry picked from commit a51b8378d516d5bd7cb4bbc8ed4317137b3c96c1) --- .../Client/Dependency/Modular/Builder.hs | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/cabal-install/Distribution/Client/Dependency/Modular/Builder.hs b/cabal-install/Distribution/Client/Dependency/Modular/Builder.hs index 236aa78efc6..f52951d544f 100644 --- a/cabal-install/Distribution/Client/Dependency/Modular/Builder.hs +++ b/cabal-install/Distribution/Client/Dependency/Modular/Builder.hs @@ -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 @@ -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