Re: [PATCH 1/5] add a separate diversion for shell functions

[Paolo Bonzini]

> Thanks for your effort on this series!  I agree that we are finally ready
> to require shell functions in ./configure scripts.  It will take me some
> time to review them all, but let's start at the beginning.

Thanks.

> Do you have a repository published with this series available for quick
> test?  If nothing else, feel free to use the mob branch on my repo.or.cz
> clone:
> 
> $ git push address@hidden:/srv/git/autoconf/ericb.git +HEAD:mob
> 
> although you'll need to ask me to copy changes off to a more stable
> location, since anyone can overwrite the mob branch.

It's 9a91dabc6c16d6744767c7ac65c31f3fb92646e0 there.

> Nice - as long as we guarantee that shell initialization finds a shell
> that supports functions, then this is the right way to ensure functions
> are declared first.  However, for languages with more diversions (think
> autoconf and autotest), it still makes sense to have as many macros
> deferred until after --help/--version is processed, to avoid time lost in
> parsing shell functions that are otherwise unused, so we need to remember
> to not make the new diversion the catchall location for all functions.

Then it'd be better to also add the diversion argument to
AS_REQUIRE_SHELL_FN.  This means applying this patch 1bis/5 which is
folded into this one in my push to the mob branch.

diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index c24ac3b..39becd6 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -148,17 +148,17 @@ m4_define([AS_REQUIRE],
                               [m4_default([$2], [$1])])])])


-# AS_REQUIRE_SHELL_FN(NAME-TO-CHECK, BODY-TO-EXPAND)
-# --------------------------------------------------
+# AS_REQUIRE_SHELL_FN(NAME-TO-CHECK, BODY-TO-EXPAND, [DIVERSION =
M4SH-INIT-FN])
+#
------------------------------------------------------------------------------
 # BODY-TO-EXPAND is the body of a shell function to be emitted in the
-# M4SH-INIT section when expanded (required or not).  Unlike other
+# given diversion when expanded (required or not).  Unlike other
 # xx_REQUIRE macros, BODY-TO-EXPAND is mandatory.
 #
 m4_define([AS_REQUIRE_SHELL_FN],
 [_AS_DETECT_REQUIRED([_AS_SHELL_FN_WORK])dnl
 AS_REQUIRE([AS_SHELL_FN_$1], [m4_provide([AS_SHELL_FN_$1])$1() {
 $2
-}], [M4SH-INIT-FN])])
+}], [m4_default([$3], [M4SH-INIT-FN])])])


 # AS_BOURNE_COMPATIBLE


>> The argument to AS_REQUIRE is used also later in the series,
>> which is why I have not inlined it in AS_REQUIRE_SHELL_FN.
> 
>> 2008-09-18  Paolo Bonzini  <address@hidden>
> 
>>      * lib/m4sugar/m4sh.m4 (M4SH-INIT-FN): New diversion.
>>      (AS_REQUIRE): Accept diversion parameter.
>>      (AS_REQUIRE_SHELL_FN): Use it.
> 
> Looks okay, except for these nits:

Done.

> This is a subtle semantic change if M4SH or INIT is defined as a macro
> name, since m4_default expands its argument (although we are unlikely to
> be using macro names that conflict like that, it never hurts to play it
> safe).  Maybe it is time to introduce an m4sugar macro m4_default_quoted,
> which does NOT expand its arguments - I have several places in mind that
> could use it, in addition to this part of your patch.  What do you think,
> should I tackle that before we apply this series?

Shouldn't it be enough to double-quote?  I haven't much thought on the
m4 behavior on that, so this part is not yet in the series I pushed.

Paolo