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

cargo fix should serialize on entire workspace #6528

Closed
baumanj opened this issue Jan 7, 2019 · 7 comments · Fixed by #9677
Closed

cargo fix should serialize on entire workspace #6528

baumanj opened this issue Jan 7, 2019 · 7 comments · Fixed by #9677
Labels

Comments

@baumanj
Copy link

baumanj commented Jan 7, 2019

(Edit ehuss): cargo fix in a workspace should serialize across the entire workspace (currently it only locks each package). This is needed if something like include! is used in multiple packages to the same file. The two packages will attempt to fix the shared file at the same time, causing problems.

Original Issue

Starting from https://github.com/habitat-sh/habitat/tree/066d60d52d4e65c6978d6a8f62962035b5fa4ba5, applying cargo fix --edition-idioms led to compilation errors, so I'm filing a bug report per the output's request. Some additional system information:

➤ rustc --version
rustc 1.31.0 (abe02cefd 2018-12-04)

09:18:05 AM jbauman@ubuntu:~
➤ cargo --version
cargo 1.31.0 (339d9f9c8 2018-11-16)

➤ uname -a
Linux ubuntu 4.13.0-16-generic #19-Ubuntu SMP Wed Oct 11 18:35:14 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

If I find any interesting information in fixing up the errors, I'll post a follow-up. In the meantime, I'm attaching the full output of the cargo fix command.
cargo-fix-output.txt

@baumanj baumanj added the C-bug Category: bug label Jan 7, 2019
@ehuss
Copy link
Contributor

ehuss commented Jan 7, 2019

Yikes, that's quite a few issues. I noticed that your initial edition transition did not fix non-linux targets. Any code marked with things like #[cfg(target_os = "macos")] is currently broken. Dealing with cfg blocks is something that cargo fix doesn't handle too well.

I can try to reduce some of the errors. I already see a few that haven't been reported before.

@baumanj
Copy link
Author

baumanj commented Jan 7, 2019

That's helpful to know about the cfg blocks. I'll make sure to deal with those by hand.

As I work through the fixes, one thing I'm noticed is that rustfmt undid a necessary fix (or perhaps I'm making the fix wrong). We're using the time crate. In the first part of my conversion, I changed the extern crate statement to time as time_crate in order to support this usage of Duration.

However, once the --edition-idioms pass removed the extern crate, I needed to change that line to

use ::time::Duration;

However, when I pass this through rustfmt, it gets re-written to

use time::Duration;

which is ambiguous again. Should I file a separate issue for that?

@baumanj
Copy link
Author

baumanj commented Jan 7, 2019

Here are the fixes that I made to get things building again (at least on linux) after running cargo fix --edition-idioms.

@ehuss
Copy link
Contributor

ehuss commented Jan 7, 2019

Should I file a separate issue for that

Nah, I'm finding quite a few different issues, and I'll file upstream and link here once I've got them all.

@ehuss
Copy link
Contributor

ehuss commented Jan 7, 2019

OK, not sure if I got all of them, but here's what I found:

@baumanj Have you been able to get everything working? Let me know if you have any questions, or anything you can't figure out.

@alexcrichton
Copy link
Member

@ehuss oh jeez, but yeah for now seems like we need to serialize workspace crates, not just packages

@baumanj
Copy link
Author

baumanj commented Jan 7, 2019

@baumanj Have you been able to get everything working? Let me know if you have any questions, or anything you can't figure out.

I think so! Everything builds locally, but we're having an issue with our CI so can't say for certain that it's working on all platforms. I don't anticipate any further cargo-related issues, but if there's anything relevant, I'll follow up here.

Thanks for being so responsive and all your awesome work here!

@ehuss ehuss changed the title cargo fix --edition-idioms introduced compilation errors cargo fix should serialize on entire workspace Jun 24, 2019
@bors bors closed this as completed in f2496ee Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants