Hi Ralf,

On Fri, Sep 25, 2009 at 5:02 AM, Ralf Wildenhues <ralf.wildenh...@gmx.de> wrote:
> Hi Jack,
>
> * Jack Kelly wrote on Wed, Sep 23, 2009 at 11:30:54AM CEST:
>> On Wed, Sep 23, 2009 at 2:02 PM, Ralf Wildenhues wrote:
>> > Uh, oh, more bikeshed color questions.  Let's try to find answers that
>> > follow some principle ...  ;-)
>> >
>> > -snip-
>> >
>> > What if you just give up the alignment of the target for tags longer
>> > than 6 characters?  Too ugly shed color?  Or we go to 8, that might
>> > still be tolerable, maybe let's see an example build.
>>
>> Sure. I added silent rules to my libfake437 Makefile.am and built it:
>
> [...]
>> If I drop it back to 6, it looks like this:
>
>> I personally prefer it with 8, but I'd rather get something I don't
>> like committed over a long bikeshed argument.
>
> Thanks.  8 is fine with me, too, after checking.
>
>> Revised patch attached. If you're happy with it, I'll cook up some
>> tests and we can try to get this committed soonish.
>
> Nits inline.  Nothing big.
>
>> --- a/automake.in
>> +++ b/automake.in
>> @@ -1199,11 +1199,25 @@ sub define_verbose_tagvar ($)
>>      my ($name) = @_;
>>      if (option 'silent-rules')
>>        {
>> -     define_verbose_var ($name, '@echo "  '. $name . ' ' x (6 - length 
>> ($name)) . '" $@;');
>> +     define_verbose_var ($name, '@echo "  '. $name . ' ' x (8 - length 
>> ($name)) . '" $@;');
>>       define_verbose_var ('at', '@');
>>        }
>>  }
>>
>> +# define_verbose_texinfo
>> +# ----------------------
>> +# Engage the needed `silent-rules' machinery for assorted texinfo commands.
>> +sub define_verbose_texinfo ()
>> +{
>
> Can we make the contents of this function optional to 'silent-rules'?

Done, but it doesn't make a difference. Everything it calls is a no-op
if silent-rules is off.

>
>> +  my @tagvars = ('DVIPS', 'MAKEINFO', 'INFOHTML', 'TEXI2DVI', 'TEXI2PDF');
>> +  foreach my $tag (@tagvars)
>> +    {
>> +      define_verbose_tagvar($tag);
>> +    }
>> +  define_verbose_var('texinfo', '-q');
>> +  define_verbose_var('texidevnull', '> /dev/null');
>> +}
>> +
>>  # define_verbose_libtool
>>  # ----------------------
>>  # Engage the needed `silent-rules' machinery for `libtool --silent'.
>> @@ -3248,6 +3262,8 @@ sub output_texinfo_build_rules ($$$@)
>>                                                      ? '$<' : $source),
>>                                 SOURCE_REAL      => $source,
>>                                 SOURCE_SUFFIX    => $ssfx,
>> +                                  TEXIQUIET        => 
>> verbose_flag('texinfo'),
>> +                                  TEXIDEVNULL      => 
>> verbose_flag('texidevnull'),
>>                                 );
>>    return ($dirstamp, "$dpfx.dvi", "$dpfx.pdf", "$dpfx.ps", "$dpfx.html");
>>  }
>> @@ -3551,6 +3567,7 @@ sub handle_texinfo ()
>>    reject_var 'TEXINFOS', "`TEXINFOS' is an anachronism; use 
>> `info_TEXINFOS'";
>>    # FIXME: I think this is an obsolete future feature name.
>>    reject_var 'html_TEXINFOS', "HTML generation not yet supported";
>> +  define_verbose_texinfo;
>
> Can we make this call optional to makefile files that contain texinfos?

Done.

> Otherwise, you end up with >20 unneeded lines in Makefile.in files.
> (Try adding silent-rules to the Automake package itself, ./bootstrap,
> and git diff.)

Didn't happen for me. Perhaps you're thinking of older code?

>>    my $info_texinfos = var ('info_TEXINFOS');
>>    my ($mostlyclean, $clean, $maintclean) = ('', '', '');
>> @@ -3567,7 +3584,8 @@ sub handle_texinfo ()
>>                                  MOSTLYCLEAN   => $mostlyclean,
>>                                  TEXICLEAN     => $clean,
>>                                  MAINTCLEAN    => $maintclean,
>> -                                'LOCAL-TEXIS' => !!$info_texinfos);
>> +                                'LOCAL-TEXIS' => !!$info_texinfos,
>> +                                   TEXIQUIET     => 
>> verbose_flag('texinfo'));
>>  }
>
>> --- a/lib/am/texibuild.am
>> +++ b/lib/am/texibuild.am
>> @@ -32,7 +32,8 @@
>>  ##    to fail, the info files are not removed.  (They are needed by the
>>  ##    developer while he writes documentation.)
>>  ## *.iNN files are used on DJGPP.  See the comments in install-info-am
>> -     restore=: && backupdir="$(am__leading_dot)am$$$$" && \
>> +     $(AM_V_MAKEINFO)
>> +     $(AM_V_at)restore=: && backupdir="$(am__leading_dot)am$$$$" && \
>
> Two issues here (several instances below):
>
> Completely empty lines in make rules are not portable (they provoke a
> warning from some make implementation), which is what $(AM_V_MAKEINFO)
> will expand to if silent-rules is disabled or V=1.  Instead, you can
> replace the first AM_V_at with AM_V_MAKEINFO and remove the preceding line
> (likewise, in the other instances, replace the first AM_V_at in a recipe).
>
> Second, so far I have been a bit keen on keeping Makefile.in files
> unchanged when 'silent-rules' was not used at all.  Arguably, this
> was mostly important for depend2.am, which can get expanded very
> often in large packages, leading to visibly larger file size.  I'm
> not so sure whether that is all that important for texinfo rules
> which are used less often, and after all, AM_V_* is in our namespace,
> so we don't need to fear that it's used for other purposes already.
>
> Anyway, the way to avoid this would be to replace $(AM_V_MAKEINFO)
> with %VERBOSE%, and any remaining instances of $(AM_V_at) with %SILENT%,
> and substituting those two in the respective %transform in automake.in,
> only if option 'silent-rules' is used, empty otherwise.

Done.

>>  ?INSRC?      am__cwd=`pwd` && $(am__cd) $(srcdir) && \
>>       rm -rf $$backupdir && mkdir $$backupdir && \
>>  ## If makeinfo is not installed we must not backup the files so
>> @@ -62,23 +63,29 @@ INFO_DEPS += %DEST_INFO_PREFIX%%DEST_SUFFIX%
>>
>>  ?GENERIC?%SOURCE_SUFFIX%.dvi:
>>  ?!GENERIC?%DEST_PREFIX%.dvi: %SOURCE% %DEPS% %DIRSTAMP%
>> -     TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
>> +     $(AM_V_TEXI2DVI)
>> +     
>> $(AM_V_at)TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
>>  ## Must set MAKEINFO like this so that version.texi will be found even
>>  ## if it is in srcdir (-I $(srcdir) is set in %MAKEINFOFLAGS%).
>>       MAKEINFO='$(MAKEINFO) $(AM_MAKEINFOFLAGS) $(MAKEINFOFLAGS) 
>> %MAKEINFOFLAGS%' \
>>  ## Do not use `-o' unless necessary: it is only supported since Texinfo 4.1.
>> -?GENERIC?    $(TEXI2DVI) %SOURCE%
>> -?!GENERIC?   $(TEXI2DVI) -o $@ `test -f '%SOURCE%' || echo 
>> '$(srcdir)/'`%SOURCE%
>> +## texi2dvi doesn't silence everything with -q, redirect to /dev/null 
>> instead.
>> +## We still want -q (%TEXIQUIET%) because it turns on batch mode.
>> +?GENERIC?    $(TEXI2DVI) %TEXIQUIET% %SOURCE% %TEXIDEVNULL%
>> +?!GENERIC?   $(TEXI2DVI) %TEXIQUIET% -o $@ `test -f '%SOURCE%' || echo 
>> '$(srcdir)/'`%SOURCE% %TEXIDEVNULL%
>
> This leads to trailing spaces in the non-silent-rules case.  Can we add
> a leading space to var('texidevnull') and remove the one before
> %TEXIDEVNULL% here (again, several instances)?
> (Don't bother with it if the solution is harder than that.)

Done.

>> -?GENERIC?    $(TEXI2PDF) %SOURCE%
>> -?!GENERIC?   $(TEXI2PDF) -o $@ `test -f '%SOURCE%' || echo 
>> '$(srcdir)/'`%SOURCE%
>> +## texi2pdf doesn't silence everything with -q, redirect to /dev/null 
>> instead.
>> +## We still want -q (%TEXIQUIET%) because it turns on batch mode.
>
> Thanks for documenting this!
>
>> +?GENERIC?    $(TEXI2PDF) %TEXIQUIET% %SOURCE% %TEXIDEVNULL%
>> +?!GENERIC?   $(TEXI2PDF) %TEXIQUIET% -o $@ `test -f '%SOURCE%' || echo 
>> '$(srcdir)/'`%SOURCE% %TEXIDEVNULL%
>>
>>  ?GENERIC?%SOURCE_SUFFIX%.html:
>>  ?!GENERIC?%DEST_PREFIX%.html: %SOURCE% %DEPS% %DIRSTAMP%
>
> Thanks,
> Ralf
>

I have also added a test: tests/silent8.test. Lastly, I changed the
tagline for the html target to MAKEINFO not INFOHTML, because MAKEINFO
is the name of the program. Besides, seeing `MAKEINFO foo.html' makes
it fairly clear what is going on. I can change this if you want.

-- Jack
From 837fd0c365ca31beae711792022438ed5501431e Mon Sep 17 00:00:00 2001
From: Jack Kelly <endgame....@gmail.com>
Date: Tue, 22 Sep 2009 12:24:04 +1000
Subject: [PATCH] Add silent rules support for texinfo outputs.

* automake.in (define_verbose_texinfo): Define several new verbose
tagvars and verbose vars.
* automake.in (define_verbose_tagvar): Increase spacing to 8 to
accommodate MAKEINFO, TEXI2DVI, TEXI2PDF.
* automake.in (handle_texinfo): Additional substitution for
silencing dvips.
* automake.in (output_texinfo_build_rules): Additional
substitutions for silencing texi2dvi and texi2pdf.
* lib/am/texibuild.am: Add silencing to makeinfo, makeinfo --html,
texi2dvi and texi2pdf rules.
* lib/am/texinfos.am: Add silencing to .dvi.ps rule.
* tests/Makefile.am: Add silent8.test.
* tests/silent8.test: New test: tests that silent texinfo rules
produce quiet messages.
---
 NEWS                |    4 +++
 automake.in         |   27 +++++++++++++++++++-
 lib/am/texibuild.am |   22 ++++++++++-------
 lib/am/texinfos.am  |    4 +-
 tests/Makefile.am   |    1 +
 tests/silent8.test  |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 110 insertions(+), 13 deletions(-)
 create mode 100755 tests/silent8.test

diff --git a/NEWS b/NEWS
index 7e14ed8..26a8b31 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,9 @@
 New in 1.11a:
 
+* Changes to automake:
+
+  - automake now generates silenced rules for texinfo outputs.
+
 Bugs fixed in 1.11a:
 
 * Bugs introduced by 1.11:
diff --git a/automake.in b/automake.in
index bab8c42..04f52e4 100755
--- a/automake.in
+++ b/automake.in
@@ -1199,11 +1199,25 @@ sub define_verbose_tagvar ($)
     my ($name) = @_;
     if (option 'silent-rules')
       {
-	define_verbose_var ($name, '@echo "  '. $name . ' ' x (6 - length ($name)) . '" $@;');
+	define_verbose_var ($name, '@echo "  '. $name . ' ' x (8 - length ($name)) . '" $@;');
 	define_verbose_var ('at', '@');
       }
 }
 
+# define_verbose_texinfo
+# ----------------------
+# Engage the needed `silent-rules' machinery for assorted texinfo commands.
+sub define_verbose_texinfo ()
+{
+  my @tagvars = ('DVIPS', 'MAKEINFO', 'INFOHTML', 'TEXI2DVI', 'TEXI2PDF');
+  foreach my $tag (@tagvars)
+    {
+      define_verbose_tagvar($tag);
+    }
+  define_verbose_var('texinfo', '-q');
+  define_verbose_var('texidevnull', ' > /dev/null');
+}
+
 # define_verbose_libtool
 # ----------------------
 # Engage the needed `silent-rules' machinery for `libtool --silent'.
@@ -3233,6 +3247,9 @@ sub output_texinfo_build_rules ($$$@)
 
   $output_rules .= file_contents ('texibuild',
 				  new Automake::Location,
+                                  AM_V_MAKEINFO    => verbose_flag('MAKEINFO'),
+                                  AM_V_TEXI2DVI    => verbose_flag('TEXI2DVI'),
+                                  AM_V_TEXI2PDF    => verbose_flag('TEXI2PDF'),
 				  DEPS             => "@deps",
 				  DEST_PREFIX      => $dpfx,
 				  DEST_INFO_PREFIX => $dipfx,
@@ -3242,12 +3259,15 @@ sub output_texinfo_build_rules ($$$@)
 				  GENERIC_INFO     => $generic_info,
 				  INSRC		   => $insrc,
 				  MAKEINFOFLAGS    => $makeinfoflags,
+                                  SILENT           => silent_flag(),
 				  SOURCE           => ($generic
 						       ? '$<' : $source),
 				  SOURCE_INFO      => ($generic_info
 						       ? '$<' : $source),
 				  SOURCE_REAL      => $source,
 				  SOURCE_SUFFIX    => $ssfx,
+                                  TEXIQUIET        => verbose_flag('texinfo'),
+                                  TEXIDEVNULL      => verbose_flag('texidevnull'),
 				  );
   return ($dirstamp, "$dpfx.dvi", "$dpfx.pdf", "$dpfx.ps", "$dpfx.html");
 }
