Skip to content

cabal update: don't show comment with builddir in the default config #11067

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
Jul 13, 2025

Conversation

ulysses4ever
Copy link
Collaborator

@ulysses4ever ulysses4ever commented Jul 11, 2025

When calling cabal update with an empty state (empty CABAL_DIR or new machine), cabal writes out a default config file with many fields commented out to show their defaults. This suggests that the user can uncomment the field and override the default. This is wrong for fields that aren't allowed in the config file like builddir. This patch avoids generating a comment with builddir in the default config.

Fix #11050.

Manual QA Notes

cabal build cabalc=$(cabal list-bin cabal)CABAL_DIR=$PWD/cabal-dir-testing $c updategrep builddir ./cabal-dir-testing/config

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

@ulysses4ever ulysses4ever added cabal-install: cmd/update re: config-file Concerning the cabal configuration file and the `--config-file` option labels Jul 11, 2025
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.

(assuming the next push fixes the comment)

@geekosaur
Copy link
Collaborator

Oh, whoops, there's a test that checks these (which IIRC you already pointed out to someone else?).

@ulysses4ever
Copy link
Collaborator Author

does anyone get why CI fails?

@geekosaur
Copy link
Collaborator

@ulysses4ever ulysses4ever force-pushed the dont-mention-builddir-in-default-config-t11050 branch from 5847556 to 8301c1e Compare July 11, 2025 15:55
@ulysses4ever ulysses4ever force-pushed the dont-mention-builddir-in-default-config-t11050 branch 2 times, most recently from 2c86641 to ad6dedf Compare July 11, 2025 16:55
@ulysses4ever
Copy link
Collaborator Author

Added changelog and manual QA notes in the PR description.

@ulysses4ever ulysses4ever marked this pull request as ready for review July 11, 2025 16:56
@ulysses4ever ulysses4ever added the merge me Tell Mergify Bot to merge label Jul 11, 2025
@ulysses4ever ulysses4ever force-pushed the dont-mention-builddir-in-default-config-t11050 branch 2 times, most recently from e65b0b5 to 0fdb5b1 Compare July 11, 2025 16:59
When calling `cabal update` with an empty state (empty CABAL_DIR or new
machine), cabal writes out a default config file with many fields
commented out to show their defaults. This suggests that the user can
uncomment the field and override the default. This is wrong for fields
that aren't allowed in the config file like `builddir`. This patch
avoids generating a comment with `builddir` in the default config.

Fix #11050.
@ulysses4ever ulysses4ever force-pushed the dont-mention-builddir-in-default-config-t11050 branch from 0fdb5b1 to a49146a Compare July 11, 2025 17:01
@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 Jul 11, 2025
mergify bot added a commit that referenced this pull request Jul 13, 2025
@mergify mergify bot merged commit 9a343d1 into master Jul 13, 2025
55 checks passed
@mergify mergify bot deleted the dont-mention-builddir-in-default-config-t11050 branch July 13, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cabal-install: cmd/update merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge re: config-file Concerning the cabal configuration file and the `--config-file` option ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cabal incorrectly adds -- builddir: to default configuration file
3 participants