569058 - Upgrade NSIS version to 2.45 or later (PCA complains when installer / uninstaller is cancelled on Windows 7)

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

[Masatoshi Kimura [:emk]]

NSIS 2.45 added a support for Win7. It's required to fix bug 522065.

Comment 1

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

Note: last I checked there were some changes to NSIS that will require changes to our installer code to fix this. I don't remember specifically what I had found and it will just require trying to compile the existing code with the newer NSIS version and fixing the code to support the newer NSIS as the problems are found. Due to this we will likely need to support the older version and the newer version side by side in Mozilla Build so it can continue to support 3.5.x and 3.6.x

Comment 2

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

Attachment: patch rev1

Ted, could you review the makefile.in changes? Thanks

Comment 3

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

Comment on attachment 448707 [details] [diff] [review]
patch rev1

makensis-2.46.exe should actually be makensisu-2.46.exe

Comment 4

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

Attachment: patch rev2 - slightly cleaner

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 448806 [details] [diff] [review]
patch rev2 - slightly cleaner

I'd much prefer if you did this check in configure with a MOZ_PATH_PROGS, like we do for makensis:
http://mxr.mozilla.org/mozilla-central/source/configure.in#6438
(you could stick another check for makensisu there, and check for makensisu-2.46 first).

Comment 6

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

Attachment: patch rev2

Comment 7

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

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

I'm going to change this so it will work with other installations of NSIS as long as they meet the required version

Comment 8

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

Attachment: patch rev3

Comment 9

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

Attachment: patch rev4 - forgot to add AC_MSG_RESULT

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 449404 [details] [diff] [review]
patch rev4 - forgot to add AC_MSG_RESULT

Nice, that looks better. Thanks!

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>+    AC_MSG_CHECKING([for Unicode NSIS and minimum required NSIS version >= $MIN_NSIS_VERSION])
>+    if test "$MAKENSIS_VERSION" == "" || test ! "$MAKENSIS_VERSION" -ge $MIN_NSIS_VERSION; then
>+        AC_MSG_RESULT([no])
>+        AC_MSG_ERROR([To build the installer you must either have the latest Mozilla Build or habe makensis v2.33-Unicode or greater in your path. To build without the installer reconfigure using --disable-installer.])

Spelling: s/habe/have/. Also, I think we generally refer to it as "MozillaBuild", one word with intercaps.

Comment 11

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

Attachment: patch - check major version also

Ted, I decided to checking the major version instead of hardcoding it in the regexp... seems a tad cleaner this way and easier to maintain when we change NSIS versions.

Comment 12

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

Attachment: overrides patch rev1

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 449720 [details] [diff] [review]
patch - check major version also

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>+        AC_MSG_ERROR([To build the installer you must either have the latest MozillaBuild or have Unicode NSIS with a major version of $REQ_NSIS_MAJOR_VER and a minimum minor version of $MIN_NSIS_MINOR_VER in your path. To build without the installer reconfigure using --disable-installer.])

I'd just say "Unicode NSIS version ${REQ_NSIS_MAJOR_VER}.${MIN_NSIS_MINOR_VER} or newer". Something similar in your AC_MSG_CHECKING below.

Looks good otherwise, r=me.

Comment 14

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

Attachment: updated patch for configure.in, autoconf.mk.in, and makensis.mk (checked in)

Carrying forward r+

Talked with Ted about how we only allow a major version of 2 so newer won't work in the message when NSIS 3.x comes out.

Comment 15

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

Comment on attachment 450238 [details] [diff] [review]
updated patch for configure.in, autoconf.mk.in, and makensis.mk (checked in)

Pushed this patch to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/41c559dce8a7

Leaving open for the second patch.

Comment 16

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

MAKENSIS was already used in configure and was cached which caused red for incrementals so I changed it to MAKENSISU

Comment 17

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

(In reply to comment #16)
> MAKENSIS was already used in configure and was cached which caused red for
> incrementals so I changed it to MAKENSISU

http://hg.mozilla.org/mozilla-central/rev/f7856191760c

Comment 18

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

Attachment: comm-central patch (checked in)

Forgot we need to do this for comm-central as well. I've backed out the changes to makensis.mk until this is fixed. Thanks

Comment 19

[Mark Banner (:standard8)]

Comment on attachment 450270 [details] [diff] [review]
comm-central patch (checked in)

Thanks Rob.

Comment 20

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

Comment on attachment 450270 [details] [diff] [review]
comm-central patch (checked in)

Thanks Mark

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/481ead815207

Comment 21

[Jim Mathies [:jimm]]

Comment on attachment 449722 [details] [diff] [review]
overrides patch rev1

Looks ok. Did you want this -

DetailPrint 'Search in: $R8'

uncommented?

Comment 22

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

(In reply to comment #21)
> (From update of attachment 449722 [details] [diff] [review])
> Looks ok. Did you want this -
> 
> DetailPrint 'Search in: $R8'
> 
> uncommented?
doh!. I'll remove that change before checkin. Thanks

Comment 23

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

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/fa0dbc5943ac
http://hg.mozilla.org/mozilla-central/rev/519a3ebf00ac
http://hg.mozilla.org/mozilla-central/rev/1cec9ed81981

The way this is written the current NSIS version should just work like it did previously and once the new MozillaBuild with the newer version of NSIS is installed on the build systems it will start using it.

So, as soon as bug 569534 is fixed and the build machines are upgraded to that version of MozillaBuild the complaints from the PCA will go away.