Since these lines are (supposed to be) used to determine inter-module relationships, it's best to keep the list minimal. It's quite a bit of work though to keep track of modules no longer needed (where, if you need a new one, the compiler on a MOZ_TRACK_MODULE_DEPS build will let you know). A friend of mine wrote a little script which looks at the .deps/*.pp files to determine which modules were needed, compares that against what's currently listed on the REQUIRES line, and gives output in the form of a cvs diff -u. I've applied this patch to a fresh tree, and built with it successfully. I'll attach the patch for review.

Making dbaron the QA contact on this. doron, hope you don't mind :-)

r=cls

r=dbaron on that, assuming it builds :-) I guess anything it breaks should be fixed by a conditional (or even just commented) |REQUIRES += ...|.

Changing the summary, I'll just use this as a tracker.

r=cls on attach 30060

After doing a clobber build it turned out I needed to fix these two too: embedding/base/Makefile.in rdf/tests/rdfcat/Makefile.in Doing a "find -name 'Makefile.in' | xargs grep MODULE" from mozilla/gfx: ./idl/Makefile.in:MODULE = gfx ./idl/Makefile.in:XPIDL_MODULE = gfx ./src/ps/Makefile.in:MODULE = gfx ./src/qt/Makefile.in:MODULE = layout ./src/gtk/Makefile.in:MODULE = gfx ./src/mac/Makefile.in:MODULE = layout ./src/os2/Makefile.in:MODULE = layout ./src/beos/Makefile.in:MODULE = layout ./src/xlib/Makefile.in:MODULE = layout ./src/xlibrgb/Makefile.in:MODULE = xlibrgb ./src/xlibrgb/Makefile.in:XPIDL_MODULE = layout ./src/motif/Makefile.in:MODULE = layout ./src/rhapsody/Makefile.in:MODULE = layout ./src/Makefile.in:MODULE = gfx ./src/photon/Makefile.in:MODULE = layout ./src/xprint/Makefile.in:MODULE = gfx ./tests/Makefile.in:MODULE = gfx ./public/Makefile.in:MODULE = gfx What should I do with all those other MODULE lines?

This might be a question for drivers. Can gfx really be separated from layout? I think everything under gfx should be set to MODULE = gfx.

gfx doesn't (shouldn't) have any direct dependancies on layout. It should be able to build without it.

Mass move to jaggernaut@netscape.com

r=cls on attachment 43229 [details] [diff] [review]

Nice. r=dbaron.

Don't you have to add XPIDL_MODULE in some places (or change packaging files, but I'd prefer not)? Are there any static build issues?

I checked for idl files but there are none where I change the MODULE name. Btw, I talked to Dauphin and we agreed on putting everything in the "content" MODULE, instead of all the seperate ones. I'm also tempted to change the existing "xul", "xuldoc" and "xultmpl" over to "content" and add XPIDL_MODULE there to keep the right .xpts. I don't know what the impact of this is on the static build though. Waterson?

There should be no impact.

Comment on attachment 46749 [details] [diff] [review] [patch] intermediate clean-up. r=cls

Comment on attachment 49309 [details] [diff] [review] Instead of adding "gfx" to manager/ssl/src/Makefile.in, I'll just remove a #include "nsIWidget.h" ;-) r=bryner

i checked in attachment 49309 [details] [diff] [review].

Comment on attachment 49311 [details] [diff] [review] Move gfx from module layout to module gfx and fix REQUIRES lines. r=cls in e-mail

Comment on attachment 49667 [details] [diff] [review] Y'all are gonna love this! this patch was checked in with sr=alecf on Sep 18. Stop confusing my module owner. And don't touch extensions/transformiix/build/Makefile.in until we moved to static libs for building.

> And don't touch extensions/transformiix/build/Makefile.in > until we moved to static libs for building. So should we remove it from the build until then? Tree-wide changes often need to be made everywhere at once.

Pike: I haven't touched anything transformiix after you mentioned it to me, but thanks for reminding me and making it public so others know too :-)

Accepting.

Comment on attachment 126627 [details] [diff] [review] Clean-up 2003-06-27 wow, so many great cleanups :) I'll apply this on linux, see what happens.

Comment on attachment 127473 [details] [diff] [review] Same as above, but diff -u for testing r=cls

Comment on attachment 127473 [details] [diff] [review] Same as above, but diff -u for testing Noting sr=bryner, requesting approval. Risk is low, if anything breaks it'll show up as build bustage on tinderboxen, easy enough to fix.

Comment on attachment 127473 [details] [diff] [review] Same as above, but diff -u for testing Minusing for 1.5a approval, since (a) we're probably going to cut a branch tomorrow (b) I'd rather not risk having the source tarball not be buildable in unusual configurations that, which is a risk here. Sometimes bustage from this type of thing for more unusual things doesn't show up for a few days.

Let's get this into 1.5b as soon as the trunk opens. /be

Neil said this patch built for him on Linux.

Some additional cleanup, particularly for firefox/toolkit. Tested the following build configurations: linux firefox opt windows firefox opt mac firefopx opt mac seamonkey debug + tests

Comment on attachment 173314 [details] [diff] [review] patch You probably don't want the intl/uconv/src/Makefile.in change (although you may want to move the ucnative into the ifdef, so it's a REQUIRES += ucnative line that doesn't get caught by this script)

Tested mac seamonkey, windows seamonkey, and windows thunderbird.

Comment on attachment 299946 [details] [diff] [review] Passes TryServer (Firefox), and SeaMonkey and Thunderbird on Mac and Linux. Ignore the non-Makefile.in changes, they're from peterv's patch on bug 407034 which make using trunk bearable. I'll attach one without them in a bit.

Comment on attachment 300043 [details] [diff] [review] Passes TryServer (Firefox), and SeaMonkey and Thunderbird on Mac and Linux. [checked in] So I'm not so happy about ifdef'd REQUIRES changes -- they make it easy for somebody in one configuration to make a change that compiles fine (since they have that thing turned on) but that breaks for somebody with the feature off. However, they are helpful for automatic testing of whether requires lines are still needed. Maybe the best thing to do would be a compromise: comment rather than ifdef? Does that seem reasonable? rubber-stamp r+sr=dbaron with that (or without it if you can convince me otherwise)

<dbaron> the places where you have ifdef ACCESSIBILITY <dbaron> just say <dbaron> # for ACCESSIBILITY <dbaron> or something like that <jag> If you're ok with that <jag> We've always done the ifdef <jag> I can change all locations to use the comment style. <dbaron> I dunno. <jag> Would you be ok with me doing that in a separate bug? <dbaron> I guess it's ok the way you have it. <dbaron> separate bug is ok, yeah <dbaron> If it's not biting people maybe it's not even worth the bother. Filed bug 418037 for further discussion.

Comment on attachment 300043 [details] [diff] [review] Passes TryServer (Firefox), and SeaMonkey and Thunderbird on Mac and Linux. [checked in] jag is on the case on irc. /be

So in certain cases people have been picking up header files from dist/sdk/include instead of from dist/include/$MODULE. This changes config.mk to only add dist/sdk/include when MOZ_ENABLE_LIBXUL is defined, and fixes up the various REQUIRES lines that were missing modules due to this. This builds on my Intel Mac Mini under SeaMonkey, Firefox, Thunderbird, Camino and XulRunner.

Comment on attachment 300043 [details] [diff] [review] Passes TryServer (Firefox), and SeaMonkey and Thunderbird on Mac and Linux. [checked in] 1.148 jag%tty.nl 2008-02-18 00:50

Comment on attachment 310745 [details] [diff] [review] Another pass, this time also putting in missing modules. This patch hasn't landed and is presumably holding this bug open. Can we spin this off to another bug? Drivers want all bugs with approval1.9+ closed ASAP.

Since bsmedberg wants to change the way this all works I've held off on getting that patch reviewed and checked in. I'm just gonna close this bug. I'll re-open it, or open a new one, if needed.

Pushed by frgrahl@gmx.net: https://hg.mozilla.org/comm-central/rev/32e089cc6736 Clean up our MODULE/REQUIRES story. rs=dbaron, a=brendan