629037 - Installer for trunk builds no longer installs extensions since omnijar landing.

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Comment 1

[Justin Wood (:Callek)]

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)]

Iiuc, there's an installation log.
Does it report anything about this?

Comment 3

[Justin Wood (:Callek)]

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)]

Fwiw, see also bug 627240, not to do unnecessary work (unless it's trivial).

Comment 5

[[e-mail]]

You'll have to be careful to remove both the old [e-mail] dir and any (not-so-old!) [e-mail] XPI.

Comment 6

[Edmund Wong (:ewong)]

Attachment: Fixed the installer to also install the extensions.

Comment 7

[Justin Wood (:Callek)]

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)]

Attachment: Fixed installer to install extensions (after omnijar)

Comment 9

[Justin Wood (:Callek)]

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)]

Attachment: Fixed installer to install extensions (after omnijar) [Checkin: comment 11]

Fixed nit from comment#9.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

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)]

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.