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 - failure with failure #5798

Closed
ExpHP opened this issue Jul 26, 2018 · 11 comments
Closed

cargo fix - failure with failure #5798

ExpHP opened this issue Jul 26, 2018 · 11 comments

Comments

@ExpHP
Copy link
Contributor

ExpHP commented Jul 26, 2018

cc @alexcrichton

Oh dear these issues sound bad! Are you sure you were testing with an up-to-date nightly? Recent changes in lint emissions should supress lints related to foreign macros (like derive in these cases) but if they’re still showing up that’s a bug!

Would you be able to share some test cases if it’s still showing problems?


#![feature(rust_2018_preview)]

#[macro_use]
extern crate failure;

#[derive(Fail, Debug)]
#[fail(display = "hello!")]
pub struct Error(::module::Thing);

pub mod module {
    #[derive(Debug)]
    pub struct Thing;
}

A complete source tree: failure-test-case.tar.gz
(Edit: I edited and broke the link because I was confused. Restored the proper link.)

I really don't think there is much that can be done about this one. cargo fix fixes the path to crate::module::Thing, then failure panics because the latest 0.1.1 release can't parse the new syntax.

This test case will be "magically" resolved when failure 0.1.2 is released.

Tested on rustc 1.29.0-nightly (6a1c0637c 2018-07-23).

@alexcrichton
Copy link
Member

Thanks for the report!

The bug here looks like it's because failure doesn't understand the crate::foo syntax. In that sense it's a bug that we say this is a bug, because it's not really a bug in cargo-fix at all! We're sort of just pessimistically taking the blame for this.

There's not much that cargo fix can do here about this, although I agree that failure 0.1.2 should solve the issue!

@alexcrichton
Copy link
Member

I also want to thank you for a minimal test case here, it's incredibly helpful!

@alexcrichton
Copy link
Member

Unfortunately I'm not sure if there's much action we can take on this issue though in cargo-fix or rustc itself :(

@alexcrichton alexcrichton added this to the Edition Preview 2 milestone Jul 26, 2018
@ExpHP
Copy link
Contributor Author

ExpHP commented Jul 26, 2018

I guess the "fix" here if the cargo team really wants to ensure a smooth transition, is to start making PRs to popular proc macro crates to make sure their dependencies are up to date.

In the case of failure it may be a bit tricky. The 0.1.2 on master is a shim around 1.0.0, but the 1.0.0 release is in limbo.

@alexcrichton
Copy link
Member

Definitely! I've make sure this comes up during triage so we can continue to prioritize fixing the specific case of the failure crate, and that's an immediately actionable item to take away from this, although it doesn't necessarily mean this should be closed out when done

@aturon
Copy link
Member

aturon commented Jul 31, 2018

cc @mitsuhiko @withoutboats, can y'all look into prioritizing this?

@withoutboats
Copy link
Contributor

I believe this is already fixed in 0.1.2, at least I thought I saw a PR pass through my email.

@alexcrichton
Copy link
Member

For context @mitsuhiko and @withoutboats this should be fixed by updating the syn dependency in 0.1.2, but the release hasn't been published yet. All that's needed for this to be fixed is an updated syn dependency, though.

@mitsuhiko
Copy link
Contributor

This should be fixed in 0.1.2. I want to push out a release for that tomorrow.

@alexcrichton
Copy link
Member

Ok, thanks for the update!

@alexcrichton
Copy link
Member

The new release is out now, thanks so much @mitsuhiko!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants