-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Rephrased error message for disallowed sections in virtual workspace #8200
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
I don't understand that test failure. Locally It seems to not be related to this patch anyway? |
It is an unrelated error. rust-lang/rust#71716 changed some behavior that is causing that test to fail, and Cargo will need to be updated for it. |
066393a
to
7bbbbd0
Compare
Rebased. The previously offending test |
Hm, I have never seen that error before, and I can't imagine how it is possible. I tried reproducing it, but I couldn't.
Can probably ignore it for now. If it shows up again, we can investigate more. |
src/cargo/util/toml/mod.rs
Outdated
@@ -1320,43 +1320,43 @@ impl TomlManifest { | |||
config: &Config, | |||
) -> CargoResult<(VirtualManifest, Vec<PathBuf>)> { | |||
if me.project.is_some() { | |||
bail!("virtual manifests do not define [project]"); | |||
bail!("This virtual manifest specifies a [project] section, which is not allowed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently all of Cargo's error messages conventionally start with a lowercase letter, mind changing that here? Other than that this seems good to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
7bbbbd0
to
d6a1428
Compare
@bors: r+ |
📌 Commit d6a1428 has been approved by |
☀️ Test successful - checks-azure |
Update cargo 9 commits in cb06cb2696df2567ce06d1a39b1b40612a29f853..500b2bd01c958f5a33b6aa3f080bea015877b83c 2020-05-08 21:57:44 +0000 to 2020-05-18 17:12:54 +0000 - Handle LTO with an rlib/cdylib crate type (rust-lang/cargo#8254) - Gracefully handle errors during a build. (rust-lang/cargo#8247) - Update `im-rc` to 15.0.0 (rust-lang/cargo#8255) - Fix `cargo update` with unused patch. (rust-lang/cargo#8243) - Rephrased error message for disallowed sections in virtual workspace (rust-lang/cargo#8200) - Ignore broken console output in some situations. (rust-lang/cargo#8236) - Expand error message to explain that a string was found (rust-lang/cargo#8235) - Add context to some fs errors. (rust-lang/cargo#8232) - Move SipHasher to an isolated module. (rust-lang/cargo#8233)
I changed the error message from
virtual manifests do not specify [target]
toThis virtual manifest specifies a [target] section, which is not allowed
because the old one confused me.I encountered it while hacking on Rustc in CLion. I made a change to the
Cargo.toml
I had copied from StackOverflow and after the next restart code analysis and go-to-definition stopped working. After some fiddling, I tracked that down to my change inCargo.toml
. It took me a while because, like I said, the error message from CLion didn't make sense to me.I hope this change avoids future confusion :)