Skip to content

Commit

Permalink
[os] fix nim-lang#8734, nim-lang#8353: parentDir, parentDirs now work…
Browse files Browse the repository at this point in the history 
… reliably
  • Loading branch information
@timotheecour
timotheecour committed Feb 4, 2019
1 parent 315abd7 commit 9ffaf16
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 86 deletions.
3 changes: 3 additions & 0 deletions changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
- `getImpl` on a `var` or `let` symbol will now return the full `IdentDefs`
tree from the symbol declaration instead of just the initializer portion.

- ``os.parentDir`` and ``os.parentDirs`` had bugs that were fixed in #10545, see details there



#### Breaking changes in the standard library

Expand Down
182 changes: 100 additions & 82 deletions lib/pure/os.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,34 +303,6 @@ proc parentDirPos(path: string): int =
if path[i] in {DirSep, AltSep}: return i
result = -1

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.
## The remainder can be obtained with `lastPathPart(path) proc
## <#lastPathPart,string>`_.
##
## See also:
## * `relativePath proc <#relativePath,string,string>`_
## * `splitPath proc <#splitPath,string>`_
## * `tailDir proc <#tailDir,string>`_
## * `parentDirs iterator <#parentDirs.i,string>`_
runnableExamples:
assert parentDir("") == ""
when defined(posix):
assert parentDir("/usr/local/bin") == "/usr/local"
assert parentDir("foo/bar/") == "foo"
assert parentDir("./foo") == "."
assert parentDir("/foo") == ""

let sepPos = parentDirPos(path)
if sepPos >= 0:
result = substr(path, 0, sepPos-1)
else:
result = ""

