Re: make automake less verbose (try 2)

[Ralf Wildenhues]

Hi Jan,

* Jan Engelhardt wrote on Thu, Oct 23, 2008 at 07:07:52PM CEST:
> 
> There have been a number of attempts at introducing a quiet behavior to
> automake, much like the Linux kernel's kbuild does.

And this one looks quite a bit better than the last, I must say.
Thank you!

I will queue this to my things to look at in detail, might be a few days
until I get to it; but below are a few quick comments.  Please post
further patches on the automake-patches list, thanks.

> Signed-off-by: Jan Engelhardt <address@hidden>
> # All rights are handed to the automake team.

Well, as nice as that is, the FSF really needs either a copyright
assignment or a copyright disclaimer, signed on a piece of paper;
sorry.  Details off-list.

> Getting your verbose output back is as simple as using `make V=1`,
> or by setting V statically in your Makefile.am.

I am a wee bit wary of changing the default for both the Makefile.am
author (the "developer") and the configure user, without leaving the
default to be backward compatible.  Also, the
  ${variable${V}}

expansion is not Posix conforming.  Luckily, all make implementations
I tried to seem to understand it correctly though; Solaris 2.6, HP-UX
10.20, AIX 4.3.3, Tru64/OSF1 4.0D, even IRIX 6.5.  I wonder whether the
IRIX system I have access to has seen a system update recently, because
it was IRIX make that used to fail this.  It also works with parentheses
instead of braces.  For one-letter variables it should even be possible
to drop them, but notably IRIX make fails here:

UX:make: ERROR: Unmatched closing curly braces or parenthesis in line 
containing:
        ${bar$V} or address@hidden

I'm not sure either whether using $@ in a variable (as opposed to: in
a rule) is portable.  Posix doesn't seem to specify it, but I haven't
found issues anywhere; it seems fairly safe.

Anyway, when we use nonconforming constructs then it's probably safer if
they are default-off, so the developer can choose to enable it and knows
the limitation.  I suppose we can have an Automake option 'silent' or so
(better name suggestions appreciated), or maybe additionally a command
line argument if there is much desire for that.

A few more nits below.  FWIW, if you are willing to work on this more,
that would be great.  If you aren't, then no problem, I (or somebody
else) will get to it eventually.  (If you won't sign papers, then it's
in fact easier for us to fix things if you don't submit patches.)

Can you please state how and on what systems/packages/compilers etc. you
tested the patch, and whether you ran Automake's testsuite?  Thanks!

FWIW, the patch needs a test or two, documentation, a NEWS and a
ChangeLog entry.

> --- automake-1.10.1.orig/automake.in
> +++ automake-1.10.1/automake.in

> +                'verbose_link'    => '${AM_VERBOSE_CXXLD}',

(wondering whether to use am__verbose_CCXLD and likewise here)

How come you use ${...} everywhere, instead of $(...) as is done in the
rest of Automake?  See HACKING for some conventions, that, stupid as
they may be, at least bring a bit of consistency.

I wonder whether verbose_compile and verbose_link can be autogenerated
resp. mapped from compiler and lder, respectively.  Hmm.

> @@ -2410,6 +2442,15 @@ sub handle_libtool
>                                  LTRMS => join ("\n", @libtool_rms));
>  }
>  
> +sub find_link_verbose($)
> +{
> +     foreach my $lang_name (keys %languages) {
> +             if ($languages{$lang_name}->linker eq $_[0]) {
> +                     return $languages{$lang_name}->verbose_link;
> +             }
> +     }
> +}

That's a bit inefficient, looping over all languages each time.
Can't you do something like this?

  if (exists $languages{$lang_name})
    {
      return $languages{$lang_name}->verbose_link;
    }
  return undef;

> @@ -2761,7 +2806,9 @@ sub handle_ltlibraries
>                                            NONLIBTOOL => 0, LIBTOOL => 1);
>  
>        # Determine program to use for link.
> -      my $xlink = &define_per_target_linker_variable ($linker, $xlib);
> +      my($xlink, $vlink) = &define_per_target_linker_variable ($linker, 
> $xlib);
> +      $vlink ||= &find_link_verbose($xlink);
> +      $vlink ||= "";
>  
>        my $rpathvar = "am_${xlib}_rpath";
>        my $rpath = "\$($rpathvar)";


> +     # We are using an indirection via __AM_VERBOSE_* here so that
> +     # ${V} is not used in any files but automake.in itself,
> +     # especially avoiding the use of ${V} template files.
> +     #
> +     # GEN is for generate, which you can use for any manual rules.
> +     $output_vars .= join("\n",
> +             '__AM_LIBTOOL_SILENT = --silent',

Variables with leading underscores are not universally portable; the
naming convention in HACKING recommends am__ as prefix.

I think this code piece should use one of the define_*variable
functions, for nicely formatted output, and variables that are not used
in the Makefile.in should not need to be defined, to avoid bloat.

> +             '__AM_VERBOSE_CC     = @echo "  CC      " $@;',

Honest question: why the leading white space in the output?
All it does is take away screen space, no?  Or is the goal
bit-for-bit compatibility with Linux output?

[...]
> +             'AM_LIBTOOL_SILENT   = ${__AM_LIBTOOL_SILENT${V}}',


> --- automake-1.10.1.orig/lib/am/depend2.am
> +++ automake-1.10.1/lib/am/depend2.am
> @@ -63,6 +63,7 @@ if %?NONLIBTOOL%
>  ?GENERIC?%EXT%.o:
>  ?!GENERIC?%OBJ%: %SOURCE%
>  if %FASTDEP%
> +     %VERBOSE% \

For a default-off automake-time decision, the .am snippets changes would
need a ?SILENT? or so modifier (can also use ternary operators for bits
with finer than line granularity).

I'm not all that happy about the extra line of output in the V=1 case
here.  Probably better if the %VERBOSE% happens on the same line as the
compile command, even if that unfortunately means more duplication
inside depend2.am; like

?!GENERIC?      %SILENT?%VERBOSE% :%%COMPILE% -MT ...

Ah, there may be an even better alternative.  At least for the gcc3
fastdep path, we can just use another command line, with @ prefixed.
Since there 

>  ## In fast-dep mode, we can always use -o.
>  ## For non-suffix rules, we must emulate a VPATH search on %SOURCE%.
>  ?!GENERIC?   %COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJ% 
> `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE% && \
> @@ -71,6 +72,7 @@ if %FASTDEP%
>  ?GENERIC??SUBDIROBJ? %COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o 
> %OBJ% %SOURCE% &&\
>       mv -f %DEPBASE%.Tpo %DEPBASE%.Po

How come you didn't address the mv calls?  Should be possible with
something like
        am__at = @
        AM__AT = $(am__at$(V))

        $(AM__AT)mv -f %DEPBASE%.Tpo %DEPBASE%.Po


>  else !%FASTDEP%
> +     %VERBOSE% @AMDEPBACKSLASH@
>  if %AMDEP%
>       source='%SOURCE%' object='%OBJ%' libtool=no @AMDEPBACKSLASH@
>       DEPDIR=$(DEPDIR) $(%FPFX%DEPMODE) $(depcomp) @AMDEPBACKSLASH@

Cheers,
Ralf