threads / patch / 8379

patchMake the installation targets a little less chatty

Subject: [PATCH] Make the installation targets a little less chatty

## tl;dr

6 messages between May 31, 2007 and Jun 3, 2007. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Alex Riesen· May 31, 2007, 22:23 UTC · lore

by default. V=1 works as usual.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Now it quite quiet. I tried top show every installed file, but it wasn't an improvement at all so I decided to just show what's being done.

 Makefile           |   32 +++++++++++++++++++-------------
 templates/Makefile |    5 +++--
 2 files changed, 22 insertions(+), 15 deletions(-)

Show changes to 2 files +22 −15

Makefile, templates/Makefile

diff --git a/Makefile b/Makefile
index cac0a4a..fccc57b 100644
--- a/Makefile
+++ b/Makefile
@@ -660,6 +660,7 @@ ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
 endif
 
+QUIET=
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -671,6 +672,8 @@ endif
 
 ifneq ($(findstring $(MAKEFLAGS),s),s)
 ifndef V
+	QUIET = @
+
 	QUIET_CC       = @echo '   ' CC $@;
 	QUIET_AR       = @echo '   ' AR $@;
 	QUIET_LINK     = @echo '   ' LINK $@;
@@ -973,33 +976,36 @@ check: common-cmds.h
 ### Installation rules
 
 install: all
-	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
-	$(MAKE) -C perl prefix='$(prefix_SQ)' install
+	$(QUIET)$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(QUIET)$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	@echo installing programs
+	$(QUIET)$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	$(QUIET)$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
+	@echo setting up builtins
+	$(QUIET)$(foreach p,$(BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
+	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) DESTDIR='$(DESTDIR_SQ)' install
+	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) prefix='$(prefix_SQ)' install
 ifndef NO_TCLTK
