threads / patch / 33926

patch, 6 partsbuild: cleanups

Subject: [PATCH 0/6] build: cleanups

## tl;dr

8 messages between May 25, 2013 and May 26, 2013. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Felipe Contreras· May 25, 2013, 02:41 UTC · lore
Hi,

Here's a bunch of cleanups, after which it's trivial to avoid the installation of test scripts, namely git-remote-testpy.

Felipe Contreras (6):
  build: trivial simplification
  build: cleanup using $^
  build: cleanup using $<
  build: be clearer about order-only prerequisites
  build: add NO_INSTALL variable
  build: do not install git-remote-testpy
 Makefile | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)
-- 
1.8.3.rc3.312.g47657de
Felipe Contreras· May 25, 2013, 02:41 UTC · re: Felipe Contreras · lore

[PATCH 1/6] build: trivial simplification

SCRIPT_PYTHON_GEN is '$(patsubst %.py,%,$(SCRIPT_PYTHON))', so replace '$(patsubst %.py,%,$(SCRIPT_PYTHON))' with it

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to Makefile +3 −3
diff --git a/Makefile b/Makefile
index 985598b..2704df4 100644
--- a/Makefile
+++ b/Makefile
@@ -1822,8 +1822,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
 endif # NO_PERL
 
 ifndef NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
+$(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
+$(SCRIPT_PYTHON_GEN): % : %.py
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
 		--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \
@@ -1835,7 +1835,7 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
 	chmod +x $@+ && \
 	mv $@+ $@
 else # NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
+$(SCRIPT_PYTHON_GEN): % : unimplemented.sh
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \

-- 
1.8.3.rc3.312.g47657de
Felipe Contreras· May 25, 2013, 02:41 UTC · re: Felipe Contreras · lore

[PATCH 2/6] build: cleanup using $^

There's no need to list again the prerequisites. No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Show changes to Makefile +4 −4
diff --git a/Makefile b/Makefile
index 2704df4..1ac52dd 100644
--- a/Makefile
+++ b/Makefile
@@ -510,11 +510,11 @@ build-python-script: $(SCRIPT_PYTHON_GEN)
 
 .PHONY: install-perl-script install-sh-script install-python-script
 install-sh-script: $(SCRIPT_SH_GEN)
-	$(INSTALL) $(SCRIPT_SH_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 install-perl-script: $(SCRIPT_PERL_GEN)
-	$(INSTALL) $(SCRIPT_PERL_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 install-python-script: $(SCRIPT_PYTHON_GEN)
-	$(INSTALL) $(SCRIPT_PYTHON_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
 .PHONY: clean-perl-script clean-sh-script clean-python-script
 clean-sh-script:
@@ -1671,7 +1671,7 @@ please_set_SHELL_PATH_to_a_more_modern_shell:
 shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 
 strip: $(PROGRAMS) git$X
-	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
+	$(STRIP) $(STRIP_OPTS) $^
 
 ### Target-specific flags and dependencies
 

-- 
1.8.3.rc3.312.g47657de
Felipe Contreras· May 25, 2013, 02:41 UTC · re: Felipe Contreras · lore

[PATCH 3/6] build: cleanup using $<

No need to list the first prerequisite. No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Show changes to Makefile +5 −5
diff --git a/Makefile b/Makefile
index 1ac52dd..28b6117 100644
--- a/Makefile
+++ b/Makefile
@@ -1731,9 +1731,9 @@ version.sp version.s version.o: EXTRA_CPPFLAGS = \
 
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
-	ln git$X $@ 2>/dev/null || \
-	ln -s git$X $@ 2>/dev/null || \
-	cp git$X $@
+	ln $< $@ 2>/dev/null || \
+	ln -s $< $@ 2>/dev/null || \
+	cp $< $@
 
 common-cmds.h: ./generate-cmdlist.sh command-list.txt
 
@@ -1798,7 +1798,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
 	    -e '	x' \
 	    -e '}' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-	    $@.perl >$@+ && \
+	    $< >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
 
@@ -1831,7 +1831,7 @@ $(SCRIPT_PYTHON_GEN): % : %.py
 	sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
 	    -e 's|\(os\.getenv("GITPYTHONLIB"\)[^)]*)|\1,"@@INSTLIBDIR@@")|' \
 	    -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
-	    $@.py >$@+ && \
+	    $< >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
 else # NO_PYTHON

-- 
1.8.3.rc3.312.g47657de
Felipe Contreras· May 25, 2013, 02:41 UTC · re: Felipe Contreras · lore

[PATCH 4/6] build: be clearer about order-only prerequisites

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to Makefile +3 −3
diff --git a/Makefile b/Makefile
index 28b6117..97ff848 100644
--- a/Makefile
+++ b/Makefile
@@ -1787,7 +1787,7 @@ perl/PM.stamp: FORCE
 perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
 
-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl | GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
 	sed -e '1{' \
@@ -1850,7 +1850,7 @@ CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \
 		   autoconf -o configure configure.ac+ && \
 		   $(RM) configure.ac+
 
-configure: configure.ac GIT-VERSION-FILE
+configure: configure.ac | GIT-VERSION-FILE
 	$(QUIET_GEN)$(CONFIGURE_RECIPE)
 
 ifdef AUTOCONFIGURED
@@ -2415,7 +2415,7 @@ quick-install-html:
 
 ### Maintainer's dist rules
 
-git.spec: git.spec.in GIT-VERSION-FILE
+git.spec: git.spec.in | GIT-VERSION-FILE
 	sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
 	mv $@+ $@
 

-- 
1.8.3.rc3.312.g47657de
Junio C Hamano· May 26, 2013, 21:50 UTC · re: Felipe Contreras · lore

Re: [PATCH 4/6] build: be clearer about order-only prerequisites

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
This patch is wrong, I think.
The canonical example of order-only is something like
  http://www.kolpackov.net/pipermail/notes/2004-January/000001.html

where you want to make sure an output directory already exists, but you do not want to list that as prerequiste _input_ to produce output from the source. G-V-F has real information that is used to affect the output, which is quite a different animal.

Try applying this 6-patch series, "make clean; make git.spec", and then "git checkout HEAD~2" to come back to the commit for this patch. And then try running "make git.spec". I think Version line will not change and still claim you are building a version with the 6-patch series fully applied.

After a quick scan of the other 5 patches in the series, they looked all sensible, and I do not think any of them depends on the change in this patch, so I think we can safely drop this one.

Administrivia: as I said in the whats cooking message, I'll be
taking things slow til the beginning of next month, so if your (not
limited to Felipe) messages are not answered by me, it merely means
they were not yet read by me (as opposed to dismissed as not worth
reading without responding).
Show 33 quoted lines
>  Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 28b6117..97ff848 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1787,7 +1787,7 @@ perl/PM.stamp: FORCE
>  perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
>  	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
>  
> -$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
> +$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl | GIT-VERSION-FILE
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
>  	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
>  	sed -e '1{' \
> @@ -1850,7 +1850,7 @@ CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \
>  		   autoconf -o configure configure.ac+ && \
>  		   $(RM) configure.ac+
>  
> -configure: configure.ac GIT-VERSION-FILE
> +configure: configure.ac | GIT-VERSION-FILE
>  	$(QUIET_GEN)$(CONFIGURE_RECIPE)
>  
>  ifdef AUTOCONFIGURED
> @@ -2415,7 +2415,7 @@ quick-install-html:
>  
>  ### Maintainer's dist rules
>  
> -git.spec: git.spec.in GIT-VERSION-FILE
> +git.spec: git.spec.in | GIT-VERSION-FILE
>  	sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
>  	mv $@+ $@
Felipe Contreras· May 25, 2013, 02:41 UTC · re: Felipe Contreras · lore

[PATCH 5/6] build: add NO_INSTALL variable

So that we can specify which scripts we do not want to install (they are for testing).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
Show changes to Makefile +10 −6
diff --git a/Makefile b/Makefile
index 97ff848..333b5d3 100644
--- a/Makefile
+++ b/Makefile
@@ -500,6 +500,10 @@ SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))
 SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))
 SCRIPT_PYTHON_GEN = $(patsubst %.py,%,$(SCRIPT_PYTHON))
 
