diff options
author | havardjohn <havard.mjaavatten@outlook.com> | 2022-09-11 22:51:39 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-11 16:51:39 -0400 |
commit | 68f92af17c4d38bdca348ae956447b2571b74198 (patch) | |
tree | 07b8d47210f380cfba658ade6f6ba766db34019d | |
parent | 088487f652638a745e8e7e440a8a3b381239597b (diff) | |
download | Nim-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.nim | 27 | ||||
-rw-r--r-- | tests/stdlib/tos.nim | 19 |
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"] |