Closed Bug 1237040 Opened 4 years ago Closed 4 years ago

Add support for NSIS 3.0b3 to configure.in

Categories

(Toolkit :: NSIS Installer, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 + fixed
firefox46 --- fixed

People

(Reporter: rstrong, Assigned: rstrong)

References

Details

Attachments

(1 file, 3 obsolete files)

There are some fixes we want to pick up in this version of NSIS.
[Tracking Requested - why for this release]:
We'd like to get this in esr45 so it has the additional dll pre-loading functionality and so we don't have to support the older versions of NSIS until esr45 is EOL.
Comment on attachment 8704302 [details] [diff] [review]
patch rev1

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

::: configure.in
@@ +6261,5 @@
>  dnl is not yet sufficient).
>  if test "$OS_ARCH" = "WINNT"; then
>      MIN_NSIS_MAJOR_VER=2
>      MIN_NSIS_MINOR_VER=46
> +    MOZ_PATH_PROGS(MAKENSISU, $MAKENSISU makensis-3.0b3.exe makensis-3.0b1.exe makensisu-3.0a2.exe makensisu-2.46.exe makensis)

Could we drop makensisu-3.0a2.exe and makensisu-2.46.exe at this point? MozillaBuild 2.x is already required to build Fx these days and it comes with 3.0b1.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #3)
> Comment on attachment 8704302 [details] [diff] [review]
> patch rev1
> 
> Review of attachment 8704302 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.in
> @@ +6261,5 @@
> >  dnl is not yet sufficient).
> >  if test "$OS_ARCH" = "WINNT"; then
> >      MIN_NSIS_MAJOR_VER=2
> >      MIN_NSIS_MINOR_VER=46
> > +    MOZ_PATH_PROGS(MAKENSISU, $MAKENSISU makensis-3.0b3.exe makensis-3.0b1.exe makensisu-3.0a2.exe makensisu-2.46.exe makensis)
> 
> Could we drop makensisu-3.0a2.exe and makensisu-2.46.exe at this point?
> MozillaBuild 2.x is already required to build Fx these days and it comes
> with 3.0b1.
With that info yes we can. Thanks!
Attachment #8704302 - Attachment is obsolete: true
Attachment #8704302 - Flags: review?(gps)
Attached patch patch rev2Splinter Review
If the build is not an alpha or beta build MAKENSISU_PRERELEASE_TYPE and MAKENSISU_PRERELEASE_VER will default to the minimum values for the check thereby allowing release versions when they are available to pass the check.
Attachment #8704399 - Flags: review?(gps)
Comment on attachment 8704399 [details] [diff] [review]
patch rev2

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

AFAICT we'll only accept beta NSIS versions with this patch. I'm not thrilled that we'll require code changes to support the non-beta release version. But I suppose we'll cross that bridge when we get to it.
Attachment #8704399 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #7)
> Comment on attachment 8704399 [details] [diff] [review]
> patch rev2
> 
> Review of attachment 8704399 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> AFAICT we'll only accept beta NSIS versions with this patch. I'm not
> thrilled that we'll require code changes to support the non-beta release
> version. But I suppose we'll cross that bridge when we get to it.
It also handles release versions and I verified that it does.
[Tracking Requested - why for this release]:
We'd like to get this in esr45 so it has the additional dll pre-loading functionality and so we don't have to support the older versions of NSIS until esr45 is EOL. I'll request approval for aurora after this makes it to m-c.
Flags: needinfo?(robert.strong.bugs)
Attached patch patch with clobber (obsolete) — Splinter Review
Attachment #8704399 - Attachment is obsolete: true
Flags: needinfo?(robert.strong.bugs)
Attachment #8705304 - Flags: review+
Attached patch patch with clobber (obsolete) — Splinter Review
Attachment #8705304 - Attachment is obsolete: true
Attachment #8705310 - Flags: review+
Backed out again in https://hg.mozilla.org/integration/fx-team/rev/c532278cd008 for other bustage:

http://archive.mozilla.org/pub/firefox/tinderbox-builds/fx-team-win64/1452199648/fx-team-win64-bm72-build1-build86.txt.gz

Seeing a bunch of failures like 12:55:07     
             INFO -  cl : Command line error D8021 : invalid numeric argument '/Werror'
12:55:07     INFO -  configure: failed program was:
12:55:07     INFO -  #line 21628 "configure"
12:55:07     INFO -  #include "confdefs.h"
12:55:07     INFO -  int main() {
12:55:07     INFO -  static char c __attribute__ ((aligned(8))) = 0; return c;
12:55:07     INFO -  ; return 0; }
12:55:07     INFO -  configure:22776: checking for java
12:55:07     INFO -  configure:22825: checking for javac
12:55:07     INFO -  configure:22874: checking for javah
12:55:07     INFO -  configure:22923: checking for jar
12:55:07     INFO -  configure:22972: checking for jarsigner
12:55:07     INFO -  configure:23021: checking for keytool
12:55:07     INFO -  configure:23900: checking for makensis-3.0b3.exe
12:55:07     INFO -  configure:23900: checking for makensis-3.0b1.exe
12:55:07     INFO -  configure:23900: checking for makensis
12:55:07     INFO -  configure: error: To build the installer you must have the latest MozillaBuild or NSIS version 3.0b1 or greater in your path.
12:55:07     INFO -  *** Fix above errors and then restart with\
12:55:07     INFO -                 "c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src/mozmake.EXE -f client.mk build"
12:55:07     INFO -  c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src/client.mk:359: recipe for target 'configure' failed
12:55:07     INFO -  mozmake.EXE[2]: *** [configure] Error 1
12:55:07     INFO -  mozmake.EXE[2]: Leaving directory 'c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src'
12:55:07     INFO -  c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src/client.mk:373: recipe for target 'c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src/obj-firefox/Makefile' failed
12:55:07     INFO -  mozmake.EXE[1]: *** [c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src/obj-firefox/Makefile] Error 2
12:55:07     INFO -  mozmake.EXE[1]: Leaving directory 'c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src'
12:55:07     INFO -  client.mk:171: recipe for target 'build' failed
12:55:07     INFO -  mozmake.EXE: *** [build] Error 2
12:55:07     INFO -  277 compiler warnings present.
12:55:14     INFO -  2
12:55:14    ERROR - Return code: 1
12:55:14  WARNING - setting return code to 2
Meh... after the clobber it is easy to see that the Win64 build slave doesn't have makensis-3.0b1.exe like the Win32 build slaves.
https://hg.mozilla.org/mozilla-central/rev/01a7297a24d0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Realized today that this is busting the configure step on l10n nightlies, 

07:18:18     INFO -   0:50.54 checking for makensis-3.0b3.exe... no
07:18:18     INFO -   0:50.56 checking for makensis-3.0b1.exe... no
07:18:18     INFO -   0:50.57 checking for makensis... no
07:18:18     INFO -   0:50.57 configure: error: To build the installer you must have the latest MozillaBuild or NSIS version 3.0b1 or greater in your path.

Rob, do you know what's in need of fixing?
Flags: needinfo?(robert.strong.bugs)
Looks like the l10n build machines either don't have NSIS 3.0b1 or they don't have it in the path. To fix, NSIS 3.0b1 needs to be on the system and included in the path.
Flags: needinfo?(robert.strong.bugs)
Blocks: 1239074
No longer blocks: 1239074
Depends on: 1239074
Tracking to make sure it is fixed before the m-a => m-b merge.
Chris, do you know who could update the build machine to have NSIS installed/configured? Thanks
Flags: needinfo?(catlee)
Ryan, did we resolve this last week in the dep bug?
Flags: needinfo?(catlee) → needinfo?(ryanvm)
No, bug 1236624 covers getting NSIS 3.0b3 installed on the build slaves. This one just needs an Aurora uplift.
Flags: needinfo?(ryanvm)
Robert, could you fill an uplift request for this bug? Thanks
Flags: needinfo?(robert.strong.bugs)
Comment on attachment 8704399 [details] [diff] [review]
patch rev2

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: We'd like to get this in esr45 so it has the additional dll pre-loading functionality and so we don't have to support the older versions of NSIS until esr45 is EOL.
[Describe test coverage new/current, TreeHerder]: This has baked on nightly for several days.
[Risks and why]: Improperly configured build systems might break as we have seen but now that the build systems have been fixed I don't expect any additional issues.
[String/UUID change made/needed]: None
Flags: needinfo?(robert.strong.bugs)
Attachment #8704399 - Flags: approval-mozilla-aurora?
Comment on attachment 8704399 [details] [diff] [review]
patch rev2

Let's do it!
Attachment #8704399 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 1248963
You need to log in before you can comment on or make changes to this bug.