Re: Segmentation fault with latest git (070c57df)

[Jeff King <peff@peff.net>]

On Thu, Jan 31, 2013 at 05:40:02PM -0800, Junio C Hamano wrote:

> 허종만 <jongman.heo@samsung.com> writes:
> 
> > But usually when I build upstream Linux kernel, I don't do "make
> > clean" after git pull..  I didn't expect that I needed "make
> > clean" for git build.
> 
> We don't expect anybody need "make clean", either.  There is
> something wrong in the dependency.

Agreed, but I cannot see what. If auto-header-dependencies is on, gcc
should find it (it is not even a recursive dependency for
builtin/fetch.c). And if it is not on, we should rebuild based on LIB_H,
which includes string-list.h (and always has, as far as I can tell).

Hmm. I do notice one oddity with the computed header dependencies,
though. We build the computed dependency files as a side effect of doing
the actual compilation. So before we have run the compilation once, we
need some way to say "you _must_ build this, because we do even know the
correct dependencies".  And to do that, we have each object file depend
on any missing .depend dirs, which bootstraps the whole process: we
build everything the first time because .depend is missing, and from
then on, we use the correct dependencies.

But that is not quite right. The .depend directory might exist, but be
missing the actual dependency file for a particular object. So if I do:

  $ make ;# builds all objects and dependency files
  $ rm builtin/.depend/fetch.o.d
  $ touch string-list.h
  $ make

we will fail to rebuild builtin/fetch.o properly. It does not see the
dependency on string-list (because we have no .d file), nor does it
realize that it needs to build the .d file (because it only checks that
builtin/.depend exists).

It seems like building each object file should depend on its dependency
file (but only when COMPUTE_HEADER_DEPENDENCIES is on, of course), since
otherwise we cannot know if we have the right dependencies or not.

Something like this almost works, I think:

diff --git a/Makefile b/Makefile
index 6b42f66..a329736 100644
--- a/Makefile
+++ b/Makefile
@@ -1843,8 +1843,8 @@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
 	@mkdir -p $@
 
 missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
-dep_file = $(dir $@).depend/$(notdir $@).d
-dep_args = -MF $(dep_file) -MMD -MP
+dep_file = $(dir $1).depend/$(notdir $1).d
+dep_args = -MF $(call dep_file, $@) -MMD -MP
 ifdef CHECK_HEADER_DEPENDENCIES
 $(error cannot compute header dependencies outside a normal build. \
 Please unset CHECK_HEADER_DEPENDENCIES and try again)
@@ -1909,9 +1909,9 @@ $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
 endif
 
 ifndef CHECK_HEADER_DEPENDENCIES
-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(call dep_file, %.o)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(call dep_file,%.o)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 endif
 

But not quite. The problem is that we put the dep file for foo/bar.o
into foo/.depend/bar.o. But when we call the dep_file function for the
dependency, it sees only "%.o", not "foo/bar.o", so it can't properly
split it apart. I don't think there is a way to force expansion before
calling the function.

And of course I have no idea if this was the problem that we saw,
anyway. I have no idea how one would get into this situation short of
manually removing the dependency file.

-Peff