Closed Bug 1393162 Opened 4 years ago Closed 4 years ago

PGO and Valgrind builds are going to be broken when Gecko 57 merges to Beta on 2017-09-20

Categories

(Firefox Build System :: General, defect)

Unspecified
All
defect
Not set
blocker

Tracking

(firefox57- verified)

VERIFIED FIXED
mozilla57
Tracking Status
firefox57 - verified

People

(Reporter: RyanVM, Unassigned)

Details

Attachments

(2 files)

[Tracking Requested - why for this release]: Broken builds on the next merge day.

This affects both Linux and Windows, where PGO is the default for builds on Beta. Looking at the logs, it appears that quitter.xpi is being blocked and as a result, the browser can't shut down after the profiling run until the run eventually times out.

https://public-artifacts.taskcluster.net/WnfamXPZRreQpUCn0Dpkvg/0/public/logs/live_backing.log

INFO -  1503504724460	addons.xpi	WARN	disabling legacy extension quitter@mozilla.org
Flags: needinfo?(aswan)
Affects Valgrind too (same problem).
https://public-artifacts.taskcluster.net/fwKO3-MoQheDihStzqmZuA/0/public/logs/live_backing.log
Summary: PGO builds are going to be broken when Gecko 57 merges to Beta on 2017-09-20 → PGO and Valgrind builds are going to be broken when Gecko 57 merges to Beta on 2017-09-20
Andy, the expedient fix is to just sign the quitter extension with "Mozilla Extensions" and that looks to be safe (the extension just adds a content-accessible api for quitting the browser).  I don't know what the actual process is for getting it signed, can you get it signed?  (actually there's an xpi checked into the tree but people have been updating the sources without updating the xpi so updating to the current sources would be a nice extra step)
Flags: needinfo?(aswan) → needinfo?(amckay)
Having a content accessible API to quit Firefox doesn't sound great, since anyone could install this and get that API in their Firefox and we haven't really got a good way to prevent its distribution. However it seems kinda pointless, I can't spot many attack vectors here.

If you can attach an .xpi to this bug, we can get Jason to sign it. If there's more to do we might want to consider setting up someone like Ryan up with API access to do this on a regular basis. Details on that can be found in the mana page from https://wiki.mozilla.org/Add-ons/InternalSigning

We should probably file a bug to remove quitter from the tree and find a work around for this.
Flags: needinfo?(amckay)
I don't know why there's an xpi in the source tree, but it's not what's used, the source is. And we can't sign the xpi that was just built from there, because that won't work for downstreams.
(In reply to Mike Hommey [:glandium] from comment #4)
> I don't know why there's an xpi in the source tree, but it's not what's
> used, the source is. And we can't sign the xpi that was just built from
> there, because that won't work for downstreams.

I assumed the checked-in xpi is what is used since it is signed.  If we build another xpi from the sources, how does it get loaded on beta/release during PGO builds?

(In reply to Andy McKay [:andym] from comment #3)
> We should probably file a bug to remove quitter from the tree and find a
> work around for this.

I didn't think too hard about this but I don't see an obvious work around, and its not like we have a lot of time to do new work.  I also don't think quitter is a big problem, if an "attacker" tricks somebody into installing it and then tricks them into visiting a page that uses the API, that would be annoying but not harmful.  I would worry more about some exploitable bug in quitter, I'm not very well-trained at looking for these but its code is pretty simple, it looks safe to me.
I looked more closely, and we do use the xpi for pgo profile and valgrind, which makes me cringe. The addon being signed is not going to make it less a legacy addon, too...
(In reply to Mike Hommey [:glandium] from comment #6)
> The addon being signed is not going to make it less a
> legacy addon, too...

Well being signed with "Mozilla Extensions" will.
Jason, can you please have the xpi from this attachment signed with "Mozilla Extensions"
Flags: needinfo?(jthomas)
For the short-term fix to unbreak tests for beta it'd be good to just get a signed xpi. For a longer-term fix we should just change these tests to use marionette to start and shutdown the browser.
Please see attached.
Flags: needinfo?(jthomas)
Comment on attachment 8900724 [details]
quitter@mozilla.org.xpi signed

I've confirmed on Try that this gets PGO and Valgrind builds working again.
Attachment #8900724 - Flags: feedback+
https://hg.mozilla.org/mozilla-central/rev/4d4296e65042
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Status: RESOLVED → VERIFIED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.