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

Avoid introducing spurious features for optional dependencies #5691

Merged
merged 2 commits into from
May 12, 2024

Conversation

bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented May 11, 2024

Connections
None

Description
If a feature depends on an optional dependency without using the dep: prefix, a feature with the same name as the optional dependency is introduced. This feature almost certainly won't have any effect when enabled other than increasing compile times and polutes the feature list shown by cargo add. Consistently use dep: for all optional dependencies to avoid this problem.

Testing
Explain how this change is tested.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

If a feature depends on an optional dependency without using the dep:
prefix, a feature with the same name as the optional dependency is
introduced. This feature almost certainly won't have any effect when
enabled other than increasing compile times and polutes the feature list
shown by cargo add. Consistently use dep: for all optional dependencies
to avoid this problem.
@bjorn3 bjorn3 requested review from a team as code owners May 11, 2024 20:08
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

Didn't know this was a thing! I tried before after cargo add --path <my wgpu path> wgpu and indeed the old version lists naga and hal when it really shouldn't.
Nice, thank you!

@Wumpf Wumpf merged commit fa48562 into gfx-rs:trunk May 12, 2024
27 checks passed
@bjorn3 bjorn3 deleted the less_features branch May 12, 2024 07:16
EriKWDev pushed a commit to bitwise-git/wgpu that referenced this pull request May 14, 2024
…#5691)

* Avoid introducing spurious features for optional dependencies

If a feature depends on an optional dependency without using the dep:
prefix, a feature with the same name as the optional dependency is
introduced. This feature almost certainly won't have any effect when
enabled other than increasing compile times and polutes the feature list
shown by cargo add. Consistently use dep: for all optional dependencies
to avoid this problem.

* Add changelog entry
jimblandy pushed a commit to jimblandy/wgpu that referenced this pull request May 23, 2024
…#5691)

* Avoid introducing spurious features for optional dependencies

If a feature depends on an optional dependency without using the dep:
prefix, a feature with the same name as the optional dependency is
introduced. This feature almost certainly won't have any effect when
enabled other than increasing compile times and polutes the feature list
shown by cargo add. Consistently use dep: for all optional dependencies
to avoid this problem.

* Add changelog entry
@fintelia
Copy link
Contributor

FYI: This is likely to cause breakage for some downstream users when they upgrade to the next release so it could be worth calling out more clearly in the CHANGELOG.

Specifically, the naga feature used to be required for anyone who wanted to use the wgpu::naga re-export with the 0.18 or earlier versions. It became a no-op in the 0.19 release, but many people upgrading probably didn't notice. For instance, Bevy still enables the feature today even though they target 0.20.

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.

3 participants