Closed Bug 569058 Opened 10 years ago Closed 10 years ago

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

Categories

(Toolkit :: NSIS Installer, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: emk, Assigned: rstrong)

References

()

Details

Attachments

(3 files, 6 obsolete files)

15.42 KB, patch
jimm
: review+
Details | Diff | Splinter Review
5.91 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
3.84 KB, patch
standard8
: review+
Details | Diff | Splinter Review
NSIS 2.45 added a support for Win7. It's required to fix bug 522065.
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
Attached patch patch rev1 (obsolete) — Splinter Review
Ted, could you review the makefile.in changes? Thanks
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Attachment #448707 - Flags: review?(ted.mielczarek)
Comment on attachment 448707 [details] [diff] [review]
patch rev1

makensis-2.46.exe should actually be makensisu-2.46.exe
Attached patch patch rev2 - slightly cleaner (obsolete) — Splinter Review
Attachment #448707 - Attachment is obsolete: true
Attachment #448806 - Flags: review?(ted.mielczarek)
Attachment #448707 - Flags: review?(ted.mielczarek)
Summary: Upgrade NSIS version to 2.45 or later → Upgrade NSIS version to 2.45 or later (PCA complains when installer is cancelled on Windows 7)
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).
Attachment #448806 - Flags: review?(ted.mielczarek) → review-
Attached patch patch rev2 (obsolete) — Splinter Review
Attachment #448806 - Attachment is obsolete: true
Attachment #449390 - Flags: review?(ted.mielczarek)
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
Attachment #449390 - Attachment is obsolete: true
Attachment #449390 - Flags: review?(ted.mielczarek)
Attached patch patch rev3 (obsolete) — Splinter Review
Attachment #449403 - Flags: review?(ted.mielczarek)
Attachment #449403 - Attachment is obsolete: true
Attachment #449404 - Flags: review?(ted.mielczarek)
Attachment #449403 - Flags: review?(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.
Attachment #449404 - Flags: review?(ted.mielczarek) → review+
Attached patch patch - check major version also (obsolete) — Splinter Review
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.
Attachment #449720 - Flags: review?(ted.mielczarek)
Attachment #449404 - Attachment is obsolete: true
Attachment #449722 - Flags: review?(jmathies)
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.
Attachment #449720 - Flags: review?(ted.mielczarek) → review+
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.
Attachment #449720 - Attachment is obsolete: true
Attachment #450238 - Flags: review+
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.
MAKENSIS was already used in configure and was cached which caused red for incrementals so I changed it to MAKENSISU
(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
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
Attachment #450270 - Flags: review?(bugzilla)
Comment on attachment 450270 [details] [diff] [review]
comm-central patch (checked in)

Thanks Rob.
Attachment #450270 - Flags: review?(bugzilla) → review+
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
Attachment #450270 - Attachment description: comm-central patch → comm-central patch (checked in)
Comment on attachment 449722 [details] [diff] [review]
overrides patch rev1

Looks ok. Did you want this -

DetailPrint 'Search in: $R8'

uncommented?
Attachment #449722 - Flags: review?(jmathies) → review+
(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
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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Summary: Upgrade NSIS version to 2.45 or later (PCA complains when installer is cancelled on Windows 7) → Upgrade NSIS version to 2.45 or later (PCA complains when installer / uninstaller is cancelled on Windows 7)
You need to log in before you can comment on or make changes to this bug.