+SCRIPT_SH_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_SH_GEN))
+SCRIPT_PERL_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PERL_GEN))
+SCRIPT_PYTHON_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PYTHON_GEN))
+
 # Individual rules to allow e.g.
 # "make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script"
 # from subdirectories like contrib/*/
@@ -509,11 +513,11 @@ 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_GEN)
+install-sh-script: $(SCRIPT_SH_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-install-perl-script: $(SCRIPT_PERL_GEN)
+install-perl-script: $(SCRIPT_PERL_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-install-python-script: $(SCRIPT_PYTHON_GEN)
+install-python-script: $(SCRIPT_PYTHON_INS)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
 .PHONY: clean-perl-script clean-sh-script clean-python-script
@@ -524,9 +528,9 @@ clean-perl-script:
 clean-python-script:
 	$(RM) $(SCRIPT_PYTHON_GEN)
 
-SCRIPTS = $(SCRIPT_SH_GEN) \
-	  $(SCRIPT_PERL_GEN) \
-	  $(SCRIPT_PYTHON_GEN) \
+SCRIPTS = $(SCRIPT_SH_INS) \
+	  $(SCRIPT_PERL_INS) \
+	  $(SCRIPT_PYTHON_INS) \
 	  git-instaweb
 
 ETAGS_TARGET = TAGS

-- 
1.8.3.rc3.312.g47657de
Felipe Contreras· May 25, 2013, 02:41 UTC · re: Felipe Contreras · lore

[PATCH 6/6] build: do not install git-remote-testpy

It's only meant for testing.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)
Show changes to Makefile +2 −0
diff --git a/Makefile b/Makefile
index 333b5d3..ad70d45 100644
--- a/Makefile
+++ b/Makefile
@@ -495,6 +495,8 @@ SCRIPT_PERL += git-svn.perl
 SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
+NO_INSTALL += git-remote-testpy
+
 # Generated files for scripts
 SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))
 SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))

-- 
1.8.3.rc3.312.g47657de

← back to recent threads