Repository navigation
stack: prefer a committed .cabal over re-running hpack (#626, #767) - #2547
Open
hamishmack wants to merge 9 commits into
Open
hamishmack wants to merge 9 commits into
hamishmack wants to merge 9 commits into
Conversation
In the stack path, findCabalFiles UsePackageYamlFirst always re-rendered a package.yaml through hpack whenever it existed, even when the package directory also contained a committed .cabal file. This discarded the committed file and historically failed outright when the .cabal had been hand-modified or was produced by a newer hpack. The cabalProject path already prefers a committed .cabal (call-cabal-project-to-nix.nix skips hpack when a .cabal exists). Mirror that in the stack path: use the on-disk .cabal when present, falling back to hpack only when there is no .cabal. --ignore-package-yaml continues to force cabal-only discovery. Adds a self-contained (no external tools/network) test-suite stack-project-tests exercising findCabalFiles.
…tack-prefer-cabal
`StackProjectTests` imports `Cabal2Nix` for the `CabalFile` constructors, which
lives in the `cabal2nix` internal library rather than the main `nix-tools`
library, so the test suite failed to compile with
Could not load module 'Cabal2Nix'
It is a member of the hidden package 'nix-tools-0.1.0.0:cabal2nix'.
Add `nix-tools:cabal2nix` to its build-depends.
…tack-prefer-cabal # Conflicts: # nix-tools/nix-tools/nix-tools.cabal
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues in the focused discovery change or test integration; compilation and test execution remain unverified.
No open issues 🎉
What changed in this PR
Updates Stack package discovery to prefer an existing .cabal file over regenerating it with hpack, aligning with the Cabal path and addressing #626 and #767.
Changes:
- Uses hpack only when no
.cabalfile exists, preservingIgnorePackageYamlbehavior. - Adds and registers four self-contained tests covering file preference and fallback behavior.
- Delivery to haskell.nix builds remains gated on a nix-tools release.
| File | Description |
|---|---|
nix-tools/nix-tools/tests/StackProjectTests.hs |
Adds regression tests for discovery and hpack fallback. |
nix-tools/nix-tools/nix-tools.cabal |
Registers the stack-project-tests suite. |
nix-tools/nix-tools/lib/Stack2nix/Project.hs |
Prefers on-disk .cabal files before invoking hpack. |
Review effort: Balanced
…tack-prefer-cabal # Conflicts: # nix-tools/nix-tools/nix-tools.cabal
plan-to-nix kept its own copy of findCabalFiles, which still rendered package.yaml through hpack whenever one existed. For a cabalProject with both files cabal-install planned against the committed .cabal, but the generated .plan.nix came from hpack and was marked cabal-generator = "hpack", so comp-builder re-ran hpack at build time and could hit the same "modified manually" failure as #767. Drop Plan2Nix.Project and use Stack2nix.Project.findCabalFiles, so both tools take the committed .cabal (even a stale one) and only fall back to hpack when there is no .cabal. The same change is applied to the nix-tools-sh copy, whose files were identical.
…tack-prefer-cabal
…tack-prefer-cabal
…tack-prefer-cabal
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
findCabalFiles UsePackageYamlFirstre-rendered apackage.yamlthrough hpack whenever it existed, even when the package directory also had a committed.cabalfile. That discarded the committed file and failed outright with ".cabal was generated with a newer version of hpack, please upgrade" (#626) or ".cabal was modified manually, please use --force" (#767).It now uses the committed
.cabalwhen one exists and falls back to hpack only when there is no.cabal.--ignore-package-yaml(IgnorePackageYaml) still forces cabal-only discovery.This applies to:
lib/Stack2nix/Project.hs).findCabalFilesthat still renderedpackage.yaml. On the Nix side,call-cabal-project-to-nix.nixonly skips the shell hpack run when a.cabalexists, so cabal-install planned against the committed.cabalwhile plan-to-nix generated.plan.nixfrom hpack output and marked itcabal-generator = "hpack".comp-builder.nixthen re-ran hpack at build time and could hit the same .cabal was modified manually, please use --force to overwrite. #767 error.Plan2Nix.Projectis removed and plan-to-nix usesStack2nix.Project.findCabalFiles; with an on-disk.cabalnocabal-generatoris emitted, so the build-time hpack run is skipped.Behaviour changes to be aware of
.cabalwins. Ifpackage.yamlwas edited without regenerating the.cabal, haskell.nix uses the committed.cabal. Stack itself would regenerate it (unless the.cabalwas hand-modified or made by a newer hpack). This is deliberate: it matches what cabal-install sees, avoids running hpack, and the fix is to commit the regenerated.cabal. No staleness warning: hpack's-- hash:header hashes the.cabalbody (to detect hand edits), notpackage.yaml, so detecting staleness would mean rendering everypackage.yamlthrough hpack, which is the cost and failure mode this change removes..cabalfiles plus apackage.yaml: all the.cabalfiles are returned (as with nopackage.yaml); cabal-name takes the first.Tests
Self-contained test suite
stack-project-tests(temp dir only, no external tools or network) asserting thatfindCabalFiles:.cabalwhen both files exist (the Do not call hpack if .cabal exists #626/.cabal was modified manually, please use --force to overwrite. #767 regression),package.yamlexists,.cabalwhen only a.cabalexists,package.yamlunderIgnorePackageYaml.plan-to-nix now calls the same function, so these cover it too.
Verification
stack-project-testswith nixpkgs GHC 9.10 + Cabal 3.16: all pass.plan2nix/Main.hs) for nix-tools, and for nix-tools-sh against nix-tools'cabal2nixlibrary (the -sh one needs the stable-haskell Cabal fork). No new warnings.check-nix-tools).Notes
--use-package-yamlopt-out (stack's regenerate-always behaviour) was considered and skipped; easy to add later if wanted.Closes #626.
Closes #767.