@@ -3551,6 +3571,7 @@ sub handle_texinfo ()
   reject_var 'TEXINFOS', "`TEXINFOS' is an anachronism; use `info_TEXINFOS'";
   # FIXME: I think this is an obsolete future feature name.
   reject_var 'html_TEXINFOS', "HTML generation not yet supported";
+  define_verbose_texinfo;
 
   my $info_texinfos = var ('info_TEXINFOS');
   my ($mostlyclean, $clean, $maintclean) = ('', '', '');
@@ -3564,10 +3585,12 @@ sub handle_texinfo ()
 
   $output_rules .=  file_contents ('texinfos',
 				   new Automake::Location,
+                                   AM_V_DVIPS    => verbose_flag('DVIPS'),
 				   MOSTLYCLEAN   => $mostlyclean,
 				   TEXICLEAN     => $clean,
 				   MAINTCLEAN    => $maintclean,
-				   'LOCAL-TEXIS' => !!$info_texinfos);
+				   'LOCAL-TEXIS' => !!$info_texinfos,
+                                   TEXIQUIET     => verbose_flag('texinfo'));
 }
 
 
diff --git a/lib/am/texibuild.am b/lib/am/texibuild.am
index dca9ce1..eba9542 100644
--- a/lib/am/texibuild.am
+++ b/lib/am/texibuild.am
@@ -32,7 +32,7 @@
 ##    to fail, the info files are not removed.  (They are needed by the
 ##    developer while he writes documentation.)
 ## *.iNN files are used on DJGPP.  See the comments in install-info-am
