Core bug 514665 dropped/replaced USE_SHORT_LIBNAME uses: port that to comm-central (apps)

RESOLVED FIXED in Thunderbird 3.3a1

Status

defect
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Trunk
Thunderbird 3.3a1
x86
Windows 2000
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(9 attachments, 3 obsolete attachments)

2.92 KB, patch
ted
: review+
Details | Diff | Splinter Review
1.04 KB, patch
kairo
: review+
Details | Diff | Splinter Review
2.88 KB, patch
standard8
: review+
standard8
: superreview+
Details | Diff | Splinter Review
824 bytes, patch
standard8
: review+
standard8
: superreview+
Details | Diff | Splinter Review
849 bytes, patch
standard8
: review+
standard8
: superreview+
Details | Diff | Splinter Review
805 bytes, patch
standard8
: review+
standard8
: superreview+
Details | Diff | Splinter Review
26.86 KB, patch
Callek
: review+
kairo
: review+
standard8
: review+
standard8
: superreview+
wuno
: feedback+
Details | Diff | Splinter Review
7.37 KB, patch
Callek
: review+
Details | Diff | Splinter Review
13.49 KB, patch
Callek
: review+
sgautherie
: feedback+
Details | Diff | Splinter Review
Assignee

Description

10 years ago
No description provided.
Flags: in-testsuite-
Assignee

Updated

9 years ago
Blocks: 550018
Assignee

Updated

9 years ago
Blocks: 528806, 534410
Assignee

Updated

9 years ago
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 3.1rc1
Assignee

Updated

9 years ago
Attachment #430216 - Flags: review? → review?(bugzilla)

Updated

9 years ago
Attachment #430211 - Flags: review?(kairo) → review+
Assignee

Comment 4

9 years ago
Comment on attachment 430211 [details] [diff] [review]
(Bv1-SM) Remove duplicate entries after bug 528806 and bug 534410
[Checkin: Comment 4]


http://hg.mozilla.org/comm-central/rev/686ca4a5b62c
Attachment #430211 - Attachment description: (Bv1) Remove duplicate entries after bug 528806 and bug 534410 → (Bv1) Remove duplicate entries after bug 528806 and bug 534410 [Checkin: Comment 4]
Assignee

Updated

9 years ago
No longer blocks: 550018
Depends on: 550018
Assignee

Updated

9 years ago
Attachment #430211 - Attachment description: (Bv1) Remove duplicate entries after bug 528806 and bug 534410 [Checkin: Comment 4] → (Bv1-SM) Remove duplicate entries after bug 528806 and bug 534410 [Checkin: Comment 4]
Assignee

Updated

9 years ago
Attachment #430216 - Flags: superreview?(bugzilla)
Assignee

Comment 7

9 years ago
This one is optional but is more explicit, helps sorting, etc.
Attachment #430537 - Flags: superreview?(bugzilla)
Attachment #430537 - Flags: review?(bugzilla)
Comment on attachment 430209 [details] [diff] [review]
(Av1-MC) Remove obsolete MOZ_MOVEMAIL and MOZ_STATIC_MAIL_BUILD
[Checkin: Comment 10]

Is this comm-central? I probably shouldn't be reviewing this.
Comment on attachment 430209 [details] [diff] [review]
(Av1-MC) Remove obsolete MOZ_MOVEMAIL and MOZ_STATIC_MAIL_BUILD
[Checkin: Comment 10]

Oh huh, that crud is in m-c. Weird.
Attachment #430209 - Flags: review?(ted.mielczarek) → review+
Assignee

Comment 10

9 years ago
Comment on attachment 430209 [details] [diff] [review]
(Av1-MC) Remove obsolete MOZ_MOVEMAIL and MOZ_STATIC_MAIL_BUILD
[Checkin: Comment 10]


http://hg.mozilla.org/mozilla-central/rev/161508365ee2
Attachment #430209 - Attachment description: (Av1-MC) Remove obsolete MOZ_MOVEMAIL and MOZ_STATIC_MAIL_BUILD → (Av1-MC) Remove obsolete MOZ_MOVEMAIL and MOZ_STATIC_MAIL_BUILD [Checkin: Comment 10]
Assignee

Updated

9 years ago
Target Milestone: Thunderbird 3.1rc1 → Thunderbird 3.1b2
Attachment #430216 - Flags: superreview?(bugzilla)
Attachment #430216 - Flags: superreview+
Attachment #430216 - Flags: review?(bugzilla)
Attachment #430216 - Flags: review+
Assignee

