From b9fa552e6b8c4493b371ff0a81027f4396920ecd Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Thu, 23 Jan 2020 00:16:02 -0800 Subject: [PATCH 1/3] fix lots of bugs with parentDir, refs #8734 --- lib/pure/os.nim | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 3b1421cb56ed..2373b8092c0f 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -86,10 +86,13 @@ proc normalizePathEnd(path: var string, trailingSep = false) = ## Ensures ``path`` has exactly 0 or 1 trailing `DirSep`, depending on ## ``trailingSep``, and taking care of edge cases: it preservers whether ## a path is absolute or relative, and makes sure trailing sep is `DirSep`, - ## not `AltSep`. + ## not `AltSep`. Trailing `/.` are compressed, see examples. if path.len == 0: return var i = path.len - while i >= 1 and path[i-1] in {DirSep, AltSep}: dec(i) + while i >= 1: + if path[i-1] in {DirSep, AltSep}: dec(i) + elif path[i-1] == '.' and i >= 2 and path[i-2] in {DirSep, AltSep}: dec(i) + else: break if trailingSep: # foo// => foo path.setLen(i) @@ -106,9 +109,10 @@ proc normalizePathEnd(path: string, trailingSep = false): string = ## outplace overload runnableExamples: when defined(posix): - assert normalizePathEnd("/lib//", trailingSep = true) == "/lib/" - assert normalizePathEnd("lib//", trailingSep = false) == "lib" + assert normalizePathEnd("/lib//.//", trailingSep = true) == "/lib/" + assert normalizePathEnd("lib/./.", trailingSep = false) == "lib" assert normalizePathEnd("", trailingSep = true) == "" # not / ! + assert normalizePathEnd("/", trailingSep = false) == "/" # not "" ! result = path result.normalizePathEnd(trailingSep) @@ -417,8 +421,8 @@ proc parentDir*(path: string): string {. noSideEffect, rtl, extern: "nos$1".} = ## Returns the parent directory of `path`. ## - ## This is the same as ``splitPath(path).head`` when ``path`` doesn't end - ## in a dir separator. + ## This is similar to ``splitPath(path).head`` when ``path`` doesn't end + ## in a dir separator, but also takes care of path normalizations. ## The remainder can be obtained with `lastPathPart(path) proc ## <#lastPathPart,string>`_. ## @@ -431,19 +435,23 @@ proc parentDir*(path: string): string {. assert parentDir("") == "" when defined(posix): assert parentDir("/usr/local/bin") == "/usr/local" - assert parentDir("foo/bar/") == "foo" assert parentDir("foo/bar//") == "foo" - assert parentDir("//foo//bar//") == "//foo" + assert parentDir("//foo//bar//.") == "/foo" assert parentDir("./foo") == "." - assert parentDir("/foo") == "" - - result = normalizePathEnd(path) + assert parentDir("/./foo//./") == "/" + assert parentDir("a//./") == "." + assert parentDir("a/b/c/..") == "a" + result = pathnorm.normalizePath(path) var sepPos = parentDirPos(result) if sepPos >= 0: - while sepPos >= 0 and result[sepPos] in {DirSep, AltSep}: dec sepPos result = substr(result, 0, sepPos) - else: + normalizePathEnd(result) + elif result == ".." or result == "." or result.len == 0 or result[^1] in {DirSep, AltSep}: + # `.` => `..` and .. => `../..`(etc) would be a sensible alternative + # `/` => `/` (as done with splitFile) would be a sensible alternative result = "" + else: + result = "." proc tailDir*(path: string): string {. noSideEffect, rtl, extern: "nos$1".} = @@ -586,7 +594,7 @@ proc splitFile*(path: string): tuple[dir, name, ext: string] {. noSideEffect, rtl, extern: "nos$1".} = ## Splits a filename into `(dir, name, extension)` tuple. ## - ## `dir` does not end in `DirSep <#DirSep>`_. + ## `dir` does not end in `DirSep <#DirSep>`_ unless it's `/`. ## `extension` includes the leading dot. ## ## If `path` has no extension, `ext` is the empty string. From 24ebbd9a140dc4a2192a0b31b7489928ade8990e Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Thu, 23 Jan 2020 00:19:17 -0800 Subject: [PATCH 2/3] fixup --- lib/pure/os.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 2373b8092c0f..e3ce3dc30fdc 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -111,6 +111,7 @@ proc normalizePathEnd(path: string, trailingSep = false): string = when defined(posix): assert normalizePathEnd("/lib//.//", trailingSep = true) == "/lib/" assert normalizePathEnd("lib/./.", trailingSep = false) == "lib" + assert normalizePathEnd(".//./.", trailingSep = false) == "." assert normalizePathEnd("", trailingSep = true) == "" # not / ! assert normalizePathEnd("/", trailingSep = false) == "/" # not "" ! result = path From 99ab727c6d5ca924e8b04432b7c4aa4e12dad3b4 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Thu, 23 Jan 2020 01:03:05 -0800 Subject: [PATCH 3/3] fix test --- tests/stdlib/tos.nim | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index 48202157ad56..6d3f5b5ff726 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -206,11 +206,11 @@ block isHidden: when defined(posix): doAssert ".foo.txt".isHidden doAssert "bar/.foo.ext".isHidden - doAssert: not "bar".isHidden - doAssert: not "foo/".isHidden - # Corner cases: paths are not normalized when determining `isHidden` - doAssert: not ".foo/.".isHidden - doAssert: not ".foo/..".isHidden + doAssert not "bar".isHidden + doAssert not "foo/".isHidden + doAssert ".foo/.".isHidden + # Corner cases: `isHidden` is not yet `..` aware + doAssert not ".foo/..".isHidden block absolutePath: doAssertRaises(ValueError): discard absolutePath("a", "b")