-	restore=: && backupdir="$(am__leading_dot)am$$$$" && \
+	%AM_V_MAKEINFO%restore=: && backupdir="$(am__leading_dot)am$$$$" && \
 ?INSRC?	am__cwd=`pwd` && $(am__cd) $(srcdir) && \
 	rm -rf $$backupdir && mkdir $$backupdir && \
 ## If makeinfo is not installed we must not backup the files so
@@ -62,23 +62,27 @@ INFO_DEPS += %DEST_INFO_PREFIX%%DEST_SUFFIX%
 
 ?GENERIC?%SOURCE_SUFFIX%.dvi:
 ?!GENERIC?%DEST_PREFIX%.dvi: %SOURCE% %DEPS% %DIRSTAMP%
-	TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
+	%AM_V_TEXI2DVI%TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
 ## Must set MAKEINFO like this so that version.texi will be found even
 ## if it is in srcdir (-I $(srcdir) is set in %MAKEINFOFLAGS%).
 	MAKEINFO='$(MAKEINFO) $(AM_MAKEINFOFLAGS) $(MAKEINFOFLAGS) %MAKEINFOFLAGS%' \
 ## Do not use `-o' unless necessary: it is only supported since Texinfo 4.1.
-?GENERIC?	$(TEXI2DVI) %SOURCE%
-?!GENERIC?	$(TEXI2DVI) -o $@ `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE%
+## texi2dvi doesn't silence everything with -q, redirect to /dev/null instead.
+## We still want -q (%TEXIQUIET%) because it turns on batch mode.
+?GENERIC?	$(TEXI2DVI) %TEXIQUIET% %SOURCE%%TEXIDEVNULL%
+?!GENERIC?	$(TEXI2DVI) %TEXIQUIET% -o $@ `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE%%TEXIDEVNULL%
 
 ?GENERIC?%SOURCE_SUFFIX%.pdf:
 ?!GENERIC?%DEST_PREFIX%.pdf: %SOURCE% %DEPS% %DIRSTAMP%
