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

Update to syn 0.12 #28

Merged
merged 1 commit into from
Mar 1, 2018
Merged

Update to syn 0.12 #28

merged 1 commit into from
Mar 1, 2018

Conversation

alexcrichton
Copy link
Contributor

In tracking down the cause of rust-lang/rust#48223 I found that the syn crate
was compiled twice and with different features than the ones that Cargo
activated. It turns out that the RLS pulls in this crate which depends on syn,
and this crate enabled the full feature in syn where a standalone build of
Cargo didn't activate such a feature. Consequently this means that when Cargo
was compiled for the RLS and for itself it was compiled twice!

I initially started out by removing the dependency on the full feature in this
crate (hurray faster compiles too!) but ended up updating to syn 0.12 as well
which is the updated version that works better with spans and such.

I've also done some reorganization of the tests cases to hopefully be more
amenable to CI as well, but let me know if any of it looks odd!

In tracking down the cause of rust-lang/rust#48223 I found that the `syn` crate
was compiled twice and with different features than the ones that Cargo
activated. It turns out that the RLS pulls in this crate which depends on `syn`,
and this crate enabled the `full` feature in `syn` where a standalone build of
Cargo didn't activate such a feature. Consequently this means that when Cargo
was compiled for the RLS and for itself it was compiled twice!

I initially started out by removing the dependency on the `full` feature in this
crate (hurray faster compiles too!) but ended up updating to syn 0.12 as well
which is the updated version that works better with spans and such.

I've also done some reorganization of the tests cases to hopefully be more
amenable to CI as well, but let me know if any of it looks odd!
@nrc
Copy link
Owner

nrc commented Mar 1, 2018

I've also done some reorganization of the tests cases to hopefully be more
amenable to CI as well, but let me know if any of it looks odd!

Looks fine, but out of curiosity, why is this easier for CI?

Thanks for the PR!

@nrc nrc merged commit 6b72141 into nrc:master Mar 1, 2018
@alexcrichton alexcrichton deleted the update-syn branch March 1, 2018 22:31
@alexcrichton
Copy link
Contributor Author

Oh I mostly figured it'd be easiest for CI to mirror just cargo test on stable/beta and have nightly go the extra mile with cargo test --manifest-path w/o extra Cargo features, but it may also be more personal preference than anything else!

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

Successfully merging this pull request may close these issues.

2 participants