{"thread":{"id":"40973","subject":"[PATCHv2] Makefile: add missing phony target","startedAt":"2015-12-11T14:22:46Z","lastAt":"2015-12-11T15:43:18Z","messageCount":4,"participants":["Elia Pinto","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"274272","messageId":"1449843766-41658-1-git-send-email-gitter.spiros@gmail.com","threadId":"40973","inReplyTo":null,"subject":"[PATCHv2] Makefile: add missing phony target","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-12-11T14:22:46Z","receivedAt":"2015-12-11T14:22:46Z","isPatch":false,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Add some missing phony target to Makefile. Also put the .PHONY\ndeclaration immediately before the target declaration, where necessary,\nfor a better readability and a uniform style.\n\nSigned-off-by: Elia Pinto <gitter.spiros@gmail.com>\nHelped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n---\nThis is the second version of this patch.\nAdded the corrections suggested by Matthieu Moy ($gmane/282221)\n\n Makefile | 25 +++++++++++++++++--------\n 1 file changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 43ceeb9..afd2358 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -534,7 +534,7 @@ install-perl-script: $(SCRIPT_PERL_INS)\n install-python-script: $(SCRIPT_PYTHON_INS)\n \t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \n-.PHONY: clean-perl-script clean-sh-script clean-python-script\n+.PHONY: clean-sh-script clean-perl-script clean-python-script\n clean-sh-script:\n \t$(RM) $(SCRIPT_SH_GEN)\n clean-perl-script:\n@@ -1644,6 +1644,9 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $^\n \n+.PHONY: all profile profile-fast\n+.PHONY: please_set_SHELL_PATH_to_a_more_modern_shell shell_compatibility_test strip\n+\n ### Target-specific flags and dependencies\n \n # The generic compilation pattern rule and automatically\n@@ -2011,6 +2014,7 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)\n \n export DEFAULT_EDITOR DEFAULT_PAGER\n \n+.PHONY: doc man html info pdf\n doc:\n \t$(MAKE) -C Documentation all\n \n@@ -2054,6 +2058,7 @@ po/git.pot: $(GENERATED_H) FORCE\n \t\t$(LOCALIZED_PERL)\n \tmv $@+ $@\n \n+.PHONY: pot\n pot: po/git.pot\n \n POFILES := $(wildcard po/*.po)\n@@ -2074,6 +2079,7 @@ $(ETAGS_TARGET): FORCE\n \t$(RM) $(ETAGS_TARGET)\n \t$(FIND_SOURCE_FILES) | xargs etags -a -o $(ETAGS_TARGET)\n \n+.PHONY: FORCE cscope\n tags: FORCE\n \t$(RM) tags\n \t$(FIND_SOURCE_FILES) | xargs ctags -a\n@@ -2189,6 +2195,7 @@ export NO_SVN_TESTS\n export TEST_NO_MALLOC_CHECK\n \n ### Testing rules\n+.PHONY: test perf\n \n test: all\n \t$(MAKE) -C t/ all\n@@ -2196,7 +2203,6 @@ test: all\n perf: all\n \t$(MAKE) -C t/perf/ all\n \n-.PHONY: test perf\n \n test-ctype$X: ctype.o\n \n@@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a\n test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n \n+.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1\n check-sha1:: test-sha1$X\n \t./test-sha1.sh\n \n@@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n \t\t$(SPARSE_FLAGS) $<\n \n-.PHONY: sparse $(SP_OBJ)\n sparse: $(SP_OBJ)\n \n check: common-cmds.h\n@@ -2237,6 +2243,7 @@ check: common-cmds.h\n \t\texit 1; \\\n \tfi\n \n+\n ### Installation rules\n \n ifneq ($(filter /%,$(firstword $(template_dir))),)\n@@ -2263,6 +2270,7 @@ mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n \n install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n \n+.PHONY: profile-install profile-fast-install install\n profile-install: profile\n \t$(MAKE) install\n \n@@ -2329,6 +2337,8 @@ endif\n \tdone && \\\n \t./check_bindir \"z$$bindir\" \"z$$execdir\" \"$$bindir/git-add$X\"\n \n+.PHONY: install-gitweb install-man install-html install-info install-pdf\n+.PHONY: quick-install-doc quick-install-man quick-install-html\n install-gitweb:\n \t$(MAKE) -C gitweb install\n \n@@ -2365,6 +2375,8 @@ git.spec: git.spec.in GIT-VERSION-FILE\n \tmv $@+ $@\n \n GIT_TARNAME = git-$(GIT_VERSION)\n+\n+.PHONY: dist rpm dist-doc  distclean profile-clean clean\n dist: git.spec git-archive$(X) configure\n \t./git-archive --format=tar \\\n \t\t--prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar\n@@ -2445,9 +2457,6 @@ endif\n \t$(RM) GIT-USER-AGENT GIT-PREFIX\n \t$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS\n \n-.PHONY: all install profile-clean clean strip\n-.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-.PHONY: FORCE cscope\n \n ### Check documentation\n #\n@@ -2499,9 +2508,9 @@ check-builtins::\n \n ### Test suite coverage testing\n #\n-.PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report\n-.PHONY: coverage-clean-results\n \n+.PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report\n+.PHONY: coverage-clean-results coverage-untested-functions cover_db cover_db_html\n coverage:\n \t$(MAKE) coverage-test\n \t$(MAKE) coverage-untested-functions\n-- \n2.5.0\n"},{"id":"274273","messageId":"vpq7fkl3v66.fsf@anie.imag.fr","threadId":"40973","inReplyTo":"1449843766-41658-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCHv2] Makefile: add missing phony target","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-12-11T14:40:49Z","receivedAt":"2015-12-11T14:40:49Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Elia Pinto <gitter.spiros@gmail.com> writes:\n\n> This is the second version of this patch.\n> Added the corrections suggested by Matthieu Moy ($gmane/282221)\n\nSorry, but my main concern was that the patch could not be reviewed in\ngood conditions as-is, and I think it still cannot be. It's very hard to\nspot which .PHONY rules you're adding and which are just code movement.\nYou should really split this into one \"code movement\" patch and one\n\"actual bugfix\" patch. Or someone with better eyes than me should review\nthe patch ;-).\n\n> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a\n>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\n>  \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n>  \n> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1\n>  check-sha1:: test-sha1$X\n>  \t./test-sha1.sh\n>  \n> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n>  \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n>  \t\t$(SPARSE_FLAGS) $<\n>  \n> -.PHONY: sparse $(SP_OBJ)\n>  sparse: $(SP_OBJ)\n\nThis \"sparse\" movement looks again contradictory with the goal announced\nin the commit message.\n\n> @@ -2237,6 +2243,7 @@ check: common-cmds.h\n>  \t\texit 1; \\\n>  \tfi\n>  \n> +\n>  ### Installation rules\n\nUseless hunk.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"274275","messageId":"CA+EOSB=a-qWP2-UfYJjPjSr8DD1ER39_rJPc8HAxV5qr+gw1qw@mail.gmail.com","threadId":"40973","inReplyTo":"vpq7fkl3v66.fsf@anie.imag.fr","subject":"Re: [PATCHv2] Makefile: add missing phony target","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-12-11T15:13:11Z","receivedAt":"2015-12-11T15:13:11Z","isPatch":false,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-12-11 15:40 GMT+01:00 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:\n> Elia Pinto <gitter.spiros@gmail.com> writes:\n>\n>> This is the second version of this patch.\n>> Added the corrections suggested by Matthieu Moy ($gmane/282221)\n>\n> Sorry, but my main concern was that the patch could not be reviewed in\n> good conditions as-is, and I think it still cannot be. It's very hard to\n> spot which .PHONY rules you're adding and which are just code movement.\n> You should really split this into one \"code movement\" patch and one\n> \"actual bugfix\" patch. Or someone with better eyes than me should review\n> the patch ;-).\n\nOk. No problem. I thought there was no need for a patch so simple. But ok.\n\nThank you. I will reroll.\n>\n>> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a\n>>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\n>>       $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n>>\n>> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1\n>>  check-sha1:: test-sha1$X\n>>       ./test-sha1.sh\n>>\n>> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n>>       $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n>>               $(SPARSE_FLAGS) $<\n>>\n>> -.PHONY: sparse $(SP_OBJ)\n>>  sparse: $(SP_OBJ)\n>\n> This \"sparse\" movement looks again contradictory with the goal announced\n> in the commit message.\nThe idea was to group all the phony before all the target, not to put\nthe phony necessarily before the closest target. but ok\n>\n>> @@ -2237,6 +2243,7 @@ check: common-cmds.h\n>>               exit 1; \\\n>>       fi\n>>\n>> +\nMy bad :=)\n>>  ### Installation rules\n>\n> Useless hunk.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n"},{"id":"274276","messageId":"vpqsi392dpl.fsf@anie.imag.fr","threadId":"40973","inReplyTo":"CA+EOSB=a-qWP2-UfYJjPjSr8DD1ER39_rJPc8HAxV5qr+gw1qw@mail.gmail.com","subject":"Re: [PATCHv2] Makefile: add missing phony target","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-12-11T15:43:18Z","receivedAt":"2015-12-11T15:43:18Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Elia Pinto <gitter.spiros@gmail.com> writes:\n\n> 2015-12-11 15:40 GMT+01:00 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:\n>> Elia Pinto <gitter.spiros@gmail.com> writes:\n>>\n>>> This is the second version of this patch.\n>>> Added the corrections suggested by Matthieu Moy ($gmane/282221)\n>>\n>> Sorry, but my main concern was that the patch could not be reviewed in\n>> good conditions as-is, and I think it still cannot be. It's very hard to\n>> spot which .PHONY rules you're adding and which are just code movement.\n>> You should really split this into one \"code movement\" patch and one\n>> \"actual bugfix\" patch. Or someone with better eyes than me should review\n>> the patch ;-).\n>\n> Ok. No problem. I thought there was no need for a patch so simple. But ok.\n\nThe point is: once a tricky bug was found in a patch (and I did on v1),\nyou cannot claim anymore that it is \"so simple\". If it was that simple,\nyou would have cought it before sending.\n\n>>> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a\n>>>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\n>>>       $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n>>>\n>>> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1\n>>>  check-sha1:: test-sha1$X\n>>>       ./test-sha1.sh\n>>>\n>>> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n>>>       $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n>>>               $(SPARSE_FLAGS) $<\n>>>\n>>> -.PHONY: sparse $(SP_OBJ)\n>>>  sparse: $(SP_OBJ)\n>>\n>> This \"sparse\" movement looks again contradictory with the goal announced\n>> in the commit message.\n> The idea was to group all the phony before all the target, not to put\n> the phony necessarily before the closest target. but ok\n\nI personally prefer the old way. I have no strong objection to changing,\nbut currently your commit message says \"Also put the .PHONY declaration\nimmediately before the target declaration\", which is clearly not a\njustification to do this change.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}