automake-patches
[Top][All Lists]
Advanced

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

Re: automake less verbose (iter 3)


From: Jan Engelhardt
Subject: Re: automake less verbose (iter 3)
Date: Wed, 24 Dec 2008 23:47:36 +0100 (CET)
User-agent: Alpine 1.10 (LNX 962 2008-03-14)

On Monday 2008-12-22 22:28, Ralf Wildenhues wrote:
>[ from <http://thread.gmane.org/gmane.comp.sysutils.automake.general/9995> ]
>
>But it is still not POSIX compliant (neither 2001 nor 2008).

Sigh. There's a nice set of C functions that are all posixly
standard, but getting restricted so hard by the posixity of make..
I attest the POSIX committee a lack of attention towards 'make'.

>There are two possible ways to solve this:
>
>1) We add an Automake option to enable the new functionality, default
>off.
>
>2) We replace it with a functionality that is compliant.
>
>I haven't found another way yet.  If we can't find one, then your choice
>of $(var1$(var2)) is certainly better than nothing, in combination with
>(1).

I seem to remember that configure produces non-portable Makefiles
anyway, so (1) should be ok; specifically, running `configure` on
Linux and then issuing `make` on NetBSD, or vice versa, got me into
shell syntax issues or something in that direction.

>Luckily, we don't really need to warn here, but can just use a default
>value; I chose 'GEN'.

Perfect.

>> @@ -3941,7 +3985,8 @@ sub handle_configure ($$$@)
>>       'USE-DEPS'            => global_option 'no-dependencies'
>>                                  ? ' --ignore-deps' : '',
>>       'MAKEFILE-AM-SOURCES' => "$makefile$colon_infile",
>> -     'REGEN-ACLOCAL-M4'    => $regen_aclocal_m4);
>> +     'REGEN-ACLOCAL-M4'    => $regen_aclocal_m4,
>> +     VERBOSE               => '${am__verbose_GEN}');
>
>Hmm.  This has no corresponding change in lib/am/configure.am (yet).

Does it need one?

