On Wednesday 03 November 2010, Paolo Bonzini wrote:
> On 11/03/2010 04:24 PM, Stefano Lattarini wrote:
> >> + # 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 `...@path_separator@' here instead of `:', for better
> > portability to windows?
>
> Yes, I think so.
>
> >> + {
> >> + push (@system_includes, $dir) if $dir ne ''&& -d $dir;
> > Hmmm... IMHO the right place where to add directories from `ACLOCAL_PATH'
> > is `...@user_includes', not `...@system_includes'. 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.
>
> This is true (and, I think, handled by pushing at the end of
> @system_includes).
>
> On the other hand, I considered @user_includes as "per-package includes"
> and @system_includes as "installation-wide includes" (albeit per
> account), which means ACLOCAL_PATH would count as a system include
> directory.
Yes, makes sense. Still, having "per-account" includes in @system_includes
doesn't seem much clear to me. Maybe we should procees with a rename like
@system_includes => @global_includes and @user_includes => @local_includes.
But that's bikeshedding, and for a follow-up patch anyway; and I'm now OK
with your patch if the tests pass (by the way, the testcase I provided are
buggy, since they use `:' rather than `$PATH_SPERATOR' in definition of
ACLOCAL_PATH; that should be fixed).
> > 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?
>
> Keep it into @system_includes? :)
Right. Refactoring and renamings can be done as follow-ups, if needed.
Thanks,
Stefano
