Closed Bug 571381 Opened 14 years ago Closed 11 years ago

Require NSIS 2.46 Unicode

Categories

(Firefox :: Installer, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

Attachments

(2 files, 5 obsolete files)

NSIS 2.46 Unicode can use UTF-8 encoded files so we can remove the usage / requirement for iconv and utf16-le-bom.bin to make things simpler for building the installer.
I think bug 570689 should be fixed before this as well.
Depends on: 570689
note: a comm-central patch will need to be landed before the mozilla-central patch or at the same time and all build systems will need the new mozilla-build that includes NSIS 2.46 Unicode so this won't happen for a while.
Attached patch 1. patch - require NSIS 2.46 (obsolete) — Splinter Review
Just getting these out of my queue

I'll create a comm-central patch when the time to do this is nearer
Attachment #450479 - Attachment is obsolete: true
Summary: After NSIS 2.46 Unicode is required remove iconv requirement, utf16-le-bom.bin, etc. → Require NSIS 2.46 Unicode and remove iconv requirement, utf16-le-bom.bin, etc.
Attachment #450573 - Attachment is obsolete: true
Attachment #450574 - Attachment is obsolete: true
Attachment #450575 - Attachment is obsolete: true
Turns out that Bug 570689 will include most of the changes without requiring NSIS 2.46 Unicode.
Summary: Require NSIS 2.46 Unicode and remove iconv requirement, utf16-le-bom.bin, etc. → Require NSIS 2.46 Unicode (possibly remove utf-16-le file creation in preprocess-locale.py)
No longer depends on: 570689
Summary: Require NSIS 2.46 Unicode (possibly remove utf-16-le file creation in preprocess-locale.py) → Require NSIS 2.46 Unicode
Depends on: 791687
Attached patch m-c patchSplinter Review
Attachment #450572 - Attachment is obsolete: true
Attachment #693274 - Flags: review?(mh+mozilla)
Attached patch c-c patchSplinter Review
Attachment #693276 - Flags: review?(bugspam.Callek)
Depends on: 822569
Comment on attachment 693274 [details] [diff] [review]
m-c patch

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

::: configure.in
@@ +6202,5 @@
>  dnl is not yet sufficient).
>  if test "$OS_ARCH" = "WINNT"; then
>      REQ_NSIS_MAJOR_VER=2
> +    MIN_NSIS_MINOR_VER=46
> +    MOZ_PATH_PROGS(MAKENSISU, $MAKENSISU makensisu-2.46 makensis)

Why not keep makensisu in there? Seems like the version test below should find non-matching versions.
Attachment #693274 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #11)
> Comment on attachment 693274 [details] [diff] [review]
> m-c patch
> 
> Review of attachment 693274 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.in
> @@ +6202,5 @@
> >  dnl is not yet sufficient).
> >  if test "$OS_ARCH" = "WINNT"; then
> >      REQ_NSIS_MAJOR_VER=2
> > +    MIN_NSIS_MINOR_VER=46
> > +    MOZ_PATH_PROGS(MAKENSISU, $MAKENSISU makensisu-2.46 makensis)
> 
> Why not keep makensisu in there? Seems like the version test below should
> find non-matching versions.
Because there is no need for it since we only renamed NSIS 2.33's makensis to makensisu and it is just one more thing to figure out when reading what is going on.
Comment on attachment 693276 [details] [diff] [review]
c-c patch

Also requesting from Mark Banner in case he has bandwidth to review this
Attachment #693276 - Flags: review?(mbanner)
Comment on attachment 693276 [details] [diff] [review]
c-c patch

Looks fine to me, thanks.
Attachment #693276 - Flags: review?(mbanner)
Attachment #693276 - Flags: review?(bugspam.Callek)
Attachment #693276 - Flags: review+
Merged to mozilla-central
https://hg.mozilla.org/mozilla-central/rev/0cc898fd66fc
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Component: NSIS Installer → Installer
Product: Toolkit → Firefox
You need to log in before you can comment on or make changes to this bug.