[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#13514: [PATCH v2 0/0] AC_CONFIG_MACRO_DIR && non-existent directory
From: |
Stefano Lattarini |
Subject: |
bug#13514: [PATCH v2 0/0] AC_CONFIG_MACRO_DIR && non-existent directory |
Date: |
Mon, 11 Feb 2013 15:23:24 +0100 |
On 02/11/2013 01:11 PM, Pavel Raiskup wrote:
> Hi, thanks for your comments!
>
Thanks for your patches, and you patience.
> Here is second iteration of my proposal.
>
> [PATCH 1/2] aclocal: just warn if the primary local m4 dir doesn't
> aclocal.in | 10 ++++++++++
> t/aclocal-macrodir.tap | 22 +++++++++++++++++++++-
>
> [PATCH 2/2] aclocal: fix for more-than-once specified directories
> aclocal.in | 31 +++++++++++++++++++++++--------
> t/aclocal-macrodir.tap | 12 ++++++++----
>
Review will follow later today, or in the next days.
>> Two meta-questions first, though:
>>
>> - Do you have copyright assignment paperwork for Automake in place?
>> Your changes definitely don't qualify as "trivial" or "very short",
>> so I fear the paperwork is required.
>
> These shouldn't be needed as Red Hat should have already signed papers
> with FSF covering all employees - if the patch it is part of the work -
> Which is this case.
>
Nice! And thanks to Red Hat for that.
>> - In the future, if it doesn't take you too much time, could you try
>> to send the patches in-line rather than as attachments? (You can
>> use the excellent "git send-email" command for that). This would
>> make it more convenient for reviewers to answer to your patches
>> separately, and to quote relevant chunks from them. Thanks.
>
> From now I'm using git send-email,
>
Thanks.
> sorry for inconvenience.
>
And please, no need to apologize!
>>> See:
>>> <http://lists.gnu.org/archive/html/bug-automake/2013-01/msg00115.html>
>>>
>>> * aclocal.in (scan_m4_files): Unique multiply specified directories.
>>>
>> s/Unique multiply/uniquify/ perhaps?
>>
>> Good catch, BTW! I think we also need a testsuite addition to cover for
>> the use case fixed by this patch (not a requirement to get your patches
>> in; if you don't want to write such a test, I'll get to it myself
>> eventually).
>
> I tried to write a test.
> Feel free to trash/rewrite it just to speed this process up.
>
OK.
>> =====
>> === PATCH [3/3]
>> =====
>>
>>> Subject: [PATCH 3/3] docs: Mention consequences with AC_CONFIG_MACRO_DIRS
>>>
>> I'm quite unconvinced of the usefulness of this patch. More details below.
>>
>> [...]
>>
>>> diff --git a/doc/automake.texi b/doc/automake.texi
>>> index e700ab9..944fd9d 100644
>>> --- a/doc/automake.texi
>>> +++ b/doc/automake.texi
>>> [...]
>>> +There is strictly encouraged not to change the 'm4' directory name to
>>> +some different name at the moment.
>>>
>> Why not? Have you examples of real-world issues caused by different
>> names?
>
> Ok, I was considering situation that we can live without ACLOCAL_AMFLAGS.
> Because we can not, there is no problem (I'm thinking about gettext).
>
> For this reason, do you think it would be possible to freeze
> ACLOCAL_AMFLAGS and AC_CONFIG_MACRO_DIR a 'little' bit longer in automake
> then until 1.14?
>
Do you mean not removing ACLOCAL_AMFLAGS in 1.14? Sure, that has never
been the plan. ACLOCAL_AMFLAGS will cause non-fatal warnings in 1.14,
but will still work. And very likely it will keep working with Automake
1.15 (and possibly 1.16 too).
<ASIDE>
Notice that if the new versioning and releasing scheme planned at
http://debbugs.gnu.org/13578 is finally picked up, what we are now
labelling as Automake 1.14 will actually be released as Automake 2.0
(to be released no sooner than an year and a half from now); similarly,
1.15 will actually be released as 3.0, and 1.16 as 4.0, and so on.
</ASIDE>
> I'm afraid that all tools will have problems to follow these
> changes...
>
Indeed, we should tread carefully here.
>>> [SNIP]
>>
>>> + The least problematic way seems to be now:
>>> +
>>> address@hidden
>>> +configure.ac: AC_CONFIG_MACRO_DIRS([m4])
>>> +Makefile.am: # **NO** ACLOCAL_AMFLAGS
>>> address@hidden example
>>> +
>> Alas, this is not true; the above will not work with Automake 1.12.x,
>
> I forgot about it, sorry.
>
>> nor with the (so far) latest version of libtool (2.4.2).
>
> This is because of AC_CONFIG_MACRO_DIRS, there should be
> AC_CONFIG_MACRO_DIR. But anyway, this is not a good idea.. as we need
> ACLOCAL_AMFLAGS.
>
>> For the moment, all the users not willing to require cutting-edge
>> autotools for their build systems will still have to specify
>> ACLOCAL_AMFLAGS.
>
> Yes, but not if they don't want (or don't necessarily need) to switch
> macros into different directory than 'm4'?
>
There might be a misunderstanding here (either from your part, or in
my understanding of what you are saying). Are you aware that none of
the autotools use *any* local m4 directory if it is not explicitly
instructed to, right? So there is not "default local m4 directory".
The 'AC_CONFIG_MACRO_DIR' and 'AC_CONFIG_MACRO_DIRS' macros are not
used to switch to an alternate name for the local m4 directory, but
to declare such directory (or directories).
> This is somehow complicated
> and inconsistent behavior among autotools :( and I'm still not sure what
> should I suggest to package maintainers which are dependant on autotools
> for that issue.
>
> Should we suggest in general something like this?
>
> configure.ac: AC_CONFIG_MACRO_DIR([*Whatever*])
> Makefile.am: ACLOCAL_AMFLAGS = -I *Whatever*
> # but *Whatever* must be the same on both lines
>
I'd suggest them to either:
- use AC_CONFIG_MACRO_DIRS if they can/want require cutting-edge
versions of the autotools;
- use ACLOCAL_AMFLAGS otherwise, for the time being (until the
now-cutting-edge autotools version have become widespread and
"old" enough).
Also notice that proper support for AC_CONFIG_MACRO_DIRS will only
be present from the *next* versions of autoconf and libtool (2.70
and 2.6 respectively I think); it is not yet available with the
current one (2.69 and 2.4.2, respectively).
>> =====
>> === FIXUP PATCH
>> =====
>>
>>> [PATCH] tests: Command 'aclocal -Inon-existent' should warn
>>>
>>> Check that this command just warns and does not fail.
>>>
>>> See:
>>> <http://lists.gnu.org/archive/html/bug-automake/2013-01/msg00115.html>
>>>
>>> * t/aclocal-macrodir.tap: Check for proper return value and do not handle
>>> warnings as errors.
>>>
>>
>> This patch should just be folded into the first one IMHO, to keep spurious
>> test failures to appear (albeit temporarily) in the testsuite.
>
> Done.
>
>> Oops. that "(1)" is just an old copy & paster error; so just drop the "(0)"
>> in your new message.
>
> Done.
>
>>> cat > configure.ac << 'END'
>>> AC_INIT([oops], [1.0])
>>> AC_CONFIG_MACRO_DIR([non-existent])
>>> END
>>>
>>> -not $ACLOCAL -Wnone 2>stderr \
>>> +$ACLOCAL -Wno-error 2>stderr \
>>> && cat stderr >&2 \
>>> && grep "couldn't open directory 'non-existent'" stderr \
>>> || r='not ok'
>>>
>>
>> You should also adjust the sister test 't/aclocal-macrodir.tap'
>> in the same way (there are three checks to adjust there).
>
> If you think something like that
>
> test ! -d foo \
> - && $ACLOCAL --install --system-acdir ./acdir \
> + && $ACLOCAL --install --system-acdir ./acdir 2>stderr \
> + && cat stderr >&2 \
> + && not grep "couldn't open directory" stderr \
> && diff acdir/bar.m4 foo/bar.m4 \
> || r='not ok'
>
> then it should fixed.
>
Yes, this is good enough IMO. If I want to further tweak it,
I can do that myself, without bothering you further.
Thanks, and best regards,
Stefano
- bug#13514: unintentional regression with AC_CONFIG_MACRO_DIR in 1.13.1, Pavel Raiskup, 2013/02/04
- bug#13514: unintentional regression with AC_CONFIG_MACRO_DIR in 1.13.1, Stefano Lattarini, 2013/02/04
- bug#13514: unintentional regression with AC_CONFIG_MACRO_DIR in 1.13.1, Pavel Raiskup, 2013/02/08
- bug#13514: unintentional regression with AC_CONFIG_MACRO_DIR in 1.13.1, Pavel Raiskup, 2013/02/09
- bug#13514: unintentional regression with AC_CONFIG_MACRO_DIR in 1.13.1, Stefano Lattarini, 2013/02/09
- bug#13514: [PATCH v2 0/0] AC_CONFIG_MACRO_DIR && non-existent directory, Pavel Raiskup, 2013/02/11
- bug#13514: [PATCH 1/2] aclocal: just warn if the primary local m4 dir doesn't exist (no error), Pavel Raiskup, 2013/02/11
- bug#13514: [PATCH 1/2] aclocal: just warn if the primary local m4 dir doesn't exist (no error), Stefano Lattarini, 2013/02/15
- bug#13514: [PATCH 2/2] aclocal: fix for more-than-once specified directories, Pavel Raiskup, 2013/02/11
- bug#13514: [PATCH v2 0/0] AC_CONFIG_MACRO_DIR && non-existent directory,
Stefano Lattarini <=
- bug#13514: [PATCH v2 0/0] AC_CONFIG_MACRO_DIR && non-existent directory, Pavel Raiskup, 2013/02/11
- bug#13514: [PATCH v3 0/2] AC_CONFIG_MACRO_DIR && non-existent directories, Stefano Lattarini, 2013/02/16
- bug#13514: [PATCH v3 2/2] aclocal: fix for more-than-once specified directories, Stefano Lattarini, 2013/02/16
- bug#13514: [PATCH v3 1/2] aclocal: just warn if the primary local m4 dir doesn't exist (don't error), Stefano Lattarini, 2013/02/16
- bug#13514: [PATCH v3 1/2] aclocal: just warn if the primary local m4 dir doesn't exist (don't error), Pavel Raiskup, 2013/02/18
- bug#13514: [PATCH v3 1/2] aclocal: just warn if the primary local m4 dir doesn't exist (don't error), Stefano Lattarini, 2013/02/20
- bug#13514: [PATCH v3 1/2] aclocal: just warn if the primary local m4 dir doesn't exist (don't error), Stefano Lattarini, 2013/02/21