Re: Segmentation fault with latest git (070c57df)
On Thu, Jan 31, 2013 at 05:40:02PM -0800, Junio C Hamano wrote:
Show 8 quoted lines
> 허종만 <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
