Closed Bug 351917 Opened 13 years ago Closed 12 years ago

Create NSIS installer for SeaMonkey (suiterunner)

Categories

(SeaMonkey :: Installer, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcsmurf, Assigned: mcsmurf)

References

Details

Attachments

(7 files, 7 obsolete files)

76.38 KB, application/x-zip-compressed
Details
43.07 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
129.78 KB, patch
Details | Diff | Splinter Review
6.16 KB, patch
Details | Diff | Splinter Review
551 bytes, patch
Details | Diff | Splinter Review
1.36 KB, patch
mcsmurf
: review+
Details | Diff | Splinter Review
2.74 KB, patch
mcsmurf
: review+
Details | Diff | Splinter Review
SeaMonkey based on toolkit/ should probably use the new NSIS installer, it's easier to maintain, provides many advantages in general and all the toolkit/ projects in the tree also use it (FF,TB,Calendar).
Attached patch First attempt at a patch (obsolete) — Splinter Review
This patch should pretty much work, a few details maybe need to be tweaked (like remove some debug foo from it, adjust the shortcuts in the Programs menu, fix the removed-files.in file).
To compile with this patch you need to install NSIS (http://developer.mozilla.org/en/docs/Windows_Build_Prerequisites#NSIS), if you want to compile the installer, you also need to install 7-zip and UPX (see at the bottom of http://developer.mozilla.org/en/docs/Build_and_Install for some info). NSIS during compile time of the app is required because the uninstaller is already built during the normal build, not when the installer itself is built. To compile the installer, execute "make installer" in suite/installer/.
Some binary files are required to build (one 7-zip file and three artwork files). Extract the ZIP file to the root of your source tree. Don't get confused by the Minefield artwork there, I just needed some artwork for testing :).
Just as a note: I'll create and update to real SeaMonkey artwork once the patch is reviewed and working, probably even after initial checkin.
Depends on: 351930
Blocks: 360048
Attached patch Patch 2 (obsolete) — Splinter Review
Attachment #237464 - Attachment is obsolete: true
Comment on attachment 250778 [details] [diff] [review]
Inter-diff of browser/installer and suite/installer patch

Robert: Could you maybe take a look at this inter-diff if there's anything wrong with my changes? The removed-files.in file will probably go away completely for now since suiterunner will install in a completely new folder with no old files in it. The patch was tested locally, it builds and the installer seems to work.
Attachment #250778 - Flags: review?(robert.bugzilla)
Depends on: 366240
Attached patch Patch 3 (obsolete) — Splinter Review
Attachment #250777 - Attachment is obsolete: true
Attached patch Patch 4 (obsolete) — Splinter Review
Attachment #250841 - Attachment is obsolete: true
Sorry about the delay... I'll try to get to this by this weekend.
The "Nur in browser/installer: ..." lines mean: "Only in browser/installer: ..." The branding stuff was partly changed for now because I did not have any proper artwork for testing ;), will be changed later.
Attachment #250778 - Attachment is obsolete: true
Attachment #251099 - Flags: review?(robert.bugzilla)
Attachment #250778 - Flags: review?(robert.bugzilla)
Depends on: 350701
Blocks: 350701
No longer depends on: 350701
Comment on attachment 251099 [details] [diff] [review]
Inter-diff two of browser/installer and suite/installer

Sorry this has taken so long...

>diff -x CVS -x unix -ur browser/installer/windows/Makefile.in suite/installer/windows/Makefile.in
>--- browser/installer/windows/Makefile.in	2006-09-02 21:28:58.875000000 +0200
>+++ suite/installer/windows/Makefile.in	2006-09-13 21:38:40.937500000 +0200
>...
>@@ -59,10 +59,9 @@
> 	$(NULL)
> 
> BRANDING_FILES = \
>-	branding.nsi \
>-	wizHeader.bmp \
>-	wizHeaderRTL.bmp \
No RTL installers?

