Merge pull request #3041 from ZyX-I/better-mkdir

Move recursive directory creation function to os/fs.c

runtime/doc/eval.txt

@@ -4573,15 +4573,16 @@ mkdir({name} [, {path} [, {prot}]])
 		If {prot} is given it is used to set the protection bits of
 		the new directory.  The default is 0755 (rwxr-xr-x: r/w for
 		the user readable for others).	Use 0700 to make it unreadable
-		for others.  This is only used for the last part of {name}.
+		for others.
-		Thus if you create /tmp/foo/bar then /tmp/foo will be created
+									{Nvim}
-		with 0755.
+		{prot} is applied for all parts of {name}.  Thus if you create
-		Example: >
+		/tmp/foo/bar then /tmp/foo will be created with 0700. Example: >
 			:call mkdir($HOME . "/tmp/foo/bar", "p", 0700)
 <		This function is not available in the |sandbox|.
-		Not available on all systems.  To check use: >
+
-			:if exists("*mkdir")
+		If you try to create an existing directory with {path} set to 
-<
+		"p" mkdir() will silently exit.
+
 							*mode()*
 mode([expr])	Return a string that indicates the current mode.
 		If [expr] is supplied and it evaluates to a non-zero Number or

runtime/doc/vim_diff.txt

@@ -64,6 +64,14 @@ are always available and may be used simultaneously in separate plugins.  The
 `neovim` pip package must be installed to use Python plugins in Nvim (see
 |nvim-python|).
 
+|mkdir()| behaviour changed:
+1. Assuming /tmp/foo does not exist and /tmp can be written to 
+   mkdir('/tmp/foo/bar', 'p', 0700) will create both /tmp/foo and /tmp/foo/bar 
+   with 0700 permissions. Vim mkdir will create /tmp/foo with 0755.
+2. If you try to create an existing directory with `'p'` (e.g. mkdir('/', 
+   'p')) mkdir() will silently exit. In Vim this was an error.
+3. mkdir() error messages now include strerror() text when mkdir fails.
+
 ==============================================================================
 4. New Features						     *nvim-features-new*
 

src/nvim/eval.c

@@ -11684,33 +11684,6 @@ static void f_min(typval_T *argvars, typval_T *rettv)
   max_min(argvars, rettv, FALSE);
 }
 
-
-/*
- * Create the directory in which "dir" is located, and higher levels when
- * needed.
- */
-static int mkdir_recurse(char_u *dir, int prot)
-{
-  char_u      *p;
-  char_u      *updir;
-  int r = FAIL;
-
-  /* Get end of directory name in "dir".
-   * We're done when it's "/" or "c:/". */
-  p = path_tail_with_sep(dir);
-  if (p <= get_past_head(dir))
-    return OK;
-
-  /* If the directory exists we're done.  Otherwise: create it.*/
-  updir = vim_strnsave(dir, (int)(p - dir));
-  if (os_isdir(updir))
-    r = OK;
-  else if (mkdir_recurse(updir, prot) == OK)
-    r = vim_mkdir_emsg(updir, prot);
-  xfree(updir);
-  return r;
-}
-
 /*
  * "mkdir()" function
  */
@@ -11735,8 +11708,19 @@ static void f_mkdir(typval_T *argvars, typval_T *rettv)
     if (argvars[1].v_type != VAR_UNKNOWN) {
       if (argvars[2].v_type != VAR_UNKNOWN)
         prot = get_tv_number_chk(&argvars[2], NULL);
-      if (prot != -1 && STRCMP(get_tv_string(&argvars[1]), "p") == 0)
+      if (prot != -1 && STRCMP(get_tv_string(&argvars[1]), "p") == 0) {
-        mkdir_recurse(dir, prot);
+        char *failed_dir;
+        int ret = os_mkdir_recurse((char *) dir, prot, &failed_dir);
+        if (ret != 0) {
+          EMSG3(_(e_mkdir), failed_dir, os_strerror(ret));
+          xfree(failed_dir);
+          rettv->vval.v_number = FAIL;
+          return;
+        } else {
+          rettv->vval.v_number = OK;
+          return;
+        }
+      }
     }
     rettv->vval.v_number = prot == -1 ? FAIL : vim_mkdir_emsg(dir, prot);
   }

