27955 - Mozilla's make depend and nsprpub

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P3)

Description

[Wan-Teh Chang]

Mozilla's make depend does not recurse into
the nsprpub directory if COMPILER_DEPEND is
defined. (See mozilla/config/rules.mk.)
NSPR's build system does not generate header
dependencies.  These two facts together break
the Linux depend Tinderbox builds whenever an
NSPR header file changes.

There are two possible solutions:
1. Fix NSPR's build system so that it generates
   header dependencies in the same way as Mozilla's
   build system does.
2. Fix Mozilla's make depend rule so that it always
   recurses into nsprpub.

I propose that we do #2 because it is more modular:
it allows NSPR to implement make depend in whatever
way it sees fit.  In fact, currently NSPR's make
depend does a make clean.  It's an ugly hack,
but it meets the requirement of Mozilla's depend
Tinderbox builds.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch for mozilla/Makefile.in.

Comment 2

[cls]

#2 is unsatisfactory as nspr's 'depend:: clean' rule will cause a complete
rebuild of xpcom and therefore the tree for each depend cycle.  For the nspr4
landing though, r=cls on the condition that added depend:: rule is removed in
the near future.  Hopefully, shortly after the tree closes.

Comment 3

[Wan-Teh Chang]

Why will xpcom be completely rebuilt?  The xpcom library
may need to be relinked but the C and C++ files shouldn't
need to be recompiled if libnspr3.so has a newer time stamp.

Comment 4

[cls]

Ok, my mistake.  I thought that either the timestamps on the symlinks would
change or the symlinks would be removed but that only happens with a 'realclean'
under nspr. r=cls

Comment 5

[Wan-Teh Chang]

cls, I have another solution that might be more to
your liking.

I figured out a way to make all the object files in
NSPR depend on a dummy header file.  Then, whenever
I want the Tinderbox depend builds to recompile
everything inside nsprpub, I just touch that dummy
header to give it a newer time stamp.

This solution requires no changes to mozilla/Makefile.in.

Comment 6

[Wan-Teh Chang]

Attachment: Proposed patch to replace the "depend: clean" hack by the dummy header file hack.

Comment 7

[Wan-Teh Chang]

cls, in NSPR makefiles, should I use MOD_DEPTH or topsrcdir?

Comment 8

[cls]

topsrcdir should work for both builds.  MOD_DEPTH will break autoconf builds
that use a separate object tree.  I like the dummy header idea. 

Comment 9

[Wan-Teh Chang]

I'm using MOD_DEPTH and it works with an autoconf build
in mozilla/nsprpub/obj-linux.  Do you know why?  MOD_DEPTH
and topsrcdir are off by one level for that build.

Comment 10

[cls]

When you test the forced rebuild, do you touch
mozilla/nsprpub/obj-linux/config/prdefend.h or
mozilla/nsprpub/config/prdepend.h.  Using MOD_DEPTH would look for the former
and topsrcdir should look for the latter.

At second glance, I don't understand that rule. If checking the dependencies for
nsprpub/lib/ds/plarena.c, that rule would add this to the Makefile...

./plarena.o: ./plarena.o: $(DUMMY_DEPEND_H)

Why do you need the first $(filter)?

Comment 11

[Wan-Teh Chang]

I touch mozilla/nsprpub/config/prdepend.h.  That's
why I don't understand why MOD_DEPTH also works.

The $(filter) is necessary to filter out files
in $(OBJS) that don't end in .o or .obj.  On
Windows, $(OBJS) may contain a compiled resource
file with the .res suffix.  So I need to filter
that out.  On Unix the $(filter) does nothing.

Comment 12

[cls]

It appears as though VPATH is forcing make to also look in
mozilla/nsprpub/config/ for prdepend.h so it uses the one found there even
though it's looking for the one in obj-linux/config/.  If you remove prdepend.h
from both places, you'll get an error about not being able to find
../config/prdepend.h.
topsrcdir is the proper solution. MOD_DEPTH just happens to work because of the
way we use VPATH.

Comment 13

[Wan-Teh Chang]

I was also suspecting VPATH but I don't understand
how it works.  Thanks for looking into this and
the code review, cls.

I've checked in the patch (using topsrcdir instead
of MOD_DEPTH) on NSPRPUB_RELEASE_4_0_BRANCH.