>diff -x CVS -x unix -ur browser/installer/windows/nsis/defines.nsi.in suite/installer/windows/nsis/defines.nsi.in
>--- browser/installer/windows/nsis/defines.nsi.in	2006-09-23 19:42:20.671875000 +0200
>+++ suite/installer/windows/nsis/defines.nsi.in	2006-11-18 23:59:09.484375000 +0100
>@@ -5,7 +5,10 @@
> !define FileInstallerEXE      "@PKG_BASENAME@.installer.exe"
> !define FileInstallerMSI      "@PKG_BASENAME@.installer.msi"
> !define FileInstallerNETRoot  "@PKG_BASENAME@.net-installer"
>+!define MOZ_DEFAULT_TOOLKIT   @MOZ_DEFAULT_TOOLKIT@
I don't see this being used by the installer itself... is this necessary?

>diff -x CVS -x unix -ur browser/installer/windows/nsis/installer.nsi suite/installer/windows/nsis/installer.nsi
>--- browser/installer/windows/nsis/installer.nsi	2006-11-30 19:14:47.203125000 +0100
>+++ suite/installer/windows/nsis/installer.nsi	2007-01-07 22:59:08.109375000 +0100
>@@ -61,6 +62,7 @@
> Var AddDesktopSC
> Var fhInstallLog
> Var fhUninstallLog
>+Var ShortPathNameToExe
note: on the trunk we've converted to using long paths and it appears that you are using long paths except for here
Software\Clients\Mail\${BrandFullNameInternal}\protocols\mailto\shell\open\command

>@@ -586,86 +584,111 @@
>-    StrCpy $0 "Software\Classes\FirefoxURL\DefaultIcon"
>+    StrCpy $0 "Software\Classes\SeaMonkeyURL\DefaultIcon"
>     StrCpy $1 "$\"$INSTDIR\${FileMainEXE}$\",0"
>     ${WriteRegStr2} $TmpVal "$0" "" "$1" 0
DefaultIcon paths are typically not quoted even when they aren't 8dot3 paths and this has been fixed in the Firefox installer. This also applies to the other places the DefaultIcon path is being set.

>...
>-    StrCpy $0 "Software\Classes\FirefoxURL\shell\open\ddeexec\ifexec"
>+    StrCpy $0 "Software\Classes\SeaMonkeyURL\shell\open\ddeexec\ifexec"
ifexec is no longer used anywhere due to IE 6 and below breaking because it doesn't remove the key when seting itself as the default browser. This also applies to the other places that ifexec is being set. See bug 355650.

>     ; Vista Capabilities registry keys
>-    StrCpy $0 "Software\Clients\StartMenuInternet\$R9\Capabilities"
>+    StrCpy $0 "Software\Mozilla\${DDEApplication}\Capabilities"
>     StrCpy $1 "$\"$INSTDIR\${FileMainEXE}$\",0"
>     ${WriteRegStr2} $TmpVal "$0" "ApplicationDescription" "$(REG_APP_DESC)" 0
>     ${WriteRegStr2} $TmpVal "$0" "ApplicationIcon" "$1" 0
> 
>-    StrCpy $0 "Software\Clients\StartMenuInternet\$R9\Capabilities\FileAssociations"
>-    ${WriteRegStr2} $TmpVal "$0" ".htm" "FirefoxHTML" 0
>-    ${WriteRegStr2} $TmpVal "$0" ".html" "FirefoxHTML" 0
>-    ${WriteRegStr2} $TmpVal "$0" ".shtml" "FirefoxHTML" 0
>-    ${WriteRegStr2} $TmpVal "$0" ".xht" "FirefoxHTML" 0
>-    ${WriteRegStr2} $TmpVal "$0" ".xhtml" "FirefoxHTML" 0
>-
>-    StrCpy $0 "Software\Clients\StartMenuInternet\$R9\Capabilities\StartMenu"
>+    StrCpy $0 "Software\Mozilla\${DDEApplication}\Capabilities\FileAssociations"
>+    ${WriteRegStr2} $TmpVal "$0" ".htm" "SeaMonkeyHTML" 0
>+    ${WriteRegStr2} $TmpVal "$0" ".html" "SeaMonkeyHTML" 0
>+    ${WriteRegStr2} $TmpVal "$0" ".shtml" "SeaMonkeyHTML" 0
>+    ${WriteRegStr2} $TmpVal "$0" ".xht" "SeaMonkeyHTML" 0
>+    ${WriteRegStr2} $TmpVal "$0" ".xhtml" "SeaMonkeyHTML" 0
>+    ${WriteRegStr2} $TmpVal "$0" ".eml" "SeaMonkeyEML" 0
So, EML is a mail app file type and not a browser app file type. You could easily separate these by placing the capabilities keys with each file / protocol handler under their appropriate Software\Clients keys.

