Add a format rejections function #9560

philderbeast · 2023-12-24T19:53:18Z

Fixes #9559. Helps with #4251.

Template Α: This PR modifies cabal behaviour

Include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
~~The documentation has been updated, if necessary.~~
Manual QA notes have been included.
Tests have been added.

To test, introduce a version bound (or conflict) that is going to reject some versions of a package. Even if this creates an unsolvable combination of versions, that should suffice to trigger the rejection message by the solver for manual testing.

I found no examples of solver rejection messages in the docs.

philderbeast · 2023-12-24T19:57:59Z

@yvan-sraka I would like to get this in before some formatting changes for showing "constraint from project". This is a subset of your #9541 changes.

philderbeast · 2023-12-26T12:13:17Z

@Mikolaj I had to rerun failed jobs three times to get all green ticks:

Failures were:

Validate ubuntu-latest ghc-8.4.4
PackageTests/NewUpdate/RejectFutureIndexStates/cabal.test.hs

-----BEGIN CABAL OUTPUT-----
Error: cabal: '/usr/bin/curl' exited with an error:
curl: (7) Failed to connect to localhost port 8000 after 0 ms: Connection refused
-----END CABAL OUTPUT-----

Validate ubuntu-latest ghc-8.10.7
PackageTests/NewUpdate/RejectFutureIndexStates/cabal.test.hs

-----BEGIN CABAL OUTPUT-----
Error: cabal: '/usr/bin/curl' exited with an error:
curl: (7) Failed to connect to localhost port 8000 after 0 ms: Connection refused
-----END CABAL OUTPUT-----

Validate macos-latest ghc-9.4.8
PackageTests/NewUpdate/UpdateIndexState/update-index-state.test.hs

-----BEGIN CABAL OUTPUT-----
Error: cabal: '/usr/local/opt/curl/bin/curl' exited with an error:
curl: (7) Failed to connect to localhost port 8000 after 0 ms: Couldn't connect to server
-----END CABAL OUTPUT-----

ulysses4ever · 2023-12-26T14:35:02Z

@philderbeast thanks for the report. The work to prevent this bug is under way in #9540 If you have energy, your help there would be greatly appreciated because I'm running out of ideas.

philderbeast · 2023-12-30T11:30:03Z

Label merge+no rebase is necessary when the pull request is from an organisation.

ulysses4ever

Could we get @grayjay's opinion on it but any chance? She's our solver code owner effectively. Would be great to get her approval on any change to it.

grayjay

@philderbeast Thanks for working on the solver error messages! Please add a note about starting with @yvan-sraka's change to the commit message.

cabal-testsuite/PackageTests/ShortRejections/pandoc.test.hs

cabal-install-solver/src/Distribution/Solver/Modular/Message.hs

philderbeast · 2023-12-31T16:33:31Z

Please add a note about starting with @yvan-sraka's change to the commit message.

@yvan-sraka I did not start from your change and sorry I didn't see your work on #4251 earlier. I was working on #9510 and the verbosity of the rejection messages started to annoy me so I raised #9559, saw a quick fix improvement and went for it, found your work and realized I hadn't accounted for hyphenated package names.

The commit history is lost after squashing but these were the 3 initial commit messages:

Format solver rejections
Take account of hyphenated package names
Don't shorten a singe rejection

grayjay · 2023-12-31T20:30:06Z

@yvan-sraka I did not start from your change and sorry I didn't see your work on #4251 earlier. I was working on #9510 and the verbosity of the rejection messages started to annoy me so I raised #9559, saw a quick fix improvement and went for it, found your work and realized I hadn't accounted for hyphenated package names.

@philderbeast Sorry, I had misunderstood your comment above. I see that the changes are independent.

philderbeast · 2024-01-01T15:31:07Z

I've left recent commits as-is for review. Please let me know if you'd like some of these squashed.

grayjay

@philderbeast Thanks for the updates and for also handling skipped versions.

I've left recent commits as-is for review. Please let me know if you'd like some of these squashed.

I would prefer squashing to remove the large test build files before you merge.

cabal-install-solver/src/Distribution/Solver/Modular/Message.hs

- Is for instances, Vs for versions

Leave commentary that goPReject and goPSkip reverse the order of instances

cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs

cabal-install-solver/src/Distribution/Solver/Modular/Message.hs

philderbeast · 2024-01-16T11:31:28Z

@ulysses4ever this is ready for your approval. It already has the merge delay passed label attached.

ulysses4ever

Perfect, thanks!

philderbeast · 2024-01-16T13:45:20Z

Label merge+no rebase is necessary when the pull request is from an organisation.

ulysses4ever · 2024-01-17T05:20:21Z

Please, try not to forget squashing before merging.

philderbeast · 2024-01-17T13:49:43Z

@ulysses4ever a momentary lapse of diligence on my part, sorry about that.

philderbeast force-pushed the fix/format-rejections-9559 branch 3 times, most recently from 73540ec to a5f2cc8 Compare December 25, 2023 17:16

philderbeast added the attention: needs-review label Dec 26, 2023

