Skip to content

Add suppress for strong var inheritance#1349

Open
jrray wants to merge 1 commit intomainfrom
suppress-var
Open

Add suppress for strong var inheritance#1349
jrray wants to merge 1 commit intomainfrom
suppress-var

Conversation

@jrray
Copy link
Copy Markdown
Collaborator

@jrray jrray commented Mar 30, 2026

Allow downstream packages to opt out of inheriting strongly-inherited variables by adding a suppress
directive to install requirements:

  install:
    requirements:
      - var: base.inherit-me
        suppress: "reason for suppressing"

The suppress directive prevents the variable from
being added to the built package's runtime
requirements and tells the InheritRequirements
validator to skip checking for it.

Closes #1324.

Allow downstream packages to opt out of inheriting
strongly-inherited variables by adding a suppress
directive to install requirements:

  install:
    requirements:
      - var: base.inherit-me
        suppress: "reason for suppressing"

The suppress directive prevents the variable from
being added to the built package's runtime
requirements and tells the InheritRequirements
validator to skip checking for it.

Closes #1324.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: J Robert Ray <jrray@jrray.org>
@jrray jrray self-assigned this Mar 30, 2026
@jrray jrray added enhancement New feature or request AI Code authored with AI assistance. labels Mar 30, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 82.60870% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/spk-schema/src/recipe.rs 0.00% 6 Missing ⚠️
...ma/crates/foundation/src/ident/pinnable_request.rs 78.26% 5 Missing ⚠️
...hema/crates/foundation/src/ident/pinned_request.rs 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

// this report will not be complete initially, but the
// additional functions called after should fill in the
// final details as the build progresses
let setup = BuildSetupReport {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think when this BuildSetupReport type was conceived, at the time there was little difference between the recipe and package types, so the fact this type only carries the (built) package didn't matter as much. But these days it would likely make sense to have both the recipe and the package here. There are fields in the recipe that don't get added to the package.

This PR currently takes a more minimal approach of adding suppressed_requirements as a field, but it could stick the whole recipe in here so the validation rules have access to it.

.spec
.downstream_runtime_requirements([component]);
for request in downstream_runtime.iter() {
if let RequestWithOptions::Var(var) = request
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was the main implementation hurdle, making it so if the var is suppressed it doesn't then violate the validation rules.

/// Prevent the package from inheriting the var named.
/// Contains the var name and an optional comment explaining
/// why the suppression is needed.
Suppress(OptNameBuf, String),
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I elected to add a 3rd arm here to not overload the VarRequest type and have that impact many more areas of the codebase. This feature should conceptually stay within the realm of recipes and building packages and not extend into the built package or solver territories.

@jrray jrray requested a review from rydrman March 30, 2026 23:41
| var | _str_ | The requested value of a package build variable in the form`name=value`, this can reference a specific package or the global variable (eg `debug=on`, or `python.abi=cp37`) |
| fromBuildEnv | _bool_ | If true, replace the requested value of this variable with the value used in the build environment |
| ifPresentInBuildEnv | _bool_ | Either true or false; if true, then `fromBuildEnv` only applies if the variable was present in the build environment. This allows different variants to have different runtime requirements. |
| suppress | _str_ | If set, prevents this variable from being inherited from upstream packages with Strong inheritance. The value serves as documentation for why the suppression is needed. |
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.

something about not compatible with any other fields?

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

Labels

AI Code authored with AI assistance. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support opting out of strong inheritance vars

2 participants