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

maybe a missing crate with workspace when edition is not declared #14108

Closed
bukowa opened this issue Jun 19, 2024 · 10 comments
Closed

maybe a missing crate with workspace when edition is not declared #14108

bukowa opened this issue Jun 19, 2024 · 10 comments
Labels
C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@bukowa
Copy link

bukowa commented Jun 19, 2024

Problem

$ cargo build
   Compiling package3 v0.1.0 (\root\package3)
error[E0432]: unresolved import `ignore`                                                                                                                                     
 --> package3\src\tree.rs:4:5
  |
4 | use ignore::{DirEntry, Error, Walk, WalkBuilder};
  |     ^^^^^^ maybe a missing crate `ignore`?
  |
  = help: consider adding `extern crate ignore` to use the `ignore` crate

error[E0432]: unresolved import `serde`

Steps

# /root/Cargo.toml
[workspace]
resolver = "2"
members = [
    "package1", "package2", "package3",
]
# /root/package3
[package]
name = "package3"
version = "0.1.0"

[dependencies]
ignore = "0.4.22"
serde = { version = "1.0.203", features = ["derive"] }

Working Solution(s)

Add edition = "2021"

# /root/package3
[package]
name = "package3"
version = "0.1.0"
edition = "2021"

Notes

Better information would be helpful.

Version

cargo 1.79.0 (ffa9cf99a 2024-06-03)
@bukowa bukowa added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jun 19, 2024
@bukowa
Copy link
Author

bukowa commented Jun 19, 2024

#14023

@weihanglo
Copy link
Member

   = help: consider adding `extern crate ignore` to use the `ignore` crate

The rust compiler already gave a help message, so people do get a hint for fixing that compile error.

Note that if a package was created by a newer cargo, the package.edition should be set to the latest stable edition (2021 as of now). I would consider missing edition as a deliberate choice and won't bother users with any further warning. See

You'll get a warning if package.rust-version is set as a version greather than 1.30.

@epage
Copy link
Contributor

epage commented Jun 19, 2024

Is that output complete? In #13505 (1.78), we warn on unset edition.

@epage
Copy link
Contributor

epage commented Jun 19, 2024

Also, this explicitly calls out that a workspace was used but this should behave the same without it. Can you confirm if there is something different with workspaces?

@epage
Copy link
Contributor

epage commented Jun 19, 2024

Rustc giving a hint to specify a new edition would be worth considering but that would be for their repo, and their decision to make.

@weihanglo
Copy link
Member

I saw what went wrong. rust_version is None here because it doesn't take the current toolchain version into account.

let msrv_edition = if let Some(pkg_msrv) = &rust_version {

There are some ways to fix this:

  • Warn if both rust-version and edition are missing
  • Make package.rust_version aware of toolchain version during to_real_manifest

@epage which do you like?

@bukowa
Copy link
Author

bukowa commented Jun 19, 2024

Is that output complete? In #13505 (1.78), we warn on unset edition.

https://github.com/bukowa/rustrover-compilebug/tree/8d684228275dacc9d6a6befc53bcf192c3a27b41

$ cargo build
   Compiling package2 v0.1.0 (C:\Users\buk\RustroverProjects\rustrover-compilebug\package2)
error[E0432]: unresolved import `serde`                                                                                                                                                        
 --> package2\src\lib.rs:1:5
  |
1 | use serde::Serialize;
  |     ^^^^^ maybe a missing crate `serde`?
  |
  = help: consider adding `extern crate serde` to use the `serde` crate

For more information about this error, try `rustc --explain E0432`.                                                                                                                            
error: could not compile `package2` (lib) due to 1 previous error
$ cargo version
cargo 1.79.0 (ffa9cf99a 2024-06-03)

@weihanglo
Copy link
Member

I saw what went wrong. rust_version is None here because it doesn't take the current toolchain version into account.

let msrv_edition = if let Some(pkg_msrv) = &rust_version {

There are some ways to fix this:

* Warn if both `rust-version` and `edition` are missing

* Make `package.rust_version` aware of toolchain version during `to_real_manifest`

@epage which do you like?

Probably the first one? People only get the warning on 1.78 or later, so it is sufficent for Cargo to know that they are already on a newer toolchain. They can alwasy set the package.rust-version field to silent it.

@bukowa
Copy link
Author

bukowa commented Jun 19, 2024

@epage which do you like?

As a new user playing around with rust and workspaces, not understanding how everything fits together, I would expect a message that allows me to track down why this issue arises.

epage added a commit to epage/cargo that referenced this issue Jun 19, 2024
This came up in rust-lang#14108.  This got overlooked when we added the MSRV
limits to the warning.

Users can silience this by setting a very old MSRV.
I didn't bother finding a way to word a specialized message for this.
Wording for this case seemed hard and I figure someone in this situation
knows how to resolve it.
Instead, we are targeting the majority of users who aren't setting a
`package.rust-version` in the first place.
epage added a commit to epage/cargo that referenced this issue Jun 19, 2024
This came up in rust-lang#14108.  This got overlooked when we added the MSRV
limits to the warning.

Users can silience this by setting a very old MSRV.
I didn't bother finding a way to word a specialized message for this.
Wording for this case seemed hard and I figure someone in this situation
knows how to resolve it.
Instead, we are targeting the majority of users who aren't setting a
`package.rust-version` in the first place.
bors added a commit that referenced this issue Jun 19, 2024
fix(toml): Warn when edition is unuset, even when MSRV is unset

### What does this PR try to resolve?

This came up in #14108.  This got overlooked when we added the MSRV
limits to the warning.

Users can silence this by setting a very old MSRV.
I didn't bother finding a way to word a specialized message for this.
Wording for this case seemed hard and I figure someone in this situation
knows how to resolve it.
Instead, we are targeting the majority of users who aren't setting a
`package.rust-version` in the first place.

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

### Additional information
bors added a commit that referenced this issue Jun 19, 2024
fix(toml): Warn when edition is unuset, even when MSRV is unset

### What does this PR try to resolve?

This came up in #14108.  This got overlooked when we added the MSRV
limits to the warning.

Users can silence this by setting a very old MSRV.
I didn't bother finding a way to word a specialized message for this.
Wording for this case seemed hard and I figure someone in this situation
knows how to resolve it.
Instead, we are targeting the majority of users who aren't setting a
`package.rust-version` in the first place.

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

### Additional information
@bukowa
Copy link
Author

bukowa commented Jun 21, 2024

Thank you by looking at the PR this is much more helpful.

@bukowa bukowa closed this as completed Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

3 participants