>> +    $output_vars .= join("\n",
>> +            'V = ${AUTOMAKE_VERBOSITY}',
>> +            'am__1libtool_silent_0 = --silent',
>> +            'am__1libtool_silent_1 = --silent',
>> +            'am__1verbose_CC_0     = @echo "  CC      " $@;',
>> +            'am__1verbose_CCLD_0   = @echo "  CCLD    " $@;',
>[...]
>
>Now, this is pretty ugly.  First, the variables could be nicely
>pretty-printed like the rest of the automake output.  Then, let's
>not have bloat: drop variable we don't need in this Makefile.in.
>And really it's much simpler to generate these variables along with
>the code that uses them.

I suppose you have the better solution at hand as the maintainer.

>> +            'am__1verbose_CC_1     = @echo "  CC      " $@ "<-" $<;',
>> +            'am__1verbose_CCLD_1   = @echo "  CCLD    " $@ "<-" $<;',
>> +            'am__1verbose_CXX_1    = @echo "  CXX     " $@ "<-" $<;',
>
>As hinted before, '$<' works only with GNU make.

Let's just nuke the V=1 variant then?

>Now, we get to some more meaty things:
>
>> --- automake-1.10.1.orig/lib/am/depend2.am
>> +++ automake-1.10.1/lib/am/depend2.am
>> @@ -65,12 +65,13 @@ if %?NONLIBTOOL%
>>  if %FASTDEP%
>>  ## 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% && \
>> -?GENERIC??!SUBDIROBJ?       %COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo 
>> %-c% -o %OBJ% %SOURCE% && \
>> -?GENERIC??SUBDIROBJ?        depbase=`echo %OBJ% | sed 
>> 's|[^/]*$$|$(DEPDIR)/&|;s|\.o$$||'`;\
>> +?!GENERIC?  %VERBOSE%%COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o 
>> %OBJ% `test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE% && \
>> +?GENERIC??!SUBDIROBJ?       %VERBOSE%%COMPILE% -MT %OBJ% -MD -MP -MF 
>> %DEPBASE%.Tpo %-c% -o %OBJ% %SOURCE% && \
>> +?GENERIC??SUBDIROBJ?        %VERBOSE%depbase=`echo %OBJ% | sed 
>> 's|[^/]*$$|$(DEPDIR)/&|;s|\.o$$||'`;\
>>  ?GENERIC??SUBDIROBJ?        %COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo 
>> %-c% -o %OBJ% %SOURCE% &&\
>>      mv -f %DEPBASE%.Tpo %DEPBASE%.Po
>
>Have you read the comment at the top of lib/am/depend2.am?
>Please read it.  There has been lots of discussion on this
>topic already, ignoring that will only get us back to those
>discussions.

Something about whether to hide stuff or not.
"They think @ commands are evil".
- So I believed that adding the runtime verbosity switch will satisfy them
"Those that produce diagnostics".
- they seem to have been left out anyway :)
"Want make -s to work as expected".
- I assume they can be pleased with make V=0.


>Another minor issue I don't quite like yet: before this change,
>the code was quite carefully laid out to be performant in the
>generic fastdep case: GNU make can avoid spawning a shell for
>a command, when the command line to be executed can be shown
>to be free of shell special variables.

Including ||, ``, \ and all those meta characters?

>  (For precise heuristics
>see the GNU make sources.)  Breaking this means one more shell
>fork per source file.

Well the question is whether this does happen, because %VERBOSE%
just adds
        @echo " CC " $@;
and this can entirely be handled by make alone, if its heuristics
are good.

>Not nice; however, I don't see a good way
>around this either, at least not in the silent case, and without
>introducing new newlines in the output in the non-silent case.

Well newlines can easily be avoided by getting rid of all the
continuation lines within an if block and writing the full command
lines out on every line. Yes, redundancy, but it's what it takes.

>In my attempt below I added a %SILENT% flag to get the 'mv' to be
>silent, at the cost of some added lines in the .am file, though.
>
>>  else !%FASTDEP%
>> +    %VERBOSE% @AMDEPBACKSLASH@
>
>Bzzt.  Untested change.  AMDEPBACKSLASH is not substituted in the
>non-AMDEP case, and tests/ansi3b.test and tests/fort4.test expose
>that, too.  What you meant here a literal backslash, except
>that you must hide it from the .am file parser here.

Argh. I bet that if the code for all the cells in the matrix
(fastdep-or-not  x  amdep-or-not) were written out, the
problems with adding backslashes or not would be gone, because
it's on a single line.

>> +AC_DEFUN([AM_VERBOSITY], [
>> +    AC_SUBST([AUTOMAKE_VERBOSITY], [$1])
>> +])
>
>This whole thing is still lacking all testing, documentation.  It still
>has the wrong default (needs to be opt-in, not opt-out),

It is an opt-out. If your configure.ac does not have AM_VERBOSITY
at all, you get the usual verbose behavior during make.

>and still
>causes a couple of test failures (I've tried to fix them in the patch
>below).  There is the question how users can adjust their own rules to
>fit (and we should have public macros/variables for that, not am__*
>ones).
>
>Probably in order to be real thorough about testing, the whole suite
>should be run once with V=1 and once without.  :-/

V=0, V= and V=lots

>There are a couple of sore points: I had to ease up the check within
>Variable::define.  I would like to fix this before committing.
>Also, I had to use the AM_BACKSLASH workaround; another "global"
>variable that should be unneeded.  :-/
>
>Good thing about this approach is that in the packages I tested,
>it increased Makefile.in size by 1% up to 7% only.  Feedback welcome.
>
>2008-12-22  Jan Engelhardt  <address@hidden>
>           Ralf Wildenhues  <address@hidden>
>
>        Implement silenced build rules.
>       * automake.in (Automake::Struct): New tag 'ccer'.
>       (verbose_var, verbose_flag, silent_flag)
>       (define_verbose_tagvar, define_verbose_libtool): New functions.
>       (handle_languages): Use them, for %VERBOSE% and %SILENT%.
>       (handle_programs, handle_libraries, handle_ltlibraries)
>       (handle_configure, define_compiler_variable)
>       (define_linker_variable, define_per_target_linker_variable):
>       Likewise, deal with verbose and silent flags.  Define the
>       respective expansion variables.
>       * lib/Automake/Variable.pm (Automake::Variable): Do not warn
>       about `$' in variable names.
>       * lib/am/depend2.am: Use %VERBOSE% and %SILENT% for Linux
>       kernel-like output when `V' is not set.
>       * lib/am/lex.am: Likewise.
>       * lib/am/library.am: Likewise.
>       * lib/am/ltlibrary.am: Likewise.
>       * lib/am/program.am: Likewise.
>       * lib/am/yacc.am: Likewise.
>       * m4/init.m4 (AM_INIT_AUTOMAKE) [AM_BACKSLASH]: New
>       substitution, not a make variable.
>       * m4/silent.m4 (AM_VERBOSITY): New file, new macro.
>       * m4/Makefile.am: Adjust.
>       * tests/pr300-ltlib.test: Adjust test.

I think it is wise to split this big commit - especially the changes
that are not strictly tranquility (silentness) -related, like the
ones to Variable::define / verbose_var / verbose_flag and the "ccer"
stuff, should be in separate commits preceding the tranquility patch.


Now testing the patch..




reply via email to

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