Re: [PATCH] Makefile: add missing phony target
From: Matthieu Moy <hidden>
Date: 2016-06-15 23:07:25
Elia Pinto [off-list ref] writes:
Also put the .PHONY declaration immediately before the target declaration, where necessary, for a better readability and a uniform style.
[...]
quoted hunk
--- 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.
quoted hunk
@@ -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.
quoted hunk
@@ -546,7 +546,6 @@ SCRIPTS = $(SCRIPT_SH_INS) \ $(SCRIPT_PERL_INS) \ $(SCRIPT_PYTHON_INS) \ git-instaweb - ETAGS_TARGET = TAGS
Is this needed/good?
quoted hunk
@@ -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/