threads / patch / 27133

v2, 9 partssparse: Fix errors due to missing target-specific variables

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

## tl;dr

4 messages between Apr 19, 2011 and Apr 21, 2011. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Ramsay Jones· Apr 19, 2011, 17:32 UTC · lore
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(-)
Show changes to Makefile +27 −17
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
Junio C Hamano· Apr 19, 2011, 18:18 UTC · re: Ramsay Jones · lore

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

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
Show 5 quoted lines
> 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?

Ramsay Jones· Apr 21, 2011, 19:06 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 6 quoted lines
> 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
[...]
Show 5 quoted lines
>> -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

Junio C Hamano· Apr 21, 2011, 20:35 UTC · re: Ramsay Jones · lore

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

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
Show 7 quoted lines
>>> -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.

← back to recent threads