Closed Bug 701371 Opened 13 years ago Closed 13 years ago

Move memory/mozutils somewhere else

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox11 fixed)

RESOLVED FIXED
mozilla12
Tracking Status
firefox11 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

(Whiteboard: [requires clobber][qa-])

Attachments

(3 files, 3 obsolete files)

Since bug 677501, we have a mozutils library which consists of:
- jemalloc
- a custom ELF linker on Android
- a modified CRT on Windows

The rationale for creating that mozutils library is that it was needed for future works, such as bug 662814, requiring to be available from libxul as well as the main binary, components, and possibly third-party libraries.

I abused the memory top-level directory and placed the mozutils directory under it. I think it would be better to make things clearer, and more long-term-oriented, by renaming the memory toplevel directory to mozutils. That way, bug 662814 and subsequent similarly centralized utils will have a place where to live.

The only downside is that mozalloc, which is also under memory is not part of mozutils. But I believe this could (and probably should) be changed.
Attachment #573507 - Flags: review?(brendan)
Both mozalloc and jemalloc live in this directory ... it seems like it would make more sense to move mozutils out of memory/ and leave everything else.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2)
> Both mozalloc and jemalloc live in this directory ... it seems like it would
> make more sense to move mozutils out of memory/ and leave everything else.

Point is, mozalloc could/should be merged into mozutils. jemalloc IS in mozutils already.
Assignee: nobody → mh+mozilla
I don't think we should do this. The directory structure doesn't need to reflect the current library structures (which can change), but should provide a human-readable structure for understanding our code.
(In reply to Mike Hommey [:glandium] from comment #3)
> Point is, mozalloc could/should be merged into mozutils. jemalloc IS in
> mozutils already.

content/, layout/ and plenty of other stuff are in libxul, but I don't think anyone wants to move them into toolkit/library

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> I don't think we should do this. The directory structure doesn't need to
> reflect the current library structures (which can change), but should
> provide a human-readable structure for understanding our code.

+1
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> (In reply to Mike Hommey [:glandium] from comment #3)
> > Point is, mozalloc could/should be merged into mozutils. jemalloc IS in
> > mozutils already.
> 
> content/, layout/ and plenty of other stuff are in libxul, but I don't think
> anyone wants to move them into toolkit/library

But their content are about content, layout, etc.
We could have a separate home for mozutils, since it's not "memory" stuff, but where ? a new top-level directory ? then why keep "memory", considering the amount of stuff in it and considering it ends up in mozutils anyways?
I am going to move tri-license things from other-licenses/android under mozutils, and will add the new ELF linker there too. In both cases, memory/ is the wrong top-level directory.

Putting the memory/ toplevel directory issue aside, may i move mozutils so that it becomes a new toplevel directory, or do you have better locations?
Summary: Rename toplevel memory directory to mozutils → Move memory/mozutils somewhere else
Attached patch Move mozutils to top-level (obsolete) — Splinter Review
Attachment #580901 - Flags: review?(brendan)
Attachment #573507 - Attachment is obsolete: true
Attachment #573507 - Flags: review?(brendan)
Blocks: 709776
Suggest new top level name should be libc-overrides or mozlibc -- not libc and not mozutils. The library name stem can follow to match, if that's doable. May help decide between explicit/long -overrides and moz- prefix names.

/be
(In reply to Brendan Eich [:brendan] from comment #9)
> Suggest new top level name should be libc-overrides or mozlibc -- not libc
> and not mozutils. The library name stem can follow to match, if that's
> doable. May help decide between explicit/long -overrides and moz- prefix
> names.

How about cglue or c-glue?
After more IRC discussion with Mike, we ended up on mozglue. It's not as likely a honey trap as mozutil(s) but glue is too generic. The libc idea didn't pan out, since we have JNI and perhaps XPCOM glue in here too. It's gluey, don't touch!

/be
(In reply to Brendan Eich [:brendan] from comment #11)
> and perhaps XPCOM glue in here too.

In case bsmedberg reads this, the idea would be to eventually have an initialization glue to replace the xpcom standalone glue he wanted to get rid of before we started requiring it for firefox.
Attachment #583735 - Flags: review?(khuey)
Attachment #580901 - Attachment is obsolete: true
Attachment #580901 - Flags: review?(brendan)
Added the missing removed-files parts. Also applied bug 707121 to mobile.
Attachment #583737 - Flags: review?(khuey)
Attachment #583735 - Attachment is obsolete: true
Attachment #583735 - Flags: review?(khuey)
Backed out because of random and unexplainable android bustages.
https://hg.mozilla.org/integration/mozilla-inbound/rev/db6e5b2bf92d
Whiteboard: [inbound]
Relanded as-is (except for a few context changes). The problem was with bug 709776.
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf890c9c3e4c
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/cf890c9c3e4c
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
This broke js shell packaging (still looking for a now-missing mozutils.dll) and also prevents mozutils.dll from being copied to dist/firefox during packaging. Somehow, mozutils.dll does end up in the firefox zip package, though.
Unless this is just an issue of my objdir needing a clobber?
All platforms apart from Linux needed a clobber when I merged, if that helps :-)
Whiteboard: [inbound] → [requires clobber]
[Approval Request Comment]
This is a dependency for bug 683127. It only moves code around and changes a library name. No disruption is expected from the former, but for aurora, we may want to skip the latter. Please tell me if we should do so and I will update the patch accordingly.
Attachment #591483 - Flags: review+
Attachment #591483 - Flags: approval-mozilla-aurora?
Attachment #591483 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Did someone forget comm-aurora again?
Comment on attachment 584452 [details] [diff] [review]
Rename mozutils to mozglue. c-c part. rs=Callek

I have rs from Callek over IRC to push to comm-aurora.

Pushed:
http://hg.mozilla.org/releases/comm-aurora/rev/b808530327a5
Depends on: 721801
Whiteboard: [requires clobber] → [requires clobber][qa-]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: