Last Comment Bug 541125 - Fix wrong packaging issues on m-1.9.2, SeaMonkey part
: Fix wrong packaging issues on m-1.9.2, SeaMonkey part
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
Depends on: 530010 540585 541203
Blocks: 511849 512763 523820
  Show dependency treegraph
 
Reported: 2010-01-21 09:19 PST by Serge Gautherie (:sgautherie)
Modified: 2010-01-27 05:28 PST (History)
2 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
(Av1) Don't package mozcpp19.dll [Checkin: Comment 2] (1.23 KB, patch)
2010-01-21 09:39 PST, Serge Gautherie (:sgautherie)
kairo: review+
Details | Diff | Splinter Review
(Bv1) Restore mozbrwsr.xpt packaging (1.63 KB, patch)
2010-01-25 12:31 PST, Serge Gautherie (:sgautherie)
bugspam.Callek: review-
Details | Diff | Splinter Review
(Bv2) Restore mozbrwsr.xpt packaging on m-1.9.2, Add removal on m-c [Checkin: Comment 9] (2.36 KB, patch)
2010-01-25 21:32 PST, Serge Gautherie (:sgautherie)
bugspam.Callek: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2010-01-21 09:19:31 PST

    
Comment 1 Serge Gautherie (:sgautherie) 2010-01-21 09:39:50 PST
Created attachment 422776 [details] [diff] [review]
(Av1) Don't package mozcpp19.dll
[Checkin: Comment 2]

