Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to disable features with command line argument #100

Closed
wants to merge 1 commit into from

Conversation

NotTheEvilOne
Copy link

What this PR does / why we need it:
The newly added command line argument --disable-feature supports removing feature dependencies normally added to an image.

Which issue(s) this PR fixes:
Closes #17

Release note:

Add `--disable-feature` as command line argument

@NotTheEvilOne NotTheEvilOne self-assigned this Dec 9, 2024
build Fixed Show fixed Hide fixed
build Fixed Show fixed Hide fixed
@NotTheEvilOne NotTheEvilOne force-pushed the prs/support-cmdline-disable-feature branch 2 times, most recently from 2698fa0 to 03de844 Compare December 9, 2024 08:16
The newly added command line argument `--disable-feature` supports removing feature dependencies normally added to an image.

Closes: #17
Signed-off-by: Tobias Wolf <[email protected]>
@NotTheEvilOne NotTheEvilOne force-pushed the prs/support-cmdline-disable-feature branch from 03de844 to f7d61bd Compare December 9, 2024 08:22
@nkraetzschmar
Copy link
Contributor

Making the set of features based on command line arguments is bad for 2 reasons:

  1. The artifact cname no longer uniquely identifies what was build: two builds, one with --disable-feature X, one without, would have the same cname, but be fundamentally different artifacts)
  2. The builder can build multiple artifacts in one. So ./build A B C, where A, B, and C are all partial cnames is totally valid. Now in this syntax, which target does --disable-features apply to? All? Just the one directly after it? This introduces ambiguity.

This should really be done via pseudo features that exclude the unwanted features. So instead of --disable-feature x, there should be a feature/modifier _noX which just has a info.yaml with

description: "exclude feature X"
type: flag
features:
  exclude:
    - X

@NotTheEvilOne
Copy link
Author

Agreed so the original issue request report should be closed :)

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.

--disable-features flag for build
2 participants