haskell
Counter fromListWith reversing constraints order
~This does not change behaviour because~ (it does, see test failure with ghc >= 9.6) all constraints are passed to the solver but when we fix #9511, the order will matter if we pick the lastest version equality constraint as the winning one and discard the rest (per-package) before solving. I'm proposing this fix now so that whoever works on this or reviews it will not be surprised (like I was) when constraints are given to the solver in reversed order.
Template Β: This PR does not modify cabal behaviour (documentation, tests, refactoring, etc.)
Include the following checklist in your PR:
- [x] Patches conform to the coding conventions.
With ghc-9.8.1 and ghc-9.6.3 the output is different for MultiRepl/CabalTooOld/cabal.test.hs, but not with ghc-9.4.8:
$ cabal run cabal-testsuite:cabal-tests -- \
--with-cabal=./dist-newstyle/build/x86_64-linux/ghc-9.8.1/cabal-install-3.11.0.0/x/cabal/build/cabal/cabal \
cabal-testsuite/PackageTests/MultiRepl/CabalTooOld/cabal.test.hs --accept
$ git diff
diff --git a/cabal-testsuite/PackageTests/MultiRepl/CabalTooOld/cabal.out b/cabal-testsuite/PackageTests/MultiRepl/CabalTooOld/cabal.out
index f2253c671..fd4691ed1 100644
--- a/cabal-testsuite/PackageTests/MultiRepl/CabalTooOld/cabal.out
+++ b/cabal-testsuite/PackageTests/MultiRepl/CabalTooOld/cabal.out
@@ -6,6 +6,7 @@ Error: [Cabal-7107]
Could not resolve dependencies:
[__0] trying: pkg-a-0 (user goal)
[__1] next goal: pkg-a:setup.Cabal (dependency of pkg-a)
-[__1] rejecting: pkg-a:setup.Cabal-<VERSION>/installed-<HASH>, pkg-a:setup.Cabal-3.8.0.0 (constraint from --enable-multi-repl requires >=3.11)
+[__1] rejecting: pkg-a:setup.Cabal-<VERSION>/installed-<HASH> (constraint from --enable-multi-repl requires >=3.11)
+[__1] rejecting: pkg-a:setup.Cabal-3.8.0.0 (constraint from minimum version of Cabal used by Setup.hs requires >=3.10)
[__1] fail (backjumping, conflict set: pkg-a, pkg-a:setup.Cabal)
After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble fulfilling: pkg-a:setup.Cabal (3), pkg-a (2)
As the author of this test @mpickering, does this surprise you?
Why is the constraint overriding being implemented in the solver? Wouldn't it be better to implement it in cabal-install, where the constraints from different files are initially combined? Then cabal-install could set the ConstraintSource and only pass the active constraints to the solver.
Here we're devaluing cabal developers' time (mine - I got stung by this unexpected reversing of constraints) to avoid some potential hazard to cabal users' time. Do users expect the cabal solver to always give the same solution? I wouldn't have this expectation as a cabal user unless I pinned down all my dependencies.
@philderbeast Sorry, I hadn't seen #9510. I think that any feature that depends on the order of the constraints in the solver would be fragile, because the solver doesn't rely on the order anywhere. The constraint order currently should only affect the solution if it changes the order that the solver chooses values for package versions or flags (goal order). (EDIT: The benefit of this property is that it leaves open the opportunity to manipulate constraint order in the future to add additional testing or optimizations) I think that a better change to make this clearer for developers would be to change the list to a set.
above i suggested that changing the order "may potentially in very rare extreme cases change which solution is found or if one is found in an allowed number of steps" and i think that's mistaken here because this change doesn't change the order in which the tree is traversed, just in which constraints are checked at each traversal step, but at each step they're all checked regardless. i.e. its not the order of "all constraints" but just the order of "constraints on a given package name" and that should affect only error messages at best.
I do agree with @grayjay that changing the order one way or the other wouldn't be any particular improvement, but moving to a set over a list is more semantically accurate.
