Merge pull request #4924 from nigredo-tori/fix-4917

Make createDir return discardable bool

2 files changed, 69 insertions, 9 deletions

diff --git a/lib/pure/os.nim b/lib/pure/os.nim
index 001d3d250..8f8f36b80 100644
--- a/lib/pure/os.nim
+++ b/lib/pure/os.nim
@@ -1051,27 +1051,61 @@ proc removeDir*(dir: string) {.rtl, extern: "nos$1", tags: [
     of pcDir: removeDir(path)
   rawRemoveDir(dir)
 
-proc rawCreateDir(dir: string) =
+proc rawCreateDir(dir: string): bool =
+  # Try to create one directory (not the whole path).
+  # returns `true` for success, `false` if the path has previously existed
+  #
+  # This is a thin wrapper over mkDir (or alternatives on other systems),
+  # so in case of a pre-existing path we don't check that it is a directory.
   when defined(solaris):
-    if mkdir(dir, 0o777) != 0'i32 and errno != EEXIST and errno != ENOSYS:
+    let res = mkdir(dir, 0o777)
+    if res == 0'i32:
+      result = true
+    elif errno in {EEXIST, ENOSYS}:
+      result = false
+    else:
       raiseOSError(osLastError())
   elif defined(unix):
-    if mkdir(dir, 0o777) != 0'i32 and errno != EEXIST:
+    let res = mkdir(dir, 0o777)
+    if res == 0'i32:
+      result = true
+    elif errno == EEXIST:
+      result = false
+    else:
+      echo res
       raiseOSError(osLastError())
   else:
     when useWinUnicode:
       wrapUnary(res, createDirectoryW, dir)
     else:
-      var res = createDirectoryA(dir)
-    if res == 0'i32 and getLastError() != 183'i32:
+      let res = createDirectoryA(dir)
+
+    if res != 0'i32:
+      result = true
+    elif getLastError() == 183'i32:
+      result = false
+    else:
       raiseOSError(osLastError())
 
-proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} =
+proc existsOrCreateDir*(dir: string): bool =
+  ## Check if a `directory`:idx: `dir` exists, and create it otherwise.
+  ##
+  ## Does not create parent directories (fails if parent does not exist).
+  ## Returns `true` if the directory already exists, and `false`
+  ## otherwise.
+  result = not rawCreateDir(dir)
+  if result:
+    # path already exists - need to check that it is indeed a directory
+    if not existsDir(dir):
+      raise newException(IOError, "Failed to create the directory")
+
+proc createDir*(dir: string) {.rtl, extern: "nos$1",
+  tags: [WriteDirEffect, ReadDirEffect].} =
   ## Creates the `directory`:idx: `dir`.
   ##
   ## The directory may contain several subdirectories that do not exist yet.
   ## The full path is created. If this fails, `OSError` is raised. It does **not**
-  ## fail if the path already exists because for most usages this does not
+  ## fail if the directory already exists because for most usages this does not
   ## indicate an error.
   var omitNext = false
   when doslike:
@@ -1081,8 +1115,12 @@ proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} =
       if omitNext:
         omitNext = false
       else:
-        rawCreateDir(substr(dir, 0, i-1))
-  rawCreateDir(dir)
+        discard existsOrCreateDir(substr(dir, 0, i-1))
+
+  # The loop does not create the dir itself if it doesn't end in separator
+  if dir.len > 0 and not omitNext and
+     dir[^1] notin {DirSep, AltSep}:
+    discard existsOrCreateDir(dir)
 
 proc copyDir*(source, dest: string) {.rtl, extern: "nos$1",
   tags: [WriteIOEffect, ReadIOEffect], benign.} =
diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim
index 1ddaacfcb..1ef02f97e 100644
--- a/tests/stdlib/tos.nim
+++ b/tests/stdlib/tos.nim
@@ -36,6 +36,9 @@ false
 false
 false
 false
+true
+true
+Raises
 '''
 """
 # test os path creation, iteration, and deletion
@@ -86,3 +89,22 @@ for file in files:
 
 removeDir(dname)
 echo dirExists(dname)
+
+# createDir should create recursive directories
+createDir(dirs[0] / dirs[1])
+echo dirExists(dirs[0] / dirs[1]) # true
+removeDir(dirs[0])
+
+# createDir should properly handle trailing separator
+createDir(dname / "")
+echo dirExists(dname) # true
+removeDir(dname)
+
+# createDir should raise IOError if the path exists
+# and is not a directory
+open(dname, fmWrite).close
+try:
+  createDir(dname)
+except IOError:
+  echo "Raises"
+removeFile(dname)