Last Comment Bug 538753 - Port |Bug 508861 - [electrolysis] Build/ship the C++ runtime with MSVC/jemalloc| to comm-central. (SeaMonkey packaging fix only)
: Port |Bug 508861 - [electrolysis] Build/ship the C++ runtime with MSVC/jemall...
Status: RESOLVED FIXED
[mostly fixed by blocking bugs in TB ...
: fixed-seamonkey2.0.4
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 Windows 2000
: -- trivial (vote)
: Thunderbird 3.1b2
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
Depends on: 508861 512763 531283 NoC192SM 552943
Blocks: C192ConfSync
  Show dependency treegraph
 
Reported: 2010-01-09 04:08 PST by Serge Gautherie (:sgautherie)
Modified: 2010-03-17 08:51 PDT (History)
2 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
(Av1) Remove AC_DEFINE(_STATIC_CPPLIB), missed in bug 512763, (1.9.2+) [Backout: Bug 552943] (1.27 KB, patch)
2010-03-06 09:01 PST, Serge Gautherie (:sgautherie)
gozer: review+
Details | Diff | Review
(Bv1-SM) Remove mozcpp19.dll (on trunk too) [Checkin: Comment 9] (646 bytes, patch)
2010-03-09 07:03 PST, Serge Gautherie (:sgautherie)
bugspam.Callek: review+
Details | Diff | Review
(Cv1-SM-191) Always remove mozcpp19.dll, wrongly fixed in bug 512763 [Checkin: Comment 11] (792 bytes, patch)
2010-03-14 07:26 PDT, Serge Gautherie (:sgautherie)
kairo: review+
kairo: approval‑seamonkey2.0.4+
Details | Diff | Review

Description Serge Gautherie (:sgautherie) 2010-01-09 04:08:34 PST

    
Comment 1 Serge Gautherie (:sgautherie) 2010-01-09 04:37:03 PST
Oh, got real confused trying to sort this one out :-<
Comment 2 Serge Gautherie (:sgautherie) 2010-03-06 09:01:10 PST
Created attachment 430849 [details] [diff] [review]
(Av1) Remove AC_DEFINE(_STATIC_CPPLIB), missed in bug 512763, (1.9.2+)
[Backout: Bug 552943]
Comment 3 Serge Gautherie (:sgautherie) 2010-03-09 07:03:52 PST
Created attachment 431354 [details] [diff] [review]
(Bv1-SM) Remove mozcpp19.dll (on trunk too)
[Checkin: Comment 9]

Firefox doesn't seem to care but SeaMonkey does.
Comment 4 Philippe M. Chiasson (:gozer) 2010-03-09 11:11:34 PST
Comment on attachment 430849 [details] [diff] [review]
(Av1) Remove AC_DEFINE(_STATIC_CPPLIB), missed in bug 512763, (1.9.2+)
[Backout: Bug 552943]

Just cleaning up my unnecessary review request
Comment 5 Justin Wood (:Callek) 2010-03-12 15:57:21 PST
Comment on attachment 431354 [details] [diff] [review]
(Bv1-SM) Remove mozcpp19.dll (on trunk too)
[Checkin: Comment 9]

Can you run me through this, I'm not exactly sure what this bug; or this patch is trying to solve. I don't see any indication this should be in removed-files on trunk at all.
Comment 6 Serge Gautherie (:sgautherie) 2010-03-13 09:12:46 PST
(In reply to comment #5)

> Can you run me through this, I'm not exactly sure what this bug; or this patch

The main purpose of this bug was fixed by blocking bugs.
The current patches are additional/missed points.

> is trying to solve. I don't see any indication this should be in removed-files
> on trunk at all.

FF doesn't care at all; TB cares on m-1.9.2- only; SM already has it on c-1.9.1.
Actually, I don't see why this file shouldn't be removed when it's not needed on trunk.
Comment 7 Serge Gautherie (:sgautherie) 2010-03-13 09:15:35 PST
Comment on attachment 430849 [details] [diff] [review]
(Av1) Remove AC_DEFINE(_STATIC_CPPLIB), missed in bug 512763, (1.9.2+)
[Backout: Bug 552943]

gozer,
why r- patch you're not looking at?
why refuse to look at a followup patch for a bug you r+?
Comment 8 Justin Wood (:Callek) 2010-03-13 22:11:13 PST
Comment on attachment 431354 [details] [diff] [review]
(Bv1-SM) Remove mozcpp19.dll (on trunk too)
[Checkin: Comment 9]

To me, this actually can land; I don't see a huge deal with us needing this in removed files (one could expect that if we enable jemalloc/"MOZ_MEMORY" that we'd keep it. But theoretically this is fine to to fix for mail/* too (to remove on trunk as well), but that is not up to me.
Comment 9 Serge Gautherie (:sgautherie) 2010-03-14 07:25:23 PDT
Comment on attachment 431354 [details] [diff] [review]
(Bv1-SM) Remove mozcpp19.dll (on trunk too)
[Checkin: Comment 9]


http://hg.mozilla.org/comm-central/rev/0f98de542f0b
Comment 10 Serge Gautherie (:sgautherie) 2010-03-14 07:26:50 PDT
Created attachment 432420 [details] [diff] [review]
(Cv1-SM-191) Always remove mozcpp19.dll, wrongly fixed in bug 512763
[Checkin: Comment 11]
Comment 11 Serge Gautherie (:sgautherie) 2010-03-14 09:41:31 PDT
Comment on attachment 432420 [details] [diff] [review]
(Cv1-SM-191) Always remove mozcpp19.dll, wrongly fixed in bug 512763
[Checkin: Comment 11]


http://hg.mozilla.org/releases/comm-1.9.1/rev/ee3cd6696010
Comment 12 Mark Banner (:standard8) 2010-03-14 10:13:56 PDT
(In reply to comment #6)
> Actually, I don't see why this file shouldn't be removed when it's not needed
> on trunk.

If its not needed, then a) why does SM stil try and package it, b) the configs explicitly define it as a default library?

Yes, I'm not sure why the tinderboxes aren't broken, but maybe that's because it you're packaging and then removing it.

Either way, what's happening here just looks wrong to me IMO (see also:

http://mxr.mozilla.org/comm-central/search?string=mozcpp19

)

Anyway, I didn't fix/implement this bug, so don't assign it to me.
Comment 13 Serge Gautherie (:sgautherie) 2010-03-14 16:20:39 PDT
(In reply to comment #12)

> If its not needed, then a) why does SM stil try and package it, b) the configs
> explicitly define it as a default library?

mozcpp19.dll is needed |#ifdef MOZ_MEMORY| only and on trunk only.
Hence, it's not needed |#ifndef MOZ_MEMORY| on trunk and never needed on m-1.9.1.
NB: And SeaMonkey doesn't care about m-1.9.2, though it looks like my trunk patch would be fine on it too.

> Yes, I'm not sure why the tinderboxes aren't broken, but maybe that's because

I'm not sure which tinderboxes you would expect to be broken and in which case.

> it you're packaging and then removing it.

I'm not, am I?

> Either way, what's happening here just looks wrong to me IMO (see also:

What, specifically?

> Anyway, I didn't fix/implement this bug, so don't assign it to me.

I had assigned it to you because you had fixed bug 512763.
I agree I'd rather be the assignee now that I'm fixing some nits here ;-)
Comment 14 Philippe M. Chiasson (:gozer) 2010-03-14 17:56:49 PDT
(In reply to comment #7)
> (From update of attachment 430849 [details] [diff] [review])
> gozer,
> why r- patch you're not looking at?

Sorry, I was trying to remove a review request for a patch I thought I was not relevant reviewer for.

> why refuse to look at a followup patch for a bug you r+?

Because I mistakenly took this patch/bug for another one. Apologies.
Comment 15 Justin Wood (:Callek) 2010-03-14 19:16:08 PDT
(In reply to comment #12)

The points you bring up actually tripped me up at first too...

> (In reply to comment #6)
> > Actually, I don't see why this file shouldn't be removed when it's not needed
> > on trunk.
> 
> If its not needed, then a) why does SM stil try and package it, b) the configs
> explicitly define it as a default library?

It is only not needed when MOZ_MEMORY is not set. So the bits you cite are also correct.

> Either way, what's happening here just looks wrong to me IMO (see also:
> 
> http://mxr.mozilla.org/comm-central/search?string=mozcpp19

It did look wrong to me too; and I did like 3 checks on things just to be sure I understood what the case was. its packaged ifdef MOZ_MEMORY; and removed ifndef MOZ_MEMORY.

As I said earlier once we do set MOZ_MEMORY I don't forsee us dropping it (so long as that config is supported by m-c anyway). So I'm not certain this is even really needed in removed-files. But even so, it is "correct" so did not stop the patch landing.
Comment 16 Mark Banner (:standard8) 2010-03-15 00:55:48 PDT
(In reply to comment #13)
> > it you're packaging and then removing it.
> 
> I'm not, am I?

No you're not, I was getting confused between ifdefs and different patches.
Comment 17 Serge Gautherie (:sgautherie) 2010-03-16 09:29:24 PDT
Comment on attachment 430849 [details] [diff] [review]
(Av1) Remove AC_DEFINE(_STATIC_CPPLIB), missed in bug 512763, (1.9.2+)
[Backout: Bug 552943]


http://hg.mozilla.org/comm-central/rev/e990fa115c50
Comment 18 Mark Banner (:standard8) 2010-03-17 08:20:02 PDT
Reopening, attachment 430849 [details] [diff] [review] caused bug 552943 as that define is still needed on 1.9.2 branch.

So in reality, I think this is wontfix and bug 512763 didn't miss anything. Although due to the additional patches for packaging changes, maybe just changing the title would do.
Comment 19 Serge Gautherie (:sgautherie) 2010-03-17 08:49:38 PDT
Comment on attachment 430849 [details] [diff] [review]
(Av1) Remove AC_DEFINE(_STATIC_CPPLIB), missed in bug 512763, (1.9.2+)
[Backout: Bug 552943]


Wow, I really got confused on this case: sorry :-(

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