[PATCH] Makefile: add missing phony target

[PATCH] Makefile: add missing phony target

[Elia Pinto]

Add some missing phony target to Makefile. Also put the .PHONY
declaration immediately before the target declaration, where necessary,
for a better readability and a uniform style.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 Makefile | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 43ceeb9..a5fab53 100644
--- a/Makefile
+++ b/Makefile
@@ -522,11 +522,11 @@ SCRIPT_PYTHON_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PYTHON_GEN))
 # "make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script"
 # from subdirectories like contrib/*/
 .PHONY: build-perl-script build-sh-script build-python-script
+.PHONY: install-perl-script install-sh-script install-python-script
 build-perl-script: $(SCRIPT_PERL_GEN)
 build-sh-script: $(SCRIPT_SH_GEN)
 build-python-script: $(SCRIPT_PYTHON_GEN)
 
-.PHONY: install-perl-script install-sh-script install-python-script
 install-sh-script: $(SCRIPT_SH_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 install-perl-script: $(SCRIPT_PERL_INS)
@@ -534,7 +534,7 @@ install-perl-script: $(SCRIPT_PERL_INS)
 install-python-script: $(SCRIPT_PYTHON_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
-.PHONY: clean-perl-script clean-sh-script clean-python-script
+.PHONY: clean-sh-script clean-perl-script  clean-python-script
 clean-sh-script:
 	$(RM) $(SCRIPT_SH_GEN)
 clean-perl-script:
@@ -546,7 +546,6 @@ SCRIPTS = $(SCRIPT_SH_INS) \
 	  $(SCRIPT_PERL_INS) \
 	  $(SCRIPT_PYTHON_INS) \
 	  git-instaweb
-
 ETAGS_TARGET = TAGS
 
 # Empty...
@@ -1644,6 +1643,9 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $^
 
+.PHONY: all profile profile-fast
+.PHONY: please_set_SHELL_PATH_to_a_more_modern_shell shell_compatibility_test strip
+
 ### Target-specific flags and dependencies
 
 # The generic compilation pattern rule and automatically
@@ -1792,7 +1794,7 @@ GIT-PERL-DEFINES: FORCE
 	    fi
 
 
-.PHONY: gitweb
+.PHONY: gitweb git-instaweb
 gitweb:
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
 
@@ -2011,6 +2013,7 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
+.PHONY: doc man html info pdf
 doc:
 	$(MAKE) -C Documentation all
 
@@ -2054,6 +2057,7 @@ po/git.pot: $(GENERATED_H) FORCE
 		$(LOCALIZED_PERL)
 	mv $@+ $@
 
+.PHONY: pot
 pot: po/git.pot
 
 POFILES := $(wildcard po/*.po)
@@ -2074,6 +2078,7 @@ $(ETAGS_TARGET): FORCE
 	$(RM) $(ETAGS_TARGET)
 	$(FIND_SOURCE_FILES) | xargs etags -a -o $(ETAGS_TARGET)
 
+.PHONY: tags cscope FORCE
 tags: FORCE
 	$(RM) tags
 	$(FIND_SOURCE_FILES) | xargs ctags -a
@@ -2189,6 +2194,7 @@ export NO_SVN_TESTS
 export TEST_NO_MALLOC_CHECK
 
 ### Testing rules
+.PHONY: test perf
 
 test: all
 	$(MAKE) -C t/ all
@@ -2196,7 +2202,6 @@ test: all
 perf: all
 	$(MAKE) -C t/perf/ all
 
-.PHONY: test perf
 
 test-ctype$X: ctype.o
 
@@ -2215,6 +2220,7 @@ test-svn-fe$X: vcs-svn/lib.a
 test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
 
+.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1
 check-sha1:: test-sha1$X
 	./test-sha1.sh
 
@@ -2224,7 +2230,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
 	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
 		$(SPARSE_FLAGS) $<
 
-.PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
 check: common-cmds.h
@@ -2237,6 +2242,7 @@ check: common-cmds.h
 		exit 1; \
 	fi
 
+
 ### Installation rules
 
 ifneq ($(filter /%,$(firstword $(template_dir))),)
@@ -2263,6 +2269,7 @@ mergetools_instdir_SQ = $(subst ','\'',$(mergetools_instdir))
 
 install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)
 
+.PHONY: profile-install profile-fast-install install
 profile-install: profile
 	$(MAKE) install
 
@@ -2329,6 +2336,8 @@ endif
 	done && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
+.PHONY: install-gitweb install-man install-html install-info install-pdf
+.PHONY: quick-install-doc quick-install-man quick-install-html
 install-gitweb:
 	$(MAKE) -C gitweb install
 
@@ -2365,6 +2374,8 @@ git.spec: git.spec.in GIT-VERSION-FILE
 	mv $@+ $@
 
 GIT_TARNAME = git-$(GIT_VERSION)
+
+.PHONY: dist rpm dist-doc  distclean profile-clean clean
 dist: git.spec git-archive$(X) configure
 	./git-archive --format=tar \
 		--prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar
@@ -2445,9 +2456,6 @@ endif
 	$(RM) GIT-USER-AGENT GIT-PREFIX
 	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
 
-.PHONY: all install profile-clean clean strip
-.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-.PHONY: FORCE cscope
 
 ### Check documentation
 #
@@ -2499,9 +2507,9 @@ check-builtins::
 
 ### Test suite coverage testing
 #
-.PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
-.PHONY: coverage-clean-results
 
+.PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
+.PHONY: coverage-clean-results coverage-untested-functions cover_db cover_db_html
 coverage:
 	$(MAKE) coverage-test
 	$(MAKE) coverage-untested-functions
@@ -2543,4 +2551,3 @@ cover_db: coverage-report
 
 cover_db_html: cover_db
 	cover -report html -outputdir cover_db_html cover_db
-
-- 
2.5.0

Re: [PATCH] Makefile: add missing phony target

[Matthieu Moy]

Elia Pinto <gitter.spiros@gmail.com> writes:

> Also put the .PHONY
> declaration immediately before the target declaration, where necessary,
> for a better readability and a uniform style.
[...]
> --- a/Makefile
> +++ b/Makefile
> @@ -522,11 +522,11 @@ SCRIPT_PYTHON_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PYTHON_GEN))
>  # "make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script"
>  # from subdirectories like contrib/*/
>  .PHONY: build-perl-script build-sh-script build-python-script
> +.PHONY: install-perl-script install-sh-script install-python-script
>  build-perl-script: $(SCRIPT_PERL_GEN)
>  build-sh-script: $(SCRIPT_SH_GEN)
>  build-python-script: $(SCRIPT_PYTHON_GEN)
>  
> -.PHONY: install-perl-script install-sh-script install-python-script
>  install-sh-script: $(SCRIPT_SH_INS)
>  	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  install-perl-script: $(SCRIPT_PERL_INS)

Isn't this hunk doing exactly the opposite of what the commit message
says? install-sh-script's .PHONY used to be right before the target, and
it's now in the paragraph above.

> @@ -534,7 +534,7 @@ install-perl-script: $(SCRIPT_PERL_INS)
>  install-python-script: $(SCRIPT_PYTHON_INS)
>  	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  
> -.PHONY: clean-perl-script clean-sh-script clean-python-script
> +.PHONY: clean-sh-script clean-perl-script  clean-python-script

double-space befroe clean-python-script.

> @@ -546,7 +546,6 @@ SCRIPTS = $(SCRIPT_SH_INS) \
>  	  $(SCRIPT_PERL_INS) \
>  	  $(SCRIPT_PYTHON_INS) \
>  	  git-instaweb
> -
>  ETAGS_TARGET = TAGS

Is this needed/good?

> @@ -1792,7 +1794,7 @@ GIT-PERL-DEFINES: FORCE
>  	    fi
>  
>  
> -.PHONY: gitweb
> +.PHONY: gitweb git-instaweb

No: git-instaweb is not a .PHONY target, it's a real file generated from
git-instaweb.sh.

This bug in your patch is hard to spot in its current form where you mix
code movement and subtle changes. The patch cannot be reviewed in good
condition as-is.

> +.PHONY: tags cscope FORCE
>  tags: FORCE
>  	$(RM) tags
>  	$(FIND_SOURCE_FILES) | xargs ctags -a

If tags depends on FORCE, it doesn't need to be .PHONY. I strongly
prefer the old style, since tags is actually a file, and I prefer using
.PHONY for targets that are not meant to generate files, and depend on
FORCE where the rule actually generate a file named after its target,
but needs to be re-ran every time it's called.

If you disagree with this, then you need to justify the change in the
commit message.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/