proc tailDir*(path: string): string {.
noSideEffect, rtl, extern: "nos$1".} =
## Returns the tail part of `path`.
Expand Down Expand Up @@ -365,58 +337,6 @@ proc isRootDir*(path: string): bool {.

result = parentDirPos(path) < 0

iterator parentDirs*(path: string, fromRoot=false, inclusive=true): string =
## Walks over all parent directories of a given `path`.
##
## If `fromRoot` is true (default: false), the traversal will start from
## the file system root diretory.
## If `inclusive` is true (default), the original argument will be included
## in the traversal.
##
## Relative paths won't be expanded by this iterator. Instead, it will traverse
## only the directories appearing in the relative path.
##
## See also:
## * `parentDir proc <#parentDir,string>`_
##
## **Examples:**
##
## .. code-block::
## let g = "a/b/c"
##
## for p in g.parentDirs:
## echo p
## # a/b/c
## # a/b
## # a
##
## for p in g.parentDirs(fromRoot=true):
## echo p
## # a/
## # a/b/
## # a/b/c
##
## for p in g.parentDirs(inclusive=false):
## echo p
## # a/b
## # a

if not fromRoot:
var current = path
if inclusive: yield path
while true:
if current.isRootDir: break
current = current.parentDir
yield current
else:
for i in countup(0, path.len - 2): # ignore the last /
# deal with non-normalized paths such as /foo//bar//baz
if path[i] in {DirSep, AltSep} and
(i == 0 or path[i-1] notin {DirSep, AltSep}):
yield path.substr(0, i)

if inclusive: yield path

proc `/../`*(head, tail: string): string {.noSideEffect.} =
## The same as ``parentDir(head) / tail``, unless there is no parent
## directory. Then ``head / tail`` is performed instead.
Expand Down Expand Up @@ -468,7 +388,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 is absolute.
## `extension` includes the leading dot.
##
## If `path` has no extension, `ext` is the empty string.
Expand Down Expand Up @@ -522,6 +442,9 @@ proc splitFile*(path: string): tuple[dir, name, ext: string] {.
result.name = substr(path, sepPos+1, dotPos-1)
result.ext = substr(path, dotPos)

# goo/foo//bar.txt => (goo/foo, bar, .txt)
normalizePathEnd(result.dir)

proc extractFilename*(path: string): string {.
noSideEffect, rtl, extern: "nos$1".} =
## Extracts the filename of a given `path`.
Expand Down Expand Up @@ -560,7 +483,7 @@ proc lastPathPart*(path: string): string {.
assert lastPathPart("foo/bar/") == "bar"
assert lastPathPart("foo/bar") == "bar"

let path = path.normalizePathEnd(trailingSep = false)
let path = path.normalizePathEnd
result = extractFilename(path)

proc changeFileExt*(filename, ext: string): string {.
Expand Down Expand Up @@ -666,6 +589,101 @@ proc isAbsolute*(path: string): bool {.rtl, noSideEffect, extern: "nos$1".} =
elif defined(posix):
result = path[0] == '/'

proc parentDir*(path: string): string {.
noSideEffect, rtl, extern: "nos$1".} =
## Returns the parent directory of `path`.
##
## Conventions below derive from analogy with the shell:
## * an absolute path remains absolute
## * trailing ``.`` and ``/`` are resolved before taking the parent dir
## It returns empty when attempting to take parent of ``.`` or ``..``
## to indicate it is invalid and distinguish from ``.``
##
## The remainder can be obtained with `lastPathPart(path) proc
## <#lastPathPart,string>`_.
##
## See also:
## * `relativePath proc <#relativePath,string,string>`_
## * `splitPath proc <#splitPath,string>`_
## * `tailDir proc <#tailDir,string>`_
## * `parentDirs iterator <#parentDirs.i,string>`_
runnableExamples:
assert parentDir("") == ""
when defined(posix):
assert parentDir("/usr/local/bin") == "/usr/local"
assert parentDir("foo//bar/") == "foo"
assert parentDir("foo") == "."
assert parentDir("/foo") == "/"
assert parentDir(".") == ""
if path == "": return ""
result = path
while true:
normalizePathEnd(result)
let (dir, name, ext) = splitFile(result)
if name == "..":
if isAbsolute(dir):
return dir
return ""
if name == ".":
result = dir
continue
if dir == "" and name != "":
return "."
return dir

iterator parentDirs*(path: string, fromRoot=false, inclusive=true): string =
## Walks over all parent directories of a given `path`.
##
## If `fromRoot` is true (default: false), the traversal will start from
## the file system root diretory.
## If `inclusive` is true (default), the original argument will be included
## in the traversal.
##
## Relative paths won't be expanded by this iterator. Instead, it will traverse
## only the directories appearing in the relative path.
##
## See also:
## * `parentDir proc <#parentDir,string>`_
##
## **Examples:**
##
## .. code-block::
## let g = "a/b/c"
##
## for p in g.parentDirs:
## echo p
## # a/b/c
## # a/b
## # a
##
## for p in g.parentDirs(fromRoot=true):
## echo p
## # a/
## # a/b/
## # a/b/c
##
## for p in g.parentDirs(inclusive=false):
## echo p
## # a/b
## # a

var path = path.normalizePathEnd
if not fromRoot:
if inclusive: yield path
var current = path
while true:
if current.isRootDir: break
current = current.parentDir
yield current
else:
for i in countup(0, path.len - 2): # ignore the last /
# deal with non-normalized paths such as /foo//bar//baz
if path[i] in {DirSep, AltSep} and
(i == 0 or path[i-1] notin {DirSep, AltSep}):
yield path.substr(0, i).normalizePathEnd

if inclusive: yield path

proc unixToNativePath*(path: string, drive=""): string {.
noSideEffect, rtl, extern: "nos$1".} =
## Converts an UNIX-like path to a native one.
Expand Down
111 changes: 107 additions & 4 deletions tests/stdlib/tos.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Raises
# test os path creation, iteration, and deletion

import os, strutils, pathnorm
import "$nim/compiler/unittest_light"

block fileOperations:
let files = @["these.txt", "are.x", "testing.r", "files.q"]
Expand Down Expand Up @@ -240,16 +241,23 @@ block splitFile:

# execShellCmd is tested in tosproc

block ospaths:
doAssert unixToNativePath("") == ""
doAssert unixToNativePath(".") == $CurDir
doAssert unixToNativePath("..") == $ParDir
block isAbsolute:
doAssert not isAbsolute("")
doAssert not isAbsolute(".")
doAssert not isAbsolute("..")
doAssert not isAbsolute("abc")
doAssert not isAbsolute(".foo")
doAssert isAbsolute(unixToNativePath("/"))
doAssert isAbsolute(unixToNativePath("/", "a"))
doAssert isAbsolute(unixToNativePath("/a"))
doAssert isAbsolute(unixToNativePath("/a", "a"))
doAssert isAbsolute(unixToNativePath("/a/b"))
doAssert isAbsolute(unixToNativePath("/a/b", "a"))

block ospaths:
doAssert unixToNativePath("") == ""
doAssert unixToNativePath(".") == $CurDir
doAssert unixToNativePath("..") == $ParDir
doAssert unixToNativePath("a/b") == joinPath("a", "b")

when defined(macos):
Expand Down Expand Up @@ -333,3 +341,98 @@ block ospaths:
doAssert joinPath("", "lib") == "lib"
doAssert joinPath("", "/lib") == unixToNativePath"/lib"
doAssert joinPath("usr/", "/lib") == unixToNativePath"usr/lib"

block parentDir:
doAssert parentDir("") == ""

let paths = [
("/usr/local/bin", "/usr/local"),
("foo/bar.nim", "foo"),
("foo//bar.nim", "foo"),
("foo//bar//", "foo"),
("foo///bar", "foo"),
("foo///bar/.", "foo"),
("./foo///bar", "./foo"),
(".//foo///bar", ".//foo"),
("/.//foo///bar", "/.//foo"),
("foo/bar//a/./.", "foo/bar"),
("/a/bar", "/a"),
("/bar", "/"),

# same as in shell, `cd ..` returns `.` when pwd = "foo"; "." is not same as empty
# see https://github.com/nim-lang/Nim/pull/10018#issuecomment-447996816
("a/./.", "."),
("./bar", "."),
(".//bar", "."),
("bar", "."),

(".git", "."),
(".git.bak1", "."),

# absolute remains absolute
("/", "/"),
("/.", "/"),
("/..", "/"),
("/./", "/"),

# return empty when no parent possible
("", ""),
(".", ""),
("./", ""),
("..", ""),
("../", ""),
("../..", ""),

# regression tests

# fix #8734 (bug 2)
("a/b//", "a"),
("a/b/", "a"),

# fix #8734 (bug 3)
("/", "/"),

# fix #8734 (bug 4)
("/a.txt", "/"),
]
var numErrors = 0
for a in paths:
let path = unixToNativePath a[0]
let expected = unixToNativePath a[1]
if parentDir(path) != expected:
echo ("parentDir error:", a[0], a[1], path, expected, parentDir(path))
numErrors.inc
doAssert numErrors == 0 # delays the assert to see all errors first

import sequtils

block parentDirs:
template test(iter: untyped, expected: seq[string]): untyped =
let lhs = toSeq(iter)
let rhs = expected.mapIt(it.unixToNativePath)
assertEquals lhs, rhs

# fromRoot=false, inclusive=true
test parentDirs("a/b/c".unixToNativePath), @["a/b/c", "a/b", "a"]
test parentDirs("/a/b/c".unixToNativePath), @["/a/b/c", "/a/b", "/a", "/"]
test parentDirs("//a/b//c//".unixToNativePath), @["//a/b//c", "//a/b", "//a", "/"]
test parentDirs("/".unixToNativePath), @["/"]
test parentDirs("".unixToNativePath), @[""]

# fromRoot=true
test parentDirs("a/b/c".unixToNativePath, fromRoot=true), @["a", "a/b", "a/b/c"]
test parentDirs("a//b//c/".unixToNativePath, fromRoot=true), @["a", "a//b", "a//b//c"]
test parentDirs("/a/b".unixToNativePath, fromRoot=true), @["/", "/a", "/a/b"]

# inclusive=false
test parentDirs("/a/b".unixToNativePath, inclusive=false), @["/a", "/"]
test parentDirs("/a//b//".unixToNativePath, inclusive=false), @["/a", "/"]
test parentDirs("/a/b//".unixToNativePath, inclusive=false), @["/a", "/"]
test parentDirs("".unixToNativePath, inclusive=false), seq[string](@[])

# fromRoot=true, inclusive=false
test parentDirs("/a/b/c/".unixToNativePath, fromRoot=true, inclusive=false), @["/", "/a", "/a/b"]

# regression test
# fix #8353
test parentDirs("/a/b".unixToNativePath), @["/a/b", "/a", "/"]

0 comments on commit 9ffaf16

Please sign in to comment.