{"thread":{"id":"27133","subject":"[PATCH v2 7/9] sparse: Fix errors due to missing target-specific variables","startedAt":"2011-04-19T17:32:25Z","lastAt":"2011-04-21T20:35:17Z","messageCount":4,"participants":["Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":9},"messages":[{"id":"166090","messageId":"4DADC729.5060705@ramsay1.demon.co.uk","threadId":"27133","inReplyTo":null,"subject":"[PATCH v2 7/9] sparse: Fix errors due to missing target-specific variables","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-04-19T17:32:25Z","receivedAt":"2011-04-19T17:32:25Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"In particular, sparse issues the following errors:\n\n    attr.c:472:43: error: undefined identifier 'ETC_GITATTRIBUTES'\n    config.c:821:43: error: undefined identifier 'ETC_GITCONFIG'\n    exec_cmd.c:14:37: error: undefined identifier 'PREFIX'\n    exec_cmd.c:83:28: error: undefined identifier 'GIT_EXEC_PATH'\n    builtin/help.c:328:46: error: undefined identifier 'GIT_MAN_PATH'\n    builtin/help.c:374:40: error: undefined identifier 'GIT_INFO_PATH'\n    builtin/help.c:382:45: error: undefined identifier 'GIT_HTML_PATH'\n    git.c:96:42: error: undefined identifier 'GIT_HTML_PATH'\n    git.c:241:35: error: invalid initializer\n    http.c:293:43: error: undefined identifier 'GIT_HTTP_USER_AGENT'\n\nwhich is caused by not passing the target-specific additions to\nthe EXTRA_CPPFLAGS variable to cgcc.\n\nIn order to fix the problem, we define a new sparse target which\ndepends on a set of non-existent \"sparse object\" files (*.sp)\nwhich correspond to the set of C source files. In addition to the\nnew target, we also provide a new pattern rule for \"creating\" the\nsparse object files from the source files by running cgcc.  This\nallows us to add '*.sp' to the rules setting the target-specific\nEXTRA_CPPFLAGS variable, which is then included in the new pattern\nrule to run cgcc.\n\nAlso, we change the 'check' target to re-direct the user to the\nnew sparse target.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n\nJunio,\n\nI decided not to mark the $(SP_OBJ) as .PHONY targets; after some\ntesting, it seems that it is not necessary, even if users do\nsomething like:\n    make git.sp 2>git.sp\nDo you know of any advantages to doing so that I have missed?\n\nChanges since v1:\n    - added missing \"common-cmds.h\" dependencies\n\nATB,\nRamsay Jones\n\n Makefile |   44 +++++++++++++++++++++++++++-----------------\n 1 files changed, 27 insertions(+), 17 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 385193a..57eacb5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1581,6 +1581,7 @@ ifndef V\n \tQUIET_LNCP     = @echo '   ' LN/CP $@;\n \tQUIET_XGETTEXT = @echo '   ' XGETTEXT $@;\n \tQUIET_GCOV     = @echo '   ' GCOV $@;\n+\tQUIET_SP       = @echo '   ' SP $<;\n \tQUIET_SUBDIR0  = +@subdir=\n \tQUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \\\n \t\t\t $(MAKE) $(PRINT_DIR) -C $$subdir\n@@ -1676,17 +1677,17 @@ strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n git.o: common-cmds.h\n-git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='\"$(GIT_VERSION)\"' \\\n+git.sp git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='\"$(GIT_VERSION)\"' \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"'\n \n git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n \t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n \n-help.o: common-cmds.h\n+help.sp help.o: common-cmds.h\n \n-builtin/help.o: common-cmds.h\n-builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n+builtin/help.sp builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n@@ -1972,30 +1973,34 @@ $(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) \\\n test-svn-fe.o: vcs-svn/svndump.h\n endif\n \n-exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n+exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n \t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n \t'-DPREFIX=\"$(prefix_SQ)\"'\n \n-builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n+builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n \t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n \n-config.s config.o: EXTRA_CPPFLAGS = -DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n+config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n+\t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n-attr.s attr.o: EXTRA_CPPFLAGS = -DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n+attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n+\t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n \n-http.s http.o: EXTRA_CPPFLAGS = -DGIT_HTTP_USER_AGENT='\"git/$(GIT_VERSION)\"'\n+http.sp http.s http.o: EXTRA_CPPFLAGS = \\\n+\t-DGIT_HTTP_USER_AGENT='\"git/$(GIT_VERSION)\"'\n \n ifdef NO_EXPAT\n-http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT\n+http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT\n endif\n \n ifdef NO_REGEX\n-compat/regex/regex.o: EXTRA_CPPFLAGS = -DGAWK -DNO_MBSUPPORT\n+compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \\\n+\t-DGAWK -DNO_MBSUPPORT\n endif\n \n ifdef USE_NED_ALLOCATOR\n-compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \\\n+compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \\\n \t-DNDEBUG -DOVERRIDE_STRDUP -DREPLACE_SYSTEM_ALLOCATOR\n endif\n \n@@ -2161,14 +2166,19 @@ test-%$X: test-%.o $(GITLIBS)\n check-sha1:: test-sha1$X\n \t./test-sha1.sh\n \n+SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n+\n+%.sp: %.c GIT-CFLAGS FORCE\n+\t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n+\t\t$(SPARSE_FLAGS) $<\n+\n+sparse: common-cmds.h $(SP_OBJ)\n+\n check: common-cmds.h\n \t@if sparse; \\\n \tthen \\\n-\t\tfor i in $(patsubst %.o, %.c, $(GIT_OBJS)); \\\n-\t\tdo \\\n-\t\t\techo '   ' SP $$i; \\\n-\t\t\tcgcc -no-compile $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; \\\n-\t\tdone; \\\n+\t\techo 2>&1 \"Use 'make sparse' instead\"; \\\n+\t\t$(MAKE) --no-print-directory sparse; \\\n \telse \\\n \t\techo 2>&1 \"Did you mean 'make test'?\"; \\\n \t\texit 1; \\\n-- \n1.7.4\n"},{"id":"166094","messageId":"7vbp025clq.fsf@alter.siamese.dyndns.org","threadId":"27133","inReplyTo":"4DADC729.5060705@ramsay1.demon.co.uk","subject":"Re: [PATCH v2 7/9] sparse: Fix errors due to missing target-specific variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-19T18:18:25Z","receivedAt":"2011-04-19T18:18:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n> I decided not to mark the $(SP_OBJ) as .PHONY targets; after some\n> testing, it seems that it is not necessary, even if users do\n> something like:\n>     make git.sp 2>git.sp\n> Do you know of any advantages to doing so that I have missed?\n\nThe info documentation on GNU make states two reasons to use .PHONY.\n\nOne is to avoid existing file (e.g. git.sp in the above) to interfere\n(especially when the target itself does not have a listed dependency), and\nthe other is that .PHONY tells make that it does not have to spend cycles\nto consider implicit rules.\n\n> -help.o: common-cmds.h\n> +help.sp help.o: common-cmds.h\n\nI am not sure if you even want any dependency listed for any %.sp target\nto begin with. If sparse _were_ a normal build target, it would be sensible\nto change the rule for sparse to create/update %.sp files, perhaps even\nlike this:\n\n> +%.sp: %.c GIT-CFLAGS FORCE\n\trm -f $@+ $@\n> +\t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n> +\t\t$(SPARSE_FLAGS) $< 2>&1 >$@+\n\tmv $@+ $@\n\nso that once you run sparse on help.c, unless you touch help.c or do\nsomething to cause common-cmds.h to be updated, you do not have to\nrun the same thing again whose outcome you have already seen.\n\nBUT. That is obviously not what we want.\n\nIn the case of sparse, you *do* want it to be run every time the user says\n\"make sparse\", even when you already know you would get exactly the same\nresult from the previous run.\n\nThe situation is exactly the same as \"make clean\"; it runs even when it is\nrun immediately after another \"make clean\".\n\nSo why list any dependency?  If it is sensible to treat \"sparse\" target\njust like \"clean\" target, it would make sense not to give it any\ndependencies and mark it as .PHONY, no?\n"},{"id":"166199","messageId":"4DB0804C.10908@ramsay1.demon.co.uk","threadId":"27133","inReplyTo":"7vbp025clq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 7/9] sparse: Fix errors due to missing target-specific variables","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-04-21T19:06:52Z","receivedAt":"2011-04-21T19:06:52Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n> \n>> I decided not to mark the $(SP_OBJ) as .PHONY targets; after some\n>> testing, it seems that it is not necessary, even if users do\n>> something like:\n>>     make git.sp 2>git.sp\n[...]\n>> -help.o: common-cmds.h\n>> +help.sp help.o: common-cmds.h\n> \n> I am not sure if you even want any dependency listed for any %.sp target\n> to begin with. \n\nwithout these common-cmds.h dependencies, I get something like:\n\n    $ make clean\n        ...\n    $ make help.sp\n    GIT_VERSION = 1.7.5.rc2.10.g4d94\n        * new build flags or prefix\n        SP help.c\n    help.c:6:10: error: unable to open 'common-cmds.h'\n    make: *** [help.sp] Error 1\n\nwhereas, with them:\n\n    $ make clean\n        ...\n    $ make help.sp\n    GIT_VERSION = 1.7.5.rc2.10.g937d2\n        * new build flags or prefix\n        GEN common-cmds.h\n        SP help.c\n\n[...]\n> So why list any dependency?  If it is sensible to treat \"sparse\" target\n> just like \"clean\" target, it would make sense not to give it any\n> dependencies and mark it as .PHONY, no?\n\nAfter some experiments, looking through the output of \"make -d ...\", I think\nI have a solution which (hopefully) addresses your concerns ...\n\nVersion 3 of the patch coming up ...\n\nATB,\nRamsay Jones\n"},{"id":"166203","messageId":"7vliz3xrzu.fsf@alter.siamese.dyndns.org","threadId":"27133","inReplyTo":"4DB0804C.10908@ramsay1.demon.co.uk","subject":"Re: [PATCH v2 7/9] sparse: Fix errors due to missing target-specific variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-21T20:35:17Z","receivedAt":"2011-04-21T20:35:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n>>> -help.o: common-cmds.h\n>>> +help.sp help.o: common-cmds.h\n>> \n>> I am not sure if you even want any dependency listed for any %.sp target\n>> to begin with. \n>\n> without these common-cmds.h dependencies, I get something like:\n\nAhh, of course.  I failed to spot that you are depending on a file that we\nbuild ourselves.  Silly me.\n"}]}