Skip to content

Commit

Permalink
Solver: Detect cycles between packages and their setup scripts (fixes h…
Browse files Browse the repository at this point in the history
…askell#4161).

The solver already detected cycles involving more than one package, but it
allowed dependencies between components within a package.  This commit treats a
dependency between a package's setup script and library as a cycle in order to
allow the solver to backtrack and try to break the cycle.  A more thorough
solution would involve tracking all dependencies between components, as in haskell#4087.

This commit also fixes the internal error in issue haskell#4980.
  • Loading branch information
grayjay committed Jan 17, 2018
1 parent 0c602fd commit 53b2dc6
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
12 changes: 10 additions & 2 deletions cabal-install/Distribution/Solver/Modular/Builder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import qualified Distribution.Solver.Modular.PSQ as P
import Distribution.Solver.Modular.Tree
import qualified Distribution.Solver.Modular.WeightedPSQ as W

import Distribution.Solver.Types.ComponentDeps
import Distribution.Solver.Types.PackagePath
import Distribution.Solver.Types.Settings

Expand Down Expand Up @@ -72,8 +73,15 @@ extendOpen qpn' gs s@(BS { rdeps = gs', open = o' }) = go gs' o' gs
go g o ((Stanza sn@(SN qpn _) t) : ngs) =
go g (StanzaGoal sn t (flagGR qpn) : o) ngs
go g o ((Simple (LDep dr (Dep _ qpn _)) c) : ngs)
| qpn == qpn' = go g o ngs
-- we ignore self-dependencies at this point; TODO: more care may be needed
| qpn == qpn' =
-- We currently only add a self-dependency to the graph if it is
-- between a package and its setup script. The edge creates a cycle
-- and causes the solver to backtrack and choose a different
-- instance for the setup script. We may need to track other
-- self-dependencies once we implement component-based solving.
case c of
ComponentSetup -> go (M.adjust (addIfAbsent (ComponentSetup, qpn')) qpn g) o ngs
_ -> go g o ngs
| qpn `M.member` g = go (M.adjust (addIfAbsent (c, qpn')) qpn g) o ngs
| otherwise = go (M.insert qpn [(c, qpn')] g) (PkgGoal qpn (DependencyGoal dr) : o) ngs
-- code above is correct; insert/adjust have different arg order
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ tests = [
, runTest $ mkTest db15 "cycleThroughSetupDep3" ["C"] (solverSuccess [("C", 2), ("D", 1)])
, runTest $ mkTest db15 "cycleThroughSetupDep4" ["D"] (solverSuccess [("D", 1)])
, runTest $ mkTest db15 "cycleThroughSetupDep5" ["E"] (solverSuccess [("C", 2), ("D", 1), ("E", 1)])
, runTest $ issue4161 "detect cycle between package and its setup script"
, runTest $ testCyclicDependencyErrorMessages "cyclic dependency error messages"
]
, testGroup "Extensions" [
Expand Down Expand Up @@ -783,6 +784,30 @@ db15 = [
, Right $ exAv "E" 1 [ExFix "C" 2]
]

-- | Detect a cycle between a package and its setup script.
--
-- This type of cycle can easily occur when new-build adds default setup
-- dependencies to packages without custom-setup stanzas. For example, cabal
-- adds 'time' as a setup dependency for 'time'. The solver should detect the
-- cycle when it attempts to link the setup and non-setup instances of the
-- package and then choose a different version for the setup dependency.
issue4161 :: String -> SolverTest
issue4161 name =
mkTest db name ["target"] $
SolverResult checkFullLog $ Right [("target", 1), ("time", 1), ("time", 2)]
where
db :: ExampleDb
db = [
Right $ exAv "target" 1 [ExFix "time" 2]
, Right $ exAv "time" 2 [] `withSetupDeps` [ExAny "time"]
, Right $ exAv "time" 1 []
]

checkFullLog :: [String] -> Bool
checkFullLog = any $ isInfixOf $
"rejecting: time:setup.time~>time-2.0.0 (cyclic dependencies; "
++ "conflict set: time:setup.time)"

-- | Packages pkg-A, pkg-B, and pkg-C form a cycle. The solver should backtrack
-- as soon as it chooses the last package in the cycle, to avoid searching parts
-- of the tree that have no solution. Since there is no way to break the cycle,
Expand Down

0 comments on commit 53b2dc6

Please sign in to comment.