Closed Bug 520114 Opened 10 years ago Closed 10 years ago

Port |Bug 441324 - Implement xmalloc regardless of whether jemalloc is disabled| to comm-central

Categories

(MailNews Core :: Build Config, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a1

People

(Reporter: cjones, Assigned: philor)

References

Details

Attachments

(3 files, 4 obsolete files)

Changing product/component to make the right people see this.  Still not sure where the blame lies.
Attached patch Fix for linux comm-central build (obsolete) — Splinter Review
Not tested on 1.9.1, but should work there too.  Windows may need more work, but I don't have a windows build machine at the moment.
Component: XPCOM → Build Config
Product: Core → MailNews Core
QA Contact: xpcom → build-config
Thanks for the start Chris, I'll be taking more of a look at this in 1/2 hour or so.
This changes the if for MOZILLA_1_9_1_BRANCH to also be for 1.9.2. I've also copy-pasted the chunk that sets ac_cv_attribute_always_inline etc.

As this is based on Chris' original patch, r+a=Standard8 and I'll push this now and we'll see how it affects the tree.
Assignee: nobody → bugzilla
Attachment #404190 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #404220 - Flags: review+
Attachment #404220 - Flags: approval-thunderbird3+
Changeset http://hg.mozilla.org/comm-central/rev/86f5531e2c87 has done the trick for TB Linux check at least. I'll check on Windows and Mac later.
Linux bloat seems to have crashed in its trace malloc step. I'm letting it cycle to see if it is intermittent or not.

Windows builds have link error but I think I know what that is, will fix later.
I just ported some more of the configure.in changes for windows:

http://hg.mozilla.org/comm-central/rev/b66d6e147cfa
Thanks Mark!  Apologies again.
(In reply to comment #8)
> Thanks Mark!  Apologies again.

No apologies necessary, we expect things changing in mozilla-central to break us from time to time.
Attached patch Backout [Checkin: Comment 16] (obsolete) — Splinter Review
If I didn't screw this (hg revert -r d4529d3c2429 configure.in) up, it should back out all the fixes, and green up Linux (which is the only platform I don't think is currently burning from something else besides this).
Bug 441324 doesn't seem to be going anywhere at the moment. Changing deps so we'll get notified if bug 441324 changes state, and unassigned myself as there's nothing to do here for now.
Assignee: bugzilla → nobody
No longer blocks: 441324
Status: ASSIGNED → NEW
Depends on: 441324
Target Milestone: --- → Future
Attached patch re-port (obsolete) — Splinter Review
This might at least save you some time during your morning "wake up to flames." Pushed to the try server, just in case that finishes in time to tell you anything about how well I did.
Attachment #430237 - Flags: review?(bugzilla)
Ima go out on a limb and say we want to actually *package* libmozalloc.dylib, not just build it and leave it on the floor.
Attachment #430237 - Attachment is obsolete: true
Attachment #430243 - Flags: review?(bugzilla)
Attachment #430237 - Flags: review?(bugzilla)
Attachment #430243 - Flags: review?(bugzilla) → review+
Windows hasn't cycled yet, but this is looking pretty much fixed now:

http://hg.mozilla.org/comm-central/rev/e1e944c8ad9e
Assignee: nobody → philringnalda
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: Future → Thunderbird 3.2a1
just one [small] addition.
Attachment #430794 - Flags: review?(kairo)
Attachment #430794 - Flags: feedback?
Attachment #430794 - Flags: feedback? → feedback?(sgautherie.bz)
Attachment #404220 - Attachment description: Fix for linux comm-central build v2 → Fix for linux comm-central build v2 [Checkin: See comment 5+7]
Attachment #404220 - Attachment is obsolete: true
Attachment #404389 - Attachment description: Backout → Backout [Checkin: Comment 16]
Attachment #404389 - Attachment is obsolete: true
Attachment #430243 - Attachment description: re-port v.2 → re-port v.2 [Checkin: Comment 14]
Comment on attachment 430794 [details] [diff] [review]
finish-port (config.mk change) [checkin c#22]


It doesn't look like c-c needs this, does it?
http://mxr.mozilla.org/comm-central/search?string=MOZALLOC_LIB&case=on
Attachment #430794 - Flags: feedback?(sgautherie.bz) → feedback-
Summary: comm-central builds broken by 8cbc47eee659 → Port |Bug 441324 - Implement xmalloc regardless of whether jemalloc is disabled| to comm-central
Comment on attachment 430794 [details] [diff] [review]
finish-port (config.mk change) [checkin c#22]

I'm not completely sure if we'll ever need it, but it does no harm either.
Attachment #430794 - Flags: review?(kairo) → review+
(In reply to comment #20)
> I'm not completely sure if we'll ever need it, but it does no harm either.

It "harms" because I/we're in the process of removing what we don't "absolutely" needs :-/
Depends on: 551294
Comment on attachment 430794 [details] [diff] [review]
finish-port (config.mk change) [checkin c#22]

http://hg.mozilla.org/comm-central/pushloghtml?fromchange=386d5b686699&tochange=a803df412b50
Attachment #430794 - Attachment description: finish-port (config.mk change) → finish-port (config.mk change) [checkin c#22]
Comment on attachment 430850 [details] [diff] [review]
(Fv1) Cleanup after too much or wrong port, Remove useless (legacy) MOZ_MEMORY_*

r+ on the non Mozilla_Memory related removals. If you insist on those in this same patch we need Mark's sr here.

If you want to do the Mozilla_Memory in a different patch, feel free to port my r+ to that, and use a new bug and sr? him.
Attachment #430850 - Flags: superreview?(bugzilla)
Attachment #430850 - Flags: review?(bugspam.Callek)
Attachment #430850 - Flags: review+
Attachment #430850 - Flags: superreview?(bugzilla) → superreview-
Comment on attachment 430850 [details] [diff] [review]
(Fv1) Cleanup after too much or wrong port, Remove useless (legacy) MOZ_MEMORY_*

>+    MOZ_JS_LIBS='-L$(LIBXUL_DIST)/bin -lmozjs'
> if test "$MOZILLA_1_9_2_BRANCH" = "1"; then
>-    MOZ_JS_LIBS='-L$(LIBXUL_DIST)/bin -lmozjs'
>-    DYNAMIC_XPCOM_LIBS='-L$(LIBXUL_DIST)/bin -lxpcom -lxpcom_core '
>+    DYNAMIC_XPCOM_LIBS='-L$(LIBXUL_DIST)/bin -lxpcom -lxpcom_core'
>+else
>+    DYNAMIC_XPCOM_LIBS='-L$(LIBXUL_DIST)/bin -lxpcom -lxpcom_core -lmozalloc'
>+fi
etc...

When I reviewed this, I probably didn't comment, but I did consciously allow it in ONE block/ifdef because it was far simpler than multiple. Therefore please drop this hunk.

>-MALLOC_H=
>-AC_CHECK_HEADER(malloc.h,        [MALLOC_H=malloc.h])
>-if test "$MALLOC_H" = ""; then
>-  AC_CHECK_HEADER(malloc/malloc.h, [MALLOC_H=malloc/malloc.h])
>-  if test "$MALLOC_H" = ""; then
>-    AC_CHECK_HEADER(sys/malloc.h,    [MALLOC_H=sys/malloc.h])
>-  fi
>-fi
>-if test "$MALLOC_H" != ""; then
>-   AC_DEFINE_UNQUOTED(MALLOC_H, <$MALLOC_H>)
>-fi

I don't see why you're removing this and not the rest. There again, I'm not a wizard on this foo, but an explanation would be useful.
You need to log in before you can comment on or make changes to this bug.