Enable MAR verification on linux32 and 64bit by default

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bbondy, Assigned: spohl)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(5 attachments, 12 obsolete attachments)

1.03 KB, patch
spohl
: review+
Details | Diff | Splinter Review
5.17 KB, patch
spohl
: review+
Details | Diff | Splinter Review
13.66 KB, patch
spohl
: review+
Details | Diff | Splinter Review
8.08 KB, patch
spohl
: review+
Details | Diff | Splinter Review
1.27 KB, patch
nthomas
: review+
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
The work to get multi platform MAR verification was done in Bug 1158866.

This bug is simply for defining the best place possible to define MOZ_VERIFY_MAR_SIGNATURE.  We only want to define it where MARs will be signed and we don't want to effect third parties that use our code as much as possible.
(Reporter)

Comment 1

3 years ago
Created attachment 8598061 [details] [diff] [review]
Option 1: Enable MAR verification everywhere
(Reporter)

Updated

3 years ago
Summary: Define MOZ_VERIFY_MAR_SIGNATURE to enable MAR verification on Linux → Enable MAR verification on linux32 and 64bit by default
(Reporter)

Comment 2

3 years ago
Created attachment 8598391 [details] [diff] [review]
Option 2: Enable on linux only

I think this is the one that we should go with. We can enable in other places when needed.

Recall from nthomas:
> To capture opt on-push and nightly builds, and debug on-push builds, the 
> top-level mozconfigs are 
>  browser/config/mozconfigs/linux32/nightly
>  browser/config/mozconfigs/linux32/debug
>  browser/config/mozconfigs/linux64/nightly
>  browser/config/mozconfigs/linux64/debug
> AFAIK no-one else is using those, but you never can tell what people copy. If 
> you want the address sanitiser builds too then add *asan*, and there are 
> other build types to check too.
>
> Those top 4 end up sourcing other mozconfigs, but all use 
> build/unix/mozconfig.linux at some point. Except, so do b2g, xulrunner, and
> l10n (possibly harmlessly!). To cover all platforms you may wish to use
> build/mozconfig.common.
Attachment #8598391 - Flags: review?(robert.strong.bugs)
Comment on attachment 8598391 [details] [diff] [review]
Option 2: Enable on linux only

For consistency please move the enabling of the other platforms to the mozconfig files. This should be reviewed by bhearsum and / or nthomas.
Attachment #8598391 - Flags: review?(robert.strong.bugs)
(Reporter)

Updated

3 years ago
Attachment #8598061 - Attachment is obsolete: true
(Reporter)

Comment 4

3 years ago
Created attachment 8598400 [details] [diff] [review]
Patch v2
Attachment #8598391 - Attachment is obsolete: true
Attachment #8598400 - Flags: review?(nthomas)
Attachment #8598400 - Flags: feedback?(robert.strong.bugs)
Comment on attachment 8598400 [details] [diff] [review]
Patch v2

Looks good. I think this covers all applicable cases but it would be a good thing to verify no platforms that should run the tests, etc. are overlooked.
Attachment #8598400 - Flags: feedback?(robert.strong.bugs) → feedback+
Would also e good to ask nthomas / bhearsum if they know of a way to protect distros etc. from using our certs accidentally.
Do you think it would be worthwhile to test Linux on oak for a few days?
(Reporter)

Comment 8

3 years ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #7)
> Do you think it would be worthwhile to test Linux on oak for a few days?

I've had a successful nightly oak update on Ubuntu. But will want to make sure I have another with new changes and that everything is green.
(Reporter)

Comment 9

3 years ago
Created attachment 8599040 [details] [diff] [review]
Patch v3.

Builds were failing on oak due to a mozconfig check failure. I think adding it to the whitelist is the way to fix that. 

This does make me concerned though that this only enables things on nightly. And so the current patch would take away win/osx once uplift happens for new versions.  Maybe there's some kind of process that handles that uplift mozconfig settings though. Please advise.

