The default bug view has changed. See this FAQ.

Stop making extensions optional in SeaMonkey's installer

RESOLVED FIXED in seamonkey2.4

Status

SeaMonkey
Installer
--
major
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Callek, Assigned: Callek)

Tracking

(Blocks: 3 bugs)

SeaMonkey 2.2 Branch
seamonkey2.4
x86
All
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.2+ fixed, seamonkey2.3 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 541306 [details] [diff] [review]
drop optional components

For the 2.2b1 run, we had some issues with extensions again, similar to what delayed 2.1

These issues is what Bug 660427 was meant to solve, but seems more stuff is wrong in this round of updates, and rather than trying to untangle and maintain that mess I feel simply dropping the optional/ aspect of this all is a better solution for SeaMonkey 2.2 and future. (We can revisit the optional/ aspect with regard to "install to profile" on a future date, but the "install to app dir from installer" I'd vote to keep unless Firefox/MoCo want to direct support that potential)

The issues:
* en-US complete mar didn't notice the files that were meant to be optional, because it only bothered with |core/| files...
* repacks again didn't catch |optional/| on unpack and stuffed the langpacks to |core/|

I'm attaching a l10n-free patch, that does not attempt any dead-code removal. If this gets r+ I'll check this in, and backout Bug 660427 from trunk m-c. and Spin 2.2b2 without that patch.

This turned out to be simple patch to do, and avoids UX issues in the installer due to http://mxr.mozilla.org/comm-central/source/suite/installer/windows/nsis/custom.nsi#38

We can do a followup bug for the l10n and dead-code-removal parts of this.

By dropping this all codepaths simply put the extensions in |core/| as expected.

Also of note is the code at http://mxr.mozilla.org/comm-central/source/mozilla/extensions/venkman/locales/Makefile.in#111 (to EOF) and similarly for chatzilla seem to no longer be being hit at all, so no need to worry about that atm.

Review for KaiRo as he knows the RelEng reasons behind this quite well. Feedback to mcsmurf for he is the current module owner of the installer so should get a glance, and for record Ratty was OK with this plan after I advocated it and explained it.

For future followup we can re-instate the optional nature here, but without specifically having "optional" files. And instead restrict the ability for the application to push the extensions to profile.
Attachment #541306 - Flags: review?(kairo)
Attachment #541306 - Flags: feedback?(bugzilla)
Attachment #541306 - Flags: approval-comm-beta?
Attachment #541306 - Flags: approval-comm-aurora?

Comment 1

6 years ago
(In reply to comment #0)
> We can do a followup bug for the l10n and dead-code-removal parts of this.

Please do (I'll look at the actual review later today).

> Also of note is the code at
> http://mxr.mozilla.org/comm-central/source/mozilla/extensions/venkman/
> locales/Makefile.in#111 (to EOF) and similarly for chatzilla seem to no
> longer be being hit at all, so no need to worry about that atm.

It is hit and must work, but only for an actual repack step in a locale other than en-US (and one that actually has a venkman L10n - try de for testing).

Comment 2

6 years ago
As a remark, I can only review from code inspection, I can't build currently as building is b0rken with a Linux 3.0 kernel due to some NSPR stuff (IIRC) that is forked by kernel version.
(Assignee)

Comment 3

6 years ago
(In reply to comment #1)
> (In reply to comment #0)
> > Also of note is the code at
> > http://mxr.mozilla.org/comm-central/source/mozilla/extensions/venkman/
> > locales/Makefile.in#111 (to EOF) and similarly for chatzilla seem to no
> > longer be being hit at all, so no need to worry about that atm.
> 
> It is hit and must work, but only for an actual repack step in a locale
> other than en-US (and one that actually has a venkman L10n - try de for
> testing).

We don't seem to when I check de. We *do* hit repackage-zip-@AB_CD@. I tried searching the repack logs for |repackage-win32|.

For ref, our release repack run # 72 was de.

We *used to* but don't seem to now. [looks like we *should* of course if we wanted to keep optional working]

Comment 4

6 years ago
Comment on attachment 541306 [details] [diff] [review]
drop optional components

That all said, if this helps the whole issue, let's do it. What's the impact of this on the installer UI? I fear it's not a good idea to ship broken UI...
Attachment #541306 - Flags: review?(kairo) → review+

Comment 5

6 years ago
(In reply to comment #3)
> We don't seem to when I check de. We *do* hit repackage-zip-@AB_CD@. I tried
> searching the repack logs for |repackage-win32|.

Hah, good catch, actually. http://hg.mozilla.org/comm-central/rev/e58c1b036d73 removed this as part of bug 586848 and made everything go through repackage-zip apparently.

Followups on ChatZilla and venkman should be filed to get that dead code removed. Actually, I wonder if followup are also in order to make toolkit installer and build system get rid of the optional extension mechanism completely, as apparently it doesn't really work in a usable way nowadays and it's complicated code that would be helpful to have cleaned up.

Updated

6 years ago
Attachment #541306 - Flags: approval-comm-beta?
Attachment #541306 - Flags: approval-comm-beta+
Attachment #541306 - Flags: approval-comm-aurora?
Attachment #541306 - Flags: approval-comm-aurora+
(Assignee)

Comment 6

6 years ago
http://hg.mozilla.org/releases/comm-beta/rev/e3aefa68c646
http://hg.mozilla.org/releases/comm-aurora/rev/f606bd2859ca
http://hg.mozilla.org/comm-central/rev/f0eadc314e20
tracking-seamonkey2.2: ? → +
tracking-seamonkey2.3: ? → ---
tracking-seamonkey2.4: ? → ---
Target Milestone: --- → seamonkey2.2
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Blocks: 666823
(Assignee)

Updated

6 years ago
Blocks: 666824
(Assignee)

Updated

6 years ago
Blocks: 666825
(Assignee)

Updated

6 years ago
Blocks: 666834
(Assignee)

Updated

6 years ago
Depends on: 666836

Updated

6 years ago
Blocks: 666836
No longer depends on: 666836
(Assignee)

Updated

6 years ago
status-seamonkey2.2: --- → fixed
status-seamonkey2.3: --- → fixed
Target Milestone: seamonkey2.2 → seamonkey2.4
(Assignee)

Updated

6 years ago
Attachment #541306 - Flags: feedback?(bugzilla)
You need to log in before you can comment on or make changes to this bug.