-	TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
+	%AM_V_TEXI2PDF%TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
 ## Must set MAKEINFO like this so that version.texi will be found even
 ## if it is in srcdir (-I $(srcdir) is set in %MAKEINFOFLAGS%).
 	MAKEINFO='$(MAKEINFO) $(AM_MAKEINFOFLAGS) $(MAKEINFOFLAGS) %MAKEINFOFLAGS%' \
 ## Do not use `-o' unless necessary: it is only supported since Texinfo 4.1.
-?GENERIC?	$(TEXI2PDF) %SOURCE%
-?!GENERIC?	$(TEXI2PDF) -o $@ `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE%
+## texi2pdf doesn't silence everything with -q, redirect to /dev/null instead.
+## We still want -q (%TEXIQUIET%) because it turns on batch mode.
+?GENERIC?	$(TEXI2PDF) %TEXIQUIET% %SOURCE%%TEXIDEVNULL%
+?!GENERIC?	$(TEXI2PDF) %TEXIQUIET% -o $@ `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE%%TEXIDEVNULL%
 
 ?GENERIC?%SOURCE_SUFFIX%.html:
 ?!GENERIC?%DEST_PREFIX%.html: %SOURCE% %DEPS% %DIRSTAMP%
@@ -88,8 +92,8 @@ INFO_DEPS += %DEST_INFO_PREFIX%%DEST_SUFFIX%
 ## in the manual change, it may leave unused pages.  Our fix
 ## is to build under a temporary name, and replace the target on
 ## success.