-	$(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
-	$(MAKE) -C git-gui install
+	$(QUIET)$(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
+	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) install
 endif
-	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
+	@if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' || \
 		cp '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
 	fi
-	$(foreach p,$(BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
 ifneq (,$X)
-	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+	@echo cleaning '$(DESTDIR_SQ)$(gitexecdir_SQ)' of old scripts
+	$(QUIET)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
 endif
 
 install-doc:
-	$(MAKE) -C Documentation install
+	$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) install
 
 quick-install-doc:
-	$(MAKE) -C Documentation quick-install
+	$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) quick-install
 
 
 
diff --git a/templates/Makefile b/templates/Makefile
index b8352e7..b9a39e2 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -45,6 +45,7 @@ clean:
 	rm -rf blt boilerplates.made
 
 install: all
-	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(template_dir_SQ)'
-	(cd blt && $(TAR) cf - .) | \
+	@echo installing templates
+	$(QUIET)$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(template_dir_SQ)'
+	$(QUIET)(cd blt && $(TAR) cf - .) | \
 	(cd '$(DESTDIR_SQ)$(template_dir_SQ)' && $(TAR) xf -)

-- 
1.5.2.162.gbaa1f

Junio C Hamano· Jun 1, 2007, 00:12 UTC · re: Alex Riesen · lore

Re: [PATCH] Make the installation targets a little less chatty

Alex Riesen <raa.lkml@gmail.com> writes:

Show 8 quoted lines

> by default. V=1 works as usual.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
>
> Now it quite quiet. I tried top show every installed file, but it
> wasn't an improvement at all so I decided to just show what's being
> done.

I agree that the long single line that installs the commands to $(bindir) may be somewhat annoying.

Cleaning up compilation step is one thing; by tidying up the output it makes compiler warnings stand out.

But I do not like playing games like this in general in installation rule. An excerpt from your patch:

> +	@echo installing programs
> +	$(QUIET)$(foreach p,$(BUILT_INS), rm -f ...

This would not even let you see what got installed. At least, less verbose compilation step we have these days lets you see what is being built. I certainly would not object if the output would look like this, though:

	CC builtin-cat-file.o
        ...
        GEN git-add--interactive
        ...
        LINK git-http-push
        ...
        BUILTIN git-add
        ...
	MKDIR -p /usr/local/bin
        INSTALL git-foo
        INSTALL git-bar
	...
        INSTALL templates/description
        INSTALL templates/hooks/post-update
        ...

Alex Riesen· Jun 1, 2007, 07:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make the installation targets a little less chatty

On 6/1/07, Junio C Hamano <junkio@cox.net> wrote:

Show 12 quoted lines

> This would not even let you see what got installed.  At least,
> less verbose compilation step we have these days lets you see
> what is being built.  I certainly would not object if the output
> would look like this, though:
>
>         MKDIR -p /usr/local/bin
>         INSTALL git-foo
>         INSTALL git-bar
>         ...
>         INSTALL templates/description
>         INSTALL templates/hooks/post-update
>         ...

As I said, it did _not_ look like an improvement (I actually did that first). There is just too much information to be printed.

I'll send that first patch, with every program on its own line.

Alex Riesen· Jun 1, 2007, 23:09 UTC · re: Alex Riesen · lore

by default. V=1 works as usual.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 Makefile           |   35 ++++++++++++++++++++++-------------
 templates/Makefile |    5 +++--
 2 files changed, 25 insertions(+), 15 deletions(-)

Show changes to 2 files +25 −15

Makefile, templates/Makefile

diff --git a/Makefile b/Makefile
index cac0a4a..f2c9b39 100644
--- a/Makefile
+++ b/Makefile
@@ -660,8 +660,10 @@ ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
 endif
 
+QUIET=
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
+QUIET_MSG      = :
 
 ifneq ($(findstring $(MAKEFLAGS),w),w)
 PRINT_DIR = --no-print-directory
@@ -671,6 +673,9 @@ endif
 
 ifneq ($(findstring $(MAKEFLAGS),s),s)
 ifndef V
+	QUIET = @
+
+	QUIET_MSG      = echo  '   '
 	QUIET_CC       = @echo '   ' CC $@;
 	QUIET_AR       = @echo '   ' AR $@;
 	QUIET_LINK     = @echo '   ' LINK $@;
@@ -973,33 +978,37 @@ check: common-cmds.h
 ### Installation rules
 
 install: all
-	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
-	$(MAKE) -C perl prefix='$(prefix_SQ)' install
+	$(QUIET)$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(QUIET)$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	$(QUIET)$(foreach p,$(ALL_PROGRAMS),$(QUIET_MSG)INSTALL $p;\
+	    $(INSTALL) '$p' '$(DESTDIR_SQ)$(gitexecdir_SQ)';)
+	$(QUIET)$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(QUIET)$(foreach p,$(BUILT_INS)   ,$(QUIET_MSG)SETUP BUILTIN $p;\
+	    rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && \
+	    ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) DESTDIR='$(DESTDIR_SQ)' install
+	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) prefix='$(prefix_SQ)' install
 ifndef NO_TCLTK
-	$(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
-	$(MAKE) -C git-gui install
+	$(QUIET)$(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
+	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) install
 endif
-	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
+	@if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' || \
 		cp '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
 	fi
-	$(foreach p,$(BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
 ifneq (,$X)
-	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+	@echo cleaning '$(DESTDIR_SQ)$(gitexecdir_SQ)' of old scripts
+	$(QUIET)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
 endif
 
 install-doc:
-	$(MAKE) -C Documentation install
+	$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) install
 
 quick-install-doc:
-	$(MAKE) -C Documentation quick-install
+	$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) quick-install
 
 
 
diff --git a/templates/Makefile b/templates/Makefile
index b8352e7..b9a39e2 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -45,6 +45,7 @@ clean:
 	rm -rf blt boilerplates.made
 
 install: all
-	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(template_dir_SQ)'
-	(cd blt && $(TAR) cf - .) | \
+	@echo installing templates
+	$(QUIET)$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(template_dir_SQ)'
+	$(QUIET)(cd blt && $(TAR) cf - .) | \
 	(cd '$(DESTDIR_SQ)$(template_dir_SQ)' && $(TAR) xf -)

-- 
1.5.2.162.g8fc0

Junio C Hamano· Jun 2, 2007, 19:58 UTC · re: Alex Riesen · lore

Re: [PATCH] Make the installation targets a little less chatty

Alex Riesen <raa.lkml@gmail.com> writes:

> by default. V=1 works as usual.

You obviously did not test that X-<. V=1 does not work at all for me. This is what I get.

    ...
    install git '/home/junio/git-test/bin'
    :SETUP BUILTIN git-format-patch; rm -f '/home/junio/git-test/bin/git-format-patch' && ln '/home/junio/git-test/bin/git' '/home/junio/git-test/bin/git-format-patch'; ...
    ...
    /bin/sh: :SETUP: command not found
    /bin/sh: :SETUP: command not found
    /bin/sh: :SETUP: command not found
    /bin/sh: :SETUP: command not found
    ...

Two problems and a half.

 (0) You do not report the creation of target directory done
     with "install -d", so without $V there is no way to tell if
     things are installed in the expected location.

 (1) Obviously : and whatever action message needs to be
     separated so that it becomes a no-op command ':' followed
     by parameters;

 (2) Even with that ":SETUP: command not found" fixed, it now
     makes the command line twice as long as before for $V case,
     which makes it harder to diagnose a problem when Makefile
     goes wrong.

I think the way QUIET_CC and friends are done is quite sensible and does not have this problem, especially (3). With V=1 it just becomes transparent, and that is what we would want.

Alex Riesen· Jun 3, 2007, 11:43 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make the installation targets a little less chatty

Junio C Hamano, Sat, Jun 02, 2007 21:58:22 +0200:

> I think the way QUIET_CC and friends are done is quite sensible

I don't think it is possible to implement this way: QUIET_CC uses target-global $@, which the "install" target does not have.