Re: cygwin semantics of ..

[Paul Eggert]

Here's a proposed patch for that Cygwin incompatibility, which also
addresses several other issues.  This is probably too big for the next
stable release, though, as it's a bit more than just a "bug fix".

One of the side effects of this change is that 'mkdir' and 'install'
may not chdir back to the original working directory.  Is this another
Cygwin issue?  I seem to recall that chdir can affect the parent
process under DOS.


NEWS item:

 * 'install -d' and 'install -D' now create parent directories with mode
   755 as adjusted by the umask, instead of ignoring the umask.  This is
   for compatibility with BSD 'install'; it's also a bit more conservative.
   It is now the user's responsibility to avoid overly-restrictive
   umasks like 777.

ChangeLog entry TBD (sorry).

Index: lib/mkdir-p.c
===================================================================
RCS file: /fetish/cu/lib/mkdir-p.c,v
retrieving revision 1.15
diff -p -u -r1.15 mkdir-p.c
--- lib/mkdir-p.c       24 Oct 2005 13:35:59 -0000      1.15
+++ lib/mkdir-p.c       27 Oct 2005 20:43:59 -0000
@@ -47,12 +47,23 @@
 
 #define WX_USR (S_IWUSR | S_IXUSR)
 
+/* Return true if DIR is a directory.  */
+static bool
+direxists (char const *dir)
+{
+  struct stat st;
+  return stat (dir, &st) == 0 && S_ISDIR (st.st_mode);
+}
+
 /* Ensure that the directory ARG exists.
 
    Create any leading directories that don't already exist, with
    permissions PARENT_MODE.
    If the last element of ARG does not exist, create it as
    a new directory with permissions MODE.
+   Assume that the umask is zero when creating directories.
+   Also, assume (PARENT_MODE & ~S_IRWXUGO) == 0.
+
    If OWNER and GROUP are non-negative, use them to set the UID and GID of
    any created directories.
    If VERBOSE_FMT_STRING is nonzero, use it as a printf format
@@ -61,7 +72,8 @@
    If PRESERVE_EXISTING is true and ARG is an existing directory,
    then do not attempt to set its permissions and ownership.
 
-   Set *CWD_ERRNO to a (nonzero) error number if this
+   Do not try to restore the working directory if CWD_ERRNO is null.
+   Otherwise, set *CWD_ERRNO to a (nonzero) error number if this
    function has changed the current working directory and is unable to
    restore it to its initial state.  Do not change
    *CWD_ERRNO otherwise.
@@ -80,215 +92,147 @@ make_dir_parents (char const *arg,
                  char const *verbose_fmt_string,
                  int *cwd_errno)
 {
-  struct stat stats;
   bool retval = true;
   bool do_chdir = false;       /* Whether to chdir before each mkdir.  */
   struct saved_cwd cwd;
   bool cwd_problem = false;
-  char const *fixup_permissions_dir = NULL;
   char const *full_dir = arg;
+  char *slash;
+  char *basename_dir;
+  char *dir;
 
-  struct ptr_list
-  {
-    char *dirname_end;
-    struct ptr_list *next;
-  };
-  struct ptr_list *leading_dirs = NULL;
+  /* Make a copy of ARG that we can scribble NULs on.  */
+  dir = (char *) alloca (strlen (arg) + 1);
+  strcpy (dir, arg);
+  strip_trailing_slashes (dir);
+  full_dir = dir;
 
