Re: [PATCH] use shell function for AS_BASENAME, AS_DIRNAME, AS_MKDIR_P

[Eric Blake]

Paolo Bonzini <bonzini <at> gnu.org> writes:

> 
> Here is what remains.
> 
>       (_AS_MKDIR_P): New, from AS_MKDIR_P.  Adjust meaning of as_mkdir_p
>       to be `false' or a full `mkdir -p' command.
>       (AS_MKDIR_P): Just dispatch to as_func_mkdir_p.
>       (_AS_MKDIR_P_PREPARE): Set as_mkdir_p according to the above change.
>       Define shell functions.

You committed something slightly different.  You proposed this:

>  
> -# AS_MKDIR_P(DIR)
> -# ---------------
> -# Emulate `mkdir -p' with plain `mkdir'.
> -m4_define([AS_MKDIR_P],
> -[AS_REQUIRE([_$0_PREPARE])dnl
> -{ as_dir=$1
> +# _AS_MKDIR_P(DIR)
> +# ----------------
> +# Emulate `mkdir -p` with plain `mkdir'.
> +m4_define([_AS_MKDIR_P],
> +[as_dir=$1

...

> +# AS_MKDIR_P(DIR)
> +# ---------------
> +# Emulate `mkdir -p' with plain `mkdir' if needed.
> +m4_define([AS_MKDIR_P],
> +[AS_REQUIRE([_$0_PREPARE])dnl
> +as_dir=$1; as_func_mkdir_p "$as_dir"])# AS_MKDIR_P
> 
>  
>  # _AS_MKDIR_P_PREPARE
>  # -------------------
>  m4_defun([_AS_MKDIR_P_PREPARE],
> -[if mkdir -p . 2>/dev/null; then
> -  as_mkdir_p=:
> +[AS_REQUIRE_SHELL_FN([as_func_mkdir_p], [
> +  _AS_MKDIR_P([$[]1])
> +])

where you pass an m4 argument to _AS_MKDIR_P which is then output as the shell 
argument to as_func_mkdir_p, and that argument is assigned to as_dir (if local 
were portable, as_dir would be marked local).

But you committed a variant where _AS_MKDIR_P no longer takes an m4 parameter, 
but instead emits a body for as_func_mkdir_p that assumes that the global 
$as_dir was already set on entry to the shell function and no longer uses shell 
$1.  This alternate approach is okay (in fact, I think it is cleaner in the 
long run, as it avoids confusion between m4 $1 and shell $1), but needs 
documentation.  So I'm committing this touchup.

Meanwhile, let's settle on a consistent style: right now, m4sh uses

func () {
  body
}

but autotest uses

func()
{
  body
}

I somewhat prefer the latter (it is more like GNU Coding Standards for C), so 
unless anyone complains, I'll commit a followup patch that changes the m4sh 
formatting to match autotest.

>From 523ee0e116b772dfc7293212e2481db10042babe Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 10 Oct 2008 08:40:09 -0600
Subject: [PATCH] Fix _AS_MKDIR_P usage.

* lib/m4sugar/m4sh.m4 (_AS_MKDIR_P): Correct documentation to
match implementation.
(_AS_PREPARE, _AS_MKDIR_P_PREPARE): Adjust callers.
* doc/autoconf.texi (Programming in M4sh) <AS_MKDIR_P>: Tweak
wording to better match behavior.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog           |   10 ++++++++++
 doc/autoconf.texi   |    5 +++--
 lib/m4sugar/m4sh.m4 |   12 +++++++-----
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2ead8c6..2ab0394 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,16 @@
+2008-10-10  Eric Blake  <address@hidden>
+
+       Fix _AS_MKDIR_P usage.
+       * lib/m4sugar/m4sh.m4 (_AS_MKDIR_P): Correct documentation to
+       match implementation.
+       (_AS_PREPARE, _AS_MKDIR_P_PREPARE): Adjust callers.
+       * doc/autoconf.texi (Programming in M4sh) <AS_MKDIR_P>: Tweak
+       wording to better match behavior.
+
 2008-10-10  Paolo Bonzini  <address@hidden>
 
        * doc/autoconf.texi: Be less wary of shell functions.
+       * NEWS: Document the increased use of shell functions.
 
 2008-10-10  Paolo Bonzini  <address@hidden>
 
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 2ea542c..e1afada 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -11872,9 +11872,10 @@ Programming in M4sh
 @defmac AS_MKDIR_P (@var{file-name})
 @asindex{MKDIR_P}
 Make the directory @var{file-name}, including intervening directories
-as necessary.  This is equivalent to @samp{mkdir -p @var{file-name}},
+as necessary.  This is equivalent to @samp{mkdir -p -- @var{file-name}},
 except that it is portable to older versions of @command{mkdir} that
-lack support for the @option{-p} option.  Also, @code{AS_MKDIR_P}
+lack support for the @option{-p} option or for the @option{--}
+delimiter.  Also, @code{AS_MKDIR_P}
 succeeds if @var{file-name} is a symbolic link to an existing directory,
 even though Posix is unclear whether @samp{mkdir -p} should
 succeed in that case.  If creation of @var{file-name} fails, exit the
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index c0a1e30..369d27a 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -257,7 +257,7 @@ fi
 # important not to forget some: config.status needs them.
 m4_defun([_AS_PREPARE],
 [as_func_mkdir_p() {
-  _AS_MKDIR_P([$[]1])
+  _AS_MKDIR_P
 }
 
 _AS_EXPR_PREPARE
@@ -1033,9 +1033,11 @@ $as_ln_s $1 $2
 ])
 
 
-# _AS_MKDIR_P(DIR)
-# ----------------
-# Emulate `mkdir -p` with plain `mkdir'.
+# _AS_MKDIR_P
+# -----------
+# Emit code that can be used to emulate `mkdir -p` with plain `mkdir';
+# the code assumes that "$as_dir" contains the directory to create.
+# $as_dir is normalized, so there is no need to worry about using --.
 m4_define([_AS_MKDIR_P],
 [case $as_dir in #(
   -*) as_dir=./$as_dir;;
@@ -1067,7 +1069,7 @@ as_dir=$1; as_func_mkdir_p])# AS_MKDIR_P
 # -------------------
 m4_defun([_AS_MKDIR_P_PREPARE],
 [AS_REQUIRE_SHELL_FN([as_func_mkdir_p], [
-  _AS_MKDIR_P([$[]1])
+  _AS_MKDIR_P
 ])
 if mkdir -p . 2>/dev/null; then
   as_mkdir_p='mkdir -p "$as_dir"'
-- 
1.6.0.2