automake
[Top][All Lists]
Advanced

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

Re: amtraces


From: Derek R. Price
Subject: Re: amtraces
Date: Sun, 04 Feb 2001 03:36:43 -0500

Akim Demaille wrote:

> "Derek R. Price" <address@hidden> writes:
>
> All these comments are related to the same idea: Automake must know as
> less as possible about macros.  It means that if needed, we have to
>
> ~/src/ace % echo "AC_INIT AC_CANONICAL_SYSTEM" | ace -t AC_CANONICAL_SYSTEM 
> -t AC_CANONICAL_HOST -
> /tmp/ac23991/stdin:1:AC_CANONICAL_SYSTEM:
> /tmp/ac23991/stdin:1:AC_CANONICAL_HOST:
>
> i.e., drop AC_CANONICAL_SYSTEM *dead*.

Are you sure?  I poked through the code a bit and it looks like 
AC_CANONICAL_SYSTEM is now an alias
for AC_CANONICAL_TARGET which requires AC_CANONICAL_HOST but sets some extra 
variables which AM seems
to have handlers for as part of its AC_CANONICAL_SYSTEM handling...


> > +    # Some things required by Automake.
> > +    AC_ARG_PROGRAM => sub { $seen_arg_prog = $_[0] },
>
> Hm, I'm in favor of having AC_ARG_PROGRAM always run.  I see no use in
> having only partial support for this option across configures.  In
> addition, AM_INIT_AUTOMAKE, IIRC, calls it by itself.
>
> Pavel, Alexandre, any problem with integrating AC_ARG_PROGRAM in AC_INIT?

>
> > +    AM_C_PROTOTYPES => sub { $am_c_prototypes = $_[0] },
>
> Should be moved to an Autoconf macro.
>
> > +    AC_CHECK_TOOL => \&scan_autoconf_traces_AC_CANONICAL_HOST,
>
> Sounds wrong: you don't need AC_CANONICAL_HOST to use AC_CHECK_TOOL.
>
> > +    AM_CONDITIONAL => sub { $configure_cond{$_[2]} = $_[0] },
> > +    AC_CONFIG_AUX_DIR => sub { @config_aux_path = $_[2] },
>
> This macro gives too many problem.  Alexandre D. knows what I'm
> referring to, I'd like him to start a thread in autoconf@ about this.

Let me know when there's a fix I can use.  My basic premise was to convert 
scan_one_autoconf_file as
directly as possible while removing the redundancies I could spot.


> > +    AC_CONFIG_FILES => sub { &scan_autoconf_config_files ($_[2]) },
> > +    # Handle configuration headers
> > +    AC_CONFIG_HEADER => \&scan_autoconf_traces_AC_CONFIG_HEADER,
> > +    AC_CONFIG_HEADERS => \&scan_autoconf_traces_AC_CONFIG_HEADER,
> > +    AM_CONFIG_HEADER => \&scan_autoconf_traces_AM_CONFIG_HEADER,
>
> Nope, they all point to only AC_CONFIG_HEADERS.  Don't trace the others.

This works for AC_CONFIG_HEADERS, but not for AM_CONFIG_HEADER.  AM requires 
AM_CONFIG_HEADERS to be
used so it needs to catch the call to AC_CONFIG_HEADER so it can warn the user 
about their mistake.


> > +    AC_DECL_YYTEXT =>
> > +     sub { unless ($seen_decl_yytext eq $_[0])
> > +           {
> > +             $seen_decl_yytext = $_[0];
> > +             &am_conf_line_warning (
> > +                 split (/:/, $_[0]),
> > +                 "\`AC_DECL_YYTEXT' is covered by \`AM_PROG_LEX'");
> > +           }
> > +         },
>
> No longer exists, there is only AC_PROG_LEX which includes this.
> AM_PROG_LEX is deprecated.

I added obsolete and deprecated warnings for AC_DECL_YYTEXT and AM_PROG_LEX, 
respectively.
AC_PROG_LEX is still setting the $seen_decl_yytext variable.  Maybe someone 
will want to change the
name?  I added a comment to this effect.