Should I be adding similar  --enable-verify-mar options in the beta and release mozconfig?  Does aurora (webdev) use nightly's mozconfig?
Attachment #8598400 - Attachment is obsolete: true
Attachment #8598400 - Flags: review?(nthomas)
Attachment #8599040 - Flags: review?(nthomas)
(In reply to Brian R. Bondy [:bbondy] from comment #9)
> Builds were failing on oak due to a mozconfig check failure. I think adding
> it to the whitelist is the way to fix that. 
>
> This does make me concerned though that this only enables things on nightly.
> And so the current patch would take away win/osx once uplift happens for new
> versions.  Maybe there's some kind of process that handles that uplift
> mozconfig settings though. Please advise.
> 
> Should I be adding similar --enable-verify-mar options in the beta and
> release mozconfig?  Does aurora (webdev) use nightly's mozconfig?

The best process is for a developer to modify all the mozconfigs at the time a new feature lands, ie also the 'beta' and release' files you've mentioned. At which point you shouldn't need the whitelist change any more. I wish I had a better solution than adding it in so many places, maybe mshal has some ideas given he has worked on these configs.

Aurora uses 'nightly' for opt builds too (it's nightly in the sense of once-a-day, rather than Nightly the branding; but then it gets use for dep opt builds too. Naming is hard, lets go shopping).

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #6)
> Would also be good to ask nthomas / bhearsum if they know of a way to protect
> distros etc. from using our certs accidentally.

Do you mean a scenario where a someone builds with MOZ_VERIFY_MAR_SIGNATURE set, then the updater will only accept mar files which just Mozilla can produce ? This is extra fun because the problem isn't apparent until you wish to update to a 2nd build.  If so, Linux distros would very likely not care due to their own packaging infrastructure, but a fork/downstream project might. AFAIK there is no cast-iron way to prevent this. We've had env variables like MOZILLA_OFFICIAL, and more recently MOZ_AUTOMATION, but I doubt you can rely on people not setting that.
Comment on attachment 8599040 [details] [diff] [review]
Patch v3.

r- because of the problems you noticed yourself. You'll need someone else to review the browser/confvars.sh part too.
Attachment #8599040 - Flags: review?(nthomas) → review-
(Reporter)

Comment 12

3 years ago
Thanks, I guess I'll remove the whitelisting and add to beta/release as well.
(In reply to Nick Thomas [:nthomas] from comment #10)
> (In reply to Brian R. Bondy [:bbondy] from comment #9)
> > Builds were failing on oak due to a mozconfig check failure. I think adding
> > it to the whitelist is the way to fix that. 
> >
> > This does make me concerned though that this only enables things on nightly.
> > And so the current patch would take away win/osx once uplift happens for new
> > versions.  Maybe there's some kind of process that handles that uplift
> > mozconfig settings though. Please advise.
> > 
> > Should I be adding similar --enable-verify-mar options in the beta and
> > release mozconfig?  Does aurora (webdev) use nightly's mozconfig?
> 
> The best process is for a developer to modify all the mozconfigs at the time
> a new feature lands, ie also the 'beta' and release' files you've mentioned.
> At which point you shouldn't need the whitelist change any more. I wish I
> had a better solution than adding it in so many places, maybe mshal has some
> ideas given he has worked on these configs.

The most recent changes I've been making to mozconfigs have generally been done in the common mozconfigs, so beta+release pull those in automatically. For example, you may just want to add --enable-verify-mar to build/unix/mozconfig.linux to cover the linux platforms. This would get beta+release as well, though I don't know if it would enable it for any platforms you'd like to specifically exclude. Other platforms have similar common files.
(Reporter)

Comment 14

3 years ago
Created attachment 8600213 [details] [diff] [review]
Patch v4.

For the confvars.sh change it's just a removal of the same options being set in the mozconfigs now. rstrong already feedback+'ed that so I'll imply an r+ for that portion of the patch.
Attachment #8599040 - Attachment is obsolete: true
Attachment #8600213 - Flags: review?(nthomas)
Sorry to have neglected the review here. I realized that we may have to consider the l10n builds as well, although it's a bit weird because we're not compiling or running tests there. Does --enable-verify-mar have any effect in the l10n case ? Maybe not because we're already adding the channel id to the mar (by way of using the mar utility from the en-US build), and signing is also the same.
(Reporter)

Comment 16

3 years ago
> Does --enable-verify-mar have any effect in the l10n case ?

I'm not sure if the l10n case is a repack or a full rebuild of the binaries. It only has effects on the updater binary itself. If specified it'll verify the MAR file's signature before extracting it.  Robert may know better than me about the l10n case.
Comment on attachment 8600213 [details] [diff] [review]
Patch v4.

I think l10n should be OK, we download mar/mbsdiff from the matching en-US build where the mozconfig is set. r+ on the mozconfig part.
Attachment #8600213 - Flags: review?(nthomas) → review+
(Reporter)

Comment 18

3 years ago
Comment on attachment 8600213 [details] [diff] [review]
Patch v4.

Thx, going to remove the marking so no one accidentally lands for now until enabled on oak and tested more there.
Attachment #8600213 - Flags: review+
Brian, are you going to land this or is there more to be done?
Flags: needinfo?(netzen)
(Reporter)

Comment 21

3 years ago
I think there was some a mozconfig check or something like that failing for some reason.  Probably pretty small but I'm going to unassign myself for now.  All of the platform work is done and passing tests it's just a matter of mozconfigs.
Assignee: netzen → nobody
Flags: needinfo?(netzen)
Since mar signing isn't available on gonk I'm tempted to change
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsUpdateDriver.cpp#405 and other instances in this file

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#118 and other instances in this file

#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
to
#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX) && !defined(MOZ_WIDGET_GONK)

When they have mar signing and if they can use that code path then it can be removed.

Also, not sure if the moconfig changes I made are acceptable but I got the mozconfig check failure to pass on oak. I also enabled marVersionDowngrade.js and marWrongChannel.js app update xpcshell tests
(Assignee)

Comment 23

3 years ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #22)
> Since mar signing isn't available on gonk I'm tempted to change
> http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsUpdateDriver.
> cpp#405 and other instances in this file
> 
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/
> updater.cpp#118 and other instances in this file
> 
> #if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) &&
> !defined(XP_MACOSX)
> to
> #if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) &&
> !defined(XP_MACOSX) && !defined(MOZ_WIDGET_GONK)
> 
> When they have mar signing and if they can use that code path then it can be
> removed.

Would you like me to go ahead and make this change as part of this patch?

> Also, not sure if the moconfig changes I made are acceptable but I got the
> mozconfig check failure to pass on oak. I also enabled
> marVersionDowngrade.js and marWrongChannel.js app update xpcshell tests

Have these mozconfig changes landed everywhere, or will I need to land those as well?
Flags: needinfo?(robert.strong.bugs)
(In reply to Stephen A Pohl [:spohl] from comment #23)
> (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from
> comment #22)
> > Since mar signing isn't available on gonk I'm tempted to change
> > http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsUpdateDriver.
> > cpp#405 and other instances in this file
> > 
> > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/
> > updater.cpp#118 and other instances in this file
> > 
> > #if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) &&
> > !defined(XP_MACOSX)
> > to
> > #if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) &&
> > !defined(XP_MACOSX) && !defined(MOZ_WIDGET_GONK)
> > 
> > When they have mar signing and if they can use that code path then it can be
> > removed.
> 
> Would you like me to go ahead and make this change as part of this patch?
Yes. If possible get bbondy's input.

> 
> > Also, not sure if the moconfig changes I made are acceptable but I got the
> > mozconfig check failure to pass on oak. I also enabled
> > marVersionDowngrade.js and marWrongChannel.js app update xpcshell tests
> 
> Have these mozconfig changes landed everywhere, or will I need to land those
> as well?
They have only landed on oak. You'll need to get review on them from releng (perhaps nthomas).
Flags: needinfo?(robert.strong.bugs)
(Assignee)

Updated

3 years ago
Assignee: nobody → spohl.mozilla.bugs
(Assignee)

Comment 25

3 years ago
Created attachment 8630638 [details] [diff] [review]
Patch v4.

Updated for current trunk. Carrying over r+ by nthomas.
Attachment #8600213 - Attachment is obsolete: true
Attachment #8630638 - Flags: review+
(Assignee)

Comment 26

3 years ago
Created attachment 8630644 [details] [diff] [review]
Fix mozconfig check failures

These are rstrong's mozconfig changes to fix the mozconfig check failures (discussed at the bottom of comment 22 and comment 24).
Attachment #8630644 - Flags: review?(nthomas)
Note that I made these changes without any thought behind them beyond getting things building.

spohl, it would be a good thing to verify that oak nightly builds update without any issues on Linux since it has been a while since bbondy verified them.
(Assignee)

Comment 28

3 years ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #27)
> spohl, it would be a good thing to verify that oak nightly builds update
> without any issues on Linux since it has been a while since bbondy verified
> them.

Yes, I was thinking I'm getting everything that we have so far into patches and reviewed. If testing shows that we need to make additional changes, I was going to capture it in a separate patch before landing.
(Assignee)

Comment 29

3 years ago
Created attachment 8630651 [details] [diff] [review]
Enable tests

Enable marVersionDowngrade.js and marWrongChannel.js on Windows and OSX, disable for gonk.
Attachment #8630651 - Flags: review?(robert.strong.bugs)
Comment on attachment 8630651 [details] [diff] [review]
Enable tests

nit: fix the comment so it states "Enable marVersionDowngrade.js and marWrongChannel.js on all platforms except gonk
Attachment #8630651 - Flags: review?(robert.strong.bugs) → review+
(Assignee)

Comment 31

3 years ago
Created attachment 8630654 [details] [diff] [review]
3. Enable tests

Thanks! Fixed comment, carrying over r+.
Attachment #8630651 - Attachment is obsolete: true
Attachment #8630654 - Flags: review+
BTW: not sure but Thunderbird and SeaMonkey might need some build tweaking or it might be possible to just enable it for the test updater for those tests to pass.
(Assignee)

Comment 33

3 years ago
Created attachment 8630687 [details] [diff] [review]
Updater changes

These are the changes discussed at the top of comment 22 and comment 24.
Attachment #8630687 - Flags: review?(netzen)
(Reporter)

Comment 34

3 years ago
Comment on attachment 8630687 [details] [diff] [review]
Updater changes

Review of attachment 8630687 [details] [diff] [review]:
-----------------------------------------------------------------

Those ifdefs are mainly because the functionality is implemented in a different way on those platforms.
It'd be even better if we can fail the build when MOZ_VERIFY_MAR_SIGNATURE is defined w/ GONK so there is no failed expectations, but I'm ok with this.
Attachment #8630687 - Flags: review?(netzen) → review+
(Assignee)

Comment 35

3 years ago
I updated from the oak-nightly after enabling MAR verification [0][1] to the latest one available at the moment[2][3]. Robert, does this cover what you wanted me to test?

Would you like me to push attachment 8630687 [details] [diff] [review] to oak and test again?

[0] https://hg.mozilla.org/projects/oak/rev/5b078c9ba78a
[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/05/2015-05-03-04-02-15-oak/
[2] https://hg.mozilla.org/projects/oak/rev/8c72a8f8896c
[3] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/07/2015-07-09-04-02-06-oak/
Flags: needinfo?(robert.strong.bugs)
Updating from May to July is a good thing but that uses the updater code from May. It would be a good thing to update from a July build to a July build as well to verify changes to the updater haven't broken the process.

I'll go ahead and push that patch to oak as well.
Flags: needinfo?(robert.strong.bugs)
(Assignee)

Comment 39

3 years ago
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #38)
> https://hg.mozilla.org/projects/oak/rev/9b39451ccbe2

The update from https://hg.mozilla.org/projects/oak/rev/8c72a8f8896c to the one mentioned above was successful as well. It looks like we're in good shape.
Comment on attachment 8630644 [details] [diff] [review]
Fix mozconfig check failures

Review of attachment 8630644 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/config/mozconfigs/win32/beta
@@ +3,5 @@
>  
>  mk_add_options MOZ_PGO=1
>  
>  ac_add_options --enable-official-branding
> +ac_add_options --enable-signmar

Could you clarify why this is being added here ? Looks like browser/confvars.sh sets MOZ_ENABLE_SIGNMAR=1 unconditionally, and our the windows on release isn't busted despite it not being set in win32/release.

I'd rather see a patch to remove --enable-signmar from everything in browser/configs/mozconfigs/, including the whitelist. See build/compare-mozconfig/compare-mozconfigs-wrapper.py for hints on how to run the comparison locally.
Attachment #8630644 - Flags: review?(nthomas) → review-
I'm assuming you've looked at again at the codecoverage, valgrind, and asan builds and confirmed there's no point turning on verification there. Are the updater tests all in xpcshell ? Looks like we do run those asan builds.
(In reply to Nick Thomas [:nthomas] from comment #40)
> Comment on attachment 8630644 [details] [diff] [review]
> Fix mozconfig check failures
> 
> Review of attachment 8630644 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/config/mozconfigs/win32/beta
> @@ +3,5 @@
> >  
> >  mk_add_options MOZ_PGO=1
> >  
> >  ac_add_options --enable-official-branding
> > +ac_add_options --enable-signmar
> 
> Could you clarify why this is being added here ?
Adding those made make check pass on Windows. 

> Looks like
> browser/confvars.sh sets MOZ_ENABLE_SIGNMAR=1 unconditionally, and our the
> windows on release isn't busted despite it not being set in win32/release.
> 
> I'd rather see a patch to remove --enable-signmar from everything in
> browser/configs/mozconfigs/, including the whitelist. See
> build/compare-mozconfig/compare-mozconfigs-wrapper.py for hints on how to
> run the comparison locally.
Looks like bug 730862 added those.

spohl, could you try to figure out if you can get make check to pass without adding those and make sure that these changes don't break bug 730862 and repackaging.
(In reply to Nick Thomas [:nthomas] from comment #41)
> I'm assuming you've looked at again at the codecoverage, valgrind, and asan
> builds and confirmed there's no point turning on verification there. Are the
> updater tests all in xpcshell ? Looks like we do run those asan builds.
The updater tests are also in mochitest-other and could be added to other suites (I've considered adding them to mochitest-browser previously). Also, asan runs xpcshell.

Please provide guidance on what builds you'd like them excluded on giving the above and how  to disable this on those builds.
spohl, if getting the mozconfig files to pass the make check continues to be a pita let's just go with changing confvars.sh and file a new build config bug to get it moved to the the mozconfig files.
Actually, let's just go with defining it in confvars.sh and file a Firefox build config bug to decide whether or not it would be better to move these into the mozconfig files. If there are 3rd parties that have issues with the build due to it being defined in confvars.sh those bugs can be duped to it and if it is decided to move them then figuring out why the compare script was failing can be figured out in that bug. Getting mar signing on Linux has been held up long enough and there are way too many other things that need to be worked on.
Flags: needinfo?(nthomas)
Created attachment 8632005 [details] [diff] [review]
Remove --enable-signmar

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #42)
> (In reply to Nick Thomas [:nthomas] from comment #40)
> > I'd rather see a patch to remove --enable-signmar from everything in
> > browser/configs/mozconfigs/, including the whitelist. See
> > build/compare-mozconfig/compare-mozconfigs-wrapper.py for hints on how to
> > run the comparison locally.
> Looks like bug 730862 added those.

Fair enough; later bug 903135 changed it. This patch is all I had mind, on top of oak rev 9646b3cdbb91. It passes the mozconfig checks locally, and I don't think it's going to make any difference to an actual build. If you would rather to do this in a followup that would be fine too.

Please note I'm not advocating enabling verification in confvars.sh.
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #43)
> Please provide guidance on what builds you'd like them excluded on giving
> the above and how  to disable this on those builds.

Sorry, not trying to be difficult here. You know where the tests run and what coverage you are happy with. Lets just leave it at opt and debug.
(Assignee)

Comment 49

3 years ago
Created attachment 8632192 [details] [diff] [review]
Patch

This patch addresses comment 46.
Attachment #8630638 - Attachment is obsolete: true
Attachment #8630644 - Attachment is obsolete: true
Attachment #8632192 - Flags: review?(nthomas)
Comment on attachment 8632192 [details] [diff] [review]
Patch

You'll need someone in https://wiki.mozilla.org/Modules/All#Firefox for this review.
Attachment #8632192 - Flags: review?(nthomas)
(Assignee)

Comment 51

3 years ago
Comment on attachment 8632192 [details] [diff] [review]
Patch

Robert, please feel free to forward the review request as needed. Thanks!
Attachment #8632192 - Flags: review?(robert.strong.bugs)
Comment on attachment 8632005 [details] [diff] [review]
Remove --enable-signmar

Rotted.
Attachment #8632005 - Attachment is obsolete: true
(Assignee)

Comment 53

3 years ago
Comment on attachment 8630638 [details] [diff] [review]
Patch v4.

After discussion with nthomas, it seems best to go back to this approach. Oak is green with these changes (see https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ebbcf47b16f).
Attachment #8630638 - Attachment is obsolete: false
(Assignee)

Updated

3 years ago
Attachment #8632192 - Attachment is obsolete: true
Attachment #8632192 - Flags: review?(robert.strong.bugs)
(Assignee)

Comment 54

3 years ago
(In reply to Stephen A Pohl [:spohl] from comment #53)
> Oak is green with these changes

s/Oak/Try/
(Assignee)

Comment 55

3 years ago
Created attachment 8634091 [details] [diff] [review]
2. Updater changes

This includes two minor whitespace fixes that landed on oak. Carrying over r+.
Attachment #8630687 - Attachment is obsolete: true
Attachment #8634091 - Flags: review+
(Assignee)

Comment 56

3 years ago
Created attachment 8634104 [details] [diff] [review]
1. Patch v4.

Updated for current trunk.
Attachment #8630638 - Attachment is obsolete: true
Attachment #8634104 - Flags: review+
(Assignee)

Comment 57

3 years ago
Created attachment 8634107 [details] [diff] [review]
Remove --enable-signmar from mozconfigs

Nick, I've updated your patch[1] to apply to current trunk on top of the other three patches in this bug. Could you advise who could/should review this? Thanks!

[1] https://hg.mozilla.org/try/raw-rev/f6a6d3390ce4
Flags: needinfo?(nthomas)
Comment on attachment 8634107 [details] [diff] [review]
Remove --enable-signmar from mozconfigs

lgtm
Flags: needinfo?(nthomas)
Attachment #8634107 - Flags: review+
(Assignee)

Comment 60

3 years ago
Created attachment 8634497 [details] [diff] [review]
4. Remove --enable-signmar from mozconfigs

Updated commit message. Carrying over r+.
Attachment #8634107 - Attachment is obsolete: true
Attachment #8634497 - Flags: review+
(Assignee)

Updated

3 years ago
Attachment #8634104 - Attachment description: Patch v4. → 1. Patch v4.
(Assignee)

Updated

3 years ago
Attachment #8634091 - Attachment description: Updater changes → 2. Updater changes
(Assignee)

Updated

3 years ago
Attachment #8630654 - Attachment description: Enable tests → 3. Enable tests
(Assignee)

Comment 61

3 years ago
Created attachment 8634503 [details] [diff] [review]
5. Fix 'make check' failures on Windows

This followup was necessary to make 'make check' happy on Windows.

Try run without this patch (showing the failures): https://treeherder.mozilla.org/#/jobs?repo=try&revision=da91cc9a42ac
Try run with this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3cf947510b89

Nick, I had a look at all the failures in this second try run and couldn't identify any that were related to the changes here. Is that right, or did I miss anything?
Attachment #8634503 - Flags: review?(nthomas)
Attachment #8634503 - Flags: review?(nthomas) → review+
The comparison is using a diff, so ordering matters. :-S   None of the other failures seems related to this work.
(Assignee)

Comment 63

3 years ago
Robert, does this look ready to go to you? Shall we land and test on oak again before landing?
Flags: needinfo?(robert.strong.bugs)
Pushed to oak
Flags: needinfo?(robert.strong.bugs)
(Assignee)

Comment 65

3 years ago
Looks like we're running into signing issues for our nightlies:
https://treeherder.mozilla.org/#/jobs?repo=oak&revision=29457cebfc44
(Assignee)

Comment 66

3 years ago
So far, I've been unable to narrow down the cause of the failures. At least on linux we seem to be failing the signing step of make-sdk, but I'm not sure why signing is failing. Nick, do you know if there are additional log files on the signing server that could help us here?
Flags: needinfo?(nthomas)
Sorry, you were bitten by bug 1139929. I suggest you rebuild the (desktop) nightly jobs on 08f8bc16527c using treeherder.
Flags: needinfo?(nthomas)
I've updated oak to latest m-c so it will kick off a new set of nightly builds tonight.
(Assignee)

Comment 69

3 years ago
Thanks, Nick and Robert! The updates from 7/15 to 7/17 and 7/17 to 7/20 oak nightly were successful. Shall we merge m-c to oak one last time and run through one more update, or are we ready to land at this point?
Flags: needinfo?(robert.strong.bugs)
Should be safe to land
Flags: needinfo?(robert.strong.bugs)
You need to log in before you can comment on or make changes to this bug.