Skip to content

Fix multi-repl when only building some internal library targets #10841

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

Merged
merged 1 commit into from
Mar 30, 2025

Conversation

mpickering
Copy link
Collaborator

When combining together --dependency and --promised-dependency flags, we were using Map.union in the wrong place. If you had a dependency and promised-dependency from the same package (ie when using an internal library) then the promised dependency wouldn't be taken into account.

The fix is straightforward, don't use Map.union. First create a list of everything and then create a map using fromListWith.

Fixes #10775

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:


Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ulysses4ever
Copy link
Collaborator

@mpickering don't hesitate to put the needs-review label when you open a PR that you think is ready for review. (I know this may look a little excessive but pragmatically the label sends a Matrix notification that will make the PR more visible.)

Copy link
Collaborator

@MangoIV MangoIV left a comment

Choose a reason for hiding this comment

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

I don't have any say but here's my review anyway: looks good to me!

@Mikolaj
Copy link
Member

Mikolaj commented Mar 25, 2025

@MangoIV: thank you for the review. Please kindly accept the invite to the cabal, I've just sent you.

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

LGTM

@Mikolaj
Copy link
Member

Mikolaj commented Mar 25, 2025

@mpickering: would you like to submit this for merging once you made the final tweaks?

@Mikolaj
Copy link
Member

Mikolaj commented Mar 28, 2025

@mpickering: a humble ping?

@mpickering mpickering added the merge me Tell Mergify Bot to merge label Mar 28, 2025
@mergify mergify bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Mar 28, 2025
When combining together --dependency and --promised-dependency flags, we
were using `Map.union` in the wrong place. If you had a dependency
and promised-dependency from the same package (ie when using an internal
library) then the promised dependency wouldn't be taken into account.

The fix is straightforward, don't use `Map.union`. First create a list
of everything and then create a map using `fromListWith`.

Fixes #10775
@mergify mergify bot merged commit c0d52b2 into master Mar 30, 2025
54 checks passed
@mergify mergify bot deleted the wip/10775 branch March 30, 2025 12:39
@Mikolaj
Copy link
Member

Mikolaj commented Mar 31, 2025

@mergify backport 3.14

Copy link
Contributor

mergify bot commented Mar 31, 2025

backport 3.14

✅ Backports have been created

mergify bot added a commit that referenced this pull request Mar 31, 2025
Backport #10841: Fix multi-repl when only building some internal library targets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-review 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.

"Error: Dependency on unbuildable library" with three internal libraries and --enable-multi-repl
4 participants