388955 - js/src/Makefile.ref dependencies are broken on Mac, Linux

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Jason Orendorff [:jorendorff]]

Attachment: use gcc -MMD to generate make dependencies

They're broken on other platforms too, but these are the ones I know how to fix.

Note:  Patch is against actionmonkey branch, should work against mozilla-central; but some tweaks will be required if you want it in CVS.  In particular the "%.cpp=" bit would be "%.c=" there.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 273122 [details] [diff] [review]
use gcc -MMD to generate make dependencies

>--- a/js/src/Makefile.ref	Fri Jul 20 10:25:27 2007 -0400
>+DEPENDENCIES    = $(CPPFILES:%.cpp=$(OBJDIR)/%.d)

CPPFILES doesn't exist. I think you want JS_CPPFILES. Once I make that change (with s/CPP/C/) this works beautifully. Thanks a bundle!

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

We should also file a bug on generating the .d files only when needed (and not every time, as this patch does) unless you want to tackle that here.

Comment 3

[Jason Orendorff [:jorendorff]]

mrbkap: What did you have in mind in comment 2?

Not all the .d files are regenerated every time you build.  They're only generated for source files that are actually compiled (it happens as a side effect of compilation).  This is a fairly close approximation of "only when needed", close enough for me anyway.

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 273122 [details] [diff] [review]
use gcc -MMD to generate make dependencies

I think this is OK, because:

Makefile.ref:288:include rules.mk

rules.mk:54:CPPFILES = $(LIB_CPPFILES) $(PROG_CPPFILES)

I tried it out and it seems to work; also "make clobber" deletes the .d files, so I think DEPENDENCIES is getting set properly.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 273122 [details] [diff] [review]
use gcc -MMD to generate make dependencies

Huh, I wonder what I was seeing before.

Comment 6

[Jason Orendorff [:jorendorff]]

Pushed to actionmonkey.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: For CVS

This patch makes CVS happy. I don't know why I didn't do this before.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 306813 [details] [diff] [review]
For CVS

NPOTB ;-) -- great to have after all these years of living in the early 80s.

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

I checked the CVS patch into ... well ... CVS.