Solver: shorten the skipping message if needed
Also add a test to prevent this regressing.
Closes: https://github.com/haskell/cabal/issues/4251
Please read Github PR Conventions and then fill in one of these two templates.
Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR:
- [x] Patches conform to the coding conventions.
- [ ] Any changes that could be relevant to users have been recorded in the changelog.
- [ ] Is the change significant? If so, remember to add
significance: significantin the changelog file.
- [ ] Is the change significant? If so, remember to add
- [ ] The documentation has been updated, if necessary.
- [ ] Manual QA notes have been included.
- [x] Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)
A trivial project that depends on aeson but requires a version more recent that the latest, built with existing cabal results in:
Resolving dependencies...
Error: [Cabal-7107]
Could not resolve dependencies:
[__0] trying: solver-fix-0.1.0.0 (user goal)
[__1] next goal: aeson (dependency of solver-fix)
[__1] rejecting: aeson-2.2.3.0 (conflict: solver-fix => aeson>=2.3)
[__1] skipping: aeson; 2.2.2.0, 2.2.1.0, 2.2.0.0, 2.1.2.1, 2.1.2.0, 2.1.1.0, 2.1.0.0, 2.0.3.0, 2.0.2.0, 2.0.1.0,
2.0.0.0, 1.5.6.0, 1.5.5.1, 1.5.5.0, 1.5.4.1, 1.5.4.0, 1.5.3.0, 1.5.2.0, 1.5.1.0, 1.5.0.0, 1.4.7.1, 1.4.7.0,
1.4.6.0, 1.4.5.0, 1.4.4.0, 1.4.3.0, 1.4.2.0, 1.4.1.0, 1.4.0.0, 1.3.1.1, 1.3.1.0, 1.3.0.0, 1.2.4.0, 1.2.3.0,
1.2.2.0, 1.2.1.0, 1.2.0.0, 1.1.2.0, 1.1.1.0, 1.1.0.0, 1.0.2.1, 1.0.2.0, 1.0.1.0, 1.0.0.0, 0.11.3.0, 0.11.2.1,
0.11.2.0, 0.11.1.4, 0.11.1.3, 0.11.1.2, 0.11.1.1, 0.11.1.0, 0.11.0.0, 0.9.0.1, 0.9.0.0, 0.8.1.1, 0.8.1.0,
0.8.0.2, 0.7.0.6, 0.7.0.4, 0.6.2.1, 0.6.2.0, 0.6.1.0, 0.6.0.2, 0.6.0.1, 0.6.0.0, 0.5.0.0, 0.4.0.1, 0.4.0.0,
0.3.2.14, 0.3.2.13, 0.3.2.12, 0.3.2.11, 0.3.2.10, 0.3.2.9, 0.3.2.8, 0.3.2.7, 0.3.2.6, 0.3.2.5, 0.3.2.4,
0.3.2.3, 0.3.2.2, 0.3.2.1, 0.3.2.0, 0.3.1.1, 0.3.1.0, 0.3.0.0, 0.2.0.0, 0.1.0.0, 0.10.0.0, 0.8.0.1, 0.8.0.0,
0.7.0.5, 0.7.0.3, 0.7.0.2, 0.7.0.1, 0.7.0.0 (has the same characteristics that caused the previous version
to fail: excluded by constraint '>=2.3' from 'solver-fix')
[__1] fail (backjumping, conflict set: aeson, solver-fix)
After searching the rest of the dependency tree exhaustively, these were the goals I've had most
trouble fulfilling: solver-fix, aeson
The version of cabal in this PR produces:
Resolving dependencies...
Error: [Cabal-7107]
Could not resolve dependencies:
[__0] trying: solver-fix-0.1.0.0 (user goal)
[__1] next goal: aeson (dependency of solver-fix)
[__1] rejecting: aeson-2.2.3.0 (conflict: solver-fix => aeson>=2.3)
[__1] skipping: aeson; 2.2.2.0, 2.2.1.0, 2.2.0.0, and earlier versions (has the same characteristics that caused the previous
version to fail: excluded by constraint '>=2.3' from 'solver-fix')
[__1] fail (backjumping, conflict set: aeson, solver-fix)
After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble
fulfilling: solver-fix, aeson
I think that -v3 should continue to show all of the versions, for debugging or when users need to see which versions cabal considered or their order. Then -v3 could also be used by any existing unit tests that would be broken by this change.
I have this in my working tree.
It looks like this change would shorten the skipping/rejecting message even when the solver doesn't reject all of the versions. Is that intentional, or did you mean to implement something more like #10926? Another idea is to shorten the list when the solver rejects/skips all of the remaining versions (from the current version to the least preferred version).
Was not even aware that my change would also affect successful solutions. IMO, it should only affect the output when there is no successful solution. I will need some pointers to move forward on this.
It might be better to continue listing any installed or linked versions, no matter where they appear in the list.
Sorry, I do not even understand this comment.
I agree with @ulysses4ever that only one version should be listed, at least when they are all non-linked source versions.
Fixed in my local version already. Already have the full list being printed when Verbosity is set to verbose and had to add setVerbose to two of the unit tests, to disable the truncation of the list of skipped versions.
There should also be a test for shortening the "rejecting" message.
Any idea what the unit test for this would look like? I tried to add a test to cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs but I cannot trigger it.
It would help to have a test where the solver rejects or skips multiple versions for one reason and then rejects another version for another reason. This test case would address (2).
Clues?
Obviously these commtis should all be rebased down to a single commit. I am keeping this separate for now as each commit addresses a separate review comment.
That's what squash+merge me is for.
I know that squahs+merge can do it, but in the past I have seen it give poor results in comparison to doing it manually.
@erikd Is this ready for review again?