Re: improved Automake test for file names with funny characters

[Paul Eggert]

Following up on your response to
<http://lists.gnu.org/archive/html/automake-patches/2005-07/msg00024.html>:

Alexandre Duret-Lutz <address@hidden> writes:

> I think some of these changes need to be synced with
> mkinstalldirs and vice-versa.

I thought mkinstalldirs was obsolete now?  Do we need to continue to
maintain it?

What changes in mkinstalldirs need to be merged into install-sh?
(Presumably these can be folded in after the changes below are installed.)

> But it's not clear to me because you seems to tune it much more than
> the ChangeLog accounts for.  What's the story with all these
> test_mode/umask things ?

In the revised patch enclosed below, I'll add some commentary about
this to help explain it.

> It does not seem to be fixed on platforms where "mkdir -p" does
> not work.  (I'm concerned by the spurious report about test
> suite failures more than by the missing feature on these
> system.)

That should be fixed now; please see below.

> Does Stephan's patch for quoting $@ before eval look right to you?

No, because echo mishandles some path names.  However, there is
a better fix for the IFS problem in general, as discussed in
<http://lists.gnu.org/archive/html/autoconf-patches/2005-08/msg00074.html>.
The revised patch below incorporates this idea into install-sh.

> On second though I don't buy it [the "sleep 1; test -d"]: it only
> works if both "mkdir -p" invocations are creating the same directory
> path.  It doesn't work if one create "a/b/", the other "a/c/", and
> both race on "a" as described in mkinstalldirs.

Good catch.  The enclosed revised patch addresses that as well, by
falling back on the slow mkdir method if mkdir -p fails.

This patch also fixes a typo in the doc patch I submitted last time
("length file names" -> "lengthy file names").

2005-09-06  Paul Eggert  <address@hidden>

        * doc/automake.texi (limitations on file names): New section.
        * lib/install-sh: Rewrite to support '*' in file names.
        Also, tune so that we don't invoke so many commands in the usual case.
        * tests/instspc.test: The "*" test is now fixed.

Index: doc/automake.texi
===================================================================
RCS file: /cvs/automake/automake/doc/automake.texi,v
retrieving revision 1.121
diff -p -u -r1.121 automake.texi
--- doc/automake.texi   27 Jul 2005 19:05:21 -0000      1.121
+++ doc/automake.texi   6 Sep 2005 19:14:46 -0000
@@ -253,6 +253,7 @@ Frequently Asked Questions about Automak
 * CVS::                         CVS and generated files
 * maintainer-mode::             missing and AM_MAINTAINER_MODE
 * wildcards::                   Why doesn't Automake support wildcards?
+* limitations on file names::   Limitations on source and installed file names
 * distcleancheck::              Files left in build directory after distclean
 * Flag Variables Ordering::     CFLAGS vs.@: AM_CFLAGS vs.@: mumble_CFLAGS
 * renamed objects::             Why are object files sometimes renamed?
@@ -8031,6 +8032,7 @@ lists.
 * CVS::                         CVS and generated files
 * maintainer-mode::             missing and AM_MAINTAINER_MODE
 * wildcards::                   Why doesn't Automake support wildcards?
+* limitations on file names::   Limitations on source and installed file names
 * distcleancheck::              Files left in build directory after distclean
 * Flag Variables Ordering::     CFLAGS vs.@: AM_CFLAGS vs.@: mumble_CFLAGS
 * renamed objects::             Why are object files sometimes renamed?
@@ -8357,6 +8359,69 @@ variables as far Automake is concerned.
 You can get warnings about @samp{$(wildcard ...}) constructs using the
 @option{-Wportability} flag.
 
address@hidden limitations on file names
address@hidden Limitations on file names
address@hidden file names, limitations on
+
+Automake attempts to support all kinds of file names, even those that
+contain unusual characters or are unusually long.  However, some
+limitations are imposed by the underlying operating system and tools.
+
+Most operating systems prohibit the use of the null byte in file
+names, and reserve @samp{/} as a directory separator.  Also, they
+require that file names are properly encoded for the user's locale.
+Automake is subject to these limits.
+
+Portable packages should limit themselves to @acronym{POSIX} file
+names.  These can contain @acronym{ASCII} letters and digits,
address@hidden, @samp{.}, and @samp{-}.  File names consist of components
+separated by @samp{/}.  File name components cannot begin with
address@hidden
+
+Portable POSIX file names cannot contain components that exceed a
+14-byte limit, but nowadays it's normally safe to assume the
+more-generous @acronym{XOPEN} limit of 255 bytes.  @acronym{POSIX}
+limits file names to 255 bytes (@acronym{XOPEN} allows 1023 bytes),
+but you may want to limit a source tarball to file names to 99 bytes
+to avoid interoperability problems with old versions of @command{tar}.
+
+If you depart from these rules (e.g., by using address@hidden
+characters in file names, or by using lengthy file names), your
+installers may have problems for reasons unrelated to Automake.
+However, if this does not concern you, you should know about the
+limitations imposed by Automake itself.  These limitations are
+undesirable, but some of them seem to be inherent to underlying tools
+like Autoconf, Make, M4, and the shell.  They fall into three
+categories: install directories, build directories, and file names.
+
+The following characters:
+
address@hidden
address@hidden " # $ ' `
address@hidden example
+
+should not appear in the names of install directories.  For example,
+the operand of @command{configure}'s @option{--prefix} option should
+not contain these characters.
+
+Build directories suffer the same limitations as install directories,
+and in addition should not contain the following characters:
+
address@hidden
+& @@ \
address@hidden example
+
+For example, the full name of the directory containing the source
+files should not contain these characters.
+
+Source and installation file names like @file{main.c} are limited even
+further: they should conform to the @acronym{POSIX}/@acronym{XOPEN}
+rules described above.  In addition, if you plan to port to
address@hidden environments, you should avoid file names that
+differ only in case (e.g., @file{makefile} and @file{Makefile}).
+Nowadays it is no longer worth worrying about the 8.3 limits of
address@hidden file systems.
+
 @node distcleancheck
 @section Files left in build directory after distclean
 @cindex @code{distclean}, diagnostic
Index: lib/install-sh
===================================================================
RCS file: /cvs/automake/automake/lib/install-sh,v
retrieving revision 1.25
diff -p -u -r1.25 install-sh
--- lib/install-sh      9 Jul 2005 10:21:12 -0000       1.25
+++ lib/install-sh      6 Sep 2005 19:14:46 -0000
@@ -58,7 +58,21 @@ stripprog="${STRIPPROG-strip}"
 rmprog="${RMPROG-rm}"
 mkdirprog="${MKDIRPROG-mkdir}"
 
-chmodcmd="$chmodprog 0755"
+posix_glob=
+posix_mkdir=
+
+# Symbolic mode for testing mkdir with directories.
+# It is the same as 755, but also tests that "u+" works.
+test_mode=u=rwx,g=rx,o=rx,u+wx
+
+# Desired mode of installed file.
+mode=0755
+
+# Desired mode of newly created intermediate directories.
+# It is empty if not known yet.
+intermediate_mode=
+
+chmodcmd=$chmodprog
 chowncmd=
 chgrpcmd=
 stripcmd=
@@ -111,7 +125,7 @@ while test -n "$1"; do
 
     --help) echo "$usage"; exit $?;;
 
-    -m) chmodcmd="$chmodprog $2"
+    -m) mode=$2
         shift
         shift
         continue;;
@@ -164,6 +178,8 @@ if test -z "$1"; then
   exit 0
 fi
 
+test -n "$dir_arg" || trap '(exit $?); exit' 1 2 13 15
+
 for src
 do
   # Protect names starting with `-'.
@@ -173,15 +189,11 @@ do
 
   if test -n "$dir_arg"; then
     dst=$src
-    src=
-
-    if test -d "$dst"; then
-      mkdircmd=:
-      chmodcmd=
-    else
-      mkdircmd=$mkdirprog
-    fi
+    dstdir=$dst
+    test -d "$dstdir"
+    dstdir_status=$?
   else
+
     # Waiting for this to be detected by the "$cpprog $src $dsttmp" command
     # might cause directories to be created, which would be especially bad
     # if $src (and thus $dsttmp) contains '*'.
@@ -208,53 +220,122 @@ do
        echo "$0: $dstarg: Is a directory" >&2
        exit 1
       fi