src/nvim/ex_docmd.c

@@ -7547,8 +7547,9 @@ static void ex_mkrc(exarg_T *eap)
 
 int vim_mkdir_emsg(char_u *name, int prot)
 {
-  if (os_mkdir((char *)name, prot) != 0) {
+  int ret;
-    EMSG2(_("E739: Cannot create directory: %s"), name);
+  if ((ret = os_mkdir((char *)name, prot)) != 0) {
+    EMSG3(_(e_mkdir), name, os_strerror(ret));
     return FAIL;
   }
   return OK;

src/nvim/globals.h

@@ -1117,6 +1117,7 @@ EXTERN char_u e_jobtblfull[] INIT(= N_("E901: Job table is full"));
 EXTERN char_u e_jobexe[] INIT(= N_("E902: \"%s\" is not an executable"));
 EXTERN char_u e_jobnotpty[] INIT(= N_("E904: Job is not connected to a pty"));
 EXTERN char_u e_libcall[] INIT(= N_("E364: Library call failed for \"%s()\""));
+EXTERN char_u e_mkdir[] INIT(= N_("E739: Cannot create directory %s: %s"));
 EXTERN char_u e_markinval[] INIT(= N_("E19: Mark has invalid line number"));
 EXTERN char_u e_marknotset[] INIT(= N_("E20: Mark not set"));
 EXTERN char_u e_modifiable[] INIT(= N_(

src/nvim/os/fs.c

@@ -316,7 +316,7 @@ int os_rename(const char_u *path, const char_u *new_path)
 
 /// Make a directory.
 ///
-/// @return `0` for success, non-zero for failure.
+/// @return `0` for success, -errno for failure.
 int os_mkdir(const char *path, int32_t mode)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -326,6 +326,54 @@ int os_mkdir(const char *path, int32_t mode)
   return result;
 }
 
+/// Make a directory, with higher levels when needed
+///
+/// @param[in]  dir  Directory to create.
+/// @param[in]  mode  Permissions for the newly-created directory.
+/// @param[out]  failed_dir  If it failed to create directory, then this
+///                          argument is set to an allocated string containing
+///                          the name of the directory which os_mkdir_recurse
+///                          failed to create. I.e. it will contain dir or any
+///                          of the higher level directories.
+///
+/// @return `0` for success, -errno for failure.
+int os_mkdir_recurse(const char *const dir, int32_t mode,
+                     char **const failed_dir)
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  // Get end of directory name in "dir".
+  // We're done when it's "/" or "c:/".
+  const size_t dirlen = strlen(dir);
+  char *const curdir = xmemdupz(dir, dirlen);
+  char *const past_head = (char *) get_past_head((char_u *) curdir);
+  char *e = curdir + dirlen;
+  const char *const real_end = e;
+  const char past_head_save = *past_head;
+  while (!os_isdir((char_u *) curdir)) {
+    e = (char *) path_tail_with_sep((char_u *) curdir);
+    if (e <= past_head) {
+      *past_head = NUL;
+      break;
+    }
+    *e = NUL;
+  }
+  while (e != real_end) {
+    if (e > past_head) {
+      *e = '/';
+    } else {
+      *past_head = past_head_save;
+    }
+    e += strlen(e);
+    int ret;
+    if ((ret = os_mkdir(curdir, mode)) != 0) {
+      *failed_dir = curdir;
+      return ret;
+    }
+  }
+  xfree(curdir);
+  return 0;
+}
+
 /// Create a unique temporary directory.
 ///
 /// @param[in] template Template of the path to the directory with XXXXXX

src/nvim/os/os_defs.h

@@ -135,4 +135,9 @@
 // For dup(3).
 #define HAVE_DUP
 
+/// Function to convert -errno error to char * error description
+///
+/// -errno errors are returned by a number of os functions.
+#define os_strerror uv_strerror
+
 #endif  // NVIM_OS_OS_DEFS_H

test/unit/os/fs_spec.lua

@@ -486,6 +486,16 @@ describe('fs function', function()
       return fs.os_rmdir(to_cstr(path))
     end
 
+    local function os_mkdir_recurse(path, mode)
+      local failed_str = ffi.new('char *[1]', {nil})
+      local ret = fs.os_mkdir_recurse(path, mode, failed_str)
+      local str = failed_str[0]
+      if str ~= nil then
+        str = ffi.string(str)
+      end
+      return ret, str
+    end
+
     describe('os_mkdir', function()
       it('returns non-zero when given an already existing directory', function()
         local mode = ffi.C.kS_IRUSR + ffi.C.kS_IWUSR + ffi.C.kS_IXUSR
@@ -501,6 +511,59 @@ describe('fs function', function()
       end)
     end)
 
+    describe('os_mkdir_recurse', function()
+      it('returns zero when given an already existing directory', function()
+        local mode = ffi.C.kS_IRUSR + ffi.C.kS_IWUSR + ffi.C.kS_IXUSR
+        local ret, failed_str = os_mkdir_recurse('unit-test-directory', mode)
+        eq(0, ret)
+        eq(nil, failed_str)
+      end)
+
+      it('fails to create a directory where there is a file', function()
+        local mode = ffi.C.kS_IRUSR + ffi.C.kS_IWUSR + ffi.C.kS_IXUSR
+        local ret, failed_str = os_mkdir_recurse(
+            'unit-test-directory/test.file', mode)
+        neq(0, ret)
+        eq('unit-test-directory/test.file', failed_str)
+      end)
+
+      it('fails to create a directory where there is a file in path', function()
+        local mode = ffi.C.kS_IRUSR + ffi.C.kS_IWUSR + ffi.C.kS_IXUSR
+        local ret, failed_str = os_mkdir_recurse(
+            'unit-test-directory/test.file/test', mode)
+        neq(0, ret)
+        eq('unit-test-directory/test.file', failed_str)
+      end)
+
+      it('succeeds to create a directory', function()
+        local mode = ffi.C.kS_IRUSR + ffi.C.kS_IWUSR + ffi.C.kS_IXUSR
+        local ret, failed_str = os_mkdir_recurse(
+            'unit-test-directory/new-dir-recurse', mode)
+        eq(0, ret)
+        eq(nil, failed_str)
+        eq(true, os_isdir('unit-test-directory/new-dir-recurse'))
+        lfs.rmdir('unit-test-directory/new-dir-recurse')
+        eq(false, os_isdir('unit-test-directory/new-dir-recurse'))
+      end)
+
+      it('succeeds to create a directory tree', function()
+        local mode = ffi.C.kS_IRUSR + ffi.C.kS_IWUSR + ffi.C.kS_IXUSR
+        local ret, failed_str = os_mkdir_recurse(
+            'unit-test-directory/new-dir-recurse/1/2/3', mode)
+        eq(0, ret)
+        eq(nil, failed_str)
+        eq(true, os_isdir('unit-test-directory/new-dir-recurse'))
+        eq(true, os_isdir('unit-test-directory/new-dir-recurse/1'))
+        eq(true, os_isdir('unit-test-directory/new-dir-recurse/1/2'))
+        eq(true, os_isdir('unit-test-directory/new-dir-recurse/1/2/3'))
+        lfs.rmdir('unit-test-directory/new-dir-recurse/1/2/3')
+        lfs.rmdir('unit-test-directory/new-dir-recurse/1/2')
+        lfs.rmdir('unit-test-directory/new-dir-recurse/1')
+        lfs.rmdir('unit-test-directory/new-dir-recurse')
+        eq(false, os_isdir('unit-test-directory/new-dir-recurse'))
+      end)
+    end)
+
     describe('os_rmdir', function()
       it('returns non_zero when given a non-existing directory', function()
         neq(0, (os_rmdir('non-existing-directory')))