663636 - Missing addons, directory distribution/extensions is not there

Status: RESOLVED FIXED

Description

[Wolfgang Germund]

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.

Comment 1

[Philip Chee]

CC the usual suspects.

Comment 2

[Frank Wein [:mcsmurf]]

Maybe Bug 660427 is related, not sure.

Comment 3

[Frank Wein [:mcsmurf]]

BTW: The import directory in the installer package is optional\distribution\extensions\ as that one includes the extensions that can be installed.

Comment 4

[Frank Wein [:mcsmurf]]

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.

Comment 5

[Justin Wood (:Callek)]

"not this again" argh!

We might need a 2.1.1 :/

Comment 6

[Justin Wood (:Callek)]

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.

Comment 7

[Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention)]

(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?

Comment 8

[Robert Kaiser]

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?

Comment 9

[Frank Wein [:mcsmurf]]

(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.

Comment 10

[Robert Kaiser]

(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.

Comment 11

[Justin Wood (:Callek)]

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

Comment 12

[Justin Wood (:Callek)]

Attachment: Add distribution/extensions always.

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

Comment 13

[Robert Kaiser]

Comment on attachment 539989 [details] [diff] [review]
Add distribution/extensions always.

Sounds exactly like what I proposed. ;-)

Comment 14

[Robert Kaiser]

(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. ;-)

Comment 15

[Justin Wood (:Callek)]

http://hg.mozilla.org/releases/comm-beta/rev/bc9932900a34

Comment 16

[Justin Wood (:Callek)]

http://hg.mozilla.org/releases/comm-aurora/rev/e2050a2ff32a
http://hg.mozilla.org/comm-central/rev/70f82b97c52c