-  if (stat (arg, &stats) == 0)
-    {
-      if (! S_ISDIR (stats.st_mode))
-       {
-         error (0, 0, _("%s exists but is not a directory"), quote (arg));
-         return false;
-       }
+  /* If we can record the current working directory, we may be able
+     to do the chdir optimization.  */
+  do_chdir = (!cwd_errno || save_cwd (&cwd) == 0);
 
-      if (!preserve_existing)
-       fixup_permissions_dir = arg;
-    }
-  else if (errno != ENOENT || !*arg)
+  /* If we've saved the cwd and DIR is an absolute file name,
+     we must chdir to `/' in order to enable the chdir optimization.
+     So if chdir ("/") fails, turn off the optimization.  */
+  if (do_chdir && dir[0] == '/')
     {
-      error (0, errno, "%s", quote (arg));
-      return false;
-    }
-  else
-    {
-      char *slash;
-      mode_t tmp_mode;         /* Initial perms for leading dirs.  */
-      bool re_protect;         /* Should leading dirs be unwritable? */
-      char *basename_dir;
-      char *dir;
-
-      /* Temporarily relax umask in case it's overly restrictive.  */
-      mode_t oldmask = umask (0);
-
-      /* Make a copy of ARG that we can scribble NULs on.  */
-      dir = (char *) alloca (strlen (arg) + 1);
-      strcpy (dir, arg);
-      strip_trailing_slashes (dir);
-      full_dir = dir;
-
-      /* If leading directories shouldn't be writable or executable,
-        or should have set[ug]id or sticky bits set and we are setting
-        their owners, we need to fix their permissions after making them.  */
-      if (((parent_mode & WX_USR) != WX_USR)
-         || ((owner != (uid_t) -1 || group != (gid_t) -1)
-             && (parent_mode & (S_ISUID | S_ISGID | S_ISVTX)) != 0))
-       {
-         tmp_mode = S_IRWXU;
-         re_protect = true;
-       }
-      else
+      /* POSIX says "//" might be special, so chdir to "//" if the
+        file name starts with exactly two slashes.  */
+      char const *root = "//" + (dir[1] != '/' || dir[2] == '/');
+      if (chdir (root) != 0)
        {
-         tmp_mode = parent_mode;
-         re_protect = false;
+         if (cwd_errno)
+           free_cwd (&cwd);
+         do_chdir = false;
        }
+    }
 
-      /* If we can record the current working directory, we may be able
-        to do the chdir optimization.  */
-      do_chdir = (save_cwd (&cwd) == 0);
-
-      /* If we've saved the cwd and DIR is an absolute file name,
-        we must chdir to `/' in order to enable the chdir optimization.
-         So if chdir ("/") fails, turn off the optimization.  */
-      if (do_chdir && dir[0] == '/')
-       {
-         /* POSIX says "//" might be special, so chdir to "//" if the
-            file name starts with exactly two slashes.  */
-         char const *root = "//" + (dir[1] != '/' || dir[2] == '/');
-         if (chdir (root) != 0)
-           {
-             free_cwd (&cwd);
-             do_chdir = false;
-           }
-       }
+  slash = dir;
 
-      slash = dir;
+  /* Skip over leading slashes.  */
+  while (*slash == '/')
+    slash++;
 
