Closed
Bug 629037
Opened 13 years ago
Closed 13 years ago
Installer for trunk builds no longer installs extensions since omnijar landing.
Categories
(SeaMonkey :: Installer, defect)
Tracking
(blocking-seamonkey2.1 b2+)
RESOLVED
FIXED
seamonkey2.1b2
Tracking | Status | |
---|---|---|
blocking-seamonkey2.1 | --- | b2+ |
People
(Reporter: Callek, Assigned: ewong)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
19.48 KB,
patch
|
ewong
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•13 years ago
|
blocking-seamonkey2.1: --- → ?
Reporter | ||
Comment 1•13 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.
Updated•13 years ago
|
Blocks: 588067
Keywords: regression
Comment 2•13 years ago
|
||
Iiuc, there's an installation log. Does it report anything about this?
Reporter | ||
Comment 3•13 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.
Comment 4•13 years ago
|
||
Fwiw, see also bug 627240, not to do unnecessary work (unless it's trivial).
Depends on: 627240
Comment 5•13 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•13 years ago
|
blocking-seamonkey2.1: ? → b2+
Assignee | ||
Comment 6•13 years ago
|
||
Assignee: bugspam.Callek → ewong
Status: NEW → ASSIGNED
Assignee | ||
Updated•13 years ago
|
Attachment #507850 -
Flags: review?(bugzilla)
Reporter | ||
Comment 7•13 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•13 years ago
|
||
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•13 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•13 years ago
|
||
Fixed nit from comment#9.
Attachment #508075 -
Attachment is obsolete: true
Attachment #508078 -
Flags: review+
Reporter | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 11•13 years ago
|
||
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]
Comment 12•13 years ago
|
||
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•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•