Closed
Bug 521523
Opened 15 years ago
Closed 15 years ago
Port |Bug 511642 - use a single packaging manifest across all three platforms (with preprocessing)| to SeaMonkey
Categories
(SeaMonkey :: Build Config, defect)
SeaMonkey
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.1a1
People
(Reporter: sgautherie, Assigned: kairo)
References
Details
Attachments
(4 files, 2 obsolete files)
74.93 KB,
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
8.69 KB,
text/plain
|
Details | |
5.00 KB,
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
948 bytes,
patch
|
kairo
:
review-
kairo
:
approval-seamonkey2.0.4-
|
Details | Diff | Splinter Review |
I guess we may want to do that for our apps too?
Comment 1•15 years ago
|
||
That would be a per-app decision, not something mailnews could or should decide for both SM and Tb.
Product: MailNews Core → SeaMonkey
QA Contact: build-config → build-config
Reporter | ||
Updated•15 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•15 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.
Comment 3•15 years ago
|
||
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•15 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted-seamonkey2.1?
Assignee | ||
Comment 4•15 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•15 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•15 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 | ||
Comment 7•15 years ago
|
||
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•15 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' ;-]
Assignee | ||
Comment 9•15 years ago
|
||
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)
Comment 10•15 years ago
|
||
Robert, there are still some issues here with actual packaging... (I stripped any - foo/* lines followed by +foo/bar etc.)
Updated•15 years ago
|
Attachment #425496 -
Attachment mime type: application/octet-stream → text/plain
Comment 11•15 years ago
|
||
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•15 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•15 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.
Comment 14•15 years ago
|
||
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•15 years ago
|
||
Ah, ted, thanks, good to know.
Comment 16•15 years ago
|
||
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•15 years ago
|
||
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•15 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.
Comment 19•15 years ago
|
||
(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•15 years ago
|
Attachment #425635 -
Flags: review?(bugspam.Callek) → review-
Comment 20•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
Attachment #425652 -
Flags: review?(bugspam.Callek) → review+
Assignee | ||
Comment 22•15 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•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 23•15 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 | ||
Comment 24•15 years ago
|
||
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?
Assignee | ||
Comment 25•15 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 | ||
Comment 26•15 years ago
|
||
(In reply to comment #25) Ah. Ftr, why is this hack not needed anymore on c-c?
Assignee | ||
Comment 27•15 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 | ||
Comment 28•15 years ago
|
||
(In reply to comment #27) I filed bug 545777.
Reporter | ||
Updated•15 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•