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

workspaces.dependencies causes ignore of default-features = false in crate #11329

Closed
leighmcculloch opened this issue Nov 3, 2022 · 2 comments · Fixed by #11409
Closed

workspaces.dependencies causes ignore of default-features = false in crate #11329

leighmcculloch opened this issue Nov 3, 2022 · 2 comments · Fixed by #11409
Labels
A-workspace-inheritance Area: workspace inheritance RFC 2906 C-bug Category: bug

Comments

@leighmcculloch
Copy link

leighmcculloch commented Nov 3, 2022

Problem

When specifying a workspace dependency it is not obvious, and rather surprising that, not specifying default-features = false in the workspace dependency, means that any default-features = false is ignored in any member crate.

This I assume is because the features of workspace dependencies are additive, and by not specifying default-features = false, the default features are automatically and always enabled.

Steps

  1. Create a workspace with a workspace dependency that has default features.
  2. Add a crate that also specifies the dependency, inherits it from the workspace, and specifies default-features = false.

Possible Solution(s)

There are I think a couple solutions to this problem:

Possible Solution 1: Change the Default in workspace dependencies

This is probably not possible now that the feature is released, but it seems reasonable that workspace depenencies by default should not contribute any features unless explicitly specified. This would mean default-features = false by default in the workspace dependency.

Position Solution 2: Display a warning if a member crate has default-features = false when workspace doesn't

If a workspace doesn't specify default-features = false, but a member crate does, this seems like an error, since the member crate will never achieve the effect of default-features = false. A warning would go a long way to help educate and catch this mistake. The warning for this would be similar to how cargo warns if you include patch entries that aren't getting used.

Notes

No response

Version

cargo 1.64.0 (387270bc7 2022-09-16)
release: 1.64.0
commit-hash: 387270bc7f446d17869c7f208207c73231d6a252
commit-date: 2022-09-16
host: aarch64-apple-darwin
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.84.0 (sys:0.4.55+curl-7.83.1 system ssl:(SecureTransport) LibreSSL/3.3.6)
os: Mac OS 13.0.0 [64-bit]
@smmalis37
Copy link

smmalis37 commented Nov 4, 2022

A quick skim of the RFC implies to me that specifying default-features = anything and workspace = true should be disallowed, but currently it seems to be silently ignored. Should this be made into some sort of hard error instead?

@Muscraft
Copy link
Member

Muscraft commented Nov 5, 2022

It appears that my (poor) workaround for derive not working on TomlDependency, is the root cause of this. I'll have to dig a bit more to make sure but default-features = false on a dependency that inherits from a workspace should throw a warning for an unused manifest key.

That being said I think that default-featues = false could be a good thing to add.

@bors bors closed this as completed in 7e2a3c2 Jan 20, 2023
bors added a commit that referenced this issue Mar 1, 2023
fix(toml): Provide a way to show unused manifest keys for dependencies

Dependencies have not been able to show unused manifest keys for some time, this problem partially resulted in #11329.

This problem is caused by having an `enum` when deserializing.  To get around this you can use:
```rust
#[serde(flatten)]
other: BTreeMap<String, toml::Value>,
```
This collects any unused keys into `other` that can later be used to show warnings. This idea was suggested in a thread I cannot find but is mentioned in [serde#941](serde-rs/serde#941).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-workspace-inheritance Area: workspace inheritance RFC 2906 C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants