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

Clarify config options and transitively included features #25

Open
Person-93 opened this issue Nov 29, 2021 · 1 comment · May be fixed by #50 or #52
Open

Clarify config options and transitively included features #25

Person-93 opened this issue Nov 29, 2021 · 1 comment · May be fixed by #50 or #52

Comments

@Person-93
Copy link
Contributor

Current Config (as I understand it)

  • skip_feature_sets: A set of sets. Any feature set from the matrix that is a superset of any members will be dropped.
  • skip_optional_dependencies: Doesn't include optional dependencies when building the matrix.
  • extra_features: a set of features that will be included in every set in the matrix.
  • allowlist: If present, the matrix will only be built using these features. Other features may be transitively included.
  • denylist: If present these features will not be used to build the matrix but others may be transitively enabled.

Possible Sources of Confusion

  • Perhaps skip_feature_sets can be renamed conflicts or conflicting for clarity.
  • Default value for skip_optional_dependencies is false but the example in the README shows it as true and says in a comment that skipping them can improve performance.
  • Perhaps allowlist should be renamed as it implies that any feature set that (directly or transitively) includes a feature that's not in the list will be dropped from the matrix. I'm not sure what a good name would be. The best I could come up with is seed because these values are the seeds from which the matrix is build.
  • Perhaps denylist should be renamed to the opposite of whatever allowlist gets named for the same reason.
  • Features that start with __ are skipped but this is not documented and there is no way to disable it.

Transitively Enabled Features

Currently, transitively enabled features are not taken into consideration when building the matrix. I think that they should be taken into account for a few reasons:

Smaller Matrix

More feature sets can be excluded from the matrix as duplicates, This would help with combinatorial explosion. Currently a crate with 10 features/optional-deps would have 1024 feature sets in its matrix. If some of those features enabled other features we can significantly cut down on testing time.

Better Restrictions

  • We can allow users to deny transitively enabled features or optional dependencies. This would allow things like having a __nightly feature that is enabled by any other feature that uses nightly features and denying __nightly.
  • If conflicting features get transitively enabled, they can be dropped from the matrix.
@demmerichs
Copy link
Contributor

I agree, that transitively features should be enabled. I am working on a PR for this.

Regarding the naming of settings perhaps a new issue should be open to keep the topics separate, however, I find allowlist and denylist acceptable names and skip_optional_dependencies could be clarified in the docs.

demmerichs added a commit to demmerichs/cargo-all-features that referenced this issue Sep 5, 2023
…e selected feature sets

Fixes frewsxcv#25. Fixes frewsxcv#43.

BREAKING CHANGE: As previously feature sets [B], [A,B] were included separatly, now there will only be [A,B] run if A is a dependency of B. The testcase [B] is redundant, as it activates feature A automatically.
demmerichs added a commit to demmerichs/cargo-all-features that referenced this issue Sep 17, 2023
…e selected feature sets

Fixes frewsxcv#25. Fixes frewsxcv#43. Even though cargo activates dependent features automatically, we can reduce
the amount of feature sets when already resolving these dependencies during creation of all feature sets.
We decide to go for the more verbose option, selecting [A, B] over [B] when A is a feature dependency of B.
This allows an easier implementation, as dependency chains C -> B -> A can be verified one by one, and it takes other rules like
skip_feature_sets correctly into account without modification.

BREAKING CHANGE: As previously feature sets [B], [A,B] were included separatly, now there will only be [A,B] run if A is a dependency of B. The testcase [B] is redundant, as it activates feature A automatically.
demmerichs added a commit to demmerichs/cargo-all-features that referenced this issue Sep 17, 2023
…e selected feature sets

Fixes frewsxcv#25. Fixes frewsxcv#43. Even though cargo activates dependent features automatically, we can reduce
the amount of feature sets when already resolving these dependencies during creation of all feature sets.
We decide to go for the more verbose option, selecting [A, B] over [B] when A is a feature dependency of B.
This allows an easier implementation, as dependency chains C -> B -> A can be verified one by one, and it takes other rules like
skip_feature_sets correctly into account without modification.

BREAKING CHANGE: As previously feature sets [B], [A,B] were included separatly, now there will only be [A,B] run if A is a dependency of B. The testcase [B] is redundant, as it activates feature A automatically.
demmerichs added a commit to demmerichs/cargo-all-features that referenced this issue Sep 18, 2023
…e selected feature sets

Fixes frewsxcv#25. Fixes frewsxcv#43. Even though cargo activates dependent features automatically, we can reduce
the amount of feature sets when already resolving these dependencies during creation of all feature sets.
We decide to go for the more verbose option, selecting [A, B] over [B] when A is a feature dependency of B.
This allows an easier implementation, as dependency chains C -> B -> A can be verified one by one, and it takes other rules like
skip_feature_sets correctly into account without modification.

BREAKING CHANGE: As previously feature sets [B], [A,B] were included separatly, now there will only be [A,B] run if A is a dependency of B. The testcase [B] is redundant, as it activates feature A automatically.
@demmerichs demmerichs linked a pull request Sep 18, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants