[PATCH 7/9] sparse: Fix errors due to missing target-specific variables

[PATCH 7/9] sparse: Fix errors due to missing target-specific variables

[Ramsay Jones]

In particular, sparse issues the following errors:

    attr.c:472:43: error: undefined identifier 'ETC_GITATTRIBUTES'
    config.c:821:43: error: undefined identifier 'ETC_GITCONFIG'
    exec_cmd.c:14:37: error: undefined identifier 'PREFIX'
    exec_cmd.c:83:28: error: undefined identifier 'GIT_EXEC_PATH'
    builtin/help.c:328:46: error: undefined identifier 'GIT_MAN_PATH'
    builtin/help.c:374:40: error: undefined identifier 'GIT_INFO_PATH'
    builtin/help.c:382:45: error: undefined identifier 'GIT_HTML_PATH'
    git.c:96:42: error: undefined identifier 'GIT_HTML_PATH'
    git.c:241:35: error: invalid initializer
    http.c:293:43: error: undefined identifier 'GIT_HTTP_USER_AGENT'

which is caused by not passing the target-specific additions to
the EXTRA_CPPFLAGS variable to cgcc.

In order to fix the problem, we define a new sparse target which
depends on an a set of non-existent "sparse object" files (*.sp)
which correspond to the set of C source files. In addition to the
new target, we also provide a new pattern rule for "creating" the
sparse object files from the source files by running cgcc.  This
allows us to add '*.sp' to the rules setting the target-specific
EXTRA_CPPFLAGS variable, which is then included in the new pattern
rule to run cgcc.

Also, we change the 'check' target to re-direct the user to the
new sparse target.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Also, note the use of $(C_OBJ) rather than $(GIT_OBJS) when setting
the $(SP_OBJ) sparse objects; this leads to some additional C source
files being run through sparse. (Namely the files in the xdiff and
vcs-svn directories)

 Makefile |   40 +++++++++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 2a100b7..4b03b8a 100644
--- a/Makefile
+++ b/Makefile
@@ -1581,6 +1581,7 @@ ifndef V
 	QUIET_LNCP     = @echo '   ' LN/CP $@;
 	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
 	QUIET_GCOV     = @echo '   ' GCOV $@;
+	QUIET_SP       = @echo '   ' SP $<;
 	QUIET_SUBDIR0  = +@subdir=
 	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
 			 $(MAKE) $(PRINT_DIR) -C $$subdir
@@ -1676,7 +1677,7 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: common-cmds.h
-git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='"$(GIT_VERSION)"' \
+git.sp git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='"$(GIT_VERSION)"' \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
@@ -1686,7 +1687,7 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 help.o: common-cmds.h
 
 builtin/help.o: common-cmds.h
-builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
+builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 	'-DGIT_INFO_PATH="$(infodir_SQ)"'
@@ -1972,30 +1973,34 @@ $(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) \
 test-svn-fe.o: vcs-svn/svndump.h
 endif
 
-exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
+exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
 	'-DPREFIX="$(prefix_SQ)"'
 
-builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \
+builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \
 	-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 
-config.s config.o: EXTRA_CPPFLAGS = -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+config.sp config.s config.o: EXTRA_CPPFLAGS = \
+	-DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
 
-attr.s attr.o: EXTRA_CPPFLAGS = -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'
+attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
+	-DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'
 
-http.s http.o: EXTRA_CPPFLAGS = -DGIT_HTTP_USER_AGENT='"git/$(GIT_VERSION)"'
+http.sp http.s http.o: EXTRA_CPPFLAGS = \
+	-DGIT_HTTP_USER_AGENT='"git/$(GIT_VERSION)"'
 
 ifdef NO_EXPAT
-http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT
+http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT
 endif
 
 ifdef NO_REGEX
-compat/regex/regex.o: EXTRA_CPPFLAGS = -DGAWK -DNO_MBSUPPORT
+compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \
+	-DGAWK -DNO_MBSUPPORT
 endif
 
 ifdef USE_NED_ALLOCATOR
-compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \
+compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \
 	-DNDEBUG -DOVERRIDE_STRDUP -DREPLACE_SYSTEM_ALLOCATOR
 endif
 
@@ -2160,14 +2165,19 @@ test-%$X: test-%.o $(GITLIBS)
 check-sha1:: test-sha1$X
 	./test-sha1.sh
 
+SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))
+
+%.sp: %.c GIT-CFLAGS FORCE
+	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
+		$(SPARSE_FLAGS) $<
+
+sparse: $(SP_OBJ)
+
 check: common-cmds.h
 	@if sparse; \
 	then \
-		for i in $(patsubst %.o, %.c, $(GIT_OBJS)); \
-		do \
-			echo '   ' SP $$i; \
-			cgcc -no-compile $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; \
-		done; \
+		echo 2>&1 "Use 'make sparse' instead"; \
+		$(MAKE) --no-print-directory sparse; \
 	else \
 		echo 2>&1 "Did you mean 'make test'?"; \
 		exit 1; \
-- 
1.7.4

Re: [PATCH 7/9] sparse: Fix errors due to missing target-specific variables

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> In particular, sparse issues the following errors:
>
>     attr.c:472:43: error: undefined identifier 'ETC_GITATTRIBUTES'
>     config.c:821:43: error: undefined identifier 'ETC_GITCONFIG'
>     exec_cmd.c:14:37: error: undefined identifier 'PREFIX'
>     exec_cmd.c:83:28: error: undefined identifier 'GIT_EXEC_PATH'
>     builtin/help.c:328:46: error: undefined identifier 'GIT_MAN_PATH'
>     builtin/help.c:374:40: error: undefined identifier 'GIT_INFO_PATH'
>     builtin/help.c:382:45: error: undefined identifier 'GIT_HTML_PATH'
>     git.c:96:42: error: undefined identifier 'GIT_HTML_PATH'
>     git.c:241:35: error: invalid initializer
>     http.c:293:43: error: undefined identifier 'GIT_HTTP_USER_AGENT'
>
> which is caused by not passing the target-specific additions to
> the EXTRA_CPPFLAGS variable to cgcc.
>
> In order to fix the problem, we define a new sparse target which
> depends on an a set of non-existent "sparse object" files (*.sp)
> which correspond to the set of C source files. In addition to the
> new target, we also provide a new pattern rule for "creating" the
> sparse object files from the source files by running cgcc.  This
> allows us to add '*.sp' to the rules setting the target-specific
> EXTRA_CPPFLAGS variable, which is then included in the new pattern
> rule to run cgcc.

Hmm, shouldn't this be part of [1/9]?  I don't think I saw these with
"make check" before applying that patch.

Also shouldn't you be marking .sp suffix as .PHONY?

Re: [PATCH 7/9] sparse: Fix errors due to missing target-specific variables

[Ramsay Jones]

[Sorry for the late reply, I'm having ISP problems and real-life is
insanely busy right now...]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> In particular, sparse issues the following errors:
>>
>>     attr.c:472:43: error: undefined identifier 'ETC_GITATTRIBUTES'
>>     config.c:821:43: error: undefined identifier 'ETC_GITCONFIG'
>>     exec_cmd.c:14:37: error: undefined identifier 'PREFIX'
>>     exec_cmd.c:83:28: error: undefined identifier 'GIT_EXEC_PATH'
>>     builtin/help.c:328:46: error: undefined identifier 'GIT_MAN_PATH'
>>     builtin/help.c:374:40: error: undefined identifier 'GIT_INFO_PATH'
>>     builtin/help.c:382:45: error: undefined identifier 'GIT_HTML_PATH'
>>     git.c:96:42: error: undefined identifier 'GIT_HTML_PATH'
>>     git.c:241:35: error: invalid initializer
>>     http.c:293:43: error: undefined identifier 'GIT_HTTP_USER_AGENT'
>>
>> which is caused by not passing the target-specific additions to
>> the EXTRA_CPPFLAGS variable to cgcc.
>>
[...]
> 
> Hmm, shouldn't this be part of [1/9]?  I don't think I saw these with
> "make check" before applying that patch.

No, these errors were not introduced by patch #1; below I have added a
transcript of a session on Linux which shows that these errors were
present prior to patch #1. I can't say I'm surprised that you didn't
notice these errors; as you can see below, they are swamped by the
(for me) 4223 warnings and 161 errors.

Patch #1 is really about using "cgcc -no-compile", rather than sparse,
in order to fix the vast majority of warnings and errors; 4209 fewer
warnings and 148 fewer errors is not a bad return!

