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

Bad error message when a weak dep features's depedency is an unused optional dependency #14015

Closed
epage opened this issue Jun 5, 2024 · 1 comment · Fixed by #14026
Closed
Assignees
Labels
A-editions Area: edition-specific issues A-features Area: features — conditional compilation C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@epage
Copy link
Contributor

epage commented Jun 5, 2024

Problem

If nothing can activate an optional dependency, cargo acts as if the dependency doesn't exist which creates poor error messages.

Also, with how things are arranged, the unused optional dependency lint doesn't get reported which could at least reduce the burden on the error message.

Steps

Baseline Cargo.toml:

cargo-features = ["edition2024"]
[package]
name = "cargo-14010"
version = "0.1.0"
edition = "2024"

[dependencies]
serde = { version = "1.0.203", optional = true }
$ cargo +nightly check -Zcargo-lints
warning: unused optional dependency
 --> Cargo.toml:8:1
  |
8 | serde = { version = "1.0.203", optional = true }
  | -----
  |
  = note: `cargo::unused_optional_dependency` is set to `warn` by default
  = help: remove the dependency or activate it in a feature with `dep:serde`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.00s

Add the following to Cargo.toml:

[features]
serde = ["serde?/derive"]
$ cargo +nightly check -Zcargo-lints
error: failed to parse manifest at `/home/epage/src/personal/dump/cargo-14010-1/Cargo.toml`

Caused by:
  feature `serde` includes `serde?/derive`, but `serde` is not a dependency

Possible Solution(s)

  • Improve the error message
  • Show the lint (which might be more difficult)

Notes

This came up in discussion in #14010

#14016 covers strong dep features as we likely will want a different solution.

Version

No response

@epage epage added C-bug Category: bug A-features Area: features — conditional compilation A-editions Area: edition-specific issues S-triage Status: This issue is waiting on initial triage. labels Jun 5, 2024
@epage epage added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-triage Status: This issue is waiting on initial triage. labels Jun 5, 2024
@linyihai
Copy link
Contributor

linyihai commented Jun 6, 2024

@rustbot claim
I have a unpolish improvement need to try.

bors added a commit that referenced this issue Jun 17, 2024
fix(fix): Address problems with implicit -> explicit feature migration

### What does this PR try to resolve?

Within the scope of `cargo fix` there  are two problems
- We wipe out existing feature activations if it has the same name as an optional dependency
- The `Cargo.toml` isn't parseable because the unused optional dependency won't "exist" if just `dep_name/feature_name` is used

Fixes #14010

### How should we test and review this PR?

As for the unused optional dependency not "existing" error,
- #14015 is for improving the message for weak dep features
- #14016 is for re-evaluating how we handle this for strong dep features

Depending on what solution we go with for #14016, we might want to revisit the second migration within this PR.  This is one reason I made the commit separate (in addition to just making it clearer whats happening as this gets into some finer details of features).

### Additional information
@bors bors closed this as completed in 838d81d Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-editions Area: edition-specific issues A-features Area: features — conditional compilation C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
2 participants