> > +    AM_ENABLE_MULTILIB => sub { $seen_multilib = $_[0] },
> > +    AC_EXEEXT => sub { $seen_exeext = 1 },
>
> No longer exists, exeext is always computed when there is some
> compilation involved, i.e., when Automake wants to use exeext and
> obkext, don't check for them: they've been checked for.  Or just look
> at $ac_subst{EXEEXT}.

Changed the areas in the code to look for $configure_vars{EXEEXT} instead of 
$seen_exeext.  If these
are always checked for then maybe when tracing is required the special casing 
can be removed
entirely.


> > +    # Check for NLS support.
> > +    AM_GNU_GETTEXT =>
> > +     sub { # FIXME: eliminate redundant $ac_gettext_line
> > +           $seen_gettext = $_[0];
> > +           $ac_gettext_line = (split /:/, $_[0])[1];
> > +         },
>
> Why does it need to know this?  Hm, will look into the details some day.
>
> > +    # This macro handles several different things.
> > +    AM_INIT_AUTOMAKE =>
> > +     sub { $seen_make_set = $_[0];
> > +           $seen_arg_prog = $_[0];
> > +           $seen_prog_install = $_[0];
> > +           $package_version = $_[3];
> > +           $package_version_line = (split /:/, $_[0])[2];
> > +           $seen_init_automake = $_[0];
> > +         },
>
> Not good: --trace looks inside, you don't need to know how
> AM_INIT_AUTOMAKE is written and what it does.  And now the proper
> means to set the name/version of a package is via AC_INIT, more
> precisely tracing _AC_INIT_PACKAGE.  Note that we can introduce a
> macro just to ask for the value of an Autoconf macro:

I will examine this more closely.  The INSTALL and MAKE_SET stuff I removed 
already.


> /tmp % cat configure.ac                                          nostromo 
> 14:40
> AC_INIT(GNU Hello, 1.0)
>
> m4_define([_AC_TRACE])
> m4_define([AC_TRACE], [_AC_TRACE(m4_defn([$1]))])
>
> AC_TRACE([AC_PACKAGE_NAME])
> AC_TRACE([AC_PACKAGE_TARNAME])
> AC_TRACE([AC_PACKAGE_STRING])
> AC_TRACE([AC_PACKAGE_VERSION])
> /tmp % ace -t _AC_TRACE                                          nostromo 
> 14:40
> configure.ac:6:_AC_TRACE:GNU Hello
> configure.ac:7:_AC_TRACE:hello
> configure.ac:8:_AC_TRACE:GNU Hello 1.0
> configure.ac:9:_AC_TRACE:1.0
>
> We can put here and there calls to AC_TRACE to announce variables.  Or
> just declare an m4_define specialized in ``broadcasted'' variables.

Ok.  Sounds like Autoconf work.  Can you do it?  I'll need at least more 
explanation of what you mean
by 'declare an m4_define specialized in ``broadcasted'' variables' if I'm to do 
it myself.


> > +    AC_LIBOBJ => sub { $libsources{"$_[2].c"} = $_[0] },
> > +    _AC_LIBOBJ_DECL =>
> > +     sub { $libsources{"$_[1].c"} = $_[0]
> > +             unless defined $libsources{"$_[2].c"};
> > +         },
>
> FYI, I applied this to Autoconf:
>
> 2001-02-03  Akim Demaille  <address@hidden>
>
>         * acfunctions.m4 (AC_FUNC_ERROR_AT_LINE, AC_FUNC_ONSTACK): Use
>         AC_LIBSOURCES.
>
> 2001-02-03  Akim Demaille  <address@hidden>
>
>         * acgeneral.m4 (AC_LIBOBJ_DECL): Remove.
>         (AC_LIBSOURCES, AC_LIBSOURCE): New.

Should this really be a part of Autoconf and not Automake?  Glancing at your 
empty macro definitions,
it seems AC_LIBSOURCES only needed for AM anyhow?


> > +    AM_MAINTAINER_MODE =>
> > +     sub { $seen_maint_mode = $_[0];
> > +           $configure_cond{'MAINTAINER_MODE'} = $_[0];
> > +         },
>
> How about finally dropping support of this feature in the next-next
> Automake?  :)
>
> > +    AC_OBJEXT => sub { $seen_objext = 1 },
>
> Same as exeext.

Same as exeext.  :)


> > +    # Like AC_CONFIG_FILES
> > +    AC_OUTPUT => sub { &scan_autoconf_config_files ($_[2]) },
>
> Nope, AC_OUTPUT dispatches its args to AC_CONFIG_FILES.  Great care
> was taken precisely for this use of --trace: old macros points to
> newer macros, i.e., it is Autoconf and Autoconf only that deals with
> obsoleted macros.

Removed completely as "Autoconf and only Autoconf ... deals with obsoleted 
macros" seems to mean
AC should handle the obsolescence warning to the user, right?


> > +    AC_PROG_LEX =>
> > +     sub { &am_conf_line_warning (
> > +               split (/:/, $_[0]),
> > +               "automake requires \`AM_PROG_LEX', not \`AC_PROG_LEX'")
> > +           unless ($seen_decl_yytext eq $_[0]);
> > +         },
> > +    AM_PROG_LEX => sub { $seen_decl_yytext = $_[0] },
>
> See above.

Hmm.  Does the AC will handle everything to do with obsoleted AC macros mean 
that I can skip the
obsoleted warning for AC_DECL_YYTEXT too?


> > +    AC_PROG_LIBTOOL =>
> > +     sub { $seen_libtool = $_[0];
> > +           $libtool_line = (split /:/, $_[0])[1];
> > +         },
> > +    AM_PROG_LIBTOOL =>
> > +     sub { &am_conf_line_warning (
> > +             split (/:/, $_[0]),
> > +             "\`AM_PROG_LIBTOOL' is obsolete, use \`AC_PROG_LIBTOOL' 
> > instead"
> > +           );
> > +           # FIXME: should we really be preserving AC_PROG_LIBTOOL behavior
> > +           # below?
> > +           $seen_libtool = $_[0];
> > +           $libtool_line = (split /:/, $_[0])[1];
> > +         },
>
> We really have to clean up the Libtool macros :(
>
> > +    AC_PROG_INSTALL => sub { $seen_prog_install = $_[0] },
>
> Tracing AC_SUBST(INSTALL) seems way enough.  Let's factor!

Factored.


> > +    AC_PROG_MAKE_SET => sub { $seen_make_set = $_[0] },
>
> Same.

Same.  :)


> > +    AC_REPLACE_FUNCS =>
> > +     sub { foreach (split /\s/, $_[2])
> > +           { $libsources{$_ . '.c'} = $_[0] }
> > +         },
>
> Don't: AC_REPLACE_FUNCS is invoking AC_LIBOBJ which point is exactly
> to relieve Automake from having to know about AC_REPLACE_FUNCS.

Removed.


> > +    AC_SUBST =>
> > +     sub { $configure_vars{$_[2]} = $_[0]
> > +           unless defined $configure_vars{$_[2]};
> > +         },
> > +    # Populate libobjs array.
> > +    # This section is an exception to the alphabetical ordering
> > +    AC_FUNC_ALLOCA => sub { $libsources{'alloca.c'} = $_[0] },
>
> Same, not needed.
>
> > +    AM_FUNC_ERROR_AT_LINE =>
> > +     sub { $libsources{'error.c'} = $_[0];
> > +           $libsources{'error.h'} = $_[0];
> > +         },
>
> Same, not needed since this morning :)

I trusted you and removed the two above.


