Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Port |Bug 511642 - use a single packaging manifest across all three platforms (with preprocessing)| to SeaMonkey

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Build Config
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: Robert Kaiser)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1a1
Dependency tree / graph
Bug Flags:
wanted-seamonkey2.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
I guess we may want to do that for our apps too?
That would be a per-app decision, not something mailnews could or should decide for both SM and Tb.
Component: Build Config → Build Config
Product: MailNews Core → SeaMonkey
QA Contact: build-config → build-config
(Reporter)

Updated

8 years ago
Summary: Port |Bug 511642 - use a single packaging manifest across all three platforms (with preprocessing)| to comm-central (apps) → Port |Bug 511642 - use a single packaging manifest across all three platforms (with preprocessing)| to SeaMonkey
(Assignee)

Comment 2

8 years ago
Yes, I think we should do per-app bugs, the patches will get large enough and the decisions are really app-specific.

That said, I want it for SeaMonkey but only on 2.1, not on 2.0, and so I didn't even file it as long as we are not branched.
The Firefox patch wasn't that complicated to write. Most of the difficulty was just verifying all the differences between the packaging manifests for different platforms. I probably could have done that a little smarter by writing a script to just print out files that were packaged in only one manifest or something. It was just a matter of removing ifdefs from installer/Makefile.in, adding a bunch of #ifdefs to the package manifest, and verifying that my package manifest name change didn't break anything else.
(Reporter)

Updated

8 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted-seamonkey2.1?
(Assignee)

Comment 4

8 years ago
The biggest obstacle there is probably Mac, as we don't have a package manifest for that right now - we probably need one if we want to run tests on debug and opt builds and not clutter packages with test files, though.
(Reporter)

Comment 5

8 years ago
Would it make sense to start by merging our Unix and Windows files, then add MacOSX as a second step?
(Assignee)

Comment 6

8 years ago
Taking, I have started to work on a patch.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Flags: wanted-seamonkey2.1? → wanted-seamonkey2.1+
Target Milestone: --- → seamonkey2.1a1
(Assignee)

Updated

8 years ago
Blocks: 543289
(Assignee)

Updated

8 years ago
Blocks: 535342
(Assignee)

Comment 7

8 years ago
Created attachment 424781 [details] [diff] [review]
patch v1: create new unified manifest, replace old package files

This patch should do it. I have tested that package-compare looks good on my Linux machines (with a shared build), but I guess the real proof that it works in the situations that are really important for us is just when it actually runs on our build boxes, i.e. after checkin.
Attachment #424781 - Flags: review?(bugspam.Callek)
(Reporter)

Comment 8

8 years ago
Comment on attachment 424781 [details] [diff] [review]
patch v1: create new unified manifest, replace old package files

>+++ b/suite/installer/package-manifest.in
>@@ -0,0 +1,917 @@
>+; Package file for the Firefox build. 

Nit: not 'Firefox' ;-]
(Reporter)

Updated

8 years ago
Blocks: 541203
(Assignee)

Comment 9

8 years ago
Created attachment 425330 [details] [diff] [review]
patch v2: new unified manifest, updated for try server results

OK, I did run the patch through the try server, updated it a bit and did run this second version through it again. Everything should look good with this version, it has only small nits corrected to the last try server run.
Attachment #424781 - Attachment is obsolete: true
Attachment #425330 - Flags: review?(bugspam.Callek)
Attachment #424781 - Flags: review?(bugspam.Callek)
Created attachment 425496 [details]
Diff of Mac package-compare [modulo some manual edits: see comment]

Robert, there are still some issues here with actual packaging...

