Skip to content

cabal-install: stage-qualified package configuration (package build:*)#378

Open
andreabedini wants to merge 5 commits into
stable-haskell/masterfrom
wip/andrea/stage-qualified-options
Open

cabal-install: stage-qualified package configuration (package build:*)#378
andreabedini wants to merge 5 commits into
stable-haskell/masterfrom
wip/andrea/stage-qualified-options

Conversation

@andreabedini

Copy link
Copy Markdown
Member

Adds package <stage>:* project-file stanzas so per-package configuration can target a single build stage.

Commits

  • Cabal-syntax: allow colons in section header arguments — the field parser rejected : in a section argument, so a header such as package build:pkg was a parse error. Colon-joined argument tokens are now combined into a single SectionArg, letting stage-qualified package stanzas be written unquoted. This accepts strictly more input; all golden parser tests still pass.

  • cabal-install: stage-qualified package configuration (package build:*) — adds package build:* / package host:* stanzas. The same package can be built in both the build and host stages, so package *, top-level fields and package <name> cannot distinguish them; a stage qualifier can.

    • ProjectConfig gains projectConfigStagePackages :: MapMappend Stage PackageConfig (+ lens, parsec parser, field grammar). The legacy parser does not support the new syntax.
    • Per-package option lookup and the shared/profiling-lib downward-closed property in ProjectPlanning are made stage-aware, e.g. package build:* shared: False keeps the build stage static even when the host stage is built dynamic.
    • Adds a parser test (project-config-stage-packages).
  • fix: cover QualBase and drop redundant import for -Werror (validate) — latent -Wincomplete-patterns / -Wunused-imports warnings that -Werror promotes to errors only under validate; unrelated to the feature above.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 11, 2026 05:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support for stage-qualified per-package configuration in cabal.project (e.g. package build:* / package host:*) so options can be applied independently to build-stage vs host-stage packages during staged builds. This is enabled by extending the Cabal fields parser to accept colon-joined section arguments, and then plumbing stage-aware package config through cabal-install’s project parsing and planning.

Changes:

  • Cabal-syntax: allow : inside section header arguments by joining adjacent arg tokens with colons.
  • cabal-install: introduce projectConfigStagePackages and apply stage-qualified config during per-package option lookup and shared/profiling closure computation.
  • Tests + cleanup: add parser test coverage for stage-qualified stanzas and fix a few validate-only -Werror warnings.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Cabal-syntax/src/Distribution/Fields/Parser.hs Permits colon-joined section arguments (enables package build:* without quoting).
cabal-install/src/Distribution/Client/ProjectPlanning.hs Makes per-package option lookup and shared/profiling closure stage-aware.
cabal-install/src/Distribution/Client/ProjectConfig/Types.hs Adds projectConfigStagePackages :: MapMappend Stage PackageConfig.
cabal-install/src/Distribution/Client/ProjectConfig/Parsec.hs Parses package <stage>:* stanzas and stores config per stage.
cabal-install/src/Distribution/Client/ProjectConfig/Lens.hs Adds lens for projectConfigStagePackages.
cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs Initializes projectConfigStagePackages = mempty (legacy parser unsupported).
cabal-install/src/Distribution/Client/ProjectConfig/FieldGrammar.hs Extends ProjectConfig grammar construction with a default for stage packages.
cabal-install/parser-tests/Tests/ParserTests.hs Adds parsec-only helper and a test asserting stage-qualified stanza parsing.
cabal-install/parser-tests/Tests/files/project-config-stage-packages/cabal.project New golden test input for package build:* / package host:*.
cabal-install-solver/src/Distribution/Solver/Types/PackagePath.hs Covers QualBase in pretty-printing/qualifier display.
cabal-install-solver/src/Distribution/Solver/Types/PackageConstraint.hs Ensures constraint scope matching doesn’t accidentally match base-qualified deps.
cabal-install-solver/src/Distribution/Solver/Modular/Validate.hs Removes unused binding to satisfy -Werror under validate.
cabal-install-solver/src/Distribution/Solver/Modular/Linking.hs Removes unused state read to satisfy -Werror under validate.
cabal-install-solver/src/Distribution/Solver/Modular/Builder.hs Avoids unused pattern binding to satisfy -Werror under validate.
cabal-install-solver/src/Distribution/Solver/Modular.hs Drops unused verbosity imports to satisfy -Werror under validate.
Cabal-described/src/Distribution/Described.hs Drops unused verbosity import to satisfy -Werror under validate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 291 to 293
Nothing -> do
lift $ parseWarning pos PWTUnknownSection "target package name or * required"
return ()
Comment on lines 361 to 365
parseName secName = case runParsecParser parser "<parsePackageName>" (fieldLineStreamFromBS secName) of
Left _ -> do
parseFailure pos ("Invalid package name" ++ fromUTF8BS secName)
return Nothing
Right cfgTarget -> return $ pure cfgTarget
Comment on lines 2412 to +2416
-- This is where we merge the options from the project config that
-- apply to all packages, all project local packages, and to specific
-- named packages
global `mappend` local `mappend` perpkg
-- apply to all packages, all packages of a given stage, all project
-- local packages, and to specific named packages. Later (more specific)
-- entries override earlier ones.
global `mappend` local `mappend` stagecfg `mappend` perpkg
Comment on lines 44 to 51
<*> blurFieldGrammar L.projectConfigLocalPackages (packageConfigFieldGrammar knownPrograms)
-- \^ PackageConfig to be applied to locally built packages, specified not inside a stanza
<*> pure mempty
where
-- \^ PackageConfig applied to explicitly named packages
<*> pure mempty
where
-- \^ PackageConfig applied to all packages of a given stage ('package build:*' etc.)
provenance = Set.singleton (Explicit source)
andreabedini and others added 2 commits June 11, 2026 16:19
These warnings are promoted to errors under validate's -Werror but were
masked behind the earlier SolverInstallPlan failure. Clean them up so the
build proceeds:

