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

[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             address@hidden
-----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 <address@hidden>
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 <address@hidden>
---
 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  <address@hidden>
 
+       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@&address@hidden 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], [^], address@hidden:@ ])])])
 # _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