Re: [PATCH 2/5] Makefile: use target-specific variable to pass flags to cc

[Jonathan Nieder <jrnieder@gmail.com>]

Jonathan Nieder wrote:

> diff --git a/Makefile b/Makefile
> index ba4d071..81190a6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1467,20 +1467,19 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>  strip: $(PROGRAMS) git$X
>  	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
>  
> -git.o: git.c common-cmds.h GIT-CFLAGS
> -	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
> -		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
> -		$(ALL_CFLAGS) -o $@ -c $(filter %.c,$^)
> +git.o: common-cmds.h
> +git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
> +	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
>  
[...]

One annoying feature I wasn't thinking of: the values of
target-specific variables propagate to the dependencies of a target
(why? I can't imagine), and GIT-CFLAGS keeps on changing because of
this.

Maybe a new CMD_CFLAGS variable is needed for this, i.e. something
like the following squashed in.

diff --git a/Makefile b/Makefile
index 2580e23..d20e456 100644
--- a/Makefile
+++ b/Makefile
@@ -1468,7 +1468,7 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: common-cmds.h
-git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
+git.o: CMD_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
@@ -1476,7 +1476,7 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
 
 builtin-help.o: common-cmds.h
-builtin-help.o: ALL_CFLAGS += \
+builtin-help.o: CMD_CFLAGS += \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 	'-DGIT_INFO_PATH="$(infodir_SQ)"'
@@ -1630,28 +1630,31 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+# This can vary by target
+CMD_CFLAGS = $(ALL_CFLAGS)
+
 %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(CMD_CFLAGS) $<
 %.s: %.c GIT-CFLAGS .FORCE-LISTING
-	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -S $(CMD_CFLAGS) $<
 %.o: %.S GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(CMD_CFLAGS) $<
 
-exec_cmd.o: ALL_CFLAGS += \
+exec_cmd.o: CMD_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
 	'-DPREFIX="$(prefix_SQ)"'
 
-builtin-init-db.o: ALL_CFLAGS += \
+builtin-init-db.o: CMD_CFLAGS += \
 	-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 
-config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+config.o: CMD_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
 
-http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
+http.o: CMD_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
 
 ifdef NO_EXPAT
 http-walker.o: http.h
-http-walker.o: ALL_CFLAGS += -DNO_EXPAT
+http-walker.o: CMD_CFLAGS += -DNO_EXPAT
 endif
 
 git-%$X: %.o $(GITLIBS)