- drop unused imports across Dependency, ProjectPlanning(.Types), InLibrary,
  SetupWrapper, ProjectBuilding.UnpackedPackage, CmdRepl and the InstallPlan
  and DSL unit-test modules
- bind the discarded result of runBuild explicitly (-Wunused-do-bind)
- underscore the unused indepGoals parameter in the solver DSL
- remove the dead internalSetupMethod, buildTypeAction and selfExecSetupMethod
  (unexported, unreferenced, not dispatched by runSetupMethod) along with the
  imports they alone used (Distribution.Make, withEnv/withEnvOverrides/
  withExtraPathEnv, System.Environment)
The field parser rejected ':' in a section argument, so a section header such as
`package build:pkg` was a parse error. Combine colon-joined argument tokens into
a single SectionArg, which lets stage-qualified package stanzas be written
unquoted. A colon in this position was previously always an error, so this
accepts strictly more input; all Cabal-tests golden parser tests still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines +647 to +648
, -- The legacy parser does not support stage-qualified package stanzas.
projectConfigStagePackages = mempty

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For the legacy configuration, the current parse would be qualified as host packages I believe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it should. I will make sure of it.

Comment on lines +1 to +6
package build:*
shared: False
executable-dynamic: False

package host:*
shared: True

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we test

package *
    ...

as well? I would assume that applies to build + host?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Isn't that in testProjectConfigStagePackages already?

  assertEqual
    "Unqualified 'package *' should apply to all packages, not a single stage"
    expectedAll
    (projectConfigAllPackages (snd (condTreeData config)))

@andreabedini

Copy link
Copy Markdown
Member Author

@angerman I think any unqualified name should be "host: + build:". Then host: will be only host and build: only build. Does it make sense to you?

andreabedini and others added 2 commits June 15, 2026 15:07
Add `package <stage>:*` project-file stanzas (e.g. `package build:*`, `package
host:*`) so per-package configuration can target a single build stage. The same
package can be built in both the build and host stages, so `package *`,
top-level fields and `package <name>` cannot distinguish them; a stage qualifier
can.

- ProjectConfig gains projectConfigStagePackages :: MapMappend Stage
  PackageConfig (+ lens, parsec parser and field grammar). The legacy parser
  does not support the new syntax.
- Per-package option lookup and the shared/profiling-lib downward-closed
  property in ProjectPlanning are made stage-aware, so e.g. `package build:*
  shared: False` keeps the build stage static even when the host stage is built
  dynamic.
- Add a parser test (project-config-stage-packages).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These are latent warnings that -Wincomplete-patterns / -Wunused-imports
promote to errors only under validate's -Werror; they predate and are
unrelated to the stage-qualified package configuration work below.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@angerman

Copy link
Copy Markdown

@angerman I think any unqualified name should be "host: + build:". Then host: will be only host and build: only build. Does it make sense to you?

I can kinda see the reasoning. Still wonder if host: would be the more ergonomical ... this is a tricky question.

@andreabedini andreabedini force-pushed the wip/andrea/stage-qualified-options branch from dc184ee to e1b10cc Compare June 15, 2026 09:29
…e default stage (validate -Werror)

- TestCaseUtils.toQPN: add missing QualIndep case (incomplete-patterns -Werror),
  mirroring QualIndepSetup which already ignores its independence argument.
- PackagePath: render the stage prefix only for Build (new dispStage helper);
  Host stays invisible as the old DefaultNamespace did. Both Pretty instances.
- Package.showI / Message.showOption: revert to the original stage-agnostic
  format; the stage now lives solely on the QPN.
- TestCaseUtils: compare the install plan as a set (sort both sides), since the
  stage-aware UnitId reshuffled the topological order.

Fixes the validate build (unit-tests, mem-use-tests) and reduces /Modular/
failures 89 -> 32. Remaining 32 are pre-existing solver-semantics regressions
(independent goals, base shims, reinstall protection, setup-dep merging,
build-tool exe staging), not rendering.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hasufell

Copy link
Copy Markdown
Member

Cabal-syntax: allow colons in section header arguments — the field parser rejected : in a section argument, so a header such as package build:pkg was a parse error. Colon-joined argument tokens are now combined into a single SectionArg, letting stage-qualified package stanzas be written unquoted. This accepts strictly more input; all golden parser tests still pass.

