817723 - Only build the stub installer when building x86 and the update channel equals nightly, aurora, beta, and release

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

[Robert Strong (they/them - no direct email)]

There have been a few complaints about the stub installer only downloading the nightly channel vs. nightly-elm, etc. and since it would be a pita to maintain entries in bouncer for all of the repos we should just enable building the stub installer when the update channel equals nightly on mozilla-central. We might want to do something similar for aurora, beta, and release but that isn't as much of an issue so this bug is only about m-c.

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1 - untested

Comment 2

[Robert Strong (they/them - no direct email)]

Note: for try builds we should be able to check in a change to confvars.sh to enable building the stub installer on m-c.

Comment 3

[Brian R. Bondy [:bbondy]]

Comment on attachment 687873 [details] [diff] [review]
patch rev1 - untested

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

Here's a snippet from channel checking we use in updater/Makefile.in for determining which cert to include inside the .rc file.

ifneq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
RCFLAGS += -DMAR_SIGNING_RELEASE_BETA=1
else
ifneq (,$(filter nightly aurora nightly-elm nightly-profiling nightly-oak,$(MOZ_UPDATE_CHANNEL)))
RCFLAGS += -DMAR_SIGNING_AURORA_NIGHTLY=1
endif
endif

Comment 4

[Robert Strong (they/them - no direct email)]