(I stripped any - foo/* lines followed by +foo/bar  etc.)

Updated

8 years ago
Attachment #425496 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 425330 [details] [diff] [review]
patch v2: new unified manifest, updated for try server results

Just some brief comments so far based on seeing the try-server mac build log

>diff --git a/suite/installer/package-manifest.in b/suite/installer/package-manifest.in
>new file mode 100644
>+#ifdef XP_MACOSX
>+@BINPATH@/plugins/DefaultPlugin.plugin/

Mean to use a * ?

>+
>+; Modules
>+@BINPATH@/modules/*

[see note later]

>+#ifdef XP_MACOSX
>+@BINPATH@/res/MainMenu.nib/
>+#endif

Meant to use a *?

>+#ifdef XP_MACOSX
>+@BINPATH@/updater.app/
>+#else

Meant to use a *?

>+#ifdef XP_MACOSX
>+@BINPATH@/crashreporter.app/

Don't you mean @BINPATH@/crashreporter.app/*

>+@BINPATH@/modules/gloda/*

Redundant with an earlier @BINPATH@/modules/*
(Though this is under [mail] and the earlier one is under [browser]. does that matter?)
(Assignee)

Comment 12

8 years ago
Comment on attachment 425496 [details]
Diff of Mac package-compare [modulo some manual edits: see comment]

>+SeaMonkey.app/Contents/MacOS/components/widget_cocoa.xpt

That's corrected in the patch, btw.

>-SeaMonkey.app/Contents/MacOS/crashreporter-override.ini

And so is this, we don't have that file, wrongly had copied it from FF.

>+SeaMonkey.app/Contents/MacOS/isp/dotmac.rdf

If anyone wonders, Mnyromyr has made us intentionally not ship this one.

>-SeaMonkey.app/Contents/MacOS/res/MainMenu.nib/

>-SeaMonkey.app/Contents/MacOS/updater.app/

I copied those and similar entries from FF/TB and apparently they work fine even without the * at the end. Actually, I wonder if that scheme would help us with modules/ to not get warnings about the gloda stuff...
(Assignee)

Comment 13

8 years ago
(In reply to comment #11)
> (From update of attachment 425330 [details] [diff] [review])
> >+@BINPATH@/modules/gloda/*
> 
> Redundant with an earlier @BINPATH@/modules/*
> (Though this is under [mail] and the earlier one is under [browser]. does that
> matter?)

Actually, it's not redundant, interestingly. the modules/* does only successfully add files directly under modules/ and not those in the subdir, which is why we need that additional line. This is what we had in the old manifests and what TB has as well.

It might be worth a followup to try if using just "@BINPATH@/modules/" without the * might actually make us add the whole subtree. I would like to only go for that in a followup, though, and potentially point TB folks at that as well if it works.
(Reporter)

Updated

8 years ago
Blocks: 536451
philor noted in some other bug that just listing a directory doesn't work on Windows. Those ones you copied from the Firefox manifest are ok because they're mac-only.
(Assignee)

Comment 15

8 years ago
Ah, ted, thanks, good to know.
Comment on attachment 425330 [details] [diff] [review]
patch v2: new unified manifest, updated for try server results

without doing a manual scan or even checking the package-compare on try win/linux. r+

If you want my closer eye on that stuff, I'll be glad to give it.

Just please do the |*| additions that I pointed out earlier based on ted's comment.

This also needs a removed-files update for the mac now. Can be a followup bug, or a new patch here, your choice.
Attachment #425330 - Flags: review?(bugspam.Callek) → review+
(Assignee)

Comment 17

8 years ago
Created attachment 425635 [details] [diff] [review]
removed-files update

Here's the removed-files update needed for not breaking Mac now that we are linking XPTs. We could go and wrap all those additions in ifdef XP_MACOSX but I'm not sure this is really need or the way to go, actually...
Callek, if you wish me to do that, please let me know.
Attachment #425635 - Flags: review?(bugspam.Callek)
(Assignee)

Comment 18

8 years ago
(In reply to comment #16)
> Just please do the |*| additions that I pointed out earlier based on ted's
> comment.

Erm, his comment said that those where I did it are OK - and from all we know, * doesn't go deeper than one directory, while we know just / does on Mac, and all those I have in there are Mac-only.
(In reply to comment #18)
> (In reply to comment #16)
> > Just please do the |*| additions that I pointed out earlier based on ted's
> > comment.
> 
> Erm, his comment said that those where I did it are OK - and from all we know,
> * doesn't go deeper than one directory, while we know just / does on Mac, and
> all those I have in there are Mac-only.

erm right; sorry-carry on.

Updated

8 years ago
Attachment #425635 - Flags: review?(bugspam.Callek) → review-
Comment on attachment 425635 [details] [diff] [review]
removed-files update

I'm more partial to a section similar to TB's:

http://mxr.mozilla.org/comm-central/source/mail/installer/removed-files.in#681

Part of the draw of that for me is "You don't want to add anything to this section" comment

But I'm ok if you would rather ifdef this as-is.
(Assignee)

Comment 21

8 years ago
Created attachment 425652 [details] [diff] [review]
removed-files update, v2

This patch moves the XPTs to the end of the file into their own section.
Attachment #425635 - Attachment is obsolete: true
Attachment #425652 - Flags: review?(bugspam.Callek)

Updated

8 years ago
Attachment #425652 - Flags: review?(bugspam.Callek) → review+
(Assignee)

Comment 22

8 years ago
Pushed both patches to comm-central:

http://hg.mozilla.org/comm-central/rev/a26c51539abe
http://hg.mozilla.org/comm-central/rev/7a8ee180d883
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Updated

8 years ago
No longer blocks: 541203
(Reporter)

Updated

8 years ago
Blocks: 539355
(Reporter)

Updated

8 years ago
Depends on: 545320
(Reporter)

Comment 23

8 years ago
(In reply to comment #22)

Ftr, this happened on waterfall page
http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey&maxdate=1265527556&hours=24&legend=0&norules=1
between changesets 789dd6cd45ef and 7a8ee180d883.
(Reporter)

Updated

8 years ago
Blocks: 545471
(Reporter)

Updated

8 years ago
Blocks: 524030

Updated

8 years ago
Depends on: 545534
(Reporter)

Comment 24

8 years ago
Created attachment 426477 [details] [diff] [review]
(Cv1-191) Fix Windows README.txt packaging on c-1.9.1 too

This line had landed as is in bug 351917.
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey2.0/1265874980.1265876732.20126.gz&fulltext=1
WINNT 5.2 comm-1.9.1 build on 2010/02/10 23:56:20

make package-compare

-bin/README.txt,bin/readme.txt
+bin/README.txt
}

You fixed it on c-c here.
Attachment #426477 - Flags: review?(kairo)
Attachment #426477 - Flags: approval-seamonkey2.0.4?
(Reporter)

Updated

8 years ago
Depends on: 545628
(Assignee)

Comment 25

8 years ago
Comment on attachment 426477 [details] [diff] [review]
(Cv1-191) Fix Windows README.txt packaging on c-1.9.1 too

This is there intentionally, as FAT32 seems to sometimes just catch the file only with lower-case spelling.
Attachment #426477 - Flags: review?(kairo)
Attachment #426477 - Flags: review-
Attachment #426477 - Flags: approval-seamonkey2.0.4?
Attachment #426477 - Flags: approval-seamonkey2.0.4-
(Reporter)

Updated

8 years ago
Blocks: 524033
(Reporter)

Updated

8 years ago
Depends on: 545631
(Reporter)

Comment 26

8 years ago
(In reply to comment #25)

Ah. Ftr, why is this hack not needed anymore on c-c?
(Assignee)

Comment 27

8 years ago
(In reply to comment #26)
> (In reply to comment #25)
> 
> Ah. Ftr, why is this hack not needed anymore on c-c?

Because I just tried to go without it - or forgot to port it over. Choose one.
(Reporter)

Updated

8 years ago
Depends on: 545777
(Reporter)

Comment 28

8 years ago
(In reply to comment #27)

I filed bug 545777.
(Reporter)

Updated

8 years ago
Blocks: 487283
Depends on: 485935
(Reporter)

Updated

8 years ago
Blocks: 509147
(Reporter)

Updated

8 years ago
Depends on: 547499
(Reporter)

Updated

8 years ago
Blocks: 550657
(Assignee)

Updated

7 years ago
Duplicate of this bug: 511851
(Reporter)

Updated

7 years ago
Depends on: 564657
(Reporter)

Updated

7 years ago
Depends on: 564606
You need to log in before you can comment on or make changes to this bug.