Bug 512763 missed m-1.9.2 case :-(
Bug 523820 removed m-1.9.1 SM case.
Bug 530010 fixed it correctly, for TB only :-|
Comment 2 Serge Gautherie (:sgautherie) 2010-01-21 12:00:43 PST
Comment on attachment 422776 [details] [diff] [review]
(Av1) Don't package mozcpp19.dll
[Checkin: Comment 2]


http://hg.mozilla.org/comm-central/rev/5895e4caa1a1
Comment 3 Serge Gautherie (:sgautherie) 2010-01-25 12:31:58 PST
Created attachment 423392 [details] [diff] [review]
(Bv1) Restore mozbrwsr.xpt packaging

Bug 511849 missed m-1.9.2 case :-(
Bug 523820 removed m-1.9.1 SM case.
Bug 530010 fixed it correctly, for TB only :-|
Comment 4 Justin Wood (:Callek) (Away until Aug 29) 2010-01-25 20:28:07 PST
Comment on attachment 423392 [details] [diff] [review]
(Bv1) Restore mozbrwsr.xpt packaging

suite needs a 1.9.3 removed-files entry for this.
Comment 5 Serge Gautherie (:sgautherie) 2010-01-25 21:32:56 PST
Created attachment 423485 [details] [diff] [review]
(Bv2) Restore mozbrwsr.xpt packaging on m-1.9.2, Add removal on m-c
[Checkin: Comment 9]

Bv1, with comment 4 suggestion(s).

(In reply to comment #4)

Does it? There was none before.
(Note I'm not saying there shouldn't have been one...)

Could you provide the history that explains why it's needed, for (MacOSX and) Unix+Windows?
(I mean that I suspect you could also file a bug for many more '.xpt'...)
Comment 6 Justin Wood (:Callek) (Away until Aug 29) 2010-01-25 21:35:45 PST
(In reply to comment #5)
> Created an attachment (id=423485) [details]
> (Bv2) Restore mozbrwsr.xpt packaging
> 
> Bv1, with comment 4 suggestion(s).
> 
> (In reply to comment #4)
> 
> Does it? There was none before.
> (Note I'm not saying there shouldn't have been one...)

Before there didn't need to be one as we didn't package this anywhere...

But this bug is adding the file to 1_9_2 ONLY:

|#ifdef MOZILLA_1_9_2_BRANCH|

Thus we need a trunk removed-files entry. [to account for if we ever do a 1.9.2 release]
Comment 7 Serge Gautherie (:sgautherie) 2010-01-25 21:49:22 PST
(In reply to comment #6)

No: this patch is _restoring_ previously-MOZILLA_1_9_1_BRANCH-only lines that should not have been removed!
See http://hg.mozilla.org/comm-central/rev/955b4e732760.
(Thus my comment 5 questions.)
Comment 8 Justin Wood (:Callek) (Away until Aug 29) 2010-01-25 21:56:33 PST
(In reply to comment #7)
> (In reply to comment #6)
> 
> No: this patch is _restoring_ previously-MOZILLA_1_9_1_BRANCH-only lines that
> should not have been removed!

Either way, file is not packaged currently, and needs to be removed; if your adding it back in for 1_9_2 we need to remove it for 1.9.3 only; if you don't add it back in for 1.9.2 we need to remove it for both; (But yes the patch I reviewed should land)
Comment 9 Serge Gautherie (:sgautherie) 2010-01-26 06:26:17 PST
Comment on attachment 423485 [details] [diff] [review]
(Bv2) Restore mozbrwsr.xpt packaging on m-1.9.2, Add removal on m-c
[Checkin: Comment 9]


http://hg.mozilla.org/comm-central/rev/db60f2b46ffc
Comment 10 Serge Gautherie (:sgautherie) 2010-01-26 06:40:09 PST
(In reply to comment #8)
> Either way, file [...] needs to be removed

Probably, but it also depends on the fact that most xpt files are merged during packaging on Unix+Windows now: thus wondering about when/how this xpt was initially added...
Comment 11 Robert Kaiser 2010-01-26 08:00:27 PST
I agree with Serge here, we don't need to remove an xpt that gets merged into either browser.xpt or mailnews.xpt anyhow and therefore is never installed in a nightly or release version.
Comment 12 Justin Wood (:Callek) (Away until Aug 29) 2010-01-26 15:41:04 PST
(In reply to comment #11)
> I agree with Serge here, we don't need to remove an xpt that gets merged into
> either browser.xpt or mailnews.xpt anyhow and therefore is never installed in a
> nightly or release version.

Hmm I didn't realize these .xpt's were being merged for us now; any pointer on where and/or what they merge into for me for future?
Comment 13 Robert Kaiser 2010-01-26 17:56:52 PST
(In reply to comment #12)
> Hmm I didn't realize these .xpt's were being merged for us now; any pointer on
> where and/or what they merge into for me for future?

The packaging process merges ("links" using an "xptlink" tool) all .xpt files inside every section of the relevant packages file, i.e. all .xpt files listed under [browser] get linked into browser.xpt, all under [mailnews] into mailnews.xpt.
Comment 14 Robert Kaiser 2010-01-26 18:16:38 PST
Hrm, and actually, I just realized that we probably don't link XPTs on Mac, as we don't use a package manifest there right now - so we might need the removed-files entry after all...

Serge?
Comment 15 Justin Wood (:Callek) (Away until Aug 29) 2010-01-26 18:39:15 PST
(In reply to comment #14)
> Hrm, and actually, I just realized that we probably don't link XPTs on Mac, as
> we don't use a package manifest there right now - so we might need the
> removed-files entry after all...

Patch as pushed has the removed-files entry here. Lets do remaining followup in another/other bug
Comment 16 Serge Gautherie (:sgautherie) 2010-01-26 21:38:03 PST
(In reply to comment #14)

Usually, I'm kind of ignoring MacOSX (non-)packaging and xpt removals, until bug 521523.

In the case here though, I was implicitly wondering if adding an |#ifdef XP_MACOSX| would be appropriate.
(And you seemed to say "99% yes" over irc, iiuc.)
Of course, it's safer to let it as is until 100% sure.

(In reply to comment #15)
> Lets do remaining followup in another/other bug

Well, if you (both) want to discuss/investigate/answer it, I would say "as well continue here" now.
Comment 17 Robert Kaiser 2010-01-27 05:28:44 PST
(In reply to comment #16)
> (In reply to comment #14)
> Usually, I'm kind of ignoring MacOSX (non-)packaging and xpt removals, until
> bug 521523.

Well, removed-files is used by the automatic update functionality, which also applies to Mac, so we need to care about it for those removals, but you are right on packaging.

Note You need to log in before you can comment on or make changes to this bug.