-      /* Skip over leading slashes.  */
-      while (*slash == '/')
-       slash++;
+  while (true)
+    {
+      bool dir_known_to_exist;
+      int mkdir_errno;
 
-      while (true)
-       {
-         bool dir_known_to_exist;
-         int mkdir_errno;
+      /* slash points to the leftmost unprocessed component of dir.  */
+      basename_dir = slash;
 
-         /* slash points to the leftmost unprocessed component of dir.  */
-         basename_dir = slash;
+      slash = strchr (slash, '/');
+      if (slash == NULL)
+       break;
 
-         slash = strchr (slash, '/');
-         if (slash == NULL)
-           break;
-
-         /* If we're *not* doing chdir before each mkdir, then we have to refer
-            to the target using the full (multi-component) directory name.  */
-         if (!do_chdir)
-           basename_dir = dir;
-
-         *slash = '\0';
-         dir_known_to_exist = (mkdir (basename_dir, tmp_mode) == 0);
-         mkdir_errno = errno;
+      /* If we're *not* doing chdir before each mkdir, then we have to refer
+        to the target using the full (multi-component) directory name.  */
+      if (!do_chdir)
+       basename_dir = dir;
 
-         if (dir_known_to_exist)
-           {
-             if (verbose_fmt_string)
-               error (0, 0, verbose_fmt_string, quote (dir));
+      *slash = '\0';
+      dir_known_to_exist = (mkdir (basename_dir, parent_mode) == 0);
+      mkdir_errno = errno;
 
-             if ((owner != (uid_t) -1 || group != (gid_t) -1)
-                 && chown (basename_dir, owner, group)
+      if (dir_known_to_exist)
+       {
+         if (verbose_fmt_string)
+           error (0, 0, verbose_fmt_string, quote (dir));
+
+         if ((owner != (uid_t) -1 || group != (gid_t) -1)
+             && chown (basename_dir, owner, group)
 #if defined AFS && defined EPERM
-                 && errno != EPERM
+             && errno != EPERM
 #endif
-                 )
-               {
-                 error (0, errno, _("cannot change owner and/or group of %s"),
-                        quote (dir));
-                 retval = false;
-                 break;
-               }
-
-             if (re_protect)
-               {
-                 struct ptr_list *new = (struct ptr_list *)
-                   alloca (sizeof *new);
-                 new->dirname_end = slash;
-                 new->next = leading_dirs;
-                 leading_dirs = new;
-               }
-           }
-
-         /* If we were able to save the initial working directory,
-            then we can use chdir to change into each directory before
-            creating an entry in that directory.  This avoids making
-            mkdir process O(n^2) file name components.  */
-         if (do_chdir)
+             )
            {
-             if (chdir (basename_dir) == 0)
-               dir_known_to_exist = true;
-             else if (dir_known_to_exist)
-               {
-                 error (0, errno, _("cannot chdir to directory %s"),
-                        quote (dir));
-                 retval = false;
-                 break;
-               }
+             error (0, errno, _("cannot change owner and/or group of %s"),
+                    quote (dir));
+             retval = false;
+             break;
            }
-         else if (!dir_known_to_exist)
-           dir_known_to_exist = (stat (basename_dir, &stats) == 0
-                                 && S_ISDIR (stats.st_mode));
+       }
 
-         if (!dir_known_to_exist)
+      /* If possible, use chdir to change into each directory before
+        creating an entry in that directory.  This avoids making
+        mkdir process O(n^2) file name components.  */
+      if (do_chdir)
+       {
+         if (chdir (basename_dir) == 0)
+           dir_known_to_exist = true;
+         else if (dir_known_to_exist)
            {
-             error (0, mkdir_errno, _("cannot create directory %s"),
+             error (0, errno, _("cannot chdir to directory %s"),
                     quote (dir));
              retval = false;
              break;
            }
+       }
+      else if (! dir_known_to_exist)
+       dir_known_to_exist = direxists (basename_dir);
 
-         *slash++ = '/';
-
-         /* Avoid unnecessary calls to mkdir when given
-            file names containing multiple adjacent slashes.  */
-         while (*slash == '/')
-           slash++;
+      if (! dir_known_to_exist)
+       {
+         error (0, mkdir_errno, _("cannot create directory %s"),
+                quote (dir));
+         retval = false;
+         break;
        }
 
-      if (!do_chdir)
-       basename_dir = dir;
+      *slash++ = '/';
 
-      /* Done creating leading directories.  Restore original umask.  */
-      umask (oldmask);
+      /* Avoid unnecessary calls to mkdir when given
+        file names containing multiple adjacent slashes.  */
+      while (*slash == '/')
+       slash++;
+    }
 
-      /* We're done making leading directories.
-        Create the final component of the file name.  */
-      if (retval)
-       {
-         bool dir_known_to_exist = (mkdir (basename_dir, mode) == 0);
-         int mkdir_errno = errno;
-         struct stat sbuf;
-
-         if ( ! dir_known_to_exist)
-           dir_known_to_exist = (stat (basename_dir, &sbuf) == 0
-                                 && S_ISDIR (sbuf.st_mode));
+  if (!do_chdir)
+    basename_dir = dir;
 
-         if ( ! dir_known_to_exist)
-           {
-             error (0, mkdir_errno,
-                    _("cannot create directory %s"), quote (dir));
-             retval = false;
-           }
-         else
-           {
-             if (verbose_fmt_string)
-               error (0, 0, verbose_fmt_string, quote (dir));
-             fixup_permissions_dir = basename_dir;
-           }
+  /* We're done making leading directories.
+     Create the final component of the file name.  */
+  if (retval)
+    {
+      bool dir_known_to_exist = (mkdir (basename_dir, mode) == 0);
+      int mkdir_errno = errno;
+
+      if (! dir_known_to_exist)
+       dir_known_to_exist = direxists (basename_dir);
+
+      if (! dir_known_to_exist)
+       {
+         error (0, mkdir_errno,
+                _("cannot create directory %s"), quote (dir));
+         retval = false;
        }
     }
 
-  if (fixup_permissions_dir)
+  if (retval)
     {
+      if (verbose_fmt_string)
+       error (0, 0, verbose_fmt_string, quote (dir));
+
       /* chown must precede chmod because on some systems,
         chown clears the set[ug]id bits for non-superusers,
         resulting in incorrect permissions.
@@ -297,7 +241,7 @@ make_dir_parents (char const *arg,
 
       if (owner != (uid_t) -1 || group != (gid_t) -1)
        {
-         if (chown (fixup_permissions_dir, owner, group) != 0
+         if (chown (basename_dir, owner, group) != 0
 #ifdef AFS
              && errno != EPERM
 #endif
@@ -314,7 +258,7 @@ make_dir_parents (char const *arg,
         required to honor only the file permission bits.  In particular,
         it need not honor the `special' bits, so if MODE includes any
         special bits, set them here.  */
-      if ((mode & ~S_IRWXUGO) && chmod (fixup_permissions_dir, mode) != 0)
+      if ((mode & ~S_IRWXUGO) && chmod (basename_dir, mode) != 0)
        {
          error (0, errno, _("cannot change permissions of %s"),
                 quote (full_dir));
@@ -322,7 +266,7 @@ make_dir_parents (char const *arg,
        }
     }
 
-  if (do_chdir)
+  if (do_chdir && cwd_errno)
     {
       if (restore_cwd (&cwd) != 0)
        {
@@ -332,19 +276,5 @@ make_dir_parents (char const *arg,
       free_cwd (&cwd);
     }
 
-  /* If the mode for leading directories didn't include owner "wx"
-     privileges, reset their protections to the correct value.  */
-  for (; leading_dirs != NULL; leading_dirs = leading_dirs->next)
-    {
-      leading_dirs->dirname_end[0] = '\0';
-      if ((cwd_problem && *full_dir != '/')
-         || chmod (full_dir, parent_mode) != 0)
-       {
-         error (0, (cwd_problem ? 0 : errno),
-                _("cannot change permissions of %s"), quote (full_dir));
-         retval = false;
-       }
-    }
-
   return retval;
 }
Index: src/install.c
===================================================================
RCS file: /fetish/cu/src/install.c,v
retrieving revision 1.189
diff -p -u -r1.189 install.c
--- src/install.c       23 Sep 2005 20:50:49 -0000      1.189
+++ src/install.c       27 Oct 2005 20:44:00 -0000
@@ -96,9 +96,16 @@ static char *group_name;
 /* The group ID corresponding to `group_name'. */
 static gid_t group_id;
 
+/* The default mode for newly created files or directories.  */
+#define DEFAULT_MODE (S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH)
+
 /* The permissions to which the files will be set.  The umask has
-   no effect. */
-static mode_t mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
+   no effect.  */
+static mode_t mode = DEFAULT_MODE;
+
+/* The permissions to which parent directories will be set.  The umask
+   has no effect.  */
+static mode_t parent_mode;
 
 /* If true, strip executable files after copying them. */
 static bool strip_files;
@@ -211,7 +218,10 @@ main (int argc, char **argv)
   group_name = NULL;
   strip_files = false;
   dir_arg = false;
-  umask (0);
+
+  /* Clear the umask.  Then use its old value to determine parent
+     permissions.  This is for compatibility with BSD install.  */
+  parent_mode = DEFAULT_MODE & ~ umask (0);
 
   /* FIXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
      we'll actually use backup_suffix_string.  */
@@ -351,6 +361,15 @@ main (int argc, char **argv)
     {
       int i;
       int cwd_errno = 0;
+
+      /* Number of directory names that are either relative, or
+        precede a relative name.  */
+      int n_relatives_etc;
+
+      for (n_relatives_etc = n_files; 0 < n_relatives_etc; n_relatives_etc--)
+       if (IS_RELATIVE_FILE_NAME (file[n_relatives_etc - 1]))
+         break;
+
       for (i = 0; i < n_files; i++)
        {
          if (cwd_errno != 0 && IS_RELATIVE_FILE_NAME (file[i]))
@@ -360,9 +379,10 @@ main (int argc, char **argv)
            }
          else
            ok &=
-             make_dir_parents (file[i], mode, mode, owner_id, group_id, false,
+             make_dir_parents (file[i], mode, parent_mode,
+                               owner_id, group_id, false,
                                (x.verbose ? _("creating directory %s") : NULL),
-                               &cwd_errno);
+                               (i + 1 < n_relatives_etc ? &cwd_errno : NULL));
        }
     }
   else
@@ -409,14 +429,15 @@ install_file_in_file_parents (char const
         owner, group, and permissions for parent directories.  Remember
         that this option is intended mainly to help installers when the
         distribution doesn't provide proper install rules.  */
-      mode_t dir_mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
       int cwd_errno = 0;
-      ok = make_dir_parents (dest_dir, dir_mode, dir_mode,
+      ok = make_dir_parents (dest_dir, parent_mode, parent_mode,
                             owner_id, group_id, true,
                             (x->verbose ? _("creating directory %s") : NULL),
-                            &cwd_errno);
-      if (ok && cwd_errno != 0
-         && (IS_RELATIVE_FILE_NAME (from) || IS_RELATIVE_FILE_NAME (to)))
+                            ((IS_RELATIVE_FILE_NAME (from)
+                              || IS_RELATIVE_FILE_NAME (to))
+                             ? &cwd_errno
+                             : NULL));
+      if (ok && cwd_errno != 0)
        {
          error (0, cwd_errno, _("cannot return to current directory"));
          ok = false;
Index: src/mkdir.c
===================================================================
RCS file: /fetish/cu/src/mkdir.c,v
retrieving revision 1.101
diff -p -u -r1.101 mkdir.c
--- src/mkdir.c 14 Jun 2005 23:55:47 -0000      1.101
+++ src/mkdir.c 27 Oct 2005 20:44:00 -0000
@@ -87,6 +87,10 @@ main (int argc, char **argv)
   int optc;
   int cwd_errno = 0;
 
+  /* Number of arguments that are either relative, or precede a
+     relative file name.  */
+  int n_relatives_etc;
+
   initialize_main (&argc, &argv);
   program_name = argv[0];
   setlocale (LC_ALL, "");
@@ -139,9 +143,13 @@ main (int argc, char **argv)
          free (change);
        }
       else
-       umask (umask_value);
+       newmode &= ~umask_value;
     }
 
+  for (n_relatives_etc = argc; optind < n_relatives_etc; n_relatives_etc--)
+    if (IS_RELATIVE_FILE_NAME (argv[n_relatives_etc - 1]))
+      break;
+
   for (; optind < argc; ++optind)
     {
       char *dir = argv[optind];
@@ -157,7 +165,9 @@ main (int argc, char **argv)
          else
            ok = make_dir_parents (dir, newmode, parent_mode,
                                   -1, -1, true, verbose_fmt_string,
-                                  &cwd_errno);
+                                  (optind + 1 < n_relatives_etc
+                                   ? &cwd_errno
+                                   : NULL));
        }
       else
        {
Index: tests/mkdir/p-3
===================================================================
RCS file: /fetish/cu/tests/mkdir/p-3,v
retrieving revision 1.4
diff -p -u -r1.4 p-3
--- tests/mkdir/p-3     15 Jun 2005 08:32:11 -0000      1.4
+++ tests/mkdir/p-3     27 Oct 2005 20:44:00 -0000
@@ -38,13 +38,13 @@ b=`ls $p/a|tr -d '\n'`
 # With coreutils-5.3.0, this would fail with $b=bu.
 test "x$b" = xb || fail=1
 
-# Ensure that the re_protect code is run on absolute names, even
-# after failure to return to the initial working directory.
+# Ensure that install fails when the parent directory permissions are messed 
up.
+# This is for compatibility with ginstall.
 # This is actually a test of the underlying mkdir-p.c code.
 # The part in question cannot be tested via mkdir(1) because that
 # program cannot create leading directories that lack u=wx permissions,
 # so we have to test with install (aka ginstall in the build directory).
-(cd no-acce3s; chmod 0 . && ginstall -m 0 -d $p/c/b $p/y/z) || fail=1
+(cd no-acce3s; umask 777; ginstall -d $p/c/b $p/y/z) && fail=1
 p=`ls -ld $p/y|sed 's/ .*//'`
 case $p in d---------);; *) fail=1;; esac