1472538 - Update bindgen.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1472538: Update bindgen.

Surprisingly painless.

I'll update to 0.37.4 instead of .3, but that only contains
https://github.com/rust-lang-nursery/rust-bindgen/pull/1343 on top to prevent
build warnings.

Review commit: https://reviewboard.mozilla.org/r/254128/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254128/

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1472538: Revendor dependencies.

Review commit: https://reviewboard.mozilla.org/r/254130/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254130/

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8989047 [details]
Bug 1472538: Update bindgen.

https://reviewboard.mozilla.org/r/254128/#review260846

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e71ff3c9bd3
Update bindgen. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad59c8bf30b4
Revendor dependencies.

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/2e71ff3c9bd3
https://hg.mozilla.org/mozilla-central/rev/ad59c8bf30b4

Comment 7

[Charles Robertson]

Here at SUSE we have encountered this exact same bug running Firefox 60 ESR as described in https://bugzilla.mozilla.org/show_bug.cgi?id=1462623 (and fixed in this bug) but on a s390x IBM (big-endian) machine. 

I am asking you to push/uplift this fix to the ESR branch and make it available for Firefox 60.3 ESR when it is released in October.

We have "BIG" customers waiting for a clean build on s390x and so far no ESR 60 versions of Firefox will run on a big-endian
machine without crashing due to this bug.

Thank you!

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Rebased patch for esr60.

Yeah, I think uplifting this is reasonable. I'll request it.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Firefox is unusable without this patch in big-endian machines (see bug 1462623).
User impact if declined: Crashes at startup in tier-3 platforms.
Fix Landed on Version: 63
Risk to taking this patch (and alternatives if risky): I think it's not very risky.

This updates a build-time-only dependency, which is maintained by us (by me mostly, in fact), and this particular version we've been using since FF63, so it's pretty well tested (and again, it's build-time only).

Note that the patch looks way bigger and more complex than what it really is (the patch is mostly https://hg.mozilla.org/mozilla-central/rev/2e71ff3c9bd3) because I merged the automated dependency update / vendored code update in the patch (the result of ./mach vendor rust after applying the previous patch and resolving conflicts). I could try to make that part a bit more minimal if wanted / needed.

String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9011149 [details] [diff] [review]
Rebased patch for esr60.

This looks mildly terrifying, but I'll take your word for it that it's not as bad as it looks. Fixes crashes for some Tier 3 platforms, approved for ESR 60.3.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/11462f2b98f2

Comment 11

[Federico Mena Quintero]

One thing I had to do to make esr60 build with the rebased patch is to update just the cc crate and re-vendor it.  Otherwise it was creating the argv incorrectly, passing both the program name and the first argument in argv[0] - something like "gcc -first-option" "-second-option" "etc".  Emilio mentioned that he had seen this bug before.

I'm not sure if that cc update should go into the same rebased patch - I'm not sure if the patch is "update just bindgen and its dependencies", or "update all the crates that need bugfixes in esr60" :)

Comment 12

[Emilio Cobos Álvarez (:emilio)]

(In reply to Federico Mena Quintero from comment #11)
> One thing I had to do to make esr60 build with the rebased patch is to
> update just the cc crate and re-vendor it.  Otherwise it was creating the
> argv incorrectly, passing both the program name and the first argument in
> argv[0] - something like "gcc -first-option" "-second-option" "etc".  Emilio
> mentioned that he had seen this bug before.

That was bug 1445528. Probably you should request an uplift of that one if you need it. It's build-time only, so should be pretty safe as well.

Comment 13

[Federico Mena Quintero]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)
> That was bug 1445528. Probably you should request an uplift of that one if
> you need it. It's build-time only, so should be pretty safe as well.

Thanks, I'll request an uplift there.