Add builder conditional for Cabal version#11973
Closed
tfausak wants to merge 15 commits into
Closed
Conversation
Renames the spike's `build(cabal ...)` conditional to `builder(<tool> ...)` and generalizes it to carry the build tool, parallel to `impl(<compiler> ...)`. - New `Distribution.Types.BuildTool` module: `BuildTool = Cabal | MCabal | OtherBuildTool String`, mirroring `CompilerFlavor` (knownBuildTools, classifyBuildTool, Pretty/Parsec instances). Unknown tool names parse to `OtherBuildTool` rather than failing, so a `builder(...)` for a tool a given builder doesn't know about simply evaluates to False for it. - `ConfVar` constructor is now `Builder BuildTool VersionRange`. - Both condition parsers and the pretty-printer use the `builder` keyword. - All three evaluators (simplifyWithSysParams and the two solver paths in IndexConversion) match on the tool: only `Builder Cabal vr` checks against buildToolVersion; MCabal/OtherBuildTool evaluate to False under Cabal. See haskell#10386. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`parseConditionConfVar` now takes a `CabalSpecVersion` and emits a parse error when a `builder(...)` conditional is used with a `cabal-version` older than 3.18, mirroring how other syntactic features are spec-gated. - Package-description callers (`PackageDescription.Parsec`) thread the section parser's `specVer`. - cabal.project callers (`Client.ProjectConfig.Parsec`) pass `maxBound`: project files carry no `cabal-version`, so their conditionals are not spec-gated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The zlib Haskell package (a transitive dependency of cabal-install) configures via pkg-config, which was absent from the dev shell, so `cabal build cabal-install` failed at zlib's configure step. Add pkg-config as a tool and zlib as a build input (so pkg-config finds it on PKG_CONFIG_PATH). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two fixtures registered in Cabal-tests parser-tests: - regressions/builder-conditional.cabal: a cabal-version 3.18 package exercising builder(cabal ...), builder(mcabal ...), and an unknown builder(frobnicate ...) (parsing to OtherBuildTool). Covers the format pretty-print golden, the format round-trip, and the tree-diff expr golden. No new ToExpr instance is needed: ToExpr (Condition a) uses defaultExprViaShow and ConfVar/BuildTool derive Show. - errors/builder-spec-version.cabal: a cabal-version 3.16 package using builder(...), exercising the spec-version gating error. All 194 parser-tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- User guide: add a `builder({tool})` entry to the Conditions list in
doc/cabal-package-description-file.rst, alongside os/arch/impl/flag.
Covers the cabal/mcabal/unknown-tool semantics, the cabal-version 3.18
requirement, and the caveat that the conditional is evaluated against
the Cabal version that actually performs the build.
- changelog.d entry covering Cabal-syntax, Cabal, and cabal-install.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parity with the existing _OS, _Arch, _PackageFlag, and _Impl traversals in Distribution.Types.GenericPackageDescription.Lens, now that ConfVar has the Builder constructor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`cabal check` already warns when a conditional names an unknown OS,
arch, or compiler (UnknownOS/UnknownArch/UnknownCompiler). Add the
parallel UnknownBuildTool check so that a builder(<tool> ...) naming a
tool cabal doesn't recognise (an OtherBuildTool, which evaluates to
False) is surfaced rather than silently accepted.
Adds the UnknownBuildTool CheckExplanation, its CIUnknownBuildTool id
("unknown-build-tool"), the ppExplanation rendering, and the vcheck arm
matching Builder (OtherBuildTool _).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Builder constructor and new BuildTool type change the derived Structured encoding of GenericPackageDescription, bumping its pinned structure hash from fd3ceefd3ccba5b23fa3688aefb363a9 to 006484ba50ee652bb0c5e090fb229401. This bump is what invalidates cabal-install's on-disk caches: they are written via structuredEncodeFile/structuredDecodeFileOrFail, which prepend the structure hash as a magic header, so a stale cache fails to decode and is regenerated. Only GenericPackageDescription's hash changes; LocalBuildInfo (a flattened PackageDescription with no ConfVar) is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New unit-test module UnitTests.Distribution.PackageDescription.Configuration exercising how finalizePD evaluates the builder(...) conditional. Each case hand-builds a GenericPackageDescription with a conditional library whose two branches set distinct cpp-options markers, finalizes it, and checks which marker survives. Cases: version satisfied (cabal >= self), an omitted range defaulting to anyVersion, version not satisfied (cabal > self), a tool mismatch (mcabal), an unknown tool (frobnicate), and that the chosen branch is independent of the configured compiler. Version ranges are built from the exported buildToolVersion, so the tests don't depend on the exact Cabal version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generalize the modular-solver DSL's conditional channel from flag-only to any Condition ConfVar, and add an exBuilder/ExBuilder example dependency. This lets tests express dependencies and buildability guarded by a builder(<tool> <version range>) conditional. New "Builder conditional dependencies" group in Solver.hs exercises both IndexConversion code paths: - convBranch: a builder(...) condition statically picks the then/else dependencies (satisfied version -> then, unsatisfied version -> else, tool mismatch -> else). - testConditionForComponent: a builder(...) condition gating a component's buildability is statically evaluated (buildable accepted, unbuildable rejected). Version ranges are built from the exported buildToolVersion so the tests don't depend on the exact Cabal version. The unknown-tool (OtherBuildTool) case is intentionally not a solver test: the DSL validates packages with cabal check, and the UnknownBuildTool warning rejects such a package before solving; that convBranch arm is identical to the tool-mismatch arm and is covered by the parser and finalizePD tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PackageTests/BuilderConditional is a cabal-version: 3.18 library whose Lib.hs has CPP #error guards that only compile if cabal evaluated the builder(...) conditionals correctly: builder(cabal >= 1.0) true, builder(cabal >= 999999) false, and builder(mcabal) false. A successful v2-build is the assertion (regression test for haskell#10386). The test is guarded with skipUnlessAnyCabalVersion ">= 3.18": a build-type: Simple package with cabal-version 3.18 needs a Cabal library that both implements builder(...) and satisfies the implicit 3.18 setup dependency. That library doesn't exist until Cabal itself is bumped to 3.18 (it is 3.17.0.0 in tree, matching cabalSpecLatest = 3.16), so the test skips cleanly until then and runs once Cabal is 3.18. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two code-review follow-ups, both in the buildToolVersion Haddock comment: - Correct the stale spike syntax `build(cabal ...)` to `builder(cabal ...)`. - Document that the bootstrap fallback (digits of cabalSpecLatest) can be older than the spec version gating builder(...), so during a bootstrap build a `builder(cabal >= ...)` guard may evaluate against a version older than the Cabal actually performing the build. This resolves once cabalSpecLatest reaches that spec version. The fallback formula is left as is by design (Cabal-syntax and cabalSpecLatest move in lockstep). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
I opened this mostly to show a rough level of effort for implementing this feature. Obviously this branch would need quite a bit of work to be ready for review. But hopefully it's useful to show that adding |
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
1 participant
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.
This is a vibe coded proof of concept that fixes #10386.
Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR:
significance: significantin the changelog file.