Comment 11

9 years ago
Comment on attachment 430216 [details] [diff] [review]
(Cv1) Get rid of WINNT msgcompo.xpt special case
[Checkin: Comment 11]


http://hg.mozilla.org/comm-central/rev/9140881c8cca
Attachment #430216 - Attachment description: (Cv1) Get rid of WINNT msgcompo.xpt special case → (Cv1) Get rid of WINNT msgcompo.xpt special case [Checkin: Comment 11]
Attachment #430524 - Flags: superreview?(bugzilla)
Attachment #430524 - Flags: superreview+
Attachment #430524 - Flags: review?(bugzilla)
Attachment #430524 - Flags: review+
Attachment #430532 - Flags: superreview?(bugzilla)
Attachment #430532 - Flags: superreview+
Attachment #430532 - Flags: review?(bugzilla)
Attachment #430532 - Flags: review+
Attachment #430537 - Flags: superreview?(bugzilla)
Attachment #430537 - Flags: superreview+
Attachment #430537 - Flags: review?(bugzilla)
Attachment #430537 - Flags: review+
Assignee

Updated

9 years ago
Blocks: 550018
No longer depends on: 550018
Assignee

Comment 12

9 years ago
Comment on attachment 430216 [details] [diff] [review]
(Cv1) Get rid of WINNT msgcompo.xpt special case
[Checkin: Comment 11]


"approval-thunderbird3.0.4=?":
Zero risk, build config only.
Not really needed, but good cleanup along the others.
Attachment #430216 - Flags: approval-thunderbird3.0.4?
Assignee

Comment 13

9 years ago
Comment on attachment 430524 [details] [diff] [review]
(Dv1) Get rid of WINNT msgbase.dll special case: s/mailnews/msgbase/ for all
[Checkin: Comment 13]


http://hg.mozilla.org/comm-central/rev/aabc36111e25

"approval-thunderbird3.0.4=?":
No risk, NPOTDB.
To ease bug 550018 backport, and good cleanup anyway...
Attachment #430524 - Attachment description: (Dv1) Get rid of WINNT msgbase.dll special case: s/mailnews/msgbase/ for all → (Dv1) Get rid of WINNT msgbase.dll special case: s/mailnews/msgbase/ for all [Checkin: Comment 12]
Attachment #430524 - Flags: approval-thunderbird3.0.4?
Assignee

Comment 14

9 years ago
Comment on attachment 430532 [details] [diff] [review]
(Ev1) Get rid of msglocal.dll special case: s/localmail/msglocal/ for all
[Checkin: Comment 14]


http://hg.mozilla.org/comm-central/rev/2c83f673f6a4
Attachment #430532 - Attachment description: (Ev1) Get rid of msglocal.dll special case: s/localmail/msglocal/ for all → (Ev1) Get rid of msglocal.dll special case: s/localmail/msglocal/ for all [Checkin: Comment 13]
Attachment #430532 - Flags: approval-thunderbird3.0.4?
Assignee

Updated

9 years ago
Attachment #430532 - Attachment description: (Ev1) Get rid of msglocal.dll special case: s/localmail/msglocal/ for all [Checkin: Comment 13] → (Ev1) Get rid of msglocal.dll special case: s/localmail/msglocal/ for all [Checkin: Comment 14]
Assignee

Updated

9 years ago
Attachment #430524 - Attachment description: (Dv1) Get rid of WINNT msgbase.dll special case: s/mailnews/msgbase/ for all [Checkin: Comment 12] → (Dv1) Get rid of WINNT msgbase.dll special case: s/mailnews/msgbase/ for all [Checkin: Comment 13]
Assignee

Comment 15

9 years ago
Comment on attachment 430537 [details] [diff] [review]
(Fv1) s/emitter/mimeemit/ SHORT_LIBNAME
[Checkin: Comment 15]


http://hg.mozilla.org/comm-central/rev/080ae7e74aa3
Attachment #430537 - Attachment description: (Fv1) s/emitter/mimeemit/ SHORT_LIBNAME → (Fv1) s/emitter/mimeemit/ SHORT_LIBNAME [Checkin: Comment 15]
Attachment #430537 - Flags: approval-thunderbird3.0.4?
Comment on attachment 430216 [details] [diff] [review]
(Cv1) Get rid of WINNT msgcompo.xpt special case
[Checkin: Comment 11]

Denying approval. Sorry, even though this isn't the default config we're not going to rename libraries on a stable branch as we're bound to break someone.

