Skip to content

Use proper GHC option handling for passing multi-repl flags #10995

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
Jun 26, 2025

Conversation

mpickering
Copy link
Collaborator

This refactoring modifies the logic to start the multi-repl to use runGHCWithResponseFile rather than directly constructing a command line.

Towards #10881

Please read Github PR Conventions and then fill in one of these two templates.


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).

@mpickering mpickering force-pushed the wip/refactor-multi-lib-options branch from bc7fc99 to f6c0a3a Compare June 18, 2025 08:25
Copy link
Collaborator

@alt-romes alt-romes left a comment

Choose a reason for hiding this comment

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

Looks good!

@mpickering mpickering added merge me Tell Mergify Bot to merge and removed attention: needs-review labels Jun 23, 2025
@mpickering
Copy link
Collaborator Author

Thanks for the review Brandon and Rodrigo.

@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Jun 23, 2025
@ulysses4ever ulysses4ever force-pushed the wip/refactor-multi-lib-options branch 2 times, most recently from fd0e0f8 to 3c92519 Compare June 25, 2025 21:06
@ulysses4ever
Copy link
Collaborator

@geekosaur
Copy link
Collaborator

Yes, and I noticed a few minutes ago that cabal-plan here showed the old version of splitmix until I ran cabal update and cabal build all --dry-run locally. I think the new broken-on-Macs version got uploaded very recently (within the past week, and potentially today).

@geekosaur
Copy link
Collaborator

Anyway if you rebase on master it should work. But: it's a transitive dependency, and this breakage looks to be a PVP violation so slipped past everyone.

The good news is, it's not part of any of the released packages, it's part of the tests.

@ulysses4ever
Copy link
Collaborator

@geekosaur we have the index-state pinned (your PR that bumps it and runs into the splitmix issue isn't merged yet). So, how would you explain the failure here — with the old index-state?

@geekosaur
Copy link
Collaborator

geekosaur commented Jun 26, 2025

cabal.validate.project isn't pinned, so it picked up the new version of splitmix as soon as Hackage was updated. Only the bootstrap and release project files are pinned.

@ulysses4ever
Copy link
Collaborator

Oh, my bad. I'm curious now what we were thinking when decided to pin one but not the other...

@geekosaur
Copy link
Collaborator

The idea was that we would find out about transitive deps that broke things before we moved the pin as part of a release. Which fails when someone breaks us while we're making a release....

@geekosaur
Copy link
Collaborator

Speaking of which, I need to move the patchup to where it'll be caught by the other project files: I bet bootstrap on Macs is now broken.

This refactoring modifies the logic to start the multi-repl to use
runGHCWithResponseFile rather than directly constructing a command line.

Towards #10881
@mpickering mpickering force-pushed the wip/refactor-multi-lib-options branch from 3c92519 to 02b6d8c Compare June 26, 2025 08:50
@mpickering mpickering added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jun 26, 2025
@mergify mergify bot merged commit 25e87fd into master Jun 26, 2025
55 checks passed
@mergify mergify bot deleted the wip/refactor-multi-lib-options branch June 26, 2025 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants