[PATCH 2/5] avoid duplicate tests for shell functions

[Paolo Bonzini-2]

This one-line patch ensures that if a feature is deemed required,
its test is not emitted in the suggested tests.  It is not
perfect, but it works for the case at hand which is right now
the most complex use of the _AS_DETECT_BETTER_SHELL machinery.

A more complete fix would require more m4fu and time that I
have, sorry. :-(

2008-09-18  Paolo Bonzini  <bonzini@...>

        * lib/m4sugar/m4sh.m4 (_AS_DETECT_REQUIRED): Provide
        _AS_DETECT_SUGGESTED_provide($1).
---
 lib/m4sugar/m4sh.m4 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index 7ebb1a3..0a87a97 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -210,6 +210,7 @@ _ASEOF
 m4_define([_AS_DETECT_REQUIRED_BODY], [:])
 m4_defun([_AS_DETECT_REQUIRED],
 [m4_require([_AS_DETECT_BETTER_SHELL])dnl
+m4_provide([_AS_DETECT_SUGGESTED_provide($1)])dnl
 m4_expand_once([m4_append([_AS_DETECT_REQUIRED_BODY], [
 ($1) || AS_EXIT(1)
 ])], [_AS_DETECT_REQUIRED_provide($1)])])
--
1.5.5

[Paolo Bonzini-2]

Paolo Bonzini wrote:
> This one-line patch ensures that if a feature is deemed required,
> its test is not emitted in the suggested tests.  It is not
> perfect, but it works for the case at hand which is right now
> the most complex use of the _AS_DETECT_BETTER_SHELL machinery.

Ping (also 2..5/5).

Paolo

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paolo Bonzini on 9/18/2008 8:13 AM:
> This one-line patch ensures that if a feature is deemed required,
> its test is not emitted in the suggested tests.  It is not
> perfect, but it works for the case at hand which is right now
> the most complex use of the _AS_DETECT_BETTER_SHELL machinery.
>
> A more complete fix would require more m4fu and time that I
> have, sorry. :-(

I'm still thinking about that.  As long as you require a shell fn snippet
before someone else suggests it, then your solution is perfect.  The
problem comes that if someone else suggests the snippet first, then it
still gets duplicated when you later require it.

I'm almost wondering whether this is a use-case for m4_set.  Both
DETECT_REQUIRED and DETECT_SUGGESTED are maintaining a set of snippets to
use, with the further coordination that the output of the DETECT_SUGGESTED
test should exclude any snippets that are in the required set.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjrXuUACgkQ84KuGfSFAYAacACgxxnFAewbd7OS/fheuwiJkh/H
wFsAn0TAVve8ITUAr+jjeGw611Z9PFUV
=PJ48
-----END PGP SIGNATURE-----

[Paolo Bonzini-2]

> I'm still thinking about that.  As long as you require a shell fn snippet
> before someone else suggests it, then your solution is perfect.  The
> problem comes that if someone else suggests the snippet first, then it
> still gets duplicated when you later require it.

Yes, that's it.

> I'm almost wondering whether this is a use-case for m4_set.  Both
> DETECT_REQUIRED and DETECT_SUGGESTED are maintaining a set of snippets to
> use, with the further coordination that the output of the DETECT_SUGGESTED
> test should exclude any snippets that are in the required set.

I submitted the imperfect patch because it's a oneliner so it's easy to
back out when we have a better solution.  But this is more of an excuse
than anything else. :-)

Paolo

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paolo Bonzini on 9/18/2008 8:13 AM:
> This one-line patch ensures that if a feature is deemed required,
> its test is not emitted in the suggested tests.  It is not
> perfect, but it works for the case at hand which is right now
> the most complex use of the _AS_DETECT_BETTER_SHELL machinery.
>
> A more complete fix would require more m4fu and time that I
> have, sorry. :-(

Maybe my m4fu is better?  (It helps to have the m4 maintainer reviewing
your patches ;)

How about this patch, instead of yours?  Rather than using m4_expand_once
and m4_append to create the list of tests, I use m4_set to track the set
of tests, then filter the suggested set inside the m4_wrap to drop
required tests, no matter whether the requirement request occurred before
or after the suggestion.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjsIeIACgkQ84KuGfSFAYAYkACdHACRpee4kNbFlWKgEI64GoWp
XaIAoJcEYzOYXS6HtFtyt2gWTFBvaWLX
=EQI3
-----END PGP SIGNATURE-----

From c9d58598adc4e2e77fe6ffa5ada5f59dea3b0852 Mon Sep 17 00:00:00 2001
From: Eric Blake <ebb9@...>
Date: Tue, 7 Oct 2008 20:41:13 -0600
Subject: [PATCH] Avoid repeating required shell tests in suggested set.

* lib/m4sugar/m4sh.m4 (_AS_DETECT_REQUIRED, _AS_DETECT_SUGGESTED):
Use m4_set, rather than m4_expand_once/m4_append.
(_AS_DETECT_SUGGESTED): Adjust to new storage layout, and filter
required tests out of suggested tests.
Reported by Paolo Bonzini.

Signed-off-by: Eric Blake <ebb9@...>
---
 ChangeLog           |    7 +++++++
 lib/m4sugar/m4sh.m4 |   37 ++++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f072ae4..ea637a5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2008-10-07  Eric Blake  <ebb9@...>
 
+ Avoid repeating required shell tests in suggested set.
+ * lib/m4sugar/m4sh.m4 (_AS_DETECT_REQUIRED, _AS_DETECT_SUGGESTED):
+ Use m4_set, rather than m4_expand_once/m4_append.
+ (_AS_DETECT_SUGGESTED): Adjust to new storage layout, and filter
+ required tests out of suggested tests.
+ Reported by Paolo Bonzini.
+
  Ensure _AS_CLEANUP is defined.
  * lib/m4sugar/m4sh.m4 (_AS_CLEANUP): Give initial definition.
  * tests/m4sh.at (AS@&t@_INIT cleanup): Expose the need for this.
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index b49f302..4d6e79c 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -155,23 +155,21 @@ $1], [^], [@%:@ ])])])
 # _AS_DETECT_REQUIRED(TEST)
 # -------------------------
 # Refuse to execute under a shell that does not pass the given TEST.
-m4_define([_AS_DETECT_REQUIRED_BODY], [:])
 m4_defun([_AS_DETECT_REQUIRED],
 [m4_require([_AS_DETECT_BETTER_SHELL])dnl
-m4_expand_once([m4_append([_AS_DETECT_REQUIRED_BODY], [
-($1) || AS_EXIT(1)
-])], [_AS_DETECT_REQUIRED_provide($1)])])
+m4_set_add([_AS_DETECT_REQUIRED_BODY],
+   [($1) || AS_EXIT(1)
+])])
 
 
 # _AS_DETECT_SUGGESTED(TEST)
 # --------------------------
 # Prefer to execute under a shell that passes the given TEST.
-m4_define([_AS_DETECT_SUGGESTED_BODY], [:])
 m4_defun([_AS_DETECT_SUGGESTED],
 [m4_require([_AS_DETECT_BETTER_SHELL])dnl
-m4_expand_once([m4_append([_AS_DETECT_SUGGESTED_BODY], [
-($1) || AS_EXIT(1)
-])], [_AS_DETECT_SUGGESTED_provide($1)])])
+m4_set_add([_AS_DETECT_SUGGESTED_BODY],
+   [($1) || AS_EXIT(1)
+])])
 
 
 # _AS_DETECT_BETTER_SHELL
@@ -192,11 +190,18 @@ m4_defun_once([_AS_DETECT_BETTER_SHELL],
 [m4_append([_AS_CLEANUP], [m4_divert_text([M4SH-SANITIZE], [
 AS_REQUIRE([_AS_UNSET_PREPARE])dnl
 if test "x$CONFIG_SHELL" = x; then
-  AS_IF([_AS_RUN([_AS_DETECT_REQUIRED_BODY]) 2>/dev/null],
- [as_have_required=yes],
- [as_have_required=no])
+dnl Remove any tests from suggested that are also required
+  m4_set_foreach([_AS_DETECT_SUGGESTED_BODY], [AS_snippet],
+ [m4_set_contains([_AS_DETECT_REQUIRED_BODY],
+  _m4_defn([AS_snippet]),
+  [m4_set_remove([_AS_DETECT_SUGGESTED_BODY],
+ _m4_defn([AS_snippet]))])])dnl
+  m4_set_empty([_AS_DETECT_REQUIRED_BODY], [as_have_required=yes],
+    [AS_IF([_AS_RUN(m4_set_contents([_AS_DETECT_REQUIRED_BODY])) 2>/dev/null],
+  [as_have_required=yes],
+  [as_have_required=no])])
   AS_IF([test $as_have_required = yes &&dnl
- _AS_RUN([_AS_DETECT_SUGGESTED_BODY]) 2> /dev/null],
+ _AS_RUN(m4_set_contents([_AS_DETECT_SUGGESTED_BODY])) 2> /dev/null],
     [],
     [as_candidate_shells=
     _AS_PATH_WALK([/bin$PATH_SEPARATOR/usr/bin$PATH_SEPARATOR$PATH],
@@ -210,12 +215,14 @@ if test "x$CONFIG_SHELL" = x; then
       for as_shell in $as_candidate_shells $SHELL; do
  # Try only shells that exist, to save several forks.
  AS_IF([{ test -f "$as_shell" || test -f "$as_shell.exe"; } &&
- _AS_RUN([_AS_DETECT_REQUIRED_BODY],
+ _AS_RUN(m4_set_contents([_AS_DETECT_REQUIRED_BODY]),
  [("$as_shell") 2> /dev/null])],
        [CONFIG_SHELL=$as_shell
        as_have_required=yes
-       AS_IF([_AS_RUN([_AS_DETECT_SUGGESTED_BODY], ["$as_shell" 2> /dev/null])],
-     [break])])
+       m4_set_empty([_AS_DETECT_SUGGESTED_BODY], [break],
+         [AS_IF([_AS_RUN(m4_set_contents([_AS_DETECT_SUGGESTED_BODY]),
+         ["$as_shell" 2> /dev/null])],
+        [break])])])
       done
 
       AS_IF([test "x$CONFIG_SHELL" != x],
--
1.6.0.2

[Paolo Bonzini-2]

Eric Blake wrote:

> According to Paolo Bonzini on 9/18/2008 8:13 AM:
>> This one-line patch ensures that if a feature is deemed required,
>> its test is not emitted in the suggested tests.  It is not
>> perfect, but it works for the case at hand which is right now
>> the most complex use of the _AS_DETECT_BETTER_SHELL machinery.
>
>> A more complete fix would require more m4fu and time that I
>> have, sorry. :-(
>
> Maybe my m4fu is better?

Definitely, though this is m4sugar-fu.

> How about this patch, instead of yours?

Looks great, please push it.  I will then rebase the series and push it
somewhere (including possibly your mob branch, and/or reposting on
autoconf-patches).

Paolo