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

[PATCH v2 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 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>
---

Junio,

I decided not to mark the $(SP_OBJ) as .PHONY targets; after some
testing, it seems that it is not necessary, even if users do
something like:
    make git.sp 2>git.sp
Do you know of any advantages to doing so that I have missed?

Changes since v1:
    - added missing "common-cmds.h" dependencies

ATB,
Ramsay Jones

 Makefile |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 385193a..57eacb5 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,17 +1677,17 @@ 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)
 	$(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.s builtin/help.o: EXTRA_CPPFLAGS = \
+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)"' \
 	'-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
 
@@ -2161,14 +2166,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: common-cmds.h $(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 v2 7/9] sparse: Fix errors due to missing target-specific variables

[Junio C Hamano]

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

> I decided not to mark the $(SP_OBJ) as .PHONY targets; after some
> testing, it seems that it is not necessary, even if users do
> something like:
>     make git.sp 2>git.sp
> Do you know of any advantages to doing so that I have missed?

The info documentation on GNU make states two reasons to use .PHONY.

One is to avoid existing file (e.g. git.sp in the above) to interfere
(especially when the target itself does not have a listed dependency), and
the other is that .PHONY tells make that it does not have to spend cycles
to consider implicit rules.

> -help.o: common-cmds.h
> +help.sp help.o: common-cmds.h

I am not sure if you even want any dependency listed for any %.sp target
to begin with. If sparse _were_ a normal build target, it would be sensible
to change the rule for sparse to create/update %.sp files, perhaps even
like this:

> +%.sp: %.c GIT-CFLAGS FORCE
	rm -f $@+ $@
> +	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
> +		$(SPARSE_FLAGS) $< 2>&1 >$@+
	mv $@+ $@

so that once you run sparse on help.c, unless you touch help.c or do
something to cause common-cmds.h to be updated, you do not have to
run the same thing again whose outcome you have already seen.

BUT. That is obviously not what we want.

In the case of sparse, you *do* want it to be run every time the user says
"make sparse", even when you already know you would get exactly the same
result from the previous run.

The situation is exactly the same as "make clean"; it runs even when it is
run immediately after another "make clean".

So why list any dependency?  If it is sensible to treat "sparse" target
just like "clean" target, it would make sense not to give it any
dependencies and mark it as .PHONY, no?

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

[Ramsay Jones]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> I decided not to mark the $(SP_OBJ) as .PHONY targets; after some
>> testing, it seems that it is not necessary, even if users do
>> something like:
>>     make git.sp 2>git.sp
[...]
>> -help.o: common-cmds.h
>> +help.sp help.o: common-cmds.h
> 
> I am not sure if you even want any dependency listed for any %.sp target
> to begin with. 

without these common-cmds.h dependencies, I get something like:

    $ make clean
        ...
    $ make help.sp
    GIT_VERSION = 1.7.5.rc2.10.g4d94
        * new build flags or prefix
        SP help.c
    help.c:6:10: error: unable to open 'common-cmds.h'
    make: *** [help.sp] Error 1

whereas, with them:

    $ make clean
        ...
    $ make help.sp
    GIT_VERSION = 1.7.5.rc2.10.g937d2
        * new build flags or prefix
        GEN common-cmds.h
        SP help.c

[...]
> So why list any dependency?  If it is sensible to treat "sparse" target
> just like "clean" target, it would make sense not to give it any
> dependencies and mark it as .PHONY, no?

After some experiments, looking through the output of "make -d ...", I think
I have a solution which (hopefully) addresses your concerns ...

Version 3 of the patch coming up ...

ATB,
Ramsay Jones

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

[Junio C Hamano]

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

>>> -help.o: common-cmds.h
>>> +help.sp help.o: common-cmds.h
>> 
>> I am not sure if you even want any dependency listed for any %.sp target
>> to begin with. 
>
> without these common-cmds.h dependencies, I get something like:

Ahh, of course.  I failed to spot that you are depending on a file that we
build ourselves.  Silly me.