-	rm -rf $(@:.html=.htp)
-	if $(MAKEINFOHTML) $(AM_MAKEINFOHTMLFLAGS) $(MAKEINFOFLAGS) %MAKEINFOFLAGS% \
+	%AM_V_MAKEINFO%rm -rf $(@:.html=.htp)
+	%SILENT%if $(MAKEINFOHTML) $(AM_MAKEINFOHTMLFLAGS) $(MAKEINFOFLAGS) %MAKEINFOFLAGS% \
 ?GENERIC?	 -o $(@:.html=.htp) %SOURCE%; \
 ?!GENERIC?	 -o $(@:.html=.htp) `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE%; \
 	then \
diff --git a/lib/am/texinfos.am b/lib/am/texinfos.am
index c9dcd9d..bfde665 100644
--- a/lib/am/texinfos.am
+++ b/lib/am/texinfos.am
@@ -55,8 +55,8 @@ endif %?LOCAL-TEXIS%
 if %?LOCAL-TEXIS%
 DVIPS = dvips
 .dvi.ps:
-	TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
-	$(DVIPS) -o $@ $<
+	%AM_V_DVIPS%TEXINPUTS="$(am__TEXINFO_TEX_DIR)$(PATH_SEPARATOR)$$TEXINPUTS" \
+	$(DVIPS) %TEXIQUIET% -o $@ $<
 endif %?LOCAL-TEXIS%
 
 .PHONY: dvi dvi-am html html-am info info-am pdf pdf-am ps ps-am
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 62529a6..3ed0cab 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -583,6 +583,7 @@ silent4.test \
 silent5.test \
 silent6.test \
 silent7.test \
+silent8.test \
 sinclude.test \
 srcsub.test \
 srcsub2.test \
diff --git a/tests/silent8.test b/tests/silent8.test
new file mode 100755
index 0000000..f416bf7
--- /dev/null
+++ b/tests/silent8.test
@@ -0,0 +1,65 @@
+#!/bin/sh
+# Copyright (C) 2009  Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check texinfo rules in silent-rules mode.
+
+. ./defs
+
+set -e
+
+cat >>configure.in <<'EOF'
+AM_SILENT_RULES
+AC_OUTPUT
+EOF
+
+cat > Makefile.am <<'EOF'
+info_TEXINFOS = foo.texi
+EOF
+
+cat > foo.texi <<'EOF'
+\input texinfo
+...@c %**start of header
+...@setfilename foo.info
+...@settitle foo manual
+...@c %**end of header
+...@bye
+EOF
+
+$ACLOCAL
+$AUTOMAKE --add-missing
+$AUTOCONF
+
+./configure --disable-silent-rules
+
+# Make sure that all labels work in silent-mode
+$MAKE V=0 dvi html info ps pdf>stdout || { cat stdout; Exit 1; }
+cat stdout
+grep -q 'DVIPS    foo.ps' stdout || Exit 1
+grep -q 'MAKEINFO foo.html' stdout || Exit 1
+grep -q 'MAKEINFO foo.info' stdout || Exit 1
+grep -q 'TEXI2DVI foo.dvi' stdout || Exit 1
+grep -q 'TEXI2PDF foo.pdf' stdout || Exit 1
+
+# Now make sure the labels don't appear in verbose mode.
+$MAKE clean || Exit 1
+$MAKE V=1 dvi html info ps pdf>stdout || { cat stdout; Exit 1; }
+grep -q 'DVIPS    foo.ps' stdout && Exit 1
+grep -q 'MAKEINFO foo.html' stdout && Exit 1
+grep -q 'MAKEINFO foo.info' stdout && Exit 1
+grep -q 'TEXI2DVI foo.dvi' stdout && Exit 1
+grep -q 'TEXI2PDF foo.pdf' stdout && Exit 1
+
+:
-- 
1.6.0.4

Reply via email to