Closed
Bug 1429816
Opened 6 years ago
Closed 6 years ago
Update xpcom-macros to syn 0.12
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
Details
Attachments
(4 files)
syn 0.12 was released earlier this week. It updates the API which is used by proc macros. We should move our custom derives in tree over to using this new interface, including xpcom-macros (being added in bug 1293362).
Assignee | ||
Comment 1•6 years ago
|
||
MozReview-Commit-ID: 5LhWjnMstdE
Attachment #8942236 -
Flags: review?(nfroyd)
Assignee | ||
Comment 2•6 years ago
|
||
MozReview-Commit-ID: LKr42ccjxBr
Attachment #8942237 -
Flags: review?(nfroyd)
Comment 3•6 years ago
|
||
Comment on attachment 8942236 [details] [diff] [review] Part 1: Revendor to bring in `syn` 0.12 and dependencies Review of attachment 8942236 [details] [diff] [review]: ----------------------------------------------------------------- Questions below before we do this. ::: third_party/rust/quote-0.3.15/Cargo.toml @@ +1,3 @@ > +[package] > +name = "quote" > +version = "0.3.15" # don't forget to update version in readme for breaking changes Is there any way we could not have two versions of quote in the tree? ::: third_party/rust/syn-0.11.11/Cargo.toml @@ +1,3 @@ > +[package] > +name = "syn" > +version = "0.11.11" # don't forget to update version in readme for breaking changes Uh. Is there any way we could not have two versions of syn in the tree?
Attachment #8942236 -
Flags: review?(nfroyd)
Comment 4•6 years ago
|
||
Comment on attachment 8942237 [details] [diff] [review] Part 2: Update xpcom-macros to build with syn 0.12 Review of attachment 8942237 [details] [diff] [review]: ----------------------------------------------------------------- Sure, sure, just need to get the vendoring details worked out.
Attachment #8942237 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 5•6 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #3) > Questions below before we do this. > > ::: third_party/rust/quote-0.3.15/Cargo.toml > @@ +1,3 @@ > > +[package] > > +name = "quote" > > +version = "0.3.15" # don't forget to update version in readme for breaking changes > > Is there any way we could not have two versions of quote in the tree? > > ::: third_party/rust/syn-0.11.11/Cargo.toml > @@ +1,3 @@ > > +[package] > > +name = "syn" > > +version = "0.11.11" # don't forget to update version in readme for breaking changes > > Uh. Is there any way we could not have two versions of syn in the tree? Unfortunately we currently have some custom derives in tree which still build against the old syn API. It'd be a pretty big change to replace all of them at the same time. I'm guessing the answer to that question is no. If we really don't want it right now, I can perform the upgrade later whenever syn 0.12 ends up in tree due to a different procedural macro or dependency updating.
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(nfroyd)
Comment 6•6 years ago
|
||
Ugh, crate ecosystem. OK. Let's do this: I think all the syn 0.11 dependecies are coming through servo/servo (and serde, which servo also depends on?). Let's file a servo bug to get everything onto the new release. If that happens by the end of January, great, let's land this. If it doesn't happen, that's unfortunate, and we can land this anyway? (Or we can just hold off until servo *does* update; your call.) I'm assuming here that updating the xpcom-macros crate to accommodate different revisions is not super time-consuming (or you already have code somewhere that works with both)? If that's a bad assumption on my part, please let me know, and we can reconsider the timeline, probably just by landing everything now.
Flags: needinfo?(nfroyd)
Assignee | ||
Comment 7•6 years ago
|
||
I filed a bug on servo: https://github.com/servo/servo/issues/19786. I figure we can hold off on landing these patches until we end up with syn 0.12 in tree for some other reason.
Comment 8•6 years ago
|
||
For the record, syn 0.12 has been in the tree for a while now, and in fact we'll probably add syn 0.13 soon. It would be great if these patches could be rebased to make xpcom_macros just use syn 0.13. Since xpcom_macros is the only thing still using 0.11, it would drop 0.11 and add 0.13 which should be a net no-op :) And then other things can start using 0.13 as well.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 11•6 years ago
|
||
mozreview-review |
Comment on attachment 8966433 [details] Bug 1429816 - Part 2: Revendor dependencies. https://reviewboard.mozilla.org/r/235136/#review240826 ::: third_party/rust/clap/Cargo.toml:77 (Diff revision 1) > > [dependencies.bitflags] > version = "1.0" > > [dependencies.clippy] > -version = "~0.0.166" > +version = "0.0.181" Why is there a change to clap showing up here? That's weird.
Comment 12•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8966433 [details] Bug 1429816 - Part 2: Revendor dependencies. https://reviewboard.mozilla.org/r/235136/#review240826 > Why is there a change to clap showing up here? That's weird. That's... Interesting... This commit was created via `./mach vendor rust` so I guess the file changed on the crates.io side ? Is that even possible ?
Comment 13•6 years ago
|
||
mozreview-review |
Comment on attachment 8966432 [details] Bug 1429816 - Part 1: Bump syn and quote in xpcom. https://reviewboard.mozilla.org/r/235134/#review240966 rs=me
Attachment #8966432 -
Flags: review?(nfroyd) → review+
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8966433 [details] Bug 1429816 - Part 2: Revendor dependencies. https://reviewboard.mozilla.org/r/235136/#review240972
Attachment #8966433 -
Flags: review?(nfroyd) → review+
Comment 15•6 years ago
|
||
(In reply to eijebong from comment #12) > That's... Interesting... This commit was created via `./mach vendor rust` so > I guess the file changed on the crates.io side ? Is that even possible ? If I run ./mach vendor rust on a clean tree I don't see any changes to clap. If I pull your changes and run ./mach vendor rust on top of those it undoes the changes to clap (and also to the time crate). I suspect somehow your crates.io cache might be polluted?
Assignee | ||
Comment 16•6 years ago
|
||
mozreview-review |
Comment on attachment 8966432 [details] Bug 1429816 - Part 1: Bump syn and quote in xpcom. https://reviewboard.mozilla.org/r/235134/#review241004 Thanks! :-) ::: xpcom/rust/xpcom/xpcom_macros/src/lib.rs:238 (Diff revision 1) > + let value = if let Lit::Str(ref s) = attr.lit { > + s.value() > + } else { > + Err("Unexpected non-string value in #[refcnt]")? > + }; > + nit: unnecessary newline
Attachment #8966432 -
Flags: review?(nika) → review+
Assignee | ||
Comment 17•6 years ago
|
||
mozreview-review |
Comment on attachment 8966433 [details] Bug 1429816 - Part 2: Revendor dependencies. https://reviewboard.mozilla.org/r/235136/#review241006
Attachment #8966433 -
Flags: review?(nika) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 20•6 years ago
|
||
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5fa8c1abc6b6 Part 1: Bump syn and quote in xpcom. r=froydnj,mystor https://hg.mozilla.org/integration/autoland/rev/8ef553153fc0 Part 2: Revendor dependencies. r=froydnj,mystor
Comment 21•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5fa8c1abc6b6 https://hg.mozilla.org/mozilla-central/rev/8ef553153fc0
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•