>@@ -674,12 +697,112 @@
>   ${WriteRegStr2} $TmpVal "$0" "" "$INSTDIR\${FileMainEXE}" 0
>   ${WriteRegStr2} $TmpVal "$0" "Path" "$INSTDIR" 0
> 
>-  StrCpy $0 "Software\Classes\MIME\Database\Content Type\application/x-xpinstall;app=firefox"
>-  ${WriteRegStr2} $TmpVal "$0" "Extension" ".xpi" 0
>+  ; XXX - why?
>+  ; StrCpy $0 "MIME\Database\Content Type\application/x-xpinstall;app=firefox"
>+  ; ${WriteRegStrHKCR} "HKCR" "$0" "Extension" ".xpi" 0
Not used and has been removed from the Firefox installer since this never worked.

>+  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>+  ; Add the Mail registry keys
>+  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>+  GetFullPathName /SHORT $ShortPathNameToExe "$INSTDIR\${FileMainEXE}"
See note above about ShortPathNameToExe

more to come
Comment on attachment 251099 [details] [diff] [review]
Inter-diff two of browser/installer and suite/installer

>diff -x CVS -x unix -ur browser/installer/windows/nsis/uninstaller.nsi suite/installer/windows/nsis/uninstaller.nsi
>--- browser/installer/windows/nsis/uninstaller.nsi	2006-11-30 19:14:47.218750000 +0100
>+++ suite/installer/windows/nsis/uninstaller.nsi	2007-01-03 09:58:55.578125000 +0100
>...
>@@ -187,8 +211,8 @@
>     StrCpy $0 "Software\Microsoft\MediaPlayer\ShimInclusionList\$R9"
>     DeleteRegKey HKLM "$0"
>     DeleteRegKey HKCU "$0"
>-    StrCpy $0 "MIME\Database\Content Type\application/x-xpinstall;app=firefox"
>-    DeleteRegKey HKCR "$0"
>+;    StrCpy $0 "MIME\Database\Content Type\application/x-xpinstall;app=firefox"
>+;    DeleteRegKey HKCR "$0"
This can be removed

mscott is separating the capabilities keys in bug 354897 which may provide some assistance in doing this for SeaMonkey.

I *should* have more time now that I am done with 1.8.1.2 and the majority of the Vista work so it hopefully won't take as long on the next go around. Thanks!
Attachment #251099 - Flags: review?(robert.bugzilla) → review-
Attached patch Patch 5Splinter Review
Attachment #251095 - Attachment is obsolete: true
Comment on attachment 260911 [details] [diff] [review]
Inter-diff three of browser/installer and suite/installer

Robert: Possibly the Show/HideShortcuts function still needs tweaking, but for now (for experimental installer builds) this should be ok. I can (and will) fix this later, possible even before check-in with a seperate patch.
Attachment #260911 - Flags: review?(robert.bugzilla)
Blocks: 377953
Blocks: 379110
Blocks: 332358
Comment on attachment 260915 [details] [diff] [review]
Patch 5

+;these Palm Sync files are conditionally built so would not exist unless built
+bin\mozABConduit.dll
+bin\PalmSyncProxy.dll
+bin\PalmSyncInstall.exe
+bin\components\palmsync.dll
+bin\components\palmsync.xpt
+bin\palm.html

Palm sync has now moved to extensions/p@m so these lines at least will need changing.

I also can't find any palm.html in mxr.

