Closed Bug 506845 Opened 12 years ago Closed 12 years ago
targets are not rebuilt when archives from EXPAND
With --enable-application=browser: % make -f client.mk build % touch obj/memory/jemalloc/libjemalloc.a % make -C obj/browser/app Expected results: obj/browser/app/firefox-bin newer than obj/memory/jemalloc/libjemalloc.a. Actual results: obj/browser/app/firefox-bin older than obj/memory/jemalloc/libjemalloc.a.
Using a filename rather than -Ldir -lname allows LIBS_DEPS from config/rules.mk to work.
Hrm... is this in particular because jemalloc is a static library? If not, we should add all -lfoo to LIBS_DEPS as well (gmake searches for -lfoo dependencies correctly). We use this pattern a lot...
(In reply to comment #2) > Hrm... is this in particular because jemalloc is a static library? Yes. The lack of a dependency is always a problem for static libraries. For shared libraries, it is not usually an issue. > If not, we should add all -lfoo to LIBS_DEPS as well Maybe would should anyway, if we are being thorough, though it is less important. It may be useful to rebuild objects depending on shared libraries even to check that symbols removed from the shared libraries are not needed, for example. > (gmake searches for -lfoo dependencies correctly). I had forgotten that, but I'm not sure how to deal with -Lpath. http://www.gnu.org/software/automake/manual/make/Libraries_002fSearch.html Should we add this to rules.mk?: vpath $(LIB_PREFIX)%.$(LIB_SUFFIX) \ $(patsubst -L%,%,$(filter -L%, $(LIBS) $(HOST_LIBS) $(EXTRA_DSO_LDOPTS))) What is meant by "import libs" here? http://hg.mozilla.org/mozilla-central/file/74962214e308/config/rules.mk#l106 What might be the problem with always expanding $(call EXPAND_LIBNAME_PATH,foo,dir) to dir/$(LIB_PREFIX)foo.$(LIB_SUFFIX)? Readability I s'pose.
(In reply to comment #3) > vpath $(LIB_PREFIX)%.$(LIB_SUFFIX) \ > $(patsubst -L%,%,$(filter -L%, $(LIBS) $(HOST_LIBS) $(EXTRA_DSO_LDOPTS))) And another for DLL_PREFIX/SUFFIX.
(In reply to comment #3) > What might be the problem with always expanding > $(call EXPAND_LIBNAME_PATH,foo,dir) > to dir/$(LIB_PREFIX)foo.$(LIB_SUFFIX)? I guess we'd need to be specific with a EXPAND_DLLNAME_PATH.
Comment on attachment 391001 [details] [diff] [review] patch EXPAND_LIBNAME_PATH is also being used for compression and image static libraries, so we need to fix that too.
EXPAND_LIBNAME_PATH is being used for both static and shared libraries. I don't think shared libraries are really need to be dependencies in practice. If the exported symbols change, there will probably be a header that changes and causes the targets to be rebuilt, which is enough if the library is rebuilt first. However, it could be inconvenient to have to use a different macro depending on whether the library is built static or shared. I think I have enough of an understanding re import libs now. The $(path)/$(LIB_PREFIX)$(lib).$(LIB_SUFFIX) works on the platforms on which we set _LIBNAME_RELATIVE_PATHS on because $(LIB_SUFFIX) is the same as $(IMPORT_LIB_SUFFIX) and $(LIB_PREFIX) is the same as $(DLL_PREFIX). I'll try out setting vpath.
Summary: Programs are not rebuilt when jemalloc changes → targets are not rebuilt when archives from EXPAND_LIBNAME change
The search path that make uses is not the same as the search path that gcc uses so this doesn't try to generate deps from OS_LIBS. (We'd need to start using gcc --print-search-dirs or gcc --print-file-name, I think.) I don't know the precise purpose of EXTRA_LIBS, but there doesn't seem to be an EXTRA_LIBS_DEPS so deps are not generated from EXTRA_LIBS either.
Comment on attachment 391561 [details] [diff] [review] add libs to _DEPS and set up vpaths The semantics of vpath with one argument (only a pattern) is a little different and not what we want here.
Check that there are some directories before using vpath.
Comment on attachment 391703 [details] [diff] [review] add libs to _DEPS and set up vpaths v2 [Checkin: Comment 12] Assuming this passed tryserver, looks good.
Attachment #391703 - Flags: review?(benjamin) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
Target Milestone: mozilla1.9.3 → mozilla1.9.3a1
Attachment #428607 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 428607 [details] [diff] [review] (Bv1-CC) Copy it to comm-central [Checkin: Comment 14] http://hg.mozilla.org/comm-central/rev/05a786323772
Attachment #428607 - Attachment description: (Bv1-CC) Copy it to comm-central → (Bv1-CC) Copy it to comm-central [Checkin: Comment 14]
Attachment #391703 - Attachment description: add libs to _DEPS and set up vpaths v2 → add libs to _DEPS and set up vpaths v2 [Checkin: Comment 12]
You need to log in before you can comment on or make changes to this bug.