Re: Site Macro Directory

[Akim Demaille]

>>>>> "Mark" == Mark D Roth <address@hidden> writes:

Mark> I just got word from RMS indicating that he received the
Mark> copyright assignment papers.  Is there anything else that we
Mark> need to take care of before the patch can be committed to CVS?
Mark> Thanks!

It should be ok.  But I still have a few comments about the patch
itself:

| cvs server: Diffing .

Please, write a ChangeLog.


| Index: configure.ac
| ===================================================================
| RCS file: /cvsroot/autoconf/autoconf/configure.ac,v
| retrieving revision 1.38
| diff -u -r1.38 configure.ac
| --- configure.ac      22 Apr 2002 14:33:05 -0000      1.38
| +++ configure.ac      27 May 2002 07:21:17 -0000
| @@ -31,6 +31,14 @@
|  
|  AM_INIT_AUTOMAKE
|  
| +AC_ARG_ENABLE([site-macro-dir],
| +  AC_HELP_STRING([--enable-site-macro-dir=DIR],
| +                 [use site macro directory DIR 
@<:@DATADIR/autoconf/site_macros@:>@]),

80 columns!

| +  ,
| +  [enable_site_macro_dir='${datadir}/autoconf/site_macros'])
| +SITE_MACRO_DIR="${enable_site_macro_dir}";
| +AC_SUBST([SITE_MACRO_DIR])

AC_SUBST(var, val) instead of two lines.

| +
|  # Initialize the test suite and build position independent wrappers.
|  AC_CONFIG_TESTDIR([tests])
|  AC_CONFIG_FILES([tests/Makefile tests/atlocal])
| cvs server: Diffing bin
| Index: bin/Makefile.am
| ===================================================================
| RCS file: /cvsroot/autoconf/autoconf/bin/Makefile.am,v
| retrieving revision 1.5
| diff -u -r1.5 Makefile.am
| --- bin/Makefile.am   10 Apr 2002 15:58:19 -0000      1.5
| +++ bin/Makefile.am   27 May 2002 07:21:17 -0000
| @@ -45,7 +45,8 @@
|       -e 's,@M4\@,$(M4),g' \
|       -e 's,@AWK\@,$(AWK),g' \
|       -e 's,@VERSION\@,$(VERSION),g' \
| -     -e 's,@PACKAGE_NAME\@,$(PACKAGE_NAME),g'
| +     -e 's,@PACKAGE_NAME\@,$(PACKAGE_NAME),g' \
| +     -e 's,@SITE_MACRO_DIR\@,$(SITE_MACRO_DIR),g'
|  
|  # autoconf is written in M4sh.
|  AUTOM4SH = $(top_builddir)/tests/autom4te --language M4sh
| Index: bin/autoconf.in
| ===================================================================
| RCS file: /cvsroot/autoconf/autoconf/bin/autoconf.in,v
| retrieving revision 1.147
| diff -u -r1.147 autoconf.in
| --- bin/autoconf.in   22 Apr 2002 09:13:43 -0000      1.147
| +++ bin/autoconf.in   27 May 2002 07:21:17 -0000
| @@ -252,6 +252,16 @@
|    esac
|  done
|  
| +# Add $AC_MACRO_PATH
| +if test -n "${AC_MACRO_PATH}"; then
| +  AUTOM4TE="$AUTOM4TE -I `echo ${AC_MACRO_PATH}|sed 's/:/ -I /'`";
| +fi

We don't ${foo} where useless, but that's really benign.

| +
| +# Add site macro dir
| +if test "@SITE_MACRO_DIR@" != "no"; then
| +  AUTOM4TE="$AUTOM4TE address@hidden@";
| +fi
| +
|  # Find the input file.
|  case $# in
|    0)
| Index: bin/autom4te.in
| ===================================================================
| RCS file: /cvsroot/autoconf/autoconf/bin/autom4te.in,v
| retrieving revision 1.61
| diff -u -r1.61 autom4te.in
| --- bin/autom4te.in   20 Apr 2002 06:09:01 -0000      1.61
| +++ bin/autom4te.in   27 May 2002 07:21:17 -0000
| @@ -389,7 +389,8 @@
|  The environment variable \`WARNINGS\' is honored.
|  
|  Library directories:
| -  -I, --include=DIR  look for FILES in DIR (cumulative)
| +  -i, --prepend-include=DIR  prepend directory DIR to search path
| +  -I, --include=DIR          append directory DIR to search path
|  
|  Tracing:
|    -t, --trace=MACRO      report the MACRO invocations
| @@ -484,6 +485,7 @@
|  
|    # Process the arguments for real this time.
|    my @trace;
| +  my @prepend_include;
|    getopt
|      (
|       # Operation modes:
| @@ -493,6 +495,7 @@
|       "M|melt"       => \$melt,
|  
|       # Library directories:
| +     "i|prepend-include=s" => address@hidden,
|       "I|include=s" => address@hidden,
|  
|       # Tracing:
| @@ -525,7 +528,10 @@
|    # a pain since it introduces a useless difference in the path which
|    # invalidates the cache.  And strip `.' which is implicit and always
|    # first.
| -  @include = grep { !/^\.$/ } uniq (@include);
| +  @include = grep { !/^\.$/ } uniq (reverse(@prepend_include), @include);
| +  #print "$me: include path:\n";
| +  #foreach (@include) { print "\t$_\n"; }
| +  #print "\n";

Please, remove these.


|    # Convert @trace to %trace, and work around the M4 builtins tracing
|    # problem.
| @@ -599,13 +605,10 @@
|    #
|    # Neutralize its stdin, so that GNU M4 1.5 doesn't neutralize SIGINT.
|    #
| -  # Be extremely cautious to reverse the includes when talking to M4:
| -  # it doesn't speak the same --include as we do.
| -  #
|    # We don't output directly to the cache files, to avoid problems
|    # when we are interrupted (that leaves corrupted files).
|    my $command = ("$m4"
| -              . join (' --include=', '', reverse @include)
| +              . join (' --include=', '', @include)
|                . " --define=m4_warnings=$m4_warnings"
|                . ' --debug=aflq'
|                . " --error-output=$tcache" . $req->id . "t"
| @@ -1047,7 +1050,7 @@
|    # output but comments and empty lines.
|    my $command = ("$m4"
|                . ' --fatal-warning'
| -              . join (' --include=', '', reverse @include)
| +              . join (' --include=', '', @include)
|                . ' --define=divert'
|                . " @ARGV"
|                . ' </dev/null');
| @@ -1070,7 +1073,7 @@
|    # or an improper paren etc.
|    $command = ("$m4"
|             . ' --fatal-warning'
| -           . join (' --include=', '', reverse @include)
| +           . join (' --include=', '', @include)
|             . " --freeze-state=$output"
|             . " @ARGV"
|             . ' </dev/null');



| cvs server: Diffing lib
| Index: lib/autom4te.in
| ===================================================================
| RCS file: /cvsroot/autoconf/autoconf/lib/autom4te.in,v
| retrieving revision 1.11
| diff -u -r1.11 autom4te.in
| --- lib/autom4te.in   3 Apr 2002 13:35:59 -0000       1.11
| +++ lib/autom4te.in   27 May 2002 07:21:21 -0000
| @@ -126,7 +126,7 @@
|  ## ---------- ##
|  
|  begin-language: "Autoconf"
| -args: --include @datadir@
| +args: --prepend-include @datadir@
|  args: autoconf/autoconf.m4f
|  args: acsite.m4?
|  args: aclocal.m4?
| @@ -144,7 +144,7 @@
|  ## -------- ##
|  
|  begin-language: "Autotest"
| -args: --include @datadir@
| +args: --prepend-include @datadir@
|  args: autotest/autotest.m4f
|  args: package.m4?
|  args: --mode 777
| @@ -157,7 +157,7 @@
|  ## ---- ##
|  
|  begin-language: "M4sh"
| -args: --include @datadir@
| +args: --prepend-include @datadir@
|  args: m4sugar/m4sh.m4f
|  args: --mode 777
|  args: --language M4sugar
| @@ -169,7 +169,7 @@
|  ## ------- ##
|  
|  begin-language: "M4sugar"
| -args: --include @datadir@
| +args: --prepend-include @datadir@
|  args: m4sugar/m4sugar.m4f
|  args: --warning syntax
|  end-language: "M4sugar"

This is good!  Thanks.

There is one thing I don't like too much though: I have paid attention
to make autoconf == automake -l autoconf.  This is no longer the case
with your change.  Could you try to move the -I stuff into
autom4te.cfg?

And most importantly: did you run the testsuite?

If you ran the testsuite, I propose that we go in two steps: we apply
your patch as is (but the few minor points above), and then we work
are making autoconf == automake -l autoconf again.