Additionally KaiRo thinks that bug 550018 isn't totally necessary on branch either so there's no compelling reason to take this.
Attachment #430216 - Flags: approval-thunderbird3.0.4? → approval-thunderbird3.0.4-
Attachment #430524 - Flags: approval-thunderbird3.0.4? → approval-thunderbird3.0.4-
Attachment #430532 - Flags: approval-thunderbird3.0.4? → approval-thunderbird3.0.4-
Attachment #430537 - Flags: approval-thunderbird3.0.4? → approval-thunderbird3.0.4-
Assignee

Updated

9 years ago
Depends on: 520418
Assignee

Comment 17

9 years ago
c-c has 5 SHORT_LIBNAMEs:
*Calendar
 *calbasecomps > calbscmp
*Mail
 *mailcomps > mailcmp
*Mailnews
 *msgbaseutil > msgbsutl, that's the one copied "everywhere"...
 *msgcompose > msgcompo
 *mimeemitter > mimeemit

If we want to simplify 1+ of them, now is the right time.!.
Attachment #432063 - Flags: superreview?(bugzilla)
Attachment #432063 - Flags: review?(bugzilla)
Assignee

Updated

9 years ago
Blocks: 536451
Assignee

Comment 18

9 years ago
Comment on attachment 432063 [details] [diff] [review]
(Gv1) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs, Copy bug 520418 too, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging


Ping for review.
Depends on: 558518
Blocks: 558518
No longer depends on: 558518
Attachment #432063 - Flags: review?(philipp)
Attachment #432063 - Flags: review?(bugzilla)
Attachment #432063 - Flags: review?(bugspam.Callek)
Comment on attachment 432063 [details] [diff] [review]
(Gv1) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs, Copy bug 520418 too, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging

The calendar/ changes need review by Fallen, if not dropping completely (as Sunbird is not being supported on trunk).

I'm going to pass the review of the rest of this to Justin as he can probably take a bit more time to check things over than I can.

I can then come back and do a quick check of the mailnews parts.
Comment on attachment 432063 [details] [diff] [review]
(Gv1) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs, Copy bug 520418 too, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging

slightly bitrotted and missing the changeset from Bug 421353. Please update and re-request.
Attachment #432063 - Flags: review?(bugspam.Callek) → review-
Assignee

Comment 21

