summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorhavardjohn <havard.mjaavatten@outlook.com>2022-09-11 22:51:39 +0200
committerGitHub <noreply@github.com>2022-09-11 16:51:39 -0400
commit68f92af17c4d38bdca348ae956447b2571b74198 (patch)
tree07b8d47210f380cfba658ade6f6ba766db34019d
parent088487f652638a745e8e7e440a8a3b381239597b (diff)
downloadNim-68f92af17c4d38bdca348ae956447b2571b74198.tar.gz
Fix cannot create Windows directory in root (#20311)
* Fix cannot create Windows directory in root

Fixes #20306, a regression bug with `createDir` caused by
`23e0160af283bb0bb573a86145e6c1c792780d49`.

The issue is that, if the path consists only of a drive and a single
directory (e.g. "Y:\nimcache2" in the original issue), then no
directories will be created. This works fine if there are multiple
directories (e.g. "Y:\nimcache2\test").

In the case of "Y:\nimcache2", `omitNext` in `createDir` is `false` on
the last condition in `createDir`. This means that the "nimcache2"
directory will not be created, and no exception will be raised.

Fixed by refactoring to use `parentDirs` iterator instead of iterating
over the string characters. Motivation is reduced code complexity.

Will not test the specific "C:\test" `createDir` case, since there is no
standard Windows drive with write permissions in the root. Creating a
custom drive-mapping to Windows Temp is a non-option. That could mess
up some users running the test.

Added `parentDirs` tests since they are lacking on for POSIX paths.

* Fix `createDir("")` causing error

The change to `createDir` caused `createDir("")` to raise an error,
where it previously didn't. Fixed so `createDir("")` does not fail, and
added test case.
-rw-r--r--lib/pure/os.nim27
-rw-r--r--tests/stdlib/tos.nim19
2 files changed, 26 insertions, 20 deletions
diff --git a/lib/pure/os.nim b/lib/pure/os.nim
index 5541e6f15..5c387ec03 100644
--- a/lib/pure/os.nim
+++ b/lib/pure/os.nim
@@ -2553,25 +2553,14 @@ proc createDir*(dir: string) {.rtl, extern: "nos$1",
   ## * `copyDir proc`_
   ## * `copyDirWithPermissions proc`_
   ## * `moveDir proc`_
-  var omitNext = false
-  when doslikeFileSystem:
-    var start = 1
-    if isAbsolute(dir):
-      omitNext = true
-      start = dir.splitDrive.drive.len + 1
-  else:
-    let start = 1
-  for i in start.. dir.len-1:
-    if dir[i] in {DirSep, AltSep}:
-      if omitNext:
-        omitNext = false
-      else:
-        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)
+  if dir == "":
+    return
+  var omitNext = isAbsolute(dir)
+  for p in parentDirs(dir, fromRoot=true):
+    if omitNext:
+      omitNext = false
+    else:
+      discard existsOrCreateDir(p)
 
 proc copyDir*(source, dest: string) {.rtl, extern: "nos$1",
   tags: [ReadDirEffect, WriteIOEffect, ReadIOEffect], benign, noWeirdTarget.} =
diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim
index 08088d707..d02fed714 100644
--- a/tests/stdlib/tos.nim
+++ b/tests/stdlib/tos.nim
@@ -156,6 +156,9 @@ block fileOperations:
   doAssert fileExists("../dest/a/file.txt")
   removeDir("../dest")
 
+  # createDir should not fail if `dir` is empty
+  createDir("")
+
   # Symlink handling in `copyFile`, `copyFileWithPermissions`, `copyFileToDir`,
   # `copyDir`, `copyDirWithPermissions`, `moveFile`, and `moveDir`.
   block:
@@ -712,8 +715,10 @@ block: # isAdmin
   # In Azure on POSIX tests run as a normal user
   if isAzure and defined(posix): doAssert not isAdmin()
 
+import std/sequtils
+
 when doslikeFileSystem:
-  import std/[sequtils, private/ntpath]
+  import std/private/ntpath
 
   block: # Bug #19103 UNC paths
 
@@ -775,3 +780,15 @@ when doslikeFileSystem:
     doAssert splitFile("//?/c:") == ("//?/c:", "", "")
     doAssert splitFile("//?/c:/Users") == ("//?/c:", "Users", "")
     doAssert splitFile(r"\\localhost\c$\test.txt") == (r"\\localhost\c$", "test", ".txt")
+
+else:
+  block: # parentDirs
+    doAssert parentDirs("/home", fromRoot=true).toSeq == @["/", "/home"]
+    doAssert parentDirs("/home", fromRoot=false).toSeq == @["/home", "/"]
+    doAssert parentDirs("home", fromRoot=true).toSeq == @["home"]
+    doAssert parentDirs("home", fromRoot=false).toSeq == @["home"]
+
+    doAssert parentDirs("/home/user", fromRoot=true).toSeq == @["/", "/home/", "/home/user"]
+    doAssert parentDirs("/home/user", fromRoot=false).toSeq == @["/home/user", "/home", "/"]
+    doAssert parentDirs("home/user", fromRoot=true).toSeq == @["home/", "home/user"]
+    doAssert parentDirs("home/user", fromRoot=false).toSeq == @["home/user", "home"]