Skip to content

Fix #12473 cabal-install supports SetupHooks.lhs (literate Haskell) - #12475

Merged
mergify[bot] merged 3 commits into
masterfrom
fix12473
Oct 26, 2026
Merged

mergify[bot] merged 3 commits into
masterfrom
fix12473

Conversation

@mpilgrem

@mpilgrem mpilgrem commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:


See:

EDIT: Also warns if both Haskell and literate Haskell files are present and the latter is being ignored, both for Setup and SetupHooks (being the existing behaviour for Setup).

The change to the code follows the pattern of the lines immediately above.

The change to the documentation now distinguishes between a module and its source file.

@ulysses4ever ulysses4ever left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for picking this up! The docs changes are a clear improvement — separating the SetupHooks module from its source file reads much better.

The code change needs one more step, though: as written it finds SetupHooks.lhs but then copies it verbatim to dist-newstyle/.../setup/SetupHooks.hs, so GHC will try to parse the literate source as plain Haskell when it builds the Setup and hooks executables. The Custom branch just above avoids this by running ppUnlit on the .lhs source — we need the same here. Details and a suggestion inline.

The new test also can't see this, since cabal check doesn't go through updateSetupScript (and already accepted SetupHooks.lhs before this PR). A build-based test would catch both the original bug and the unlit issue; see inline.

Comment thread cabal-install/src/Distribution/Client/SetupWrapper.hs Outdated
Comment thread Cabal-hooks/README.md Outdated
Comment thread changelog.d/pr-12475.md
Comment thread cabal-testsuite/PackageTests/Check/SetupHooksLhs/cabal.test.hs Outdated
@mpilgrem
mpilgrem force-pushed the fix12473 branch 3 times, most recently from 8a89b59 to b7eeb5d Compare October 22, 2026 22:51

@ulysses4ever ulysses4ever left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great! Thanks for the quick turnaround!

Comment thread cabal-install/src/Distribution/Client/SetupWrapper.hs Outdated
@mpilgrem
mpilgrem force-pushed the fix12473 branch 2 times, most recently from 4610d64 to 8a12442 Compare October 24, 2026 17:42
Also expresses differently the existing algorithm.
@mpilgrem mpilgrem added the merge me Tell Mergify Bot to merge label Oct 24, 2026
@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 queued labels Oct 24, 2026
@mergify

mergify Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-10-26 20:29 UTC · Rule: default · triggered by rule Put pull requests in the rebase+merge queue
  • ✅ Checks passed · on draft merge queue: checking #12475 on master (64c2cc0) #12493
  • ✅ Merged — 2026-10-26 22:35 UTC · at bc1f17542cb07a9b7bebeedfebf085a7e2d06245 · merge

This pull request spent 2 hours 5 minutes 56 seconds in the queue, including 1 hour 55 minutes 13 seconds running CI.

Required conditions to merge
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-approved [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Doctest Cabal
    • check-neutral = Doctest Cabal
    • check-skipped = Doctest Cabal
  • any of [🛡 GitHub branch protection]:
    • check-success = Meta checks
    • check-neutral = Meta checks
    • check-skipped = Meta checks
  • any of [🛡 GitHub branch protection]:
    • check-success = docs/readthedocs.org:cabal
    • check-neutral = docs/readthedocs.org:cabal
    • check-skipped = docs/readthedocs.org:cabal
  • any of [🛡 GitHub branch protection]:
    • check-success = Validate post job
    • check-neutral = Validate post job
    • check-skipped = Validate post job
  • any of [🛡 GitHub branch protection]:
    • check-success = fourmolu
    • check-neutral = fourmolu
    • check-skipped = fourmolu
  • any of [🛡 GitHub branch protection]:
    • check-success = hlint
    • check-neutral = hlint
    • check-skipped = hlint
  • any of [🛡 GitHub branch protection]:
    • check-success = Bootstrap post job
    • check-neutral = Bootstrap post job
    • check-skipped = Bootstrap post job
  • any of [🛡 GitHub branch protection]:
    • check-success = whitespace
    • check-neutral = whitespace
    • check-skipped = whitespace
  • any of [🛡 GitHub branch protection]:
    • check-success = Check sdist post job
    • check-neutral = Check sdist post job
    • check-skipped = Check sdist post job
  • any of [🛡 GitHub branch protection]:
    • check-success = Changelogs
    • check-neutral = Changelogs
    • check-skipped = Changelogs

@mergify mergify Bot mentioned this pull request Oct 26, 2026
44 of 84 tasks
@mergify
mergify Bot merged commit bc1f175 into master Oct 26, 2026
72 checks passed
@mergify
mergify Bot deleted the fix12473 branch October 26, 2026 22:35
@mergify mergify Bot removed the queued label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

attention: needs-review Cabal: hooks 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.

3 participants