Mikolaj requested review from grayjay and yvan-sraka December 27, 2023 10:58

philderbeast force-pushed the fix/format-rejections-9559 branch from a5f2cc8 to c0c36ec Compare December 28, 2023 20:27

philderbeast requested review from geekosaur and ulysses4ever December 30, 2023 03:11

geekosaur approved these changes Dec 30, 2023

View reviewed changes

yvan-sraka approved these changes Dec 30, 2023

View reviewed changes

philderbeast added merge+no rebase and removed attention: needs-review labels Dec 30, 2023

ulysses4ever requested changes Dec 30, 2023

View reviewed changes

grayjay reviewed Dec 31, 2023

View reviewed changes

grayjay added the cabal-install: solver label Dec 31, 2023

philderbeast force-pushed the fix/format-rejections-9559 branch from c0c36ec to 472d2f1 Compare December 31, 2023 16:28

philderbeast force-pushed the fix/format-rejections-9559 branch from 70f8a76 to a79cf19 Compare January 1, 2024 15:29

philderbeast requested review from ulysses4ever and grayjay January 1, 2024 15:30

grayjay reviewed Jan 3, 2024

View reviewed changes

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jan 5, 2024

philderbeast force-pushed the fix/format-rejections-9559 branch from 5eefeee to cf9161f Compare January 6, 2024 20:39

philderbeast added 19 commits January 13, 2024 17:26

Don't pass strings to abbreviatePkgVers

8e3f600

Rename to abbreviateOptions

ccc8457

Abbreviate to versions

36e3b18

Switch to using the I and V to match other naming

7a22768

- Is for instances, Vs for versions

Add haddocks to tryVs and showIsOrVs

a760817

Add mixed tests

2e2d7c9

Simplify versions for doctests

dcc8568

Remove PackageTests/ShortRejections

8a91a64

Update expectations of cabal-install:unit-tests

c9ee431

Don't use show instance for showing ProgressAction

a4e70f5

Let intercalate check for singleton lists

6eb24c3

Don't reverse list in showIsOrVs

b20f1a9

Leave commentary that goPReject and goPSkip reverse the order of instances

Use blurb for showing a progress action

8897ff6

Add blurbOption

6c34eb6

Change wording to "linked or installed"

8b27270

Change foo to foo-bar, a hyphenated package name

5e3dace

Add skipping abbreviation unit tests

b15a515

Add skipping installed tests

a474965

Add cabal-install-solver to doctest target

5bceb40

philderbeast force-pushed the fix/format-rejections-9559 branch from 7bb55b7 to 5bceb40 Compare January 13, 2024 22:27

Satisfy fourmolu.

18bc485

grayjay approved these changes Jan 16, 2024

View reviewed changes

cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs Outdated Show resolved Hide resolved

cabal-install-solver/src/Distribution/Solver/Modular/Message.hs Show resolved Hide resolved

Add /installed-x.y.z to expectations in unit tests

f80bb15

ulysses4ever approved these changes Jan 16, 2024

View reviewed changes

philderbeast added the merge+no rebase label Jan 16, 2024

mergify bot merged commit d89087b into haskell:master Jan 16, 2024
50 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a format rejections function #9560

Add a format rejections function #9560

philderbeast commented Dec 24, 2023 •

edited

Loading

philderbeast commented Dec 24, 2023

philderbeast commented Dec 26, 2023 •

edited

Loading

ulysses4ever commented Dec 26, 2023

philderbeast commented Dec 30, 2023

ulysses4ever left a comment

grayjay left a comment

philderbeast commented Dec 31, 2023 •

edited

Loading

grayjay commented Dec 31, 2023

philderbeast commented Jan 1, 2024

grayjay left a comment

philderbeast commented Jan 16, 2024 •

edited

Loading

ulysses4ever left a comment

philderbeast commented Jan 16, 2024

ulysses4ever commented Jan 17, 2024

philderbeast commented Jan 17, 2024

Add a format rejections function #9560

Add a format rejections function #9560

Conversation

philderbeast commented Dec 24, 2023 • edited Loading

philderbeast commented Dec 24, 2023

philderbeast commented Dec 26, 2023 • edited Loading

ulysses4ever commented Dec 26, 2023

philderbeast commented Dec 30, 2023

ulysses4ever left a comment

Choose a reason for hiding this comment

grayjay left a comment

Choose a reason for hiding this comment

philderbeast commented Dec 31, 2023 • edited Loading

grayjay commented Dec 31, 2023

philderbeast commented Jan 1, 2024

grayjay left a comment

Choose a reason for hiding this comment

philderbeast commented Jan 16, 2024 • edited Loading

ulysses4ever left a comment

Choose a reason for hiding this comment

philderbeast commented Jan 16, 2024

ulysses4ever commented Jan 17, 2024

philderbeast commented Jan 17, 2024

philderbeast commented Dec 24, 2023 •

edited

Loading

philderbeast commented Dec 26, 2023 •

edited

Loading

philderbeast commented Dec 31, 2023 •

edited

Loading

philderbeast commented Jan 16, 2024 •

edited

Loading