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: Thu, 24 Sep 2009 21:02:51 +0200
User-agent: Mutt/1.5.20 (2009-08-09)

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

> +  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?
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.)

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

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

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




reply via email to

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