(In reply to Brian R. Bondy [:bbondy] from comment #3)
> Comment on attachment 687873 [details] [diff] [review]
> patch rev1 - untested
> 
> Review of attachment 687873 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Here's a snippet from channel checking we use in updater/Makefile.in for
> determining which cert to include inside the .rc file.
> 
> ifneq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
> RCFLAGS += -DMAR_SIGNING_RELEASE_BETA=1
> else
> ifneq (,$(filter nightly aurora nightly-elm nightly-profiling
> nightly-oak,$(MOZ_UPDATE_CHANNEL)))
> RCFLAGS += -DMAR_SIGNING_AURORA_NIGHTLY=1
> endif
> endif
Since confvars.sh doesn't get merged I was just going to go with the simple test on m-c. Is there any reason to go with the more inclusive test?

Comment 5

[Brian R. Bondy [:bbondy]]

that's fine, was just including it in case it was helpful.

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

Successful push to try and verified that the stub doesn't build on both win 32 and 64
https://tbpl.mozilla.org/?tree=Try&rev=a628da3c719d&noignore=1

Verifying locally that everything builds fine with the update channel set to nightly before requesting review.

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 688136 [details] [diff] [review]
patch rev2

Brian, could you sanity check the confvars.sh changes.

btw: I had to move MOZ_UPDATE_CHANNEL prior to confvars.sh in configure.in and I went with moving it to the existing location in js/src/configure.in

Comment 8

[Robert Strong (they/them - no direct email)]

catlee, juanb, jsmith: just a heads up about this upcoming change. If / when it lands, please let anyone else know that should know.

Comment 9

[Robert Strong (they/them - no direct email)]

Attachment: patch rev3

Comment 10

[Brian R. Bondy [:bbondy]]

Comment on attachment 688157 [details] [diff] [review]
patch rev3

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

Looks good to me as long as there are no objections from releng and as long as it passes try tests.

Comment 11

[Chris AtLee [:catlee]]

Please also include aurora, beta, release and esr117 here so that releng doesn't have to build stubs for these channels manually. Or perhaps building when official branding is enabled is sufficient (although that misses aurora).

Comment 12

[Robert Strong (they/them - no direct email)]

(In reply to Brian R. Bondy [:bbondy] from comment #10)
> Comment on attachment 688157 [details] [diff] [review]
> patch rev3
> 
> Review of attachment 688157 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me as long as there are no objections from releng and as long
> as it passes try tests.
It passes the l10n tests... any other tests you think it should pass?
https://tbpl.mozilla.org/?tree=Try&rev=a628da3c719d&noignore=1

Comment 13

[Robert Strong (they/them - no direct email)]

(In reply to Chris AtLee [:catlee] from comment #11)
> Please also include aurora, beta, release and esr117 here so that releng
> doesn't have to build stubs for these channels manually. Or perhaps building
> when official branding is enabled is sufficient (although that misses
> aurora).
catlee, this is in confvars.sh on m-c which doesn't build aurora, beta, release or esr117 without manual intervention. To build any of these channel's stub installer on m-c the nightly stub installer will first need to be built. Then branding files and channel name will need to be updated in the nightly directory as has been done previously to build these channels.

Comment 14

[Chris AtLee [:catlee]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> (In reply to Chris AtLee [:catlee] from comment #11)
> > Please also include aurora, beta, release and esr117 here so that releng
> > doesn't have to build stubs for these channels manually. Or perhaps building
> > when official branding is enabled is sufficient (although that misses
> > aurora).
> catlee, this is in confvars.sh on m-c which doesn't build aurora, beta,
> release or esr117 without manual intervention. To build any of these
> channel's stub installer on m-c the nightly stub installer will first need
> to be built. Then branding files and channel name will need to be updated in
> the nightly directory as has been done previously to build these channels.

As this code rides the trains, it's important that the stub is getting built properly by automation. If you want to update the merge day checklist to update this file on every uplift, that would be fine by me. We should not be planning on doing manual builds for beta/release stub installers in the long term.

Comment 15

[Brian R. Bondy [:bbondy]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #12)
> (In reply to Brian R. Bondy [:bbondy] from comment #10)
> > Comment on attachment 688157 [details] [diff] [review]
> > patch rev3
> > 
> > Review of attachment 688157 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Looks good to me as long as there are no objections from releng and as long
> > as it passes try tests.
> It passes the l10n tests... any other tests you think it should pass?
> https://tbpl.mozilla.org/?tree=Try&rev=a628da3c719d&noignore=1

Nope.  I was more concerned with something on releng side that would maybe look for the stub exe to sign it and then fail.  But maybe that code just looks for any exe and signs it, or maybe this is already covered by the confar variable. I don't think tbpl builds get signed anyway.  If you want to try it out though you can push to oak and do a nightly build there (via https://secure.pub.build.mozilla.org/buildapi/self-serve/oak ). It is a lvl3 repo so other exes would be signed there.  You'd know better than me though if that's not needed.

Comment 16

[Robert Strong (they/them - no direct email)]

(In reply to Chris AtLee [:catlee] from comment #14)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> > (In reply to Chris AtLee [:catlee] from comment #11)
> > > Please also include aurora, beta, release and esr117 here so that releng
> > > doesn't have to build stubs for these channels manually. Or perhaps building
> > > when official branding is enabled is sufficient (although that misses
> > > aurora).
> > catlee, this is in confvars.sh on m-c which doesn't build aurora, beta,
> > release or esr117 without manual intervention. To build any of these
> > channel's stub installer on m-c the nightly stub installer will first need
> > to be built. Then branding files and channel name will need to be updated in
> > the nightly directory as has been done previously to build these channels.
> 
> As this code rides the trains, it's important that the stub is getting built
> properly by automation. If you want to update the merge day checklist to
> update this file on every uplift, that would be fine by me. We should not be
> planning on doing manual builds for beta/release stub installers in the long
> term.
Unless I am badly mistaken, confvars.sh doesn't ride the trains (confvars.sh had to be updated manually in aurora, beta, and release and this is how we make ACCEPTED_MAR_CHANNEL_IDS and MAR_CHANNEL_ID train repo specifc... also, see comment #0) and if this is actually the case all other train repos will continue to build the stub installer unconditionally with this patch.

Comment 17

[(no longer active)]

(In reply to comment #16)
> (In reply to Chris AtLee [:catlee] from comment #14)
> > (In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> > > (In reply to Chris AtLee [:catlee] from comment #11)
> > > > Please also include aurora, beta, release and esr117 here so that releng
> > > > doesn't have to build stubs for these channels manually. Or perhaps building
> > > > when official branding is enabled is sufficient (although that misses
> > > > aurora).
> > > catlee, this is in confvars.sh on m-c which doesn't build aurora, beta,
> > > release or esr117 without manual intervention. To build any of these
> > > channel's stub installer on m-c the nightly stub installer will first need
> > > to be built. Then branding files and channel name will need to be updated in
> > > the nightly directory as has been done previously to build these channels.
> > 
> > As this code rides the trains, it's important that the stub is getting built
> > properly by automation. If you want to update the merge day checklist to
> > update this file on every uplift, that would be fine by me. We should not be
> > planning on doing manual builds for beta/release stub installers in the long
> > term.
> Unless I am badly mistaken, confvars.sh doesn't ride the trains.

AFAIK, it does.

Comment 18

[Chris AtLee [:catlee]]

> > As this code rides the trains, it's important that the stub is getting built
> > properly by automation. If you want to update the merge day checklist to
> > update this file on every uplift, that would be fine by me. We should not be
> > planning on doing manual builds for beta/release stub installers in the long
> > term.
> Unless I am badly mistaken, confvars.sh doesn't ride the trains (confvars.sh
> had to be updated manually in aurora, beta, and release and this is how we
> make ACCEPTED_MAR_CHANNEL_IDS and MAR_CHANNEL_ID train repo specifc... also,
> see comment #0) and if this is actually the case all other train repos will
> continue to build the stub installer unconditionally with this patch.

It does ride the trains, but is modified after each uplift. So if you want to add another step to the merge checklist to update this block of code, that's fine with releng as I indicated previously, and you should make sure relman is aware and OK with this change.

Comment 19

[Robert Strong (they/them - no direct email)]

Thanks for the info and sounds like a pita. ;)

I think I'll go with just adding them but what about release-test and friends?

Comment 20

[Robert Strong (they/them - no direct email)]

Attachment: patch rev4

catlee, will this suffice?

Comment 21

[Chris AtLee [:catlee]]

Comment on attachment 688399 [details] [diff] [review]
patch rev4

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

Looks good to me.

Builds are never built with "X-test" as the channel, so beta-test/release-test aren't a concern.

We'll need to update this file if we ever want to have stub installer support for ESR.

Comment 22

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec6a21cbe516

Comment 23

[Robert Strong (they/them - no direct email)]

Backed out :(
https://hg.mozilla.org/integration/mozilla-inbound/rev/6be47ffba716

Comment 24

[Robert Strong (they/them - no direct email)]

btw: this likely just needs a clobber but I want to verify that is the case first.

Comment 25

[Robert Strong (they/them - no direct email)]

Attachment: patch rev4 minus configure changes

Carrying forward r+

Comment 26

[Robert Strong (they/them - no direct email)]

Attachment: patch rev5 - configure changes

try run
https://tbpl.mozilla.org/?tree=Try&rev=b2186845498b

Comment 27

[Robert Strong (they/them - no direct email)]

Comment on attachment 690268 [details] [diff] [review]
patch rev5 - configure changes

Kyle, the move of MOZ_UPDATER broke the build so I split out the configure.in changes into a separate patch and just moved MOZ_UPDATE_CHANNEL to above where confvars.sh is called. The try runs look good. Thanks!
https://tbpl.mozilla.org/?tree=Try&rev=b2186845498b

Comment 28

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/4528be937f5c

Comment 29

[Robert Strong (they/them - no direct email)]

Attachment: combined patch as checked in

Comment 30

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4528be937f5c

Comment 31

[Alex Keybl [:akeybl]]

Comment on attachment 690624 [details] [diff] [review]
combined patch as checked in

Approving for Aurora, as this blocks the wonderful merge help found in bug 818333. The risk here is very manageable, given the testing that occurs over the aurora/beta merge.

Comment 32

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/90c64bebd52d

Comment 33

[Robert Strong (they/them - no direct email)]

Backed out but this one is safe to land on aurora

Comment 34

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/46ab2a290ac7