automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: AM_SILENT_RULES doesn't silence texinfo rules


From: Ralf Wildenhues
Subject: Re: AM_SILENT_RULES doesn't silence texinfo rules
Date: Fri, 25 Sep 2009 21:22:49 +0200
User-agent: Mutt/1.5.20 (2009-08-09)

* Jack Kelly wrote on Fri, Sep 25, 2009 at 08:42:34AM CEST:
> On Fri, Sep 25, 2009 at 5:02 AM, Ralf Wildenhues wrote:
> > * Jack Kelly wrote on Wed, Sep 23, 2009 at 11:30:54AM CEST:
> >> +# 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.

Oops, you're right.  Thanks for not changing it in the patch.

> >> @@ -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.

Not in the patch; this is what caused the additional lines.  I've
squashed the diff below in your patch.

diff --git a/automake.in b/automake.in
index 04f52e4..bb03484 100755
--- a/automake.in
+++ b/automake.in
@@ -3571,12 +3571,12 @@ 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) = ('', '', '');
   if ($info_texinfos)
     {
+      define_verbose_texinfo;
       ($mostlyclean, $clean, $maintclean) = handle_texinfo_helper 
($info_texinfos);
       chomp $mostlyclean;
       chomp $clean;

> > 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?

Without the above, it did happen to me like this:
  add 'silent-rules' to the configure.ac of the Automake package
  ./bootstrap
  git diff Makefile.in

> >> --- a/lib/am/texibuild.am
> >> +++ b/lib/am/texibuild.am

> >> +     $(AM_V_MAKEINFO)
> >> +     $(AM_V_at)restore=: && backupdir="$(am__leading_dot)am$$$$" && \
> >
> > Two issues here (several instances below):
[...]
> Done.

Thanks, that looks good.

> >> +## 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.

Hmmyes; it does remove the trailing space, but the leading space to
%TEXIDEVNULL% doesn't get through to 'make'.  The reason is that
automake expands your
  define_verbose_var('texidevnull', ' > /dev/null');

to
  texidevnull =  > /dev/null

and any blanks right after '=' are eaten by make, so this ends up
running
  texi2pdf ... foo.texi> /dev/null

which might confuse users, even if it's syntactically ok.  I've reverted
that change for now, we have more problems with trailing spaces.

> 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.

MAKEINFO sounds cool.  The test looks good, apart from 'grep -q' which
is not portable.

I've merged your change to master now, including a followup change to use
silent-rules within Automake.  I'm still pondering whether to pull this
in branch-1.11, probably ok to do so, but I might wait a couple of weeks
in case there are regressions.

Thanks!
Ralf




reply via email to

[Prev in Thread] Current Thread [Next in Thread]