[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] aclocal: handle ACLOCAL_PATH environment variable
From: |
Stefano Lattarini |
Subject: |
Re: [PATCH] aclocal: handle ACLOCAL_PATH environment variable |
Date: |
Wed, 3 Nov 2010 16:24:51 +0100 |
User-agent: |
KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; ) |
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