Patch #2 is about fixing up the target-specific additions in the
Makefile (ie, it's not a general sparse usage issue, but specific to
the git Makefile).

Having said that, I certainly don't mind squashing this in with patch #1
if that's what you would prefer. I have to make some additions to this
patch anyway; I forgot some "common-cmds.h" dependencies, thus:

--- >8 ---
diff --git a/Makefile b/Makefile
index 4b03b8a..4dee43f 100644
--- a/Makefile
+++ b/Makefile
@@ -1684,9 +1684,9 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
 		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
 
-help.o: common-cmds.h
+help.sp help.o: common-cmds.h
 
-builtin/help.o: common-cmds.h
+builtin/help.sp builtin/help.o: common-cmds.h
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
@@ -2171,7 +2171,7 @@ SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))
 	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
 		$(SPARSE_FLAGS) $<
 
-sparse: $(SP_OBJ)
+sparse: common-cmds.h $(SP_OBJ)
 
 check: common-cmds.h
 	@if sparse; \
--- >8 ---

NOTE: The Makefile has "git.o: common-cmds.h" (line 1679), but I don't
think git.c has depended on that header since commit 70827b15 (21-Apr-2006).

> Also shouldn't you be marking .sp suffix as .PHONY?

Hmmm, I didn't think so ... but I could easily be wrong!
Given that the .sp files never actually exist, and that the pattern rule
has a .PHONY dependency (FORCE), I didn't think marking $(SP_OBJ) as
phony would add anything.
Ah, well I suppose it would help if somebody did (say) "make git.sp 2>git.sp".
OK, I'll add that in the re-roll.

Let me know if you want me to squash this in with patch #1; I'll wait a while
before I re-roll.

Thanks!

ATB,
Ramsay Jones

$ git checkout 52d269da  # Stephen's "Makefile: cover more files" patch.
$ make check >/dev/null 2>sp-out
$ grep warning sp-out | wc -l
4223
$ grep error sp-out | wc -l
161
$ grep error sp-out | grep ETC_
attr.c:472:43: error: undefined identifier 'ETC_GITATTRIBUTES'
config.c:813:43: error: undefined identifier 'ETC_GITCONFIG'
$ grep error sp-out | grep PREFIX
exec_cmd.c:14:37: error: undefined identifier 'PREFIX'
$ grep error sp-out | grep GIT_
exec_cmd.c:83:28: error: undefined identifier 'GIT_EXEC_PATH'
builtin/help.c:328:46: error: undefined identifier 'GIT_MAN_PATH'
builtin/help.c:374:40: error: undefined identifier 'GIT_INFO_PATH'
builtin/help.c:382:45: error: undefined identifier 'GIT_HTML_PATH'
git.c:96:42: error: undefined identifier 'GIT_HTML_PATH'
http.c:293:43: error: undefined identifier 'GIT_HTTP_USER_AGENT'

$ git checkout 0343332f  # patch #1
$ make check >/dev/null 2>sp-out
$ grep warning sp-out | wc -l
14
$ grep error sp-out | wc -l
13
$ grep error sp-out | grep ETC_
attr.c:472:43: error: undefined identifier 'ETC_GITATTRIBUTES'
config.c:821:43: error: undefined identifier 'ETC_GITCONFIG'
$ grep error sp-out | grep PREFIX
exec_cmd.c:14:37: error: undefined identifier 'PREFIX'
$ grep error sp-out | grep GIT_
exec_cmd.c:83:28: error: undefined identifier 'GIT_EXEC_PATH'
builtin/help.c:328:46: error: undefined identifier 'GIT_MAN_PATH'
builtin/help.c:374:40: error: undefined identifier 'GIT_INFO_PATH'
builtin/help.c:382:45: error: undefined identifier 'GIT_HTML_PATH'
git.c:96:42: error: undefined identifier 'GIT_HTML_PATH'
http.c:293:43: error: undefined identifier 'GIT_HTTP_USER_AGENT'

$ git checkout 6f8a9fe3  # Patch #7
$ make check >/dev/null 2>sp-out
$ grep warning sp-out | wc -l
10
$ grep error sp-out | wc -l
3
$ grep error sp-out | grep ETC_
$ grep error sp-out | grep PREFIX
$ grep error sp-out | grep GIT_