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

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-MC) Remove obsolete MOZ_MOVEMAIL and MOZ_STATIC_MAIL_BUILD [Checkin: Comment 10]

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-SM) Remove duplicate entries after bug 528806 and bug 534410 [Checkin: Comment 4]

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) Get rid of WINNT msgcompo.xpt special case [Checkin: Comment 11]

Comment 4

[Serge Gautherie (:sgautherie)]

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

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) Get rid of WINNT msgbase.dll special case: s/mailnews/msgbase/ for all [Checkin: Comment 13]

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1) Get rid of msglocal.dll special case: s/localmail/msglocal/ for all [Checkin: Comment 14]

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Fv1) s/emitter/mimeemit/ SHORT_LIBNAME [Checkin: Comment 15]

This one is optional but is more explicit, helps sorting, etc.

Comment 8

[(not currently active) Ted Mielczarek]

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 9

[(not currently active) Ted Mielczarek]

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.

Comment 10

[Serge Gautherie (:sgautherie)]

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

Comment 11

[Serge Gautherie (:sgautherie)]

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

Comment 12

[Serge Gautherie (:sgautherie)]

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.

Comment 13

[Serge Gautherie (:sgautherie)]

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...

Comment 14

[Serge Gautherie (:sgautherie)]

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

Comment 15

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/comm-central/rev/080ae7e74aa3

Comment 16

[Mark Banner (:standard8)]

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.

Comment 17

[Serge Gautherie (:sgautherie)]

Attachment: (Gv1) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs, Copy bug 520418 too, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging

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.!.

Comment 18

[Serge Gautherie (:sgautherie)]

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.

Comment 19

[Mark Banner (:standard8)]

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 20

[Justin Wood (: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

slightly bitrotted and missing the changeset from Bug 421353. Please update and re-request.

Comment 21

[Serge Gautherie (:sgautherie)]

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

Which changeset exactly?

Comment 22

[Serge Gautherie (:sgautherie)]

Attachment: (Gv2) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs [Checkin: Comment 45]

Gv1, unbitrotted, without useless 192 checks, with Calendar part spun off.

Comment 23

[Serge Gautherie (:sgautherie)]

Attachment: (Hv1-SB) Get rid of USE_SHORT_LIBNAME, Remove MOZILLA_1_9_1_BRANCH in Calendar packaging [Checkin: Comment 47]

Gv1, Calendar part only.

Comment 24

[Justin Wood (:Callek)]

(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 25

[Justin Wood (:Callek)]

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.

Comment 26

[Serge Gautherie (:sgautherie)]

(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

[Walter Meinl]

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

[Robert Kaiser]

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

You mean "good" actually, right? ;-)

Comment 29

[Justin Wood (:Callek)]

(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 :/

Comment 30

[Serge Gautherie (:sgautherie)]

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

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

Comment 31

[Serge Gautherie (:sgautherie)]

Attachment: (Iv1) Copy bug 536451 rules.mk part too

Per Walter's comment 27.

Comment 32

[Walter Meinl]

Attachment: (Gv3) Get rid of USE_SHORT_LIBNAME and useless SHORT_LIBNAMEs

(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.

Comment 33

[Serge Gautherie (:sgautherie)]

(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 34

[Justin Wood (:Callek)]

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.

Comment 35

[Justin Wood (:Callek)]

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.

Comment 36

[Robert Kaiser]

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.

Comment 37

[Walter Meinl]

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.

Comment 38

[Serge Gautherie (:sgautherie)]

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

Here would be fine ;-)

Comment 39

[Walter Meinl]

Attachment: (Iv2) Copy bug 536451 rules.mk part and adjust Makefiles [Checked in: comment 52]

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.)

Comment 40

[Serge Gautherie (:sgautherie)]

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 ;-)

Comment 41

[Justin Wood (:Callek)]

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.

Comment 42

[Mark Banner (:standard8)]

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.

Comment 43

[Justin Wood (:Callek)]

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.

Comment 44

[Robert Kaiser]

(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.

Comment 45

[Serge Gautherie (:sgautherie)]

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

Comment 46

[Serge Gautherie (:sgautherie)]

(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 ;-)

Comment 47

[Serge Gautherie (:sgautherie)]

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

Comment 48

[Serge Gautherie (:sgautherie)]

Walter, can you check your patch in?

Comment 49

[Justin Wood (:Callek)]

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

I don't believe Walter has commit access for c-c.

Comment 50

[Serge Gautherie (:sgautherie)]

(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

[Walter Meinl]

(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 52

[Mark Banner (:standard8)]

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