Last Comment Bug 629037 - Installer for trunk builds no longer installs extensions since omnijar landing.
: Installer for trunk builds no longer installs extensions since omnijar landing.
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: Installer (show other bugs)
: Trunk
: x86 Windows XP
: -- blocker (vote)
: seamonkey2.1b2
Assigned To: Edmund Wong (:ewong)
: installer
Mentors:
Depends on:
Blocks: 588067
  Show dependency treegraph
 
Reported: 2011-01-26 10:02 PST by Justin Wood (:Callek)
Modified: 2011-01-29 14:31 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
b2+


Attachments
Fixed the installer to also install the extensions. (17.83 KB, patch)
2011-01-28 07:21 PST, Edmund Wong (:ewong)
bugspam.Callek: feedback+
Details | Diff | Review
Fixed installer to install extensions (after omnijar) (19.60 KB, patch)
2011-01-28 20:20 PST, Edmund Wong (:ewong)
bugspam.Callek: review+
Details | Diff | Review
Fixed installer to install extensions (after omnijar) [Checkin: comment 11] (19.48 KB, patch)
2011-01-28 22:09 PST, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Review

Description Justin Wood (:Callek) 2011-01-26 10:02:32 PST

    
Comment 1 Justin Wood (:Callek) 2011-01-26 10:03:44 PST
err no c#0...

When installing into a fresh directory a SeaMonkey trunk build since our omnijar changes, the windows installer does not install any extensions other than our themes.

The Zip unpacks are fine.
Comment 2 Serge Gautherie (:sgautherie) 2011-01-26 13:57:40 PST
Iiuc, there's an installation log.
Does it report anything about this?
Comment 3 Justin Wood (:Callek) 2011-01-26 20:59:34 PST
Ok, I did some brief testing here... staging works fine both for installer-stage and the package-stage steps... so it IS inside the NSIS installer code as far as where the issue lies.

And some brief digging [and I do mean brief] turns up:

http://mxr.mozilla.org/comm-central/source/suite/installer/windows/nsis/installer.nsi?mark=229-229,233-233,241-241,479-500,502-516,721-739#232

See all my marked sections, these are at a minimum what needs to change, I have not yet looked at other installer files.