> > +    AC_FUNC_GETLOADAVG => sub { $libsources{'getloadavg.c'} = $_[0] },
> > +    AC_FUNC_MEMCMP => sub { $libsources{'memcmp.c'} = $_[0] },
> > +    AC_FUNC_MKTIME => sub { $libsources{'mktime.c'} = $_[0] },
> > +    AM_FUNC_OBSTACK =>
> > +     sub { $libsources{'obstack.c'} = $_[0];
> > +           $libsources{'obstack.h'} = $_[0];
> > +         },
> > +    AM_FUNC_STRTOD => sub { $libsources{'strtod.c'} = $_[0] },
> > +    AC_REPLACE_GNU_GETOPT =>
> > +     sub { $libsources{'getopt.c'} = $_[0];
> > +           $libsources{'getopt1.c'} = $_[0];
> > +         },
> > +    AM_REPLACE_GNU_GETOPT =>
> > +     sub { $libsources{'getopt.c'} = $_[0];
> > +           $libsources{'getopt1.c'} = $_[0];
> > +         },
> > +    AC_STRUCT_ST_BLOCKS => sub { $libsources{'fileblocks.c'} = $_[0] },
> > +    AM_WITH_REGEX =>
> > +     sub { $libsources{'rx.c'} = $_[0];
> > +           $libsources{'rx.h'} = $_[0];
> > +           $libsources{'regex.c'} = $_[0];
> > +           $libsources{'regex.h'} = $_[0];
> > +         },
> > +);
>
> We have to check each Automake and Autoconf macros which requires some
> file, and make this ad hoc code useless.  AFAIR, these macros are
> AC_REPLACE_FUNCSing, so this code is already useless.

I checked and that doesn't appear to be happening yet.  Someone else can 
probably write that
AC (well, 3 AM macros too) patch quicker than me.


> > +sub scan_autoconf_traces_AC_CONFIG_HEADER
> > +{
> > +    # make pattern safe
> > +    $_[2] =~ s/\W/\\$&/;
> > +    &am_conf_line_error
> > +         (split (/:/, $_[0]),
> > +          "\`automake requires \`AM_CONFIG_HEADER', not 
> > \`AC_CONFIG_HEADER'")
> > +     # but make sure we're not called from AM_CONFIG_HEADER
> > +     unless grep /^$_[2]$/, @config_fullnames;
> > +}
>
> Hm, we should find a means to make AM_CONFIG_HEADER an obsolete
> concept :(  Maybe 2.51, not 2.50.

It's pretty much just creating the stamp files in addition to calling 
AC_CONFIG_HEADER and it's even
broken.  I've submitted two patches to fix this, one for the next release and 
one for the one after
that and they're still not perfect.


> > +    foreach $traced (keys %traced_macro_function)
> > +    {
> > +     # FIXME: I imagine we'll run into the system argument
> > +     # length limits eventually...
>
> Good call :( But then, we are doomed, because (watch your step),
> although it seems that obviously autoconf can have an option to read
> the trace requests from a file, and, instead of using the -t option of
> m4 to use the traceon macros (so finally we never have a command line
> which length depends upon the number of things to trace), so in spite
> of the fact that the solution seems obvious, ISTR there are some
> difference between -t and traceon which can hurt us.
>
> But if this problem actually happens, there is a way out.  Just
> adjustments will be needed.
>
> > -        local ($file, $line, $macro, @args) = split /:/;
> > +     /^([^:]*):([^:]*):([^:]*):/;
> > +        local ($file, $line, $macro, $rest) = ($1, $2, $3, $');
>
> Why do you prefer this?
>
> Let's use `local' for what's not local, and `my' for what is!  :)

Sure.  I just noticed you were changing those.  I was just sticking with what I 
thought was the
existing coding style.


> I think we are doing real good!  Your work is exiting...  Please, fill
> the papers real fast.  If I were you, at the same time I'd sign those
> for Autoconf.  You'll probably want to introduce generic macros for traces.

Will do it as soon as I have them.  :(

I haven't included a patch for the above changes yet since I haven't tested 
them.  I'll try and post
something tomorrow in case anybody wants to play with it.

Derek

--
Derek Price                      CVS Solutions Architect ( http://CVSHome.org )
mailto:address@hidden     OpenAvenue ( http://OpenAvenue.com )
--
If at first you don't succeed, skydiving is not for you.






reply via email to

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