Hi Ralf,
On Fri, Sep 25, 2009 at 5:02 AM, Ralf Wildenhues <[email protected]> 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 <[email protected]>
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
