1347191 - Add support for NSIS 3.01 to moz.configure

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

+++ This bug was initially created as a clone of Bug #1237040 +++

Bug 1236624 is getting it on the build slaves and I'm trying to play around with it locally too for a new MozillaBuild release.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: add support for NSIS 3.01 to moz.configure

Appears to get the job done locally. Wow that was easy :)

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

Maybe you're a better reviewer here, Matt.

Comment 3

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

We need to make sure the updated zip file of bug 1236624 comment 63 has been made available before this can land. Builds will break otherwise. I'll r+ this once we know that's done.

I'd also like to remove support for 3.0b1 and 3.0b3. But that seems like a bad idea without having a MozillaBuild release available that includes 3.01, and I can't ask you to do that just to get this landed. So maybe that's a followup. ;)

Comment 4

[Ryan VanderMeulen [:RyanVM]]

(In reply to Matt Howell [:mhowell] from comment #3)
> We need to make sure the updated zip file of bug 1236624 comment 63 has been
> made available before this can land. Builds will break otherwise. I'll r+
> this once we know that's done.

Wouldn't they just fall back to 3.0b3/3.0b1 if 3.01 isn't found in the mean time?

> I'd also like to remove support for 3.0b1 and 3.0b3. But that seems like a
> bad idea without having a MozillaBuild release available that includes 3.01,
> and I can't ask you to do that just to get this landed. So maybe that's a
> followup. ;)

Are you planning to backport 3.01 support to all active branches (including ESR45)? Otherwise, we're stuck supporting older NSIS for awhile still one way or another.

But again, maybe I'm misunderstanding things here. I figured adding this was safe enough with proper fallbacks in place. Future cleanups can happen whenever needed.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

BTW, I'll note that our buildbot builders are already falling back to 3.0b1 for production builds (ESR45 - Trunk). Only Taskcluster builds (which we don't actually ship) have 3.0b3 available to them.

Comment 6

[Molly Howell (she/her) (no longer active)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)
> Wouldn't they just fall back to 3.0b3/3.0b1 if 3.01 isn't found in the mean
> time?

The particular way that the previous package was broken causes the script to select 3.01 but then die out on line 331 because that makensis can't be run. Bug 1236624 comment 53 has a try run that shows what that looks like (on buildbot; the TC failures are tests that I broke).

> Are you planning to backport 3.01 support to all active branches (including
> ESR45)? Otherwise, we're stuck supporting older NSIS for awhile still one
> way or another.
> 
> But again, maybe I'm misunderstanding things here. I figured adding this was
> safe enough with proper fallbacks in place. Future cleanups can happen
> whenever needed.

No, I hadn't been thinking about doing any backporting. Leaving that part for future cleanup is fine.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> BTW, I'll note that our buildbot builders are already falling back to 3.0b1
> for production builds (ESR45 - Trunk). Only Taskcluster builds (which we
> don't actually ship) have 3.0b3 available to them.

On buildbot the 3.01 directory is there but doesn't make it into PATH (presumably 3.0b3 is the same). I think I got that taken care of in the try push I mentioned above, but I'm not confident I hit all the right places.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

(In reply to Matt Howell [:mhowell] from comment #6)
> The particular way that the previous package was broken causes the script to
> select 3.01 but then die out on line 331 because that makensis can't be run.
> Bug 1236624 comment 53 has a try run that shows what that looks like (on
> buildbot; the TC failures are tests that I broke).

Aha! Yeah, that'd be a problem :)

> No, I hadn't been thinking about doing any backporting. Leaving that part
> for future cleanup is fine.

Yeah, my understanding is that we really wanted 3.0b3 already back for ESR45 due to some improvements it had over 3.0b1. Shame that never ended up working out. Backport to at least as far back as ESR52 may be something worth considering anyway. FWIW, for MozillaBuild, I was planning to include 3.01 and 3.0b3 with the new release but drop the others.

> On buildbot the 3.01 directory is there but doesn't make it into PATH
> (presumably 3.0b3 is the same). I think I got that taken care of in the try
> push I mentioned above, but I'm not confident I hit all the right places.

Makes sense.

Comment 8

[Molly Howell (she/her) (no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=17470de57620

Comment 9

[Molly Howell (she/her) (no longer active)]

Pushed to try because I think the issue with the NSIS package might have been fixed, and I want to find out for sure.

Comment 10

[Molly Howell (she/her) (no longer active)]

So, that try push really did not work; now the configure script's call to makensis hangs forever instead of immediately failing. :(

Comment 11

[Molly Howell (she/her) (no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb3826eddb71

Comment 12

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

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

Above try run is green, I think we can finally land this.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e3976de934e
Add support for NSIS 3.01 to moz.configure. r=mhowell

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4e3976de934e

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: bug 1215648
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: bug 1351482
[Is the change risky?]: no
[Why is the change risky/not risky?]: we've been shipping with alpha/beta level NSIS 3.0 installers for a long time now, this just updates us to a final stable release
[String changes made/needed]: none

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

Add support for NSIS 3.01. Aurora54+.

Comment 17

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b368c9dcac20

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

This fix updates the win32 installer/uninstaller system. ESR52.2+

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/6797dac51b6e