Preferably Palmsync should be an optional install, but we could always do that in a follow up patch.
Depends on: 381039
Blocks: 381039
No longer depends on: 381039
This is the last piece of code we're missing from having suiterunner match xpfe nightlies. We would like to have this for the suiterunner landing which will happen this week, but it's no hard blocker - we could go without a trunk installer for a bit, if if I'd rather have it available.

This code has been lying around waiting for review (or re-review) for quite some time - Rob, could we get it in and fix your further comments "post-mortem"? Or will you get around to do the re-review in the next few days?
Comment on attachment 260915 [details] [diff] [review]
Patch 5

Neil, could you sr/moa this for a checkin with "post-mortem" re-review from Rob?

It would be nice if we didn't need to leave nightly testers without an installer when switching to suiterunner - And the testing would surely help to improve this installer code.
Attachment #260915 - Flags: superreview?(neil)
Attachment #260915 - Flags: superreview?(neil) → superreview+
After this has been checked in, tpol suiterunner tinderbox is running into this error:


Processing script file:
"installer.nsi"
 - ShellLink::GetShortCutArgs
 -
ShellLink::GetShortCutDescription
 - ShellLink::GetShortCutHotkey
 - ShellLink::GetShortCutIconIndex
 -
ShellLink::GetShortCutIconLocation
 - ShellLink::GetShortCutShowMode
 - ShellLink::GetShortCutTarget
 -
ShellLink::GetShortCutWorkingDirectory
 - ShellLink::SetShortCutArgs
 -
ShellLink::SetShortCutDescription
 - ShellLink::SetShortCutHotkey
 - ShellLink::SetShortCutIconIndex
 -
ShellLink::SetShortCutIconLocation
 - ShellLink::SetShortCutShowMode
 - ShellLink::SetShortCutTarget
 -
ShellLink::SetShortCutWorkingDirectory
 - nsProcess::_FindProcess
 - nsProcess::_KillProcess
 - nsProcess::_Unload
