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 which, quote and proc_macro. #1409

Merged
merged 6 commits into from
Nov 30, 2018
Merged

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Oct 4, 2018

Fixes #1407.

@emilio
Copy link
Contributor Author

emilio commented Oct 4, 2018

cc @Eijebong, last commit is a rebase of yours. If you could take a look it'd be great.

@ignatenkobrain
Copy link

One test failed due to missing conversion to #[doc

@emilio emilio force-pushed the update-some-crates branch from 2496a23 to a71f3ec Compare October 4, 2018 14:30
@ignatenkobrain
Copy link

Meh, it seems that this triggers rust-lang/cargo#2589.

See cbindgen's similar issues: mozilla/cbindgen#203

@ignatenkobrain
Copy link

it's enough to keep which on 1.x in order to avoid triggering that bug in cargo.

@emilio
Copy link
Contributor Author

emilio commented Oct 15, 2018

Why don't you need #[cfg_attr(serde_derive, derive(Serialize)]?

That's fine for me.

@Eijebong did you manage to take a look at this? Mostly as a sanity check.

@Eijebong
Copy link
Contributor

From a quick look it seemed fine. We're running into dtolnay/proc-macro2#60 a lot though

@emilio
Copy link
Contributor Author

emilio commented Oct 16, 2018

Err, my above comment meant to quote:

it's enough to keep which on 1.x in order to avoid triggering that bug in cargo.

So @ignatenkobrain I have no preference on landing this as is or with which kept at 1.0. If the latter makes things easier, let me know and I'll remove it from the PR.

@ignatenkobrain
Copy link

IIUC, if you push this as is, you would have problems with doing cargo install from users because when compiler is updated, binary would be broken.

@cuviper, did i describe problem correctly?

@cuviper
Copy link
Member

cuviper commented Oct 16, 2018

IIUC, if you push this as is, you would have problems with doing cargo install from users because when compiler is updated, binary would be broken.

This, but also problems if the user tries to run bindgen with a different toolchain active. e.g. having compiled bindgen with your default-stable rustc, then using it in one of your projects on nightly. Or vice versa. Any scenario where the bindgen gets linked to rustc shared libraries will mean your installation is tied to that exact toolchain.

This arises from which 2.0 -> failure -> failure_derive -> ... proc-macro2, forcing you into linking libproc_macro even though bindgen wanted it without default features. which 1.x didn't use failure at all.

@cuviper
Copy link
Member

cuviper commented Nov 27, 2018

With harryfei/which-rs#9 in which 2.0.1, the proc-macro feature will be avoided.

@Eijebong
Copy link
Contributor

:party:

@bors-servo
Copy link

☔ The latest upstream changes (presumably #1453) made this pull request unmergeable. Please resolve the merge conflicts.

Eijebong and others added 3 commits November 30, 2018 06:25
I give up on the doc comments. This is a rebase of rust-lang#1334 keeping the formatting
of the comments and using TokenStream::from_str instead because one can hope.

Fixes rust-lang#1407.
@emilio emilio force-pushed the update-some-crates branch from b8b8247 to 70298c2 Compare November 30, 2018 05:35
@emilio
Copy link
Contributor Author

emilio commented Nov 30, 2018

Ok, so I rebased this, and bumped which as well. Assuming this is green I'll merge. Thanks everyone for all the effort, specially @Eijebong :)

@emilio emilio force-pushed the update-some-crates branch from 1c28e5a to 7b1406d Compare November 30, 2018 10:59
@emilio emilio merged commit 29a6347 into rust-lang:master Nov 30, 2018
@emilio emilio deleted the update-some-crates branch November 30, 2018 11:00
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 30, 2018
Update syn

- servo/html5ever#353
- servo/rust-cssparser#229
- servo/webrender#3264
- servo/media#162
- rust-lang/rust-bindgen#1409

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22085)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants