diff options
| -rw-r--r-- | changelog.md | 12 | ||||
| -rw-r--r-- | compiler/scriptconfig.nim | 2 | ||||
| -rw-r--r-- | lib/pure/os.nim | 36 | ||||
| -rw-r--r-- | lib/system/nimscript.nim | 6 | ||||
| -rw-r--r-- | tests/newconfig/tfoo.nims | 11 | ||||
| -rw-r--r-- | tests/stdlib/tos.nim | 20 |
6 files changed, 56 insertions, 31 deletions
diff --git a/changelog.md b/changelog.md index e7e3aaf0c..d3c85d666 100644 --- a/changelog.md +++ b/changelog.md @@ -7,7 +7,7 @@ on GCC's `__builtin_sadd_overflow` family of functions. (Clang also supports these). Some versions of GCC lack this feature and unfortunately we cannot detect this case reliably. So if you get compilation errors like - "undefined reference to '__builtin_saddll_overflow'" compile your programs + "undefined reference to `__builtin_saddll_overflow`" compile your programs with `-d:nimEmulateOverflowChecks`. @@ -34,8 +34,8 @@ - `options` now treats `proc` like other pointer types, meaning `nil` proc variables are converted to `None`. - `relativePath("foo", "foo")` is now `"."`, not `""`, as `""` means invalid path - and shouldn't be conflated with `"."`; use -d:nimOldRelativePathBehavior to restore the old - behavior + and shouldn't be conflated with `"."`; use -d:nimOldRelativePathBehavior to + restore the old behavior - `joinPath(a,b)` now honors trailing slashes in `b` (or `a` if `b` = "") - `times.parse` now only uses input to compute its result, and not `now`: `parse("2020", "YYYY", utc())` is now `2020-01-01T00:00:00Z` instead of @@ -44,6 +44,12 @@ - `httpcore.==(string, HttpCode)` is now deprecated due to lack of practical usage. The `$` operator can be used to obtain the string form of `HttpCode` for comparison if desired. +- `os.walkDir` and `os.walkDirRec` now have new flag, `checkDir` (default: false). + If it is set to true, it will throw if input dir is invalid instead of a noop + (which is the default behaviour, as it was before this change), + `os.walkDirRec` only throws if top-level dir is invalid, but ignores errors for + subdirs, otherwise it would be impossible to resume iteration. + ### Breaking changes in the compiler diff --git a/compiler/scriptconfig.nim b/compiler/scriptconfig.nim index d53664912..13ab03426 100644 --- a/compiler/scriptconfig.nim +++ b/compiler/scriptconfig.nim @@ -65,7 +65,7 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; if defined(nimsuggest) or graph.config.cmd == cmdCheck: discard else: - os.removeDir getString(a, 0) + os.removeDir(getString(a, 0), getBool(a, 1)) cbos removeFile: if defined(nimsuggest) or graph.config.cmd == cmdCheck: discard diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 1ee71c1fb..e90d3c514 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -2028,8 +2028,8 @@ proc staticWalkDir(dir: string; relative: bool): seq[ tuple[kind: PathComponent, path: string]] = discard -iterator walkDir*(dir: string; relative=false): tuple[kind: PathComponent, path: string] {. - tags: [ReadDirEffect].} = +iterator walkDir*(dir: string; relative = false, checkDir = false): + tuple[kind: PathComponent, path: string] {.tags: [ReadDirEffect].} = ## Walks over the directory `dir` and yields for each directory or file in ## `dir`. The component type and full path for each item are returned. ## @@ -2038,7 +2038,6 @@ iterator walkDir*(dir: string; relative=false): tuple[kind: PathComponent, path: ## Example: This directory structure:: ## dirA / dirB / fileB1.txt ## / dirC - ## / fileA1.txt ## / fileA2.txt ## ## and this code: @@ -2069,7 +2068,10 @@ iterator walkDir*(dir: string; relative=false): tuple[kind: PathComponent, path: elif defined(windows): var f: WIN32_FIND_DATA var h = findFirstFile(dir / "*", f) - if h != -1: + if h == -1: + if checkDir: + raiseOSError(osLastError(), dir) + else: defer: findClose(h) while true: var k = pcFile @@ -2087,7 +2089,10 @@ iterator walkDir*(dir: string; relative=false): tuple[kind: PathComponent, path: else: raiseOSError(errCode.OSErrorCode) else: var d = opendir(dir) - if d != nil: + if d == nil: + if checkDir: + raiseOSError(osLastError(), dir) + else: defer: discard closedir(d) while true: var x = readdir(d) @@ -2122,7 +2127,7 @@ iterator walkDir*(dir: string; relative=false): tuple[kind: PathComponent, path: iterator walkDirRec*(dir: string, yieldFilter = {pcFile}, followFilter = {pcDir}, - relative = false): string {.tags: [ReadDirEffect].} = + relative = false, checkDir = false): string {.tags: [ReadDirEffect].} = ## Recursively walks over the directory `dir` and yields for each file ## or directory in `dir`. ## @@ -2159,14 +2164,20 @@ iterator walkDirRec*(dir: string, ## * `walkDir iterator <#walkDir.i,string>`_ var stack = @[""] + var checkDir = checkDir while stack.len > 0: let d = stack.pop() - for k, p in walkDir(dir / d, relative = true): + for k, p in walkDir(dir / d, relative = true, checkDir = checkDir): let rel = d / p if k in {pcDir, pcLinkToDir} and k in followFilter: stack.add rel if k in yieldFilter: yield if relative: rel else: dir / rel + checkDir = false + # We only check top-level dir, otherwise if a subdir is invalid (eg. wrong + # permissions), it'll abort iteration and there would be no way to + # continue iteration. + # Future work can provide a way to customize this and do error reporting. proc rawRemoveDir(dir: string) {.noNimScript.} = when defined(windows): @@ -2181,13 +2192,13 @@ proc rawRemoveDir(dir: string) {.noNimScript.} = else: if rmdir(dir) != 0'i32 and errno != ENOENT: raiseOSError(osLastError(), dir) -proc removeDir*(dir: string) {.rtl, extern: "nos$1", tags: [ +proc removeDir*(dir: string, checkDir = false) {.rtl, extern: "nos$1", tags: [ WriteDirEffect, ReadDirEffect], benign, noNimScript.} = ## Removes the directory `dir` including all subdirectories and files ## in `dir` (recursively). ## ## If this fails, `OSError` is raised. This does not fail if the directory never - ## existed in the first place. + ## existed in the first place, unless `checkDir` = true ## ## See also: ## * `tryRemoveFile proc <#tryRemoveFile,string>`_ @@ -2197,10 +2208,13 @@ proc removeDir*(dir: string) {.rtl, extern: "nos$1", tags: [ ## * `copyDir proc <#copyDir,string,string>`_ ## * `copyDirWithPermissions proc <#copyDirWithPermissions,string,string>`_ ## * `moveDir proc <#moveDir,string,string>`_ - for kind, path in walkDir(dir): + for kind, path in walkDir(dir, checkDir = checkDir): case kind of pcFile, pcLinkToFile, pcLinkToDir: removeFile(path) - of pcDir: removeDir(path) + of pcDir: removeDir(path, true) + # for subdirectories there is no benefit in `checkDir = false` + # (unless perhaps for edge case of concurrent processes also deleting + # the same files) rawRemoveDir(dir) proc rawCreateDir(dir: string): bool {.noNimScript.} = diff --git a/lib/system/nimscript.nim b/lib/system/nimscript.nim index b8abdaa38..f2a843652 100644 --- a/lib/system/nimscript.nim +++ b/lib/system/nimscript.nim @@ -30,7 +30,7 @@ proc listDirsImpl(dir: string): seq[string] {. tags: [ReadIOEffect], raises: [OSError].} = builtin proc listFilesImpl(dir: string): seq[string] {. tags: [ReadIOEffect], raises: [OSError].} = builtin -proc removeDir(dir: string) {. +proc removeDir(dir: string, checkDir = true) {. tags: [ReadIOEffect, WriteIOEffect], raises: [OSError].} = builtin proc removeFile(dir: string) {. tags: [ReadIOEffect, WriteIOEffect], raises: [OSError].} = builtin @@ -204,10 +204,10 @@ proc listFiles*(dir: string): seq[string] = result = listFilesImpl(dir) checkOsError() -proc rmDir*(dir: string) {.raises: [OSError].} = +proc rmDir*(dir: string, checkDir = false) {.raises: [OSError].} = ## Removes the directory `dir`. log "rmDir: " & dir: - removeDir dir + removeDir(dir, checkDir = checkDir) checkOsError() proc rmFile*(file: string) {.raises: [OSError].} = diff --git a/tests/newconfig/tfoo.nims b/tests/newconfig/tfoo.nims index 8dcc7d6f1..9e6405a27 100644 --- a/tests/newconfig/tfoo.nims +++ b/tests/newconfig/tfoo.nims @@ -24,8 +24,10 @@ doAssert(existsEnv("dummy") == false) # issue #7283 putEnv("dummy", "myval") -doAssert(existsEnv("dummy") == true) +doAssert(existsEnv("dummy")) doAssert(getEnv("dummy") == "myval") +delEnv("dummy") +doAssert(existsEnv("dummy") == false) # issue #7393 let wd = getCurrentDir() @@ -64,6 +66,8 @@ else: assert toDll("nim") == "libnim.so" rmDir("tempXYZ") +doAssertRaises(OSError): + rmDir("tempXYZ", checkDir = true) assert dirExists("tempXYZ") == false mkDir("tempXYZ") assert dirExists("tempXYZ") == true @@ -81,8 +85,3 @@ when false: rmDir("tempXYZ") assert dirExists("tempXYZ") == false - -putEnv("dummy", "myval") -doAssert(existsEnv("dummy") == true) -delEnv("dummy") -doAssert(existsEnv("dummy") == false) diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index d713bfe0c..be5384015 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -163,13 +163,19 @@ block walkDirRec: removeDir("walkdir_test") -when not defined(windows): - block walkDirRelative: - createDir("walkdir_test") - createSymlink(".", "walkdir_test/c") - for k, p in walkDir("walkdir_test", true): - doAssert k == pcLinkToDir - removeDir("walkdir_test") +block: # walkDir + doAssertRaises(OSError): + for a in walkDir("nonexistant", checkDir = true): discard + doAssertRaises(OSError): + for p in walkDirRec("nonexistant", checkDir = true): discard + + when not defined(windows): + block walkDirRelative: + createDir("walkdir_test") + createSymlink(".", "walkdir_test/c") + for k, p in walkDir("walkdir_test", true): + doAssert k == pcLinkToDir + removeDir("walkdir_test") block normalizedPath: doAssert normalizedPath("") == "" |