!insertmacro: macro named
"DisplayCopyErrMsg" not found!
Error in script
"installer.nsi" on line 113 --
aborting creation process
make[2]: ***
[instgen/setup.exe] Error 1
make[2]: Leaving directory
`/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/suite/installer/windows'
make[1]: *** [installer] Error 2
make[1]: Leaving directory
`/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/suite/installer/windows'
make: *** [installer] Error 2
make: Leaving directory
`/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/suite/installer'
cp
c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/../build//dist/install/sea/*.exe
c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/../build//dist/install/tpol-trunk/
cp: cannot stat
`c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/../build//dist/install/sea/*.exe':
No such file or directory
(In reply to comment #19)
> After this has been checked in, tpol suiterunner tinderbox is running into this
> error:
...
> Processing script file:
> "installer.nsi"
...
> !insertmacro: macro named
> "DisplayCopyErrMsg" not found!

Looks like toolkit removed the DisplayCopyErrMsg macro recently - see bug 369221.
Attached patch Bustage fixSplinter Review
Applied the patch from Bug 369221 to the SeaMonkey NSIS code and committed it.
client.mk needed a new entry for checking out other-licenses/7zstub/seamonkey from CVS for packaging the SeaMonkey build. Patch has already been committed with r=bsmedberg via IRC.
Should Bug 383154 – NO option to install browser only and Bug 383161 – keep in memory (turbo mode?) for faster startup not working block this bug?
I can't find the .jsm that moved from components/ to modules/ this night anywhere in the package file... are we missing it completely? Firefox might as well, but that does just mean they have a bug in that list as well, from what I know...
Which .jsm file do you mean?
(In reply to comment #24)
> I can't find the .jsm that moved from components/ to modules/ this night
> anywhere in the package file... are we missing it completely? Firefox might as
> well, but that does just mean they have a bug in that list as well, from what I
> know...

I spoke to the person who originally implemented it on irc a while before that XPCOMUtils.jsm moving from components to modules - the response was along the lines of "I suppose that it should be added but it may be moving soon".

So lets just add it. It looks like someone may get round to doing it on FF sometime (see bug 380970 comment 30 and 31).
No longer blocks: 360048
As this is not marked FIXED yet, do we need to adopt something like in bug 384350 as well?
Blocks: 385377
Due to bug 237964 we need to add:

bin\res\contenteditable.css                                                     
bin\res\designmode.css
 
to our windows packaging list.

Could I suggest that we do a patch to either cvs remove the xpinstall windows packaging lists or leave them with empty files but a pointer to where our packaging list now is? This would at least help us when folks do want to help update our packaging lists. I would just come up with one, but I haven't got time at the moment.

I don't know if we want to keep our os2/unix packaging lists - I expect we should update them and move them to suite/installer so that we only package the things that we should be shipping.

btw the XPCOMUtils.jsm addition has now been done.
AFAIK, bsmedberg plans/wants to remove xpinstall/packager completely in the near future...
Syncs changes from bug 237964 and bug 386002 into the package lists.
Attachment #272529 - Flags: review?
Attachment #272529 - Flags: review? → review?(bugzilla)
Attached patch Sync bug 384350 (obsolete) — Splinter Review
I'm going to add patches to sync a few of the recent changes
Attachment #272535 - Flags: review?(bugzilla)
Attachment #272535 - Attachment is obsolete: true
Attachment #272538 - Flags: review?(bugzilla)
Attachment #272535 - Flags: review?(bugzilla)
Comment on attachment 260911 [details] [diff] [review]
Inter-diff three of browser/installer and suite/installer

A day late and a dollar short... sorry about that. I'll try to provide a couple of patches to sync up changes since this has landed.

>diff -x CVS -x unix -rup8 browser/installer/windows/nsis/shared.nsh suite/installer/windows/nsis/shared.nsh
>--- browser/installer/windows/nsis/shared.nsh	Fri Feb 23 14:41:05 2007
>+++ suite/installer/windows/nsis/shared.nsh	Sat Apr  7 20:58:59 2007
>@@ -132,40 +134,61 @@
>@@ -190,50 +213,159 @@
>...
>+  StrCpy $1 "Software\Classes\CLSID\{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}"
>+  WriteRegStr HKLM "$1\LocalServer32" "" "$\"$8$\" /MAPIStartup"
>+  WriteRegStr HKLM "$1\ProgID" "" "MozillaMapi.1"
>+  WriteRegStr HKLM "$1\VersionIndependentProgID" "" "MozillaMAPI"
>+  StrCpy $1 "SOFTWARE\Classes"
>+  WriteRegStr HKLM "$1\MozillaMapi" "" "Mozilla MAPI"
>+  WriteRegStr HKLM "$1\MozillaMapi\CLSID" "" "{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}"
>+  WriteRegStr HKLM "$1\MozillaMapi\CurVer" "" "MozillaMapi.1"
>+  WriteRegStr HKLM "$1\MozillaMapi.1" "" "Mozilla MAPI"
>+  WriteRegStr HKLM "$1\MozillaMapi.1\CLSID" "" "{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}"
Isn't this handled by the call to RegDLL above? Seems to work this way for Thunderbird by checking if the key is present.
Attachment #260911 - Flags: review?(robert.bugzilla) → review+
Scratch that about the call to RegDLL... iirc this should be done by the dll itself but it appears that Thunderbird is also doing it this way.
Comment on attachment 272529 [details] [diff] [review]
Sync changes from bug 237964 and bug 386002

Remove the "bin/" part in the removed-files.in change, rest looks ok.
r+
Attachment #272529 - Flags: review?(bugzilla) → review+
(In reply to comment #35)
> (From update of attachment 272529 [details] [diff] [review])
> Remove the "bin/" part in the removed-files.in change, rest looks ok.
> r+
> 
Checked in with the bin/ removed.
Attachment #272538 - Flags: review?(bugzilla) → review+
Attachment #272538 - Attachment description: Sync bug 384350 rev2 → Sync bug 384350 rev2 (checked in)
Ok, this bug here is FIXED :). Please file new bugs for any remaining issues.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 428819
No longer blocks: 310637
No longer blocks: 319300
You need to log in before you can comment on or make changes to this bug.