-
Notifications
You must be signed in to change notification settings - Fork 714
Solver: shorten the skipping message if needed #11062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
A trivial project that depends on aeson but requires a version more recent that the latest, built with existing
The version of
|
70aef9e
to
1fdf18e
Compare
| x@(POption i linkedTo) <- xs | ||
]) | ||
| x@(POption i linkedTo) <- take 3 xs | ||
] ++ if length xs >= 3 then " and earlier versions" else "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I use a number less that 3
here, a number of the UnitTest
tests (4 to 6 from memory) will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, when length of xs is exactly 3, cabal will print the entirety of the list but still says "and earlier versions"?..
3 looks like too many still. I'd suggest print 1 and fix the unit tests.
97c1600
to
009e43f
Compare
Also add a test to prevent this regressing. Closes: haskell#4251
009e43f
to
d961f94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few comments:
- 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.
- 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).
- It might be better to continue listing any installed or linked versions, no matter where they appear in the list.
- I agree with @ulysses4ever that only one version should be listed, at least when they are all non-linked source versions.
- There should also be a test for shortening the "rejecting" message.
- 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).
Also add a test to prevent this regressing.
Closes: #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:
significance: significant
in the changelog file.