Skip to content
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

Show why configuring dependencies failed #10273

Merged
merged 7 commits into from
Oct 1, 2024

Conversation

9999years
Copy link
Collaborator

@9999years 9999years commented Aug 23, 2024

When act-as-setup configure fails, it doesn't indicate which dependencies are missing entirely and which ones don't satisfy the version constraints. The errors from cabal build are more informative, but aren't available in all contexts (nixpkgs Haskell builds, for example, use the act-as-setup interface). This makes it immediately clear what sort of dependency error has occurred.

Before (cabal-install 3.12.1.0):

Configuring test-pkg-0.1.0.0...
Error: [Cabal-8010]
Encountered missing or private dependencies:
    base <=4.18,
    foobar,
    test-pkg:{bar-internal, foo-internal}

After:

Configuring test-pkg-0.1.0.0...
Error: [Cabal-8010]
Encountered missing or private dependencies:
    base <=4.18 (installed: 4.19.1.0),
    foobar (missing),
    test-pkg:{bar-internal,foo-internal} (missing: :bar-internal)

Please let me know if you're interested in this patch and I can write tests and polish up the documentation.


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:

  • 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. (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!)

Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I'm not qualified to determine whether this is logically correct, only that it looks reasonable (and being a spell checker 😛).

Cabal/src/Distribution/Simple/PackageIndex.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/PackageIndex.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/PackageIndex.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/PackageIndex.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/PackageIndex.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/Configure.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/Configure.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/Configure.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/Configure.hs Outdated Show resolved Hide resolved
@9999years 9999years force-pushed the missing-or-private-details branch 5 times, most recently from 2b37276 to 310a3ed Compare August 26, 2024 22:54
@9999years
Copy link
Collaborator Author

Ah, the tests are failing because they pass the exact ID of the base library (which varies per GHC version under test), and that shows up in the output. Not sure how to work around that with the expect tests.

@jasagredo
Copy link
Collaborator

You should probably work out how to identify those and replace them with a common string in OutputNormalizer.hs

@9999years 9999years marked this pull request as draft September 17, 2024 20:14
@9999years 9999years force-pushed the missing-or-private-details branch 8 times, most recently from 063b848 to dd47021 Compare September 23, 2024 23:38
@9999years
Copy link
Collaborator Author

@jasagredo Fixed the tests and added a new one to demonstrate the error messages I showed off in the PR description, PTAL!

@9999years 9999years marked this pull request as ready for review September 24, 2024 00:06
Copy link
Collaborator

@jasagredo jasagredo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good in general 👍. I am not very knowledgeable in dependency resolution, so I cannot tell if there are other places that would need changes, but the changes that were done here look correct. Basically propagating some more information.

I added some comments, in particular I would merge those 3 new modules into Dependency.hs, but that I guess is personal preference.

Cabal-syntax/src/Distribution/Types/LibraryName.hs Outdated Show resolved Hide resolved
Cabal-syntax/src/Distribution/Pretty.hs Show resolved Hide resolved
@jasagredo
Copy link
Collaborator

Aaaarg so sorry @9999years I accidentally pressed Update branch while scrolling. Let me try to re-push your previous version.

@jasagredo
Copy link
Collaborator

Restored 👍

@9999years 9999years added the merge me Tell Mergify Bot to merge label Sep 26, 2024
@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Sep 26, 2024
fgaz
fgaz previously requested changes Sep 26, 2024
Copy link
Member

@fgaz fgaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a changelog entry

@9999years 9999years requested a review from fgaz September 26, 2024 20:13
@9999years
Copy link
Collaborator Author

9999years commented Sep 26, 2024

@fgaz Added a changelog entry, PTAL.

CI failure doesn't look like it's related:

curl: (28) Failed to connect to codeberg.org port 443 after 134854 ms: Connection timed out

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 28, 2024
When `act-as-setup configure` fails, it doesn't indicate which
dependencies are missing entirely and which ones don't satisfy the
version constraints. The errors from `cabal build` are more informative,
but aren't available in all contexts (nixpkgs Haskell builds, for
example, use the `act-as-setup` interface). This makes it immediately
clear what sort of dependency error has occurred.

Before (cabal-install 3.12.1.0):

    Configuring test-pkg-0.1.0.0...
    Error: [Cabal-8010]
    Encountered missing or private dependencies:
        base <=4.18,
        foobar,
        test-pkg:{bar-internal, foo-internal}

After:

    Configuring test-pkg-0.1.0.0...
    Error: [Cabal-8010]
    Encountered missing or private dependencies:
        base <=4.18 (installed: 4.19.1.0),
        foobar (missing),
        test-pkg:{bar-internal,foo-internal} (missing: :bar-internal)
From @geekosaur on Matrix:

> since changelog-d uses the cabal file parser to parse changelog files,
> internal braces have to be escaped
@mergify mergify bot merged commit ba507b1 into haskell:master Oct 1, 2024
42 checks passed
@9999years 9999years deleted the missing-or-private-details branch October 1, 2024 19:26
@geekosaur
Copy link
Collaborator

This is an API change, so can't be backported to 3.12. Additionally, 3.10 is now frozen (largely because major merge conflicts will happen thanks to fourmolu) so no backports can be done to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants