Closed Bug 1347191 Opened 3 years ago Closed 3 years ago

Add support for NSIS 3.01 to moz.configure

Categories

(Toolkit :: NSIS Installer, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: RyanVM, Assigned: RyanVM)

References

Details

Attachments

(1 file)

+++ 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.
Summary: Add support for NSIS 3.01 to configure.in → Add support for NSIS 3.01 to moz.configure
Appears to get the job done locally. Wow that was easy :)
Attachment #8847151 - Flags: review?(robert.strong.bugs)
Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

Maybe you're a better reviewer here, Matt.
Attachment #8847151 - Flags: review?(mhowell)
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. ;)
Attachment #8847151 - Flags: review?(robert.strong.bugs)
(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.
Flags: needinfo?(mhowell)
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.
(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.
Flags: needinfo?(mhowell)
(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.
Pushed to try because I think the issue with the NSIS package might have been fixed, and I want to find out for sure.
So, that try push really did not work; now the configure script's call to makensis hangs forever instead of immediately failing. :(
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.
Attachment #8847151 - Flags: review?(mhowell) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e3976de934e
Add support for NSIS 3.01 to moz.configure. r=mhowell
https://hg.mozilla.org/mozilla-central/rev/4e3976de934e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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
Attachment #8847151 - Flags: approval-mozilla-esr52?
Attachment #8847151 - Flags: approval-mozilla-aurora?
Comment on attachment 8847151 [details] [diff] [review]
add support for NSIS 3.01 to moz.configure

Add support for NSIS 3.01. Aurora54+.
Attachment #8847151 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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+
Attachment #8847151 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.