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

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
Installer
--
blocker
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Callek, Assigned: ewong)

Tracking

({regression})

Trunk
seamonkey2.1b2
x86
Windows XP
regression

Firefox Tracking Flags

(blocking-seamonkey2.1 b2+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Reporter)

Updated

7 years ago
blocking-seamonkey2.1: --- → ?
(Reporter)

Comment 1

7 years ago
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.
Blocks: 588067
Keywords: regression
Iiuc, there's an installation log.
Does it report anything about this?
(Reporter)

Comment 3

7 years ago
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.
Fwiw, see also bug 627240, not to do unnecessary work (unless it's trivial).
Depends on: 627240

Comment 5

7 years ago
You'll have to be careful to remove both the old inspector@mozilla.org dir and any (not-so-old!) inspector@mozilla.org XPI.

Updated

7 years ago
No longer depends on: 627240

Updated

7 years ago
blocking-seamonkey2.1: ? → b2+
(Assignee)

Comment 6

7 years ago
Created attachment 507850 [details] [diff] [review]
Fixed the installer to also install the extensions.
Assignee: bugspam.Callek → ewong
Status: NEW → ASSIGNED
(Assignee)

Updated

7 years ago
Attachment #507850 - Flags: review?(bugzilla)
(Reporter)

Comment 7

7 years ago
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.
Attachment #507850 - Flags: feedback+
(Assignee)

Comment 8

7 years ago
Created attachment 508075 [details] [diff] [review]
Fixed installer to install extensions (after omnijar)
Attachment #507850 - Attachment is obsolete: true
Attachment #508075 - Flags: review?(bugzilla)
Attachment #508075 - Flags: feedback?(bugspam.Callek)
Attachment #507850 - Flags: review?(bugzilla)
(Reporter)

Comment 9

7 years ago
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.
Attachment #508075 - Flags: review?(bugzilla)
Attachment #508075 - Flags: review+
Attachment #508075 - Flags: feedback?(bugspam.Callek)
(Assignee)

Comment 10

7 years ago
Created attachment 508078 [details] [diff] [review]
Fixed installer to install extensions (after omnijar) [Checkin: comment 11]

Fixed nit from comment#9.
Attachment #508075 - Attachment is obsolete: true
Attachment #508078 - Flags: review+
(Reporter)

Updated

7 years ago
Keywords: checkin-needed
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
Attachment #508078 - Attachment description: Fixed installer to install extensions (after omnijar) → Fixed installer to install extensions (after omnijar) [Checkin: comment 11]
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.
Keywords: checkin-needed
Target Milestone: --- → seamonkey2.1b2
(Reporter)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.