Closed
Bug 528806
Opened 15 years ago
Closed 15 years ago
Needs to package htmlpars.dll instead of gkparser.dll after bug 514665, in comm-central (apps)
Categories
(MailNews Core :: Build Config, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 3.3a1
People
(Reporter: sgautherie, Assigned: sgautherie)
References
()
Details
(Keywords: fixed-seamonkey2.0.3)
Attachments
(2 files, 3 obsolete files)
2.91 KB,
patch
|
kairo
:
review+
|
Details | Diff | Splinter Review |
580 bytes,
patch
|
kairo
:
review+
kairo
:
approval-seamonkey2.0.3+
|
Details | Diff | Splinter Review |
Bug 518632 http://hg.mozilla.org/mozilla-central/rev/6c4f06ec0300
removed m-c "embedding" packaging part.
It looks like SeaMonkey should be updated:
{
Warning: package error or possible missing or unnecessary file: bin/components/gkparser.dll (packages, 76).
}
and Thunderbird too, fwiw.
Phil, Benjamin, do you agree with that?
Or should something else be done?
NB: This file was packaged in non-static SM 1.0.x, SM 1.1.x and SM 2.0.x.
Flags: in-testsuite-
Assignee | ||
Comment 1•15 years ago
|
||
This should be it, if I guessed right about this bug.
TB clean-up:
*Add a missing '#ifdef XP_WIN', afaict.
*Remove a duplicated (and misplaced) 'components/xmlextras.xpt'.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #412478 -
Flags: review?(bugzilla)
Comment 2•15 years ago
|
||
gkparser.dll no longer existing has nothing to do with having removed an unused file that would have tried to package it except nobody was using it because it was obsolete in any number of other ways. gkparser.dll no longer exists, but htmlpars.dll does, and is pretty much essential, and this is just a duplicate of bug 519068, which I can't believe is still unfixed more than a month after I explained that all it needs is one person to build shared and look at make package-compare.
No longer depends on: 518632
Comment 3•15 years ago
|
||
Comment on attachment 412478 [details] [diff] [review]
(Av1) Just remove it on m-c, Plus a little clean-up on TB side
Per Phil's comments, this patch only addresses part of the issue.
Attachment #412478 -
Flags: review?(bugzilla) → review-
Assignee | ||
Comment 4•15 years ago
|
||
(In reply to comment #2)
> gkparser.dll no longer existing has nothing to do with having removed an unused
> file that would have tried to package it except nobody was using it because it
> was obsolete in any number of other ways. gkparser.dll no longer exists, but
> htmlpars.dll does, and is pretty much essential,
Thanks for the explanation: I thought I was missing something ;-<
> and this is just a duplicate
> of bug 519068, which I can't believe is still unfixed more than a month after I
> explained that all it needs is one person to build shared and look at make
> package-compare.
As you may have noticed, I have already filed a bunch of bugs and landed some patches +/- part of this issue.
You might have noticed too that comm-central has been branching, there is 3 apps to fix, releases are going on, reviewers are focused on the latters, ...
So, yes, it takes some time to fully fix!
Severity: trivial → major
Depends on: 514665
Summary: Stop trying to package obsolete gkparser.dll, in comm-central (apps) → Needs to package htmlpars.dll instead of gkparser.dll after bug 514665, in comm-central (apps)
Target Milestone: --- → Thunderbird 3.1a1
Assignee | ||
Comment 5•15 years ago
|
||
Av1, with comment 3 suggestion(s).
Attachment #412478 -
Attachment is obsolete: true
Attachment #412835 -
Flags: review?(bugzilla)
Assignee | ||
Comment 6•15 years ago
|
||
Mark, ping for review.
Assignee | ||
Updated•15 years ago
|
Attachment #412835 -
Flags: review?(kairo)
Assignee | ||
Comment 7•15 years ago
|
||
Comment on attachment 412835 [details] [diff] [review]
(Av2) Fix m-c and m-1.9.2, Plus a little clean-up on TB side
KaiRo, in hope to move forward on the SM part at least.
Assignee | ||
Comment 8•15 years ago
|
||
Phil told me (irc) that he would handle the TB part in bug 533043.
Depends on: 533043
Assignee | ||
Comment 9•15 years ago
|
||
Attachment #412835 -
Attachment is obsolete: true
Attachment #416487 -
Flags: review?(kairo)
Attachment #412835 -
Flags: review?(kairo)
Attachment #412835 -
Flags: review?(bugzilla)
![]() |
||
Updated•15 years ago
|
Attachment #416487 -
Flags: review?(kairo) → review-
![]() |
||
Comment 10•15 years ago
|
||
Comment on attachment 416487 [details] [diff] [review]
(Av2a-SM) Fix m-c and m-1.9.2
>diff --git a/suite/installer/removed-files.in b/suite/installer/removed-files.in
>+#ifndef XP_WIN
> components/@DLL_PREFIX@htmlpars@DLL_SUFFIX@
>+#endif
> #ifdef XP_WIN
> jsj3250.dll
> components/appshell.dll
> components/cmdlines.dll
>+#ifdef MOZILLA_1_9_2_BRANCH
> components/gkparser.dll
>+#else
>+components/htmlpars.dll
>+#endif
No need to exclude it above and add it here, just let the first variant handle all platforms, please.
> #ifdef XP_WIN
> #ifdef MOZILLA_1_9_2_BRANCH
>+components/htmlpars.dll
> mozjs.dll
> #else
>+components/gkparser.dll
And why add them here again? That's very unclean. we should very much try to mention every file only once in removed-files.
And don't we need to patch the unix packages file as well?
Assignee | ||
Comment 11•15 years ago
|
||
Comment on attachment 416487 [details] [diff] [review]
(Av2a-SM) Fix m-c and m-1.9.2
(In reply to comment #10)
> we should very much try to mention every file only once in removed-files.
I know that, but I don't know how to do it (better) in this case as m-c and m-1.9.2 need "opposite" behaviors ... and static/non-static doesn't help at that either :-|
I think we have to live with such things until c-1.9.2 is created ... much like we had to add a lot of #ifdef before c-1.9.1 existed.
Re-asking for review ... or suggestion on how to achieve a better result.
> And don't we need to patch the unix packages file as well?
No, bug 514665 is Windows only.
Attachment #416487 -
Flags: review?(kairo)
![]() |
||
Updated•15 years ago
|
Attachment #416487 -
Flags: review?(kairo) → review-
![]() |
||
Comment 12•15 years ago
|
||
Comment on attachment 416487 [details] [diff] [review]
(Av2a-SM) Fix m-c and m-1.9.2
Still minus as I think you shouldn't ifdef the file in the beginning just to add it twice below.
Also, I think we should go for a more inclusive solution, as this is only the tip of the iceberg, as philor already mentioned, and I now analyzed a bit more, see bug 519068 comment#17
Assignee | ||
Comment 13•15 years ago
|
||
Av2a-SM, with comment 12 suggestion(s).
(In reply to comment #12)
> (From update of attachment 416487 [details] [diff] [review])
> Still minus as I think you shouldn't ifdef the file in the beginning just to
> add it twice below.
Done (per our irc discussion).
> Also, I think we should go for a more inclusive solution, as this is only the
> tip of the iceberg, as philor already mentioned, and I now analyzed a bit more,
> see bug 519068 comment#17
I know about that, but I wanted a review of a single case first.
Also, it may be easier (for me) to continue to do it in multiple patches: I'll see when I work on the next one(s).
Attachment #416487 -
Attachment is obsolete: true
Attachment #417151 -
Flags: review?(kairo)
Assignee | ||
Updated•15 years ago
|
Attachment #416487 -
Flags: review-
![]() |
||
Comment 14•15 years ago
|
||
Comment on attachment 417151 [details] [diff] [review]
(Av3-SM) Fix m-c and m-1.9.2
[Checkin: See comment 15]
r=me when you also remove the whitespace changes, which are unrelated and I don't see a need for them.
Also, don't expect my reviewes on any of the other patches to be faster, as I need to look into the surroundings of the patch every time and need to call everything into mind so that I understand it. Taking that into account and looking at the pace we're at, it will take a few months until we have working builds from trunk again.
On the other hand, if you stick all the mentioned changes (see previously cited comment) I only need to look everything up one time and we could have working builds next week...
Attachment #417151 -
Flags: review?(kairo) → review+
Assignee | ||
Comment 15•15 years ago
|
||
Comment on attachment 417151 [details] [diff] [review]
(Av3-SM) Fix m-c and m-1.9.2
[Checkin: See comment 15]
http://hg.mozilla.org/comm-central/rev/315c6fa822e0
Av3-SM, with comment 14 suggestion(s).
Attachment #417151 -
Attachment description: (Av3-SM) Fix m-c and m-1.9.2 → (Av3-SM) Fix m-c and m-1.9.2
[Checkin: Comment 15]
Assignee | ||
Updated•15 years ago
|
Attachment #417151 -
Attachment description: (Av3-SM) Fix m-c and m-1.9.2
[Checkin: Comment 15] → (Av3-SM) Fix m-c and m-1.9.2
[Checkin: See comment 15]
Assignee | ||
Updated•15 years ago
|
Assignee | ||
Comment 17•15 years ago
|
||
Support downgrading.
Attachment #417309 -
Flags: review?(kairo)
Attachment #417309 -
Flags: approval-seamonkey2.0.2?
![]() |
||
Updated•15 years ago
|
Attachment #417309 -
Flags: review?(kairo)
Attachment #417309 -
Flags: review+
Attachment #417309 -
Flags: approval-seamonkey2.0.2?
Attachment #417309 -
Flags: approval-seamonkey2.0.2+
Assignee | ||
Comment 18•15 years ago
|
||
Comment on attachment 417309 [details] [diff] [review]
(Bv1-SM-191) Fix m-1.9.1
[Checkin: Comment 18]
http://hg.mozilla.org/releases/comm-1.9.1/rev/f5d52ff41bc0
Attachment #417309 -
Attachment description: (Bv1-SM-191) Fix m-1.9.1. → (Bv1-SM-191) Fix m-1.9.1
[Checkin: Comment 18]
Assignee | ||
Updated•15 years ago
|
Keywords: fixed-seamonkey2.0.2
Assignee | ||
Updated•15 years ago
|
Depends on: 538832
Target Milestone: Thunderbird 3.1a1 → Thunderbird 3.2a1
You need to log in
before you can comment on or make changes to this bug.
Description
•