If someone wants to steal this from me in 24 hours, that would be great; otherwise I'll churn out a patch by friday evening; hopefully in time for nightlies.
Comment 4 Serge Gautherie (:sgautherie) 2011-01-26 21:25:42 PST
Fwiw, see also bug 627240, not to do unnecessary work (unless it's trivial).
Comment 5 neil@parkwaycc.co.uk 2011-01-27 01:28:37 PST
You'll have to be careful to remove both the old inspector@mozilla.org dir and any (not-so-old!) inspector@mozilla.org XPI.
Comment 6 Edmund Wong (:ewong) 2011-01-28 07:21:55 PST
Created attachment 507850 [details] [diff] [review]
Fixed the installer to also install the extensions.
Comment 7 Justin Wood (:Callek) 2011-01-28 08:35:33 PST
Comment on attachment 507850 [details] [diff] [review]
Fixed the installer to also install the extensions.

>+++ b/suite/installer/windows/nsis/installer.nsi
>@@ -226,11 +226,11 @@ Section "-InstallStartCleanup"
>     ; from the installation directory. This will remove it if the user
>     ; deselected ChatZilla on the components page.
>     ${If} ${FileExists} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
>-    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
>+    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
>       RmDir /r "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
>     ${EndIf}
>     ${If} ${FileExists} "$INSTDIR\extensions\langpack-${AB_CD}@chatzilla.mozilla.org"
>-    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\langpack-${AB_CD}@chatzilla.mozilla.org"
>+    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\langpack-${AB_CD}@chatzilla.mozilla.org.xpi"
>       RmDir /r "$INSTDIR\extensions\langpack-${AB_CD}@chatzilla.mozilla.org"
>     ${EndIf}
> 

Nit these need to account for the new .xpi files as well as the old .xpi files in the final-install-dir.. so something like:

${If} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
  ${If} ${FileExists} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
    RmDir /r "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
  ${EndIf}
  ${If} ${FileExists} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
    ${DeleteFile} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
  ${EndIf}
${EndIf}

same with the rest of these optional components in this block

>@@ -477,79 +477,73 @@ Section "-Application" APP_IDX
> Section /o "IRC Client" CZ_IDX 
>+    ${If} ${FileExists} "$EXEDIR\optional\extensions\langpack-${AB_CD}@chatzilla.mozilla.org.xpi"
>+      CopyFiles /SILENT "$EXEDIR\optional\extensions\" \
>+                          "$INSTDIR\extensions\" 

need to Copy the langpack specifically here.

And other than that it looks good from a code skim, I'll test and do a slightly more in dept review [as in, if you missed anything] later, if mcsmurf doesn't beat me to it.
Comment 8 Edmund Wong (:ewong) 2011-01-28 20:20:11 PST
Created attachment 508075 [details] [diff] [review]
Fixed installer to install extensions (after omnijar)
Comment 9 Justin Wood (:Callek) 2011-01-28 21:33:38 PST
Comment on attachment 508075 [details] [diff] [review]
Fixed installer to install extensions (after omnijar)

>+++ b/suite/installer/windows/nsis/installer.nsi
>@@ -226,41 +226,65 @@ Section "-InstallStartCleanup"
>     ; from the installation directory. This will remove it if the user
>     ; deselected ChatZilla on the components page.
>     ${If} ${FileExists} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
>-    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
>+    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
>       RmDir /r "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
>     ${EndIf}
>     ${If} ${FileExists} "$INSTDIR\extensions\langpack-${AB_CD}@chatzilla.mozilla.org"
>-    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\langpack-${AB_CD}@chatzilla.mozilla.org"
>+    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\langpack-${AB_CD}@chatzilla.mozilla.org.xpi"
>       RmDir /r "$INSTDIR\extensions\langpack-${AB_CD}@chatzilla.mozilla.org"
>     ${EndIf}
>+    ${If} ${FileExists} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
>+    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
>+      ${DeleteFile} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
>+    ${EndIf}
>+    ${If} ${FileExists} "$INSTDIR\extensions\langpack-${AB_CD}@chatzilla.mozilla.org.xpi"
>+    ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\langpack-${AB_CD}@chatzilla.mozilla.org.xpi"
>+      ${DeleteFile} "$INSTDIR\extensions\langpack-${AB_CD}@chatzilla.mozilla.org.xpi"
>+    ${EndIf}

Thanks for this, and it is roughly how I had asked for it, BUT I just noticed a Mozilla-macro/define that makes this easier...

${DeleteFile} does the If Exists stuff itself, and ${RemoveDir} does the same for the dir [though it only works if dir is empty]...

So you could rewrite as the much cleaner:

${If} ${FileExists} "$EXEDIR\optional\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
  ${DeleteFile} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}.xpi"
  ${If} ${FileExists} "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
    RmDir /r "$INSTDIR\extensions\{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}"
  ${EndIf}
${EndIf}

...in fact I would prefer that.

r+ with that changed, pending my test I'm about to perform.
Comment 10 Edmund Wong (:ewong) 2011-01-28 22:09:11 PST
Created attachment 508078 [details] [diff] [review]
Fixed installer to install extensions (after omnijar) [Checkin: comment 11]

Fixed nit from comment#9.
Comment 11 Jens Hatlak (:InvisibleSmiley) 2011-01-29 10:47:41 PST
Comment on attachment 508078 [details] [diff] [review]
Fixed installer to install extensions (after omnijar) [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/656edcb89176
Comment 12 Jens Hatlak (:InvisibleSmiley) 2011-01-29 10:52:04 PST
I already checked that the wizard page with the extensions selection only appears with this patch applied (otherwise I wouldn't have pushed it!), still someone should verify this is indeed fixed using a nightly build installer (once available) and then change the status to VERIFIED. This should include actually installing and checking different cases, which I didn't do.

Note You need to log in before you can comment on or make changes to this bug.