9 years ago
(In reply to comment #20)
> missing the changeset from Bug 421353.

Which changeset exactly?
Assignee

Updated

9 years ago
No longer depends on: 520418
Assignee

Comment 22

9 years ago
Gv1, unbitrotted, without useless 192 checks, with Calendar part spun off.
Attachment #432063 - Attachment is obsolete: true
Attachment #439451 - Flags: superreview?(bugzilla)
Attachment #439451 - Flags: review?(bugspam.Callek)
Attachment #432063 - Flags: superreview?(bugzilla)
Attachment #432063 - Flags: review?(philipp)
Attachment #439451 - Flags: review?(bugspam.Callek) → review-
(In reply to comment #21)
> (In reply to comment #20)
> > missing the changeset from Bug 421353.
> 
> Which changeset exactly?

Sorry, cited wrong bug (no idea where I got that #)

http://hg.mozilla.org/mozilla-central/rev/550709f67284
from Bug 536451

r- pending that update.
Comment on attachment 439453 [details] [diff] [review]
(Hv1-SB) Get rid of USE_SHORT_LIBNAME, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging
[Checkin: Comment 47]

Fallen per IRC convo told me he's glad to have me review calendar/ build-config. This looks bug but can't land until the other patch (which I just r-'ed) lands.
Attachment #439453 - Flags: review?(philipp) → review+
Assignee

Comment 26

9 years ago
(In reply to comment #24)
> Bug 536451

I marked that (OS/2) bug as dependent on this one and I'd like to have this patch reviewed (and probably landed) first.
Why do you want to merge both?

Comment 27

9 years ago
The changes to rules.mk in bug536451 most probably should fit on top of your changes in Gv2 (attachment 439451 [details] [diff] [review]). OS/2 uses then for linking like the other platforms long names (we need 8+3 names only for final dll's but not for intermediate libs we link against). You then can do
-ifeq ($(USE_SHORT_LIBNAME), 1)
-LIBS +=   $(call EXPAND_LIBNAME_PATH,msgbsutl,../../base/util)
-else
 LIBS +=   $(call EXPAND_LIBNAME_PATH,msgbaseutil,../../base/util)
-endif
and
-ifeq ($(USE_SHORT_LIBNAME),1)
-EXTRA_DSO_LIBS = msgbsutl
-else
EXTRA_DSO_LIBS = msgbaseutil
-endif
in the Makefiles.
I'll try an updated patch if you want me to test whether it builds correctly on OS/2, thanks

Comment 28

9 years ago
(In reply to comment #25)
> This looks bug [...]

You mean "good" actually, right? ;-)
(In reply to comment #26)
> (In reply to comment #24)
> > Bug 536451
> 
> I marked that (OS/2) bug as dependent on this one and I'd like to have this
> patch reviewed (and probably landed) first.
> Why do you want to merge both?

Because I'd rather not break OS/2 when this lands. That changeset is a good deal of what this changeset also is for. So I'd like to see it encompassed here.

(In reply to comment #28)
> (In reply to comment #25)
> > This looks bug [...]
> 
> You mean "good" actually, right? ;-)

Yea, sorry :/
Assignee

Comment 30

9 years ago
(In reply to comment #27)
> You then can do [...]

Would that still work for msgbsutl.dll in debug builds?
Assignee

Updated

9 years ago
No longer blocks: 536451
Depends on: 536451
Assignee

Comment 31

9 years ago
Per Walter's comment 27.
Attachment #439726 - Flags: review?(bugspam.Callek)
Assignee

Updated

9 years ago
Attachment #439451 - Flags: review- → review?(bugspam.Callek)

Comment 32

9 years ago
(In reply to comment #30)
> (In reply to comment #27)
> > You then can do [...]
> 
> Would that still work for msgbsutl.dll in debug builds?

It's a must, when you apply also Iv1, because then OS/2 searches for msgbsutl during linking, which is after Iv1 named msgbaseutil like for the other platforms

I've updated your Gv2 patch, hope you don't mind that I'm hijacking this. Take a look and decide whether you want this. Gv3 and Iv1 have to be applied one after the other.
Assignee

Comment 33

9 years ago
(In reply to comment #32)

This simplification is the one I was (most) hoping for in my comment 17 ;-)

But I'm lost by your Gv3:
Previously, I thought your comment 27 was a suggestion, that could be done before (or after) my patches.
Now, checking Iv1, I think your comment 27 is a requirement, that should be merged with that patch.
Yet, you merged your changes with Gv2, but I don't understand why atm.
If I'm right, I would be all for you to take ownership of an Iv2 on top of my Gv2...
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]

r+ for build config changes. Still need r+ for the packaging changes on both mail and suite.
Attachment #439451 - Flags: review?(kairo)
Attachment #439451 - Flags: review?(bugspam.Callek)
Attachment #439451 - Flags: review+
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]

re-requesting after re-reading most recent comments here... I need to think on this in a more coherent state.
Attachment #439451 - Flags: review+ → review?(bugspam.Callek)

Comment 36

9 years ago
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]

Looks good to me, if Mark and OS/2 folks agree.

I think we might want to take another look at reducing ifdefs in the packaging files, but not in this patch.
Attachment #439451 - Flags: review?(kairo) → review+
Assignee

Updated

9 years ago
Attachment #439451 - Flags: review?(bugzilla)
Assignee

Updated

9 years ago
Attachment #439451 - Flags: feedback?(wuno)

Comment 37

9 years ago
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]

Yes, Gv2 builds on OS/2 properly. Packaging works as well.
I'm currently testing an Iv2 patch that picks up the changes callek requested in #24. If it works I could attach it in this bug, may also file a new bug if that's more appropriate.
Attachment #439451 - Flags: feedback?(wuno) → feedback+

Updated

9 years ago
Attachment #440264 - Attachment is obsolete: true
Assignee

Comment 38

9 years ago
(In reply to comment #37)
> I could attach it in this bug, may also file a new bug

Here would be fine ;-)

Comment 39

9 years ago
This goes on top of Serge's Gv2 patch and integrates the changes (for OS/2) of bug536451. It builds fine on OS/2.
(We had a couple of build breaks on OS/2 due to new gfx features in m-c. Therefore, it took me a while to get a build with the patch attached, and I wanted to be sure by testing that it is working.)
Attachment #439726 - Attachment is obsolete: true
Attachment #441280 - Flags: review?(bugspam.Callek)
Attachment #439726 - Flags: review?(bugspam.Callek)
Assignee

Comment 40

9 years ago
Comment on attachment 441280 [details] [diff] [review]
(Iv2) Copy bug 536451 rules.mk part and adjust Makefiles [Checked in: comment 52]


Yeah, that's what I was expecting from my comment 33 ;-)
Attachment #441280 - Flags: feedback+
Attachment #439451 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 441280 [details] [diff] [review]
(Iv2) Copy bug 536451 rules.mk part and adjust Makefiles [Checked in: comment 52]

Sorry these took so long; but pending a mailnews/ review on the first patch here this is now ready to land.
Attachment #441280 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]

This looks fine. r/sr=Standard8.

It does make me wonder that seeing as msgbaseutil is just a shared config that we don't use much whether it would just be a sane idea to shorten it down anyway and dig ourselves out of some of the build config complexities we've got there.
Attachment #439451 - Flags: superreview?(bugzilla)
Attachment #439451 - Flags: superreview+
Attachment #439451 - Flags: review?(bugzilla)
Attachment #439451 - Flags: review+
Assignee

Updated

9 years ago
Attachment #439451 - Flags: approval-seamonkey2.1a1?
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]

given this has a decent risk potential I'd rather avoid it until 2.1a1 is tagged at least. I'll leave the final determination up to an actual Council member though.

Updated

9 years ago
Attachment #439451 - Flags: approval-seamonkey2.1a1? → approval-seamonkey2.1a1+

Comment 44

9 years ago
(In reply to comment #43)
> (From update of attachment 439451 [details] [diff] [review])
> given this has a decent risk potential I'd rather avoid it until 2.1a1 is
> tagged at least. I'll leave the final determination up to an actual Council
> member though.

My reasoning for plussing anyhow is that we have done so much review on this and we would notice fairly soon if this creates problems. Also, the tree doesn't really look to be in a shape for cutting this to me unless we first get test runs on all platforms, and bug 563151 prohibits this for the moment. Also, our last blocker is still open. So, all in all, there should be a bit of time for this to bake or be backed out.
Assignee

Comment 45

9 years ago
Comment on attachment 439451 [details] [diff] [review]
(Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs
[Checkin: Comment 45]


http://hg.mozilla.org/comm-central/rev/6f1c84f4a88e
Attachment #439451 - Attachment description: (Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs → (Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs [Checkin: Comment 45]
Assignee

Comment 46

9 years ago
(In reply to comment #42)
> It does make me wonder that seeing as msgbaseutil is just a shared config that
> we don't use much whether it would just be a sane idea to shorten it down
> anyway and dig ourselves out of some of the build config complexities we've got
> there.

Patch Iv2 will fix that ;-)
Assignee

Comment 47

9 years ago
Comment on attachment 439453 [details] [diff] [review]
(Hv1-SB) Get rid of USE_SHORT_LIBNAME, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging
[Checkin: Comment 47]


http://hg.mozilla.org/comm-central/rev/7708352a72d6
Attachment #439453 - Attachment description: (Hv1-SB) Get rid of USE_SHORT_LIBNAME, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging → (Hv1-SB) Get rid of USE_SHORT_LIBNAME, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging [Checkin: Comment 47]
Assignee

Comment 48

9 years ago
Walter, can you check your patch in?
(In reply to comment #48)
> Walter, can you check your patch in?

I don't believe Walter has commit access for c-c.
Keywords: checkin-needed
Whiteboard: c-n for Iv2
Assignee

Comment 50

9 years ago
(In reply to comment #49)
> I don't believe Walter has commit access for c-c.

I believe he should ask for it...

Comment 51

9 years ago
(In reply to comment #50)
> (In reply to comment #49)
> > I don't believe Walter has commit access for c-c.
> 
> I believe he should ask for it...

We few OS/2 people working on Mozilla discuss at the moment, who of us should ask for it...
Comment on attachment 441280 [details] [diff] [review]
(Iv2) Copy bug 536451 rules.mk part and adjust Makefiles [Checked in: comment 52]

Checked in: http://hg.mozilla.org/comm-central/rev/4c915bca9ba8
Attachment #441280 - Attachment description: (Iv2) Copy bug 536451 rules.mk part and adjust Makefiles → (Iv2) Copy bug 536451 rules.mk part and adjust Makefiles [Checked in: comment 52]
Keywords: checkin-needed
Whiteboard: c-n for Iv2
Target Milestone: Thunderbird 3.1b2 → Thunderbird 3.2a1
Assignee

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee

Updated

7 years ago
Summary: Core bug 514665 dropped/replaced USE_SHORT_LIBNAME uses: port that to c-c (apps) → Core bug 514665 dropped/replaced USE_SHORT_LIBNAME uses: port that to comm-central (apps)
You need to log in before you can comment on or make changes to this bug.