-      dst=$dst/`basename "$src"`
+      dstdir=$dst
+      dst=$dstdir/`basename "$src"`
+      dstdir_status=0
+    else
+      # Prefer dirname, but fall back on a substitute if dirname fails.
+      dstdir=`
+       (dirname "$dst") 2>/dev/null ||
+       expr X"$dst" : 'X\(.*[^/]\)//*[^/][^/]*/*$' \| \
+            X"$dst" : 'X\(//\)[^/]' \| \
+            X"$dst" : 'X\(//\)$' \| \
+            X"$dst" : 'X\(/\)' \| \
+            .       : '\(.\)' 2>/dev/null ||
+       echo X"$dst" |
+           sed '/^X\(.*[^/]\)\/\/*[^/][^/]*\/*$/{ s//\1/; q; }
+                 /^X\(\/\/\)[^/].*/{ s//\1/; q; }
+                 /^X\(\/\/\)$/{ s//\1/; q; }
+                 /^X\(\/\).*/{ s//\1/; q; }
+                 s/.*/./; q'
+      `
+
+      test -d "$dstdir"
+      dstdir_status=$?
     fi
   fi
 
-  # This sed command emulates the dirname command.
-  dstdir=`echo "$dst" | sed -e 's,/*$,,;s,[^/]*$,,;s,/*$,,;s,^$,.,'`
+  obsolete_mkdir_used=false
 
-  # Make sure that the destination directory exists.
-
-  # Skip lots of stat calls in the usual case.
-  if test ! -d "$dstdir"; then
-    case $dstdir in
-      /*) pathcomp=/ ;;
-      -*) pathcomp=./ ;;
-      *)  pathcomp= ;;
+  if test $dstdir_status != 0; then
+    case $posix_mkdir in
+      '')
+       posix_mkdir=false
+       if $mkdirprog -m $test_mode -p -- / >/dev/null 2>&1; then
+         posix_mkdir=true
+       else
+         # Remove any dirs left behind by ancient mkdir implementations.
+         rmdir ./-m "$test_mode" ./-p ./-- 2>/dev/null
+       fi ;;
     esac
-    oIFS=$IFS
-    IFS=/
-    set fnord $dstdir
-    shift
-    IFS=$oIFS
-
-    for d
-    do
-      test "x$d" = x && continue
-
-      pathcomp=$pathcomp$d
-      if test ! -d "$pathcomp"; then
-        $mkdirprog "$pathcomp"
-       # mkdir can fail with a `File exist' error in case several
-       # install-sh are creating the directory concurrently.  This
-       # is OK.
-       test -d "$pathcomp" || exit 1
-      fi
-      pathcomp=$pathcomp/
-    done
+
+    if
+      $posix_mkdir && {
+
+       # With -d, create the new directory with the user-specified mode.
+       # Otherwise, create it using the same intermediate mode that
+       # mkdir -p would use when creating intermediate directories.
+       # POSIX says that this mode is "$(umask -S),u+wx", so use that
+       # if umask -S works.
+
+       if test -n "$dir_arg"; then
+         mkdir_mode=$mode
+       else
+         case $intermediate_mode in
+           '')
+             if umask_S=`(umask -S) 2>/dev/null`; then
+               intermediate_mode=$umask_S,u+wx
+             else
+               intermediate_mode=$test_mode
+             fi ;;
+         esac
+         mkdir_mode=$intermediate_mode
+       fi
+
+       $mkdirprog -m "$mkdir_mode" -p -- "$dstdir"
+      }
+    then :
+    else
+
+      # mkdir does not conform to POSIX, or it failed possibly due to
+      # a race condition.  Create the directory the slow way, step by
+      # step, checking for races as we go.
+
+      case $dstdir in
+       /*) pathcomp=/ ;;
+       -*) pathcomp=./ ;;
+       *)  pathcomp= ;;
+      esac
+
+      case $posix_glob in
+        '')
+         if (set -f) 2>/dev/null; then
+           posix_glob=true
+         else
+           posix_glob=false
+         fi ;;
+      esac
+
+      oIFS=$IFS
+      IFS=/
+      $posix_glob && set -f
+      set fnord $dstdir
+      shift
+      $posix_glob && set +f
+      IFS=$oIFS
+
+      for d
+      do
+       test "x$d" = x && continue
+
+       pathcomp=$pathcomp$d
+       if test ! -d "$pathcomp"; then
+         $mkdirprog "$pathcomp"
+         # Don't fail if two instances are running concurrently.
+         test -d "$pathcomp" || exit 1
+       fi
+       pathcomp=$pathcomp/
+      done
+      obsolete_mkdir_used=true
+    fi
   fi
 
   if test -n "$dir_arg"; then
-    $doit $mkdircmd "$dst" \
-      && { test -z "$chowncmd" || $doit $chowncmd "$dst"; } \
-      && { test -z "$chgrpcmd" || $doit $chgrpcmd "$dst"; } \
-      && { test -z "$stripcmd" || $doit $stripcmd "$dst"; } \
-      && { test -z "$chmodcmd" || $doit $chmodcmd "$dst"; }
-
+    { test -z "$chowncmd" || $doit $chowncmd "$dst"; } &&
+    { test -z "$chgrpcmd" || $doit $chgrpcmd "$dst"; } &&
+    { test "$obsolete_mkdir_used$chowncmd$chgrpcmd" = false ||
+      test -z "$chmodcmd" || $doit $chmodcmd "$mode" "$dst"; } || exit 1
   else
-    dstfile=`basename "$dst"`
 
     # Make a couple of temp file names in the proper directory.
     dsttmp=$dstdir/_inst.$$_
@@ -262,7 +343,6 @@ do
 
     # Trap to clean up those temp files at exit.
     trap 'ret=$?; rm -f "$dsttmp" "$rmtmp" && exit $ret' 0
-    trap '(exit $?); exit' 1 2 13 15
 
     # Copy the file name to the temp name.
     $doit $cpprog "$src" "$dsttmp" &&
@@ -276,10 +356,10 @@ do
     { test -z "$chowncmd" || $doit $chowncmd "$dsttmp"; } \
       && { test -z "$chgrpcmd" || $doit $chgrpcmd "$dsttmp"; } \
       && { test -z "$stripcmd" || $doit $stripcmd "$dsttmp"; } \
-      && { test -z "$chmodcmd" || $doit $chmodcmd "$dsttmp"; } &&
+      && { test -z "$chmodcmd" || $doit $chmodcmd "$mode" "$dsttmp"; } &&
 
     # Now rename the file to the real destination.
-    { $doit $mvcmd -f "$dsttmp" "$dstdir/$dstfile" 2>/dev/null \
+    { $doit $mvcmd -f "$dsttmp" "$dst" 2>/dev/null \
       || {
           # The rename failed, perhaps because mv can't rename something else
           # to itself, or perhaps because mv is so ancient that it does not
@@ -291,11 +371,12 @@ do
           # reasons.  In this case, the final cleanup might fail but the new
           # file should still install successfully.
           {
-            if test -f "$dstdir/$dstfile"; then
-              $doit $rmcmd -f "$dstdir/$dstfile" 2>/dev/null \
-              || $doit $mvcmd -f "$dstdir/$dstfile" "$rmtmp" 2>/dev/null \
+            if test -f "$dst"; then
+              $doit $rmcmd -f "$dst" 2>/dev/null \
+              || { $doit $mvcmd -f "$dst" "$rmtmp" 2>/dev/null \
+                    && { $doit $rmcmd -f "$rmtmp" 2>/dev/null; :; }; }\
               || {
-                echo "$0: cannot unlink or rename $dstdir/$dstfile" >&2
+                echo "$0: cannot unlink or rename $dst" >&2
                 (exit 1); exit 1
               }
             else
@@ -304,16 +385,13 @@ do
           } &&
 
           # Now rename the file to the real destination.
-          $doit $mvcmd "$dsttmp" "$dstdir/$dstfile"
+          $doit $mvcmd "$dsttmp" "$dst"
         }
-    }
-  fi || { (exit 1); exit 1; }
-done
+    } || exit 1
 
-# The final little trick to "correctly" pass the exit status to the exit trap.
-{
-  (exit 0); exit 0
-}
+    trap - 0
+  fi
+done
 
 # Local variables:
 # eval: (add-hook 'write-file-hooks 'time-stamp)
Index: tests/instspc.test
===================================================================
RCS file: /cvs/automake/automake/tests/instspc.test,v
retrieving revision 1.6
diff -p -u -r1.6 instspc.test
--- tests/instspc.test  19 Jul 2005 20:08:40 -0000      1.6
+++ tests/instspc.test  6 Sep 2005 19:14:46 -0000
@@ -152,7 +152,7 @@ done
 
 # The list of the above file names that cannot be used as a build directory
 # on a POSIX host.  This list should be empty, but is not due to limitations
-# in Autoconf, Automake, Make, or M4.
+# in Autoconf, Automake, Make, M4, or the shell.
 expected_build_failures='
 "
 #
@@ -172,7 +172,6 @@ expected_install_failures='
 #
 $
 '\''
-*
 `
 '"$lf"'
 a'"${lf}"'b'