Currently, dependentlibs.list is created from a more or less hardcoded list of libraries, which is error-prone and limited (see bug 762621 for an example of problem this creates)

Comment on attachment 632220 [details] [diff] [review] Generate dependentlibs.list from actual library dependencies Review of attachment 632220 [details] [diff] [review]: ----------------------------------------------------------------- I'm assuming that the ordering that comes out of these tools is the proper library dependency order? I suppose your recursive-traversal will make that work in any event, right? ::: xpcom/stub/Makefile.in @@ +55,5 @@ > include $(topsrcdir)/config/rules.mk > > libs:: $(FINAL_TARGET)/dependentlibs.list > > +$(FINAL_TARGET)/dependentlibs.list: dependentlibs.py $(FINAL_TARGET)/$(SHARED_LIBRARY) $(addprefix $(FINAL_TARGET)/,$(shell cat $(FINAL_TARGET)/dependentlibs.list)) I guess $(shell) doesn't actually fail if the command fails, but we'll still get some error output here. You could wrap this last dep in $(if $(wildcard $(FINAL_TARGET)/dependentlibs.list)) to make that nicer. ::: xpcom/stub/dependentlibs.py @@ +12,5 @@ > +import sys > + > +def dependentlibs_dumpbin(lib): > + '''Returns the list of dependencies declared in the given DLL''' > + proc = subprocess.Popen(['dumpbin', '-imports', lib], stdout = subprocess.PIPE, stderr = subprocess.PIPE) If you don't care about stderr in these cases (which you don't seem to), you could skip PIPEing it, skip the .communicate, and just do "for line in proc.stdout:". You'd also have to proc.wait() at the end. @@ +35,5 @@ > + tmp = line.split(' ', 3) > + if len(tmp) > 2 and tmp[2] == '(NEEDED)': > + # NEEDED lines look like: > + # 0x00000001 (NEEDED) Shared library: [libname] > + match = re.search('\[(.*)\]', tmp[3]) You only check for len(tmp) > 2 above, you probably want to check > 3. @@ +67,5 @@ > + in the same directory''' > + deps = [] > + dir = os.path.dirname(lib) > + for dep in func(lib): > + if dep in deps or os.path.isabs(dep): You could probably (prematurely) optimize this a little bit by using a set() to hold the "already seen-ness" of the files. I'm assuming you want a list because the ordering is important? You could maintain both, but that might be a pain.

(In reply to Ted Mielczarek [:ted] from comment #2) > Comment on attachment 632220 [details] [diff] [review] > Generate dependentlibs.list from actual library dependencies > > Review of attachment 632220 [details] [diff] [review]: > ----------------------------------------------------------------- > > I'm assuming that the ordering that comes out of these tools is the proper > library dependency order? I suppose your recursive-traversal will make that > work in any event, right? That's the intent, at least :) > @@ +67,5 @@ > > + in the same directory''' > > + deps = [] > > + dir = os.path.dirname(lib) > > + for dep in func(lib): > > + if dep in deps or os.path.isabs(dep): > > You could probably (prematurely) optimize this a little bit by using a set() > to hold the "already seen-ness" of the files. I'm assuming you want a list > because the ordering is important? You could maintain both, but that might > be a pain. Or use an OrderedSet ; the recipe at http://code.activestate.com/recipes/576694/ is >= 2.6, I guess it's possible to make it work on 2.5. (and we don't need the whole thing either) On the other hand, that's a lot of code for little benefit. Do we really care?

It's not worth that much effort, no. Hence my "premature" comment. Since this only gets run once per build anyway, it's not a big deal.

(In reply to Mike Hommey [:glandium] from comment #5) > https://hg.mozilla.org/integration/mozilla-inbound/rev/569f816a542b if this sticks, after merge to central, rs+=me to remove extradependlibs.mk from all c-c

Backed out. There's a race condition in xpcom/stub/Makefile.in https://hg.mozilla.org/integration/mozilla-inbound/rev/03341a974e4a

The race condition was that dependentlibs.list could be attempted to be generated before libxpcom is copied to FINAL_TARGET. Since that happens during libs, it's not really fixable through dependencies. So I just added an option for a library search path, which allows to generate dependentlibs.list without libxpcom being in FINAL_TARGET, where its dependencies are. Another problem is that b2g doesn't have a libmozalloc.so (it's built statically), but the lib appears in dependentlibs.list (sic), and the dependency breaks incremental builds from before to after. This is fixable with a clobber, but this also means this would fail if we ever rename a lib, remove a lib, etc.

Comment on attachment 634846 [details] [diff] [review] Fixup Review of attachment 634846 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/stub/dependentlibs.py @@ +66,5 @@ > +def dependentlibs(lib, libpaths, func): > + '''For a given library, returns the list of recursive dependencies that can > + be found in the given list of paths''' > + assert(libpaths) > + assert(isinstance(libpaths, list)) Seems like overkill, but okay.

Folded both patches when landing: https://hg.mozilla.org/integration/mozilla-inbound/rev/a57052eb374b