Add per-file options to extra source files - #12288
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends Cabal’s .cabal file format and internal APIs to support per-file options on the five “extra source” fields (c-sources, cxx-sources, asm-sources, cmm-sources, js-sources) gated behind cabal-version: 3.20, enabling per-source compilation flags (e.g. foo.c (-DFOO)).
Changes:
- Introduces
ExtraSource(path + per-file opts) and updates parsing/pretty-printing/field grammars so the five extra-source fields carry per-file options (guarded atcabal-version >= 3.20). - Wires per-file options into GHC invocation by routing options into the appropriate
GhcOptionsfields (including version-gating-optJSPto GHC ≥ 9.12). - Adds/updates documentation, changelog entries, and tests (parser regression + error test, QuickCheck roundtrip, rendering unit tests, and a setup testsuite case).
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| doc/file-format-changelog.rst | Documents the 3.20 format change for per-file extra-source options. |
| doc/cabal-package-description-file.rst | Adds user-facing documentation for per-file options and updates field docs. |
| changelog.d/per-file-extra-source-options.md | Changelog entry describing the new per-file options feature and API change. |
| changelog.d/jspp-options-ghc-912.md | Changelog entry for omitting -optJSP on GHC < 9.12. |
| changelog.d/cabal-spec-latest-3-20.md | Changelog entry for making 3.20 the latest known spec (PR #12275). |
| Cabal/src/Distribution/Simple/SrcDist.hs | Ensures sdist source collection includes ExtraSource file paths. |
| Cabal/src/Distribution/Simple/SetupHooks/Internal.hs | Updates autogen extra-source handling to extract ExtraSource file paths. |
| Cabal/src/Distribution/Simple/Program/GHC.hs | Gates -optJSP rendering to GHC ≥ 9.12 and updates related documentation. |
| Cabal/src/Distribution/Simple/GHCJS.hs | Adapts GHCJS build/link handling to ExtraSource-typed extra sources. |
| Cabal/src/Distribution/Simple/GHC/Internal.hs | Adds ExtraSourceKind + routing of per-file opts into correct GhcOptions. |
| Cabal/src/Distribution/Simple/GHC/Build/ExtraSources.hs | Switches extra-source builds to ExtraSource, applies opts per file, warns when JS opts are dropped (< 9.12). |
| Cabal/src/Distribution/Simple/BuildTarget.hs | Updates component file listing to extract file paths from ExtraSource. |
| Cabal/src/Distribution/Simple/Build.hs | Updates build-info augmentation helpers to wrap paths with extraSourceFromPath. |
| Cabal/src/Distribution/PackageDescription/Check/Target.hs | Updates path well-formedness checks to validate extraSourceFile. |
| Cabal-tree-diff/src/Data/TreeDiff/Instances/Cabal.hs | Adds ToExpr ExtraSource instance for tree-diff output. |
| cabal-testsuite/PackageTests/SetupHooks/SetupHooksNonHs/SetupHooks.hs | Updates setup hooks test to supply ExtraSource values. |
| cabal-testsuite/PackageTests/ExtraSources/src/MyLib.hs | Adds a testsuite library module for the new extra-sources setup test. |
| cabal-testsuite/PackageTests/ExtraSources/setup.test.hs | Adds a setup-based integration test for compiling extra sources. |
| cabal-testsuite/PackageTests/ExtraSources/setup.out | Expected output for the ExtraSources setup test. |
| cabal-testsuite/PackageTests/ExtraSources/extra-sources.cabal | Test package exercising per-file options in c-sources / cmm-sources. |
| cabal-testsuite/PackageTests/ExtraSources/cbits/testcmm.cmm | Test C-- file requiring a macro defined via per-file options. |
| cabal-testsuite/PackageTests/ExtraSources/cbits/test.c | Test C file requiring a macro defined via per-file options. |
| Cabal-tests/tests/UnitTests/Distribution/Utils/Structured.hs | Updates structured-hash golden values after type/API changes. |
| Cabal-tests/tests/UnitTests/Distribution/Types/ExtraSource.hs | Adds QuickCheck roundtrip property for ExtraSource parse/pretty. |
| Cabal-tests/tests/UnitTests/Distribution/Simple/Program/GHC.hs | Adds unit tests for -optJSP gating and C-- option rendering behavior. |
| Cabal-tests/tests/UnitTests.hs | Registers new ExtraSource unit tests. |
| Cabal-tests/tests/ParserTests/regressions/libpq2.expr | Updates expected parse AST output for cSources now being ExtraSource. |
| Cabal-tests/tests/ParserTests/regressions/libpq1.expr | Updates expected parse AST output for cSources now being ExtraSource. |
| Cabal-tests/tests/ParserTests/regressions/extra-source-opts.format | Adds formatting regression case for per-file options across all five kinds. |
| Cabal-tests/tests/ParserTests/regressions/extra-source-opts.expr | Adds AST expectation for the per-file extra-source options regression test. |
| Cabal-tests/tests/ParserTests/regressions/extra-source-opts.cabal | Adds .cabal input exercising per-file options parsing/printing. |
| Cabal-tests/tests/ParserTests/errors/extra-source-opts-old-spec.errors | Adds expected error output when using per-file opts below spec 3.20. |
| Cabal-tests/tests/ParserTests/errors/extra-source-opts-old-spec.cabal | Adds error fixture using per-file opts with cabal-version: 3.4. |
| Cabal-tests/tests/ParserTests.hs | Registers new parser regression and error tests. |
| Cabal-tests/tests/NoThunks.hs | Adds NoThunks ExtraSource instance for no-thunks testing. |
| Cabal-tests/Cabal-tests.cabal | Adds new unit test module to the Cabal-tests test-suite stanza. |
| Cabal-syntax/src/Distribution/Types/ExtraSource.hs | Introduces the ExtraSource type plus parsing/pretty-printing and version guard. |
| Cabal-syntax/src/Distribution/Types/BuildInfo/Lens.hs | Updates build-info lenses to expose extra-source fields as [ExtraSource]. |
| Cabal-syntax/src/Distribution/Types/BuildInfo.hs | Updates BuildInfo extra-source fields to [ExtraSource] with clarified comments. |
| Cabal-syntax/src/Distribution/SPDX/LicenseListVersion.hs | Maps CabalSpecV3_20 to SPDX license list 3.28. |
| Cabal-syntax/src/Distribution/PackageDescription/FieldGrammar.hs | Updates field grammars to parse/format extra-source fields as ExtraSource. |
| Cabal-syntax/src/Distribution/PackageDescription.hs | Re-exports Distribution.Types.ExtraSource from Distribution.PackageDescription. |
| Cabal-syntax/src/Distribution/CabalSpecVersion.hs | Sets cabalSpecLatest to CabalSpecV3_20. |
| Cabal-syntax/Cabal-syntax.cabal | Exposes the new Distribution.Types.ExtraSource module. |
| Cabal-QuickCheck/src/Test/QuickCheck/Instances/Cabal.hs | Adds Arbitrary ExtraSource generator/shrinker. |
| cabal-install/tests/IntegrationTests2.hs | Updates integration test fixture construction for cSources :: [ExtraSource]. |
| cabal-install/src/Distribution/Client/TargetSelector.hs | Updates known-component file collection to extract extraSourceFile. |
| cabal-install/src/Distribution/Client/SourceFiles.hs | Updates source file existence tracking to extract extraSourceFile. |
| Cabal-hooks/src/Distribution/Simple/SetupHooks.hs | Re-exports ExtraSource and extraSourceFromPath for hooks packages. |
| Cabal-described/src/Distribution/Described.hs | Adds Described ExtraSource and updates regex charsets for options syntax. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
f43c31e to
15df283
Compare
15df283 to
3dd6d91
Compare
|
Thank you for the review @zlonast |
CabalSpecV3_18 and CabalSpecV3_20 are both known to the development tree, but cabalSpecLatest remained CabalSpecV3_16. cabal-install consequently treated Simple packages that declare cabal-version 3.18 as future-format packages and selected a broken external setup path. Move the supported-version boundary to CabalSpecV3_20. Also map that spec version to SPDX license list 3.28 so default SPDX parsing does not fall through to license list 3.0. Fixes haskell#12271
The `jspp-options` field is rendered as `ghc -optJSP<opt>`, but that flag only exists since GHC 9.12. Passing it to an older GHC makes the invocation fail outright, so a package using `jspp-options` could not be built at all with GHC < 9.12. Gate the flag on the compiler version instead.
3dd6d91 to
6ffe40d
Compare
|
I don't like the current syntax, so I'll suggest two other options that I think fit better into the ecosystem design. Option A — file as a section with a nested field library
c-source: cbits/foo.c
c-options: -DFOO -O2
c-source: cbits/bar.cOption B — options as a separate map field library
c-sources:
cbits/foo.c
cbits/bar.c
c-options:
cbits/foo.c: -DFOO -O2 |
The extra-source fields (c-sources, cxx-sources, asm-sources, cmm-sources,
js-sources) now accept per-file options, in parentheses after the file name:
c-sources: foo.c (-DFOO -O2) bar.c
cmm-sources: Jumps_V32.cmm (-mavx2)
GHC gets these options only when it compiles that one file.
'ExtraSourceKind' names the kind of a source, and 'addExtraSourceOpts' maps
that kind to the one 'GhcOptions' field GHC reads for it:
c-sources CSourceKind ghcOptCcOptions -optc
cxx-sources CxxSourceKind ghcOptCxxOptions -optcxx
asm-sources AsmSourceKind ghcOptAsmOptions -opta
cmm-sources CmmSourceKind ghcOptExtra plain GHC options
js-sources JsSourceKind ghcOptJSppOptions -optJSP
Cabal already compiles each extra source with its own 'ghc -c' run, so
per-file scoping only means appending the options to that run's
component-wide ones. They come last, so they win over 'cc-options' and its
siblings.
Only C-- options are plain GHC options, because GHC compiles C-- itself and
there is no -optcmm. The other four kinds are options for a tool GHC
invokes, so they cannot go in 'ghcOptExtra': 'ghc -DFOO -c foo.c'
defines a Haskell CPP macro instead of passing -DFOO to the C compiler.
Keeping the kind, rather than an already-prefixed string, also leaves the
spelling to Cabal at render time, which it has to be, because the prefix
depends on the compiler version. GHC added -optcxx in 8.10, so below that
C++ options go through -optc and 'splitCandCxxOptions' clears the C options
to keep the two languages apart; GHC added -optJSP in 9.12, so below that the
options are dropped and Cabal warns. One shared field would also duplicate
C-- options, since GHC passes -optc flags to the C-- C pre-processor too.
An older Cabal does not reject this syntax but it misreads it, so
'cabal-version: 3.20' gates the syntax as an error and not a warning.
The five fields hold 'ExtraSource' (a path plus its options) instead of a
bare 'SymbolicPath'; 'extraSourceFromPath' makes one with no options.
'Distribution.Simple.SetupHooks' re-exports both, so a 'Hooks' package that
sets 'cSources' needs no Cabal-syntax dependency.
6ffe40d to
13dbe08
Compare
|
I don't think either of those can work, but the objection did make me change the syntax — just not to A or B. A isn't expressible. There are no nested fields in B parses, but it splits something that is currently one value. An library
c-sources: cbits/foo.c
if os(windows)
c-sources: cbits/win.c
c-options: cbits/foo.c: -DWINBoth are What I did change: the text between the parentheses is now taken verbatim and split by c-sources:
cbits/wide.c ("-DMESSAGE=hello there")
cbits/size.c (-DSIZE=f(1))
cbits/smiley.c (-DSMILEY=\))That also killed a bug: |
|
The
We already have nested blocks, and even if there were no nested blocks we we could still just add them anyway if everyone agrees this is the best way to do it. |
|
I think this change requires buy-in from the community and the Cabal maintainers before we can proceed to technical review. I say this for two main reasons:
The Cabal proposal repository can be found here. |
Adds per-file options to the five extra-source fields
This is one of the last things the Cabal library needs before it can build GHC; Hadrian makes the same per-file split today. This patch has been developed for https://github.com/stable-haskell/ghc.
The options are applied only to the relative file.
Based on #12275 which needs to go in first.
Design
ExtraSourceKindtags each source with its kind, andaddExtraSourceOptsmaps that kind to the oneGhcOptionsfield GHC consults for it:GhcOptionsfieldc-sourcesghcOptCcOptions-optccxx-sourcesghcOptCxxOptions-optcxxasm-sourcesghcOptAsmOptions-optacmm-sourcesghcOptExtrajs-sourcesghcOptJSppOptions-optJSPOnly C-- options are plain GHC options, because GHC compiles C-- itself and there is no
-optcmm. The other four are options for a tool GHC invokes, so they cannot shareghcOptExtra:ghc -DFOO -c foo.cdefines a Haskell CPP macro instead of passing-DFOOto the C compiler.Keeping the kind — rather than a string the user has already prefixed — leaves the spelling to Cabal at render time, which it has to be, because the prefix depends on the compiler version:
-optcxxonly exists since GHC 8.10 (below that, C++ options go through-optc),-optJSPonly since 9.12 (below that they are dropped, with a warning).Each extra source is already compiled by its own
ghc -crun, so per-file options are just appended to that run's component-wide ones. They come last, so they win overcc-optionsand its siblings.Interface change
The five fields hold
ExtraSource(a path plus its options) instead ofSymbolicPath Pkg File.extraSourceFromPathbuilds one with no options.Distribution.Simple.SetupHooksre-exports both, so aHookspackage that setscSourcesneeds no directCabal-syntaxdependency.Why the
cabal-versiongate is an error, and why not 3.18Gated behind
cabal-version: 3.20per #9331. Worth flagging explicitly: an older Cabal does not reject this syntax, it misreads it. A path is any non-space token, so Cabal-syntax 3.14.1.0 readsc-sources: foo.c (-DFOO -O2) bar.cas four source files —
foo.c,(-DFOO,-O2),bar.c— with no diagnostic, failing only later when it tries to compile(-DFOO. That rules out a warning, and it rules out the already-published 3.18 as the gate.Tests
ExtraSource, with a generator that produces options needing quotes.renderGhcOptionsunit tests:-optJSPrendered at 9.12 and dropped below it; C-- options plain at every version.ExtraSourcessetup test with a C source that only compiles if its per-file-Darrives.QA Notes
With
cabal-version: 3.20:#errors unless a macro is defined, and define it per-file:c-sources: greet.c (-DGREETING=hi) other.c. It should build, and-DGREETINGmust not reachother.c.c-sources: greet.c ("-DGREETING=\"hi there\""). Unquoted, it should be read as two options.cabal-version: 3.18should fail with a parse error naming 3.20 — not a missing-file error about(-DGREETING=hi.cabal sdiston a package using per-file options should ship the sources and round-trip the options through the regenerated.cabal.js-sources: foo.js (-DBAR)should warn that the options are ignored and still build;jspp-options: -DBARshould now build rather than failing with an unrecognised-flag error from GHC.Checklist
Template A: this PR modifies behaviour or interface.
significance: significantfor the feature).doc/cabal-package-description-file.rst,doc/file-format-changelog.rst).