Re: [PATCH] aclocal: handle ACLOCAL_PATH environment variable

[Stefano Lattarini]

On Tuesday 02 November 2010, Paolo Bonzini wrote:
> Hi all, this patch provides a fourth scheme of adding directories to
> the search path.  It is a simple colon-separated list of directories
> (without globbing).
> 
> It is useful when you're using a global installation of Automake but
> you want to add directories from your account as well to the search
> path.  It is also useful for people who do not like for some reason
> to touch files in /usr/share (which, unlike /etc, are usually not
> meant for user customization).
I like this idea.  This new feature could also be used in the future to
improve the way the automake testuite looks for third-party macros needed
by some tests (e.g. those requiring libtool or gettext).  But see the nits
below.
 
> The test suites leaves the user's ACLOCAL_PATH in place, for consistency
> with the treatment of ${prefix}/share/aclocal/dirlist in tests/defs.in.
I'd prefer to clobber any user-provided ACLOCAL_PATH value in the testcase,
so that we minimize the risk of spurious failures/passes due to unexpected
interactions with user environment.

> By the way, this line:
> 
>     aclocaldir='@prefix@/share/aclocal'
> 
> should probably use datadir, no?
> 
> Ok to apply?  (I have copyright assignment in place but I'm not sure if I
> have access to the repository---probably not).
> 
> * NEWS: Document new behavior.
> * aclocal.in (parse_arguments): Parse ACLOCAL_PATH as
> a colon-separated list of directories to be included in
> the search path.
> * doc/automake.texi (Macro Search Path): Document new
> behavior.
> * tests/acloca24.test: New testcase.
> * tests/Makefile.am (TESTS): Add it.
> * tests/Makefile.in: Regenerate.
Automake does not mention generated files in the changelog (even if they
are committed into the repository).
> ---
>  NEWS                |    5 ++++
>  aclocal.in          |   11 ++++++++-
>  doc/automake.texi   |   10 ++++++++
>  tests/Makefile.am   |    1 +
>  tests/Makefile.in   |    1 +
>  tests/acloca24.test |   60 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 87 insertions(+), 1 deletions(-)
>  create mode 100755 tests/acloca24.test
> 
> diff --git a/NEWS b/NEWS
> index c64ec14..7567ca6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,10 @@
>  New in 1.11a:
>  
> +* Changes to aclocal:
> +
> +  - aclocal now interprets the `ACLOCAL_PATH' environment variable as a
> +    colon-separated list of additional directories to search.
> +
>  * Changes to automake:
>  
>    - automake now generates silenced rules for texinfo outputs.
> diff --git a/aclocal.in b/aclocal.in
> index 4c81a47..bc8cd25 100644
> --- a/aclocal.in
> +++ b/aclocal.in
> @@ -1025,7 +1025,7 @@ sub parse_arguments ()
>      }
>    else
>      {
> -      # Finally, adds any directory listed in the `dirlist' file.
> +      # Add any directory listed in the `dirlist' file.
>        if (open (DIRLIST, "$system_includes[0]/dirlist"))
>       {
>         while (<DIRLIST>)
> @@ -1043,6 +1043,15 @@ sub parse_arguments ()
>         close (DIRLIST);
>       }
>      }
> +  # Add any directory listed in the `ACLOCAL_PATH' environment
> +  # variable.
> +  if (defined $ENV{"ACLOCAL_PATH"})
> +    {
> +      foreach my $dir (split /:/, $ENV{"ACLOCAL_PATH"})
Shouldn't we use address@hidden@' here instead of `:', for better
portability to windows?
> +        {
> +          push (@system_includes, $dir) if $dir ne '' && -d $dir;
Hmmm... IMHO the right place where to add directories from `ACLOCAL_PATH'
is address@hidden', not address@hidden'.  Also, the testcase should
verify that extra directories from `-I' command line options take
precedence over the ones from `ACLOCAL_PATH', and that these latter
ones take precedence over the extra directories from system includes.
I've attached two tentative testcases checking for the behaviour I'd
like to see from ACLOCAL_PATH.

But as an afterthought, I see that installing third party macros in
directories provided by `ACLOCAL_PATH' when the `--install' option
is used is probably a bad idea.  Any idea about what the best way to
address this would be?
> +        }
> +    }
>  }
>  
>  ################################################################
> diff --git a/doc/automake.texi b/doc/automake.texi
> index 7214e49..92fb0ab 100644
> --- a/doc/automake.texi
> +++ b/doc/automake.texi
> @@ -3361,6 +3361,16 @@ Macro Search Path
>  copy of Automake in your account and want @command{aclocal} to look for
>  macros installed at other places on the system.
>  
> address@hidden Modifying the Macro Search Path: @file{ACLOCAL_PATH}
> address@hidden @env{ACLOCAL_PATH}
> +
> +The fourth and last mechanism to customize the macro search path is
> +also the simplest.  Any directory included in the colon-separated
> +environment variable @env{ACLOCAL_PATH} is added to the search path.
> +
> +Conversely to @file{dirlist}, @env{ACLOCAL_PATH} is useful if you are
> +using a global copy of Automake and want @command{aclocal} to look for
> +macros somewhere under your home directory.
>  
>  @node Extending aclocal
>  @subsection Writing your own aclocal macros

> diff --git a/tests/acloca24.test b/tests/acloca24.test
> new file mode 100755
> index 0000000..a99be3d
> --- /dev/null
> +++ b/tests/acloca24.test
Please see the attached testcases.

Thanks,
   Stefano

acloca24.test
Description: application/shellscript

acloca25.test
Description: application/shellscript