799902 - need pretty branding for stub installer (file naming)

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

[[e-mail] (:bhearsum)]

In order to properly ship the stub installer when it rides the trains to Beta we need it to have support for pretty branding. Bug 795982 added basic support to the packaging code, but I think we should make it a full citizen and give it package-name.mk support for pretty and non-pretty packaging.

Comment 1

[Jason Smith [:jsmith]]

(In reply to Ben Hearsum [:bhearsum] from comment #0)
> In order to properly ship the stub installer when it rides the trains to
> Beta we need it to have support for pretty branding. Bug 795982 added basic
> support to the packaging code, but I think we should make it a full citizen
> and give it package-name.mk support for pretty and non-pretty packaging.

I take it this is a blocker shipping the English deployment of the stub installer then, right?

Comment 2

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

What is pretty branding?

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Jason Smith [:jsmith] from comment #1)
> (In reply to Ben Hearsum [:bhearsum] from comment #0)
> > In order to properly ship the stub installer when it rides the trains to
> > Beta we need it to have support for pretty branding. Bug 795982 added basic
> > support to the packaging code, but I think we should make it a full citizen
> > and give it package-name.mk support for pretty and non-pretty packaging.
> 
> I take it this is a blocker shipping the English deployment of the stub
> installer then, right?

Nope, this is only a blocker for when we hit Beta.

(In reply to Robert Strong [:rstrong] (do not email) from comment #2)
> What is pretty branding?

Pretty branding is generating the file names in their final format -- that is the directory layout + filenames that will be actually shipped: https://github.com/mozilla/mozilla-central/blob/master/toolkit/mozapps/installer/package-name.mk#L78, https://bugzilla.mozilla.org/show_bug.cgi?id=453840

Comment 4

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

Thanks and I get it now. :)

Comment 5

[Jason Smith [:jsmith]]

(In reply to Ben Hearsum [:bhearsum] from comment #3)
> (In reply to Jason Smith [:jsmith] from comment #1)
> > (In reply to Ben Hearsum [:bhearsum] from comment #0)
> > > In order to properly ship the stub installer when it rides the trains to
> > > Beta we need it to have support for pretty branding. Bug 795982 added basic
> > > support to the packaging code, but I think we should make it a full citizen
> > > and give it package-name.mk support for pretty and non-pretty packaging.
> > 
> > I take it this is a blocker shipping the English deployment of the stub
> > installer then, right?
> 
> Nope, this is only a blocker for when we hit Beta.

I don't understand. If we need this for a beta version of the stub installer, then it would block. Can you clarify?

Comment 6

[[e-mail] (:bhearsum)]

(In reply to Jason Smith [:jsmith] from comment #5)
> (In reply to Ben Hearsum [:bhearsum] from comment #3)
> > (In reply to Jason Smith [:jsmith] from comment #1)
> > > (In reply to Ben Hearsum [:bhearsum] from comment #0)
> > > > In order to properly ship the stub installer when it rides the trains to
> > > > Beta we need it to have support for pretty branding. Bug 795982 added basic
> > > > support to the packaging code, but I think we should make it a full citizen
> > > > and give it package-name.mk support for pretty and non-pretty packaging.
> > > 
> > > I take it this is a blocker shipping the English deployment of the stub
> > > installer then, right?
> > 
> > Nope, this is only a blocker for when we hit Beta.
> 
> I don't understand. If we need this for a beta version of the stub
> installer, then it would block. Can you clarify?

Sorry. I'm talking about when the stub installer code actually hits Beta -- which is at 18.0b1. The Beta stub installer builds we've done already are hacky one-off things. This bug is required for it to be properly integrated with our release automation.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Ben Hearsum [:bhearsum] from comment #6)
> (In reply to Jason Smith [:jsmith] from comment #5)
> > (In reply to Ben Hearsum [:bhearsum] from comment #3)
> > > (In reply to Jason Smith [:jsmith] from comment #1)
> > > > (In reply to Ben Hearsum [:bhearsum] from comment #0)
> > > > > In order to properly ship the stub installer when it rides the trains to
> > > > > Beta we need it to have support for pretty branding. Bug 795982 added basic
> > > > > support to the packaging code, but I think we should make it a full citizen
> > > > > and give it package-name.mk support for pretty and non-pretty packaging.
> > > > 
> > > > I take it this is a blocker shipping the English deployment of the stub
> > > > installer then, right?
> > > 
> > > Nope, this is only a blocker for when we hit Beta.
> > 
> > I don't understand. If we need this for a beta version of the stub
> > installer, then it would block. Can you clarify?
> 
> Sorry. I'm talking about when the stub installer code actually hits Beta --
> which is at 18.0b1. The Beta stub installer builds we've done already are
> hacky one-off things. This bug is required for it to be properly integrated
> with our release automation.

Ah, makes sense now.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking this for 18.0 and will note for RelMan that we need this in for beta1

Comment 9

[[e-mail] (:bhearsum)]

Just a reminder, 18 merges to Beta on November 12th.

Comment 10

[Jason Smith [:jsmith]]

Brian - Are you able to help out here?

Comment 11

[Brian R. Bondy [:bbondy]]

I don't really understand what needs to be done here so I'd prefer to wait for rstrong to do it. I won't be able to get to it today anyway.  I think he's back as of Monday.

Comment 12

[bhavana bajaj [:bajaj]]

Assigning it to rstrong for now,(feel free to reassign) based on comment 11 and this seems to be a blocker when FF18 hits beta as per comment 3.

Comment 13

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

Attachment: patch rev1

Pushed to try
https://tbpl.mozilla.org/?tree=Try&rev=2fcba655853b

Comment 14

[[e-mail] (:bhearsum)]

(In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> Created attachment 680520 [details] [diff] [review]
> patch rev1
> 
> Pushed to try
> https://tbpl.mozilla.org/?tree=Try&rev=2fcba655853b

We can't test the upload part of this on try, but we _do_ do a run of 'make installer' with MOZ_PKG_PRETTYNAMES set. From the log, it looks like this isn't working for en-US:
e:\builds\moz2_slave\try-w32\build\toolkit\mozapps\installer\windows\nsis\makensis.mk:68:0$ e:/builds/moz2_slave/try-w32/build/obj-firefox/_virtualenv/Scripts/python.exe e:/builds/moz2_slave/try-w32/build/config/nsinstall.py -D ../../../dist/win32/en-US/
e:\builds\moz2_slave\try-w32\build\toolkit\mozapps\installer\windows\nsis\makensis.mk:69:0$ cat instgen/7zSD.sfx instgen/app.tag instgen/app.7z > "../../../dist/win32/en-US/Nightly Setup 19.0a1.exe"
e:\builds\moz2_slave\try-w32\build\toolkit\mozapps\installer\windows\nsis\makensis.mk:70:0$ chmod 0755 "../../../dist/win32/en-US/Nightly Setup 19.0a1.exe"
e:\builds\moz2_slave\try-w32\build\toolkit\mozapps\installer\windows\nsis\makensis.mk:72:0$ cp instgen/stub.exe "../../../dist/win32/en-US/Nightly Setup 19.0a1-stub.exe"
e:\builds\moz2_slave\try-w32\build\toolkit\mozapps\installer\windows\nsis\makensis.mk:73:0$ chmod 0755 "../../../dist/win32/en-US/Nightly Setup 19.0a1-stub.exe"

Strangely, it is working for l10n:
rm -f "e:/builds/moz2_slave/try-w32/build/obj-firefox/browser/locales/../../dist/win32/x-test/Nightly Setup Stub 19.0a1.exe"; cp ../installer/windows/l10ngen/stub.exe "e:/builds/moz2_slave/try-w32/build/obj-firefox/browser/locales/../../dist/win32/x-test/Nightly Setup Stub 19.0a1.exe"; chmod 0755 "e:/builds/moz2_slave/try-w32/build/obj-firefox/browser/locales/../../dist/win32/x-test/Nightly Setup Stub 19.0a1.exe";

Comment 15

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

Attachment: patch rev2

Comment 16

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

Sent to try
https://tbpl.mozilla.org/?tree=Try&rev=7684a4446a22

Comment 17

[[e-mail] (:bhearsum)]

en-US seems to be good now: e:\builds\moz2_slave\try-w32\build\toolkit\mozapps\installer\windows\nsis\makensis.mk:72:0$ cp instgen/stub.exe "../../../dist/win32/en-US/Nightly Setup Stub 19.0a1.exe"

Comment 18

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

Attachment: patch rev3

Changed the pretty name to
Firefox Stub Setup 19.0a1.exe

Comment 19

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

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

actually, let's go with Setup Stub

Comment 20

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

Brian, I'm still having ISP issues... could you ping glandium on irc and ask him if he has time to review this today so it can land sooner rather than later? Thanks!

Comment 21

[Brian R. Bondy [:bbondy]]

Do you have access yet? If not do you want me to land it on m-c? Push to try first? Aurora?

Comment 22

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

It has already been through try. I have access for a short time and will push to inbound.

Comment 23

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

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

Comment 24

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

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): stub installer bug 322206
User impact if declined: needed to automate the creation pretty installer names for beta and release and without it there will be a very large increase of manual steps for releng
Testing completed (on m-c, etc.): Try and locally
Risk to taking this patch (and alternatives if risky): minimal - only affects file naming of the stub installer
String or UUID changes made by this patch: none

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25551c06a318

Comment 26

[bhavana bajaj [:bajaj]]

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

low risk patch needed for stub. Approving on Aurora

Comment 27

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

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