What does this mean? I have no idea. package "build:pkg" was accepted before, but now we also accept package build:pkg?

What does "accepts strictly more input" mean?

I'm afraid I can't follow this generated summary.

cabal-install: stage-qualified package configuration (package build:) — adds package build: / package host:* stanzas. The same package can be built in both the build and host stages, so package *, top-level fields and package cannot distinguish them; a stage qualifier can.

Can we start with a clear motivation, even if it is obvious? What are we trying to solve?

What can the end user do now that they couldn't do before?


colon, openBrace, closeBrace :: Parser ()

-- | A single section argument.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is a section argument?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Section arguments are what follows the opening a section, e.g.

-- name 'test', argument 'my-test'
test my-test
  ...

-- name 'if', arguments 'impl(ghc)'
if impl(ghc)
  ...

-- was previously a parse error, so this accepts strictly more input.
sectionArg :: Parser (SectionArg Position)
sectionArg = tokSym' <|> tokStr <|> tokOther <?> "section parameter"
sectionArg = do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is the test for the new syntax?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The change in Cabal-syntax is not specific to the host: or build: syntax but it is necessary to allow it. I guess I should add some tests to Cabal-syntax to describe this change anyway.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note: Cabal-syntax has no in-package tests. Tests for the build:*/host:* syntax are in test-parser which is in [cabal-install].

Right name -> return $ Just name

data PackageConfigTarget = AllPackages | SpecificPackage !PackageName
data PackageConfigTarget = AllPackages | AllStagePackages !Stage | SpecificPackage !PackageName

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How does this work? We don't have a stage for SpecificPackage?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually not, this PR only implements "build:" and "host:", not "build:pkg" or "host:pkg". It's the minimum required for building our ghc.

showI :: I -> String
showI (I s v (InRepo pn)) = intercalate ":" [showStage s, "source", prettyShow (PackageIdentifier pn v)]
showI (I s _v (Inst uid)) = intercalate ":" [showStage s, "installed", prettyShow uid]
showI (I _s v (InRepo _pn)) = showVer v

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are there some unit tests for this that showcase what the changes are?

@andreabedini andreabedini Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

-- ordinary (non-cross) builds, where every package is on the 'Host' stage,
-- stay free of stage noise; only the 'Build' stage is called out explicitly.
dispStage :: Stage -> Disp.Doc
dispStage Host = Disp.empty

@hasufell hasufell Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems confusing if the default is build+host?

Comment on lines +97 to +100
-- A base-qualified dependency is never matched by a toplevel/setup/exe scope.
constraintQualifierMatches (ScopeTarget _) (QualBase _) _ = False
constraintQualifierMatches (ScopeAnySetupQualifier _) (QualBase _) _ = False
constraintQualifierMatches (ScopeAnyExeQualifier _) (QualBase _) _ = False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this part related to the PR? It seems like an unrelated bugfix?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You are correct. It is unrelated.

@andreabedini andreabedini Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change is unfortunate but required to be able to parse host:pkg-name correctly, as it stands the parser will stop at :. The commit message describes this well:

Cabal-syntax: allow colons in section header arguments
The field parser rejected ':' in a section argument, so a section header such as
package build:pkg was a parse error. Combine colon-joined argument tokens into
a single SectionArg, which lets stage-qualified package stanzas be written
unquoted. A colon in this position was previously always an error, so this
accepts strictly more input; all Cabal-tests golden parser tests still pass.

480c9c2

@andreabedini

Copy link
Copy Markdown
Member Author

The changes

  • fix: cover QualBase and drop redundant import for -Werror (validate)
  • cabal-install: stage-qualified package configuration (package build:*)
    are unrelated to the change but needed to get things to work after the rebase on upstream master.

@andreabedini

Copy link
Copy Markdown
Member Author

What does this mean? I have no idea. package "build:pkg" was accepted before, but now we also accept package build:pkg?

Neither package "build:pkg" or package build:pkg were accepted before by the cabal.project parser and the issue is that Cabal-syntax's parseFields (which is a lower level parser shared with cabal files) could not accept that syntax either.

This is using cabal from stable-haskell/master:

/tmp/tmp.bXfC7W68cv 12s
λ cat cabal.project

extra-packages: lens

package build:lens
    ghc-options: -v

/tmp/tmp.bXfC7W68cv
λ cabal build --dry-run lens
Error: [Cabal-7090]
Error parsing project file cabal.project:4:
 - Parse of field '
   unexpected ":"
   expecting section parameter, "{", field or section name or end of file' failed.

/tmp/tmp.bXfC7W68cv
λ hx cabal.project

/tmp/tmp.bXfC7W68cv
λ cat cabal.project

extra-packages: lens

package "build:lens"
    ghc-options: -v

/tmp/tmp.bXfC7W68cv
λ cabal build --dry-run lens
Error: [Cabal-7090]
Error parsing project file cabal.project:4:
 - a 'package' section requires a package name as an argument

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants