Missing addons, directory distribution/extensions is not there

RESOLVED FIXED in seamonkey2.4

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: wolfgang, Assigned: Callek)

Tracking

SeaMonkey 2.1 Branch
seamonkey2.4
x86
Windows 7

Firefox Tracking Flags

(blocking-seamonkey2.1 -, seamonkey2.2+ fixed, seamonkey2.3+ fixed, seamonkey2.4+ fixed)

Details

Attachments

(1 attachment)

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20110608 Firefox/4.0.1 SeaMonkey/2.1

Hallo

On Linux installation and when i use Windows7 en-US version there is a directory distribution/extensions with ChatZilla, DOM Inspector and Venkman. 

On Windows7 national de installation the directory distribution is empty.

Reproducible: Always

Steps to Reproduce:
1. Install on Windows7 german SeaMonkey 2.1
2. Setup Type Standard
3. Create a new Profile !!!

Actual Results:  
The directory distribution is empty.

Expected Results:  
In directory distribution/extensions is ChatZilla, DOM Inspector and Venkman.

When i unpack the SeaMonkey Setup 2.1.exe with 7z, i see in the en-US version a empty directory core/distribution/extensions. In the de version this is not existing.
Version: unspecified → SeaMonkey 2.1 Branch
CC the usual suspects.
Maybe Bug 660427 is related, not sure.
BTW: The import directory in the installer package is optional\distribution\extensions\ as that one includes the extensions that can be installed.
Hrm, I know what's wrong: "When i unpack the SeaMonkey Setup 2.1.exe with 7z, i see in the en-US version a empty directory core/distribution/extensions". That's very important so that CopyFiles in NSIS will work. The repackaging code deletes that empty folder in the installer.
Status: UNCONFIRMED → NEW
Ever confirmed: true
"not this again" argh!

We might need a 2.1.1 :/
blocking-seamonkey2.1: --- → ?
two ways to fix this particular issue.

The easiest with the code we have in place that "fixed" the installer issue (we thought) is to modify the installer code to create the destdir for us (and ignore errors if exists)

That said the m-c based patch (we did in Bug 660427) has not yet had review, so we can easily modify that solution.

(The related downside to this, is that we'll want to forcibly cancel partials for the l10n users if we do a 2.1.1 since the partial generation will still see the optional/ stuff even though if they used the installer it won't be present)

Zip builds should be fine as well.
(In reply to comment #4)
> Hrm, I know what's wrong: "When i unpack the SeaMonkey Setup 2.1.exe with
> 7z, i see in the en-US version a empty directory
> core/distribution/extensions". That's very important so that CopyFiles in
> NSIS will work. The repackaging code deletes that empty folder in the
> installer.

maybe add a bogus zero-length file or a README or whatever so that braindead repackager doesn't think it can remove the directory?
So the problem is that we want to copy the files into an non-existing directory in the target (as we first copy all of core/ and then the specific files from optional/)?

Can't we make the installer *create* that directory in the target and therefore not need to bake knowledge of it into the packager hack?
(In reply to comment #8)
> So the problem is that we want to copy the files into an non-existing
> directory in the target (as we first copy all of core/ and then the specific
> files from optional/)?

That's right.

> Can't we make the installer *create* that directory in the target and
> therefore not need to bake knowledge of it into the packager hack?

Yes, would work. Though I think fixing the packager hack would be better, creating that folder in the installer would also be some kind of hack.
(In reply to comment #9)
> That's right.

"Good".

> > Can't we make the installer *create* that directory in the target and
> > therefore not need to bake knowledge of it into the packager hack?
> 
> Yes, would work. Though I think fixing the packager hack would be better,
> creating that folder in the installer would also be some kind of hack.

Actually, the packager hack doesn't really know anything about distribution/ - reading attachment 539112 [details] [diff] [review] will make clear that all it knows about is optional/ - and so I neither know why/where it does that removal nor should it be made to know about more than the generic stuff.

Either there is code in suite/locales/Makefile.in that does the removal and that should be fixed, or the installer should care to have the directories it needs to write to (the safer and more future-proof variant, not "a hack" IMHO). The fix very much should belong in suite/ in any case.
(In reply to comment #10)
> (In reply to comment #9)
> > That's right.
> 
> "Good".
> 
> > > Can't we make the installer *create* that directory in the target and
> > > therefore not need to bake knowledge of it into the packager hack?
> > 
> > Yes, would work. Though I think fixing the packager hack would be better,
> > creating that folder in the installer would also be some kind of hack.
> 
> Actually, the packager hack doesn't really know anything about distribution/
> - reading attachment 539112 [details] [diff] [review] [review] will make clear that all it
> knows about is optional/ - and so I neither know why/where it does that
> removal nor should it be made to know about more than the generic stuff.

Actually that IS the patch that does the removal of this.

|mv -t optional/ core/{};| is the part that does. the optional_list contains |distribution| on us. We should fix that, but its not as easy of a fix as this (which I would argue is more correct, even if we do fix it there as well)
Will take the first reviewer. (also pre-review requesting approval).

I have not tested my installer of this yet, (but it does build) so additional testing appreciated.

Based on docs at http://nsis.sourceforge.net/Docs/Chapter4.html
Assignee: installer → bugspam.Callek
Status: NEW → ASSIGNED
Attachment #539989 - Flags: review?(kairo)
Attachment #539989 - Flags: approval-comm-beta?
Attachment #539989 - Flags: approval-comm-aurora?
Attachment #539989 - Flags: review?(bugzilla)
Comment on attachment 539989 [details] [diff] [review]
Add distribution/extensions always.

Sounds exactly like what I proposed. ;-)
Attachment #539989 - Flags: review?(kairo)
Attachment #539989 - Flags: review+
Attachment #539989 - Flags: approval-comm-beta?
Attachment #539989 - Flags: approval-comm-beta+
Attachment #539989 - Flags: approval-comm-aurora?
Attachment #539989 - Flags: approval-comm-aurora+
(In reply to comment #11)
> |mv -t optional/ core/{};| is the part that does. the optional_list contains
> |distribution| on us. We should fix that, but its not as easy of a fix as
> this (which I would argue is more correct, even if we do fix it there as
> well)

Ah, misread that, I thought it would actually take the real files underneath, not the whole directory. In any case, I feel safer with the installer fix. ;-)
http://hg.mozilla.org/releases/comm-aurora/rev/e2050a2ff32a
http://hg.mozilla.org/comm-central/rev/70f82b97c52c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.2
blocking-seamonkey2.1: ? → -
Attachment #539989 - Flags: review?(bugzilla)
Target Milestone: seamonkey2.2 → seamonkey2.4
You need to log in before you can comment on or make changes to this bug.