From 48ef6761d8f5183139920b05b2b084edb312bc40 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 22 Oct 2016 12:29:03 +0700 Subject: [PATCH 1/5] Make createDir return discardable bool --- lib/pure/os.nim | 46 +++++++++++++++++++++++++++++++++----------- tests/stdlib/tos.nim | 19 ++++++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index cdbe170ccc..34819b3962 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1009,29 +1009,51 @@ proc removeDir*(dir: string) {.rtl, extern: "nos$1", tags: [ of pcDir: removeDir(path) rawRemoveDir(dir) -proc rawCreateDir(dir: string) = +proc rawCreateDir(dir: string): bool = + # Create directory. + # Does not create parent directories (fails if parent does not exist). + # Returns `true` if the directory was created, `false` if it already exists. when defined(solaris): - if mkdir(dir, 0o777) != 0'i32 and errno != EEXIST and errno != ENOSYS: + let res = mkdir(dir, 0o777) + case res + of 0'i32: + result = true + of EEXIST, ENOSYS: + result = false + else: raiseOSError(osLastError()) elif defined(unix): - if mkdir(dir, 0o777) != 0'i32 and errno != EEXIST: + let res = mkdir(dir, 0o777) + case res + of 0'i32: + result = true + of EEXIST: + result = false + else: raiseOSError(osLastError()) else: when useWinUnicode: wrapUnary(res, createDirectoryW, dir) else: - var res = createDirectoryA(dir) - if res == 0'i32 and getLastError() != 183'i32: + let res = createDirectoryA(dir) + + if res != 0'i32: + result = true + elif getLastError() == 183'i32: + result = false + else: raiseOSError(osLastError()) -proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} = +proc createDir*(dir: string): bool {.discardable, rtl, + extern: "nos$1", tags: [WriteDirEffect].} = ## Creates the `directory`:idx: `dir`. ## ## The directory may contain several subdirectories that do not exist yet. - ## The full path is created. If this fails, `OSError` is raised. It does **not** - ## fail if the path already exists because for most usages this does not - ## indicate an error. + ## The full path is created. If this fails, `OSError` is raised. + ## + ## Returns `true` if the directory did not previously exist var omitNext = false + result = false when doslike: omitNext = isAbsolute(dir) for i in 1.. dir.len-1: @@ -1039,8 +1061,10 @@ proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} = if omitNext: omitNext = false else: - rawCreateDir(substr(dir, 0, i-1)) - rawCreateDir(dir) + result = rawCreateDir(substr(dir, 0, i-1)) + # The loop does not create the dir itself if it doesn't end in separator + if dir[^1] notin {DirSep, AltSep}: + result = rawCreateDir(dir) proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", tags: [WriteIOEffect, ReadIOEffect], benign.} = diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index 1ddaacfcbd..0d726de567 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -36,6 +36,10 @@ false false false false +true +true +true +false ''' """ # test os path creation, iteration, and deletion @@ -86,3 +90,18 @@ for file in files: removeDir(dname) echo dirExists(dname) + +# createDir should create recursive directories +createDir(dirs[0] / dirs[1]) +echo dirExists(dirs[0] / dirs[1]) # true +removeDir(dirs[0]) + +# createDir should properly handle trailing separator +createDir(dname / "") +echo dirExists(dname) # true +removeDir(dname) + +# Check createDir return value +echo createDir(dname) # true +echo createDir(dname) # false +removeDir(dname) From 1f641b1337bad6fed9fbbea6f0610c54c065c3ea Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 22 Oct 2016 12:52:16 +0700 Subject: [PATCH 2/5] Fix for unix and solaris --- lib/pure/os.nim | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 34819b3962..5f957fea9b 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1015,21 +1015,20 @@ proc rawCreateDir(dir: string): bool = # Returns `true` if the directory was created, `false` if it already exists. when defined(solaris): let res = mkdir(dir, 0o777) - case res - of 0'i32: + if res == 0'i32: result = true - of EEXIST, ENOSYS: + elif errno in {EEXIST, ENOSYS}: result = false else: raiseOSError(osLastError()) elif defined(unix): let res = mkdir(dir, 0o777) - case res - of 0'i32: + if res == 0'i32: result = true - of EEXIST: + elif errno == EEXIST: result = false else: + echo res raiseOSError(osLastError()) else: when useWinUnicode: From 45b432b9010d267ef423338ffa70a1298eff7555 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 22 Oct 2016 17:17:06 +0700 Subject: [PATCH 3/5] Revert createDir signature, expose rawCreateDir --- lib/pure/os.nim | 27 +++++++++++++++------------ tests/stdlib/tos.nim | 7 ------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 5f957fea9b..88b1bd2808 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1009,10 +1009,15 @@ proc removeDir*(dir: string) {.rtl, extern: "nos$1", tags: [ of pcDir: removeDir(path) rawRemoveDir(dir) -proc rawCreateDir(dir: string): bool = - # Create directory. - # Does not create parent directories (fails if parent does not exist). - # Returns `true` if the directory was created, `false` if it already exists. +proc tryCreateDir*(dir: string): bool = + ## Try to create a `directory`:idx: `dir`. + ## + ## Does not create parent directories (fails if parent does not exist). + ## Returns `true` if a new directory was created, `false` if *path* + ## (not necessarily a directory) `dir` already exists. + + # This is a thin wrapper over mkDir (or alternatives on other systems), + # so in case of a pre-existing path we don't check that it is a directory. when defined(solaris): let res = mkdir(dir, 0o777) if res == 0'i32: @@ -1043,16 +1048,14 @@ proc rawCreateDir(dir: string): bool = else: raiseOSError(osLastError()) -proc createDir*(dir: string): bool {.discardable, rtl, - extern: "nos$1", tags: [WriteDirEffect].} = +proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} = ## Creates the `directory`:idx: `dir`. ## ## The directory may contain several subdirectories that do not exist yet. - ## The full path is created. If this fails, `OSError` is raised. - ## - ## Returns `true` if the directory did not previously exist + ## The full path is created. If this fails, `OSError` is raised. It does **not** + ## fail if the path already exists because for most usages this does not + ## indicate an error. var omitNext = false - result = false when doslike: omitNext = isAbsolute(dir) for i in 1.. dir.len-1: @@ -1060,10 +1063,10 @@ proc createDir*(dir: string): bool {.discardable, rtl, if omitNext: omitNext = false else: - result = rawCreateDir(substr(dir, 0, i-1)) + discard tryCreateDir(substr(dir, 0, i-1)) # The loop does not create the dir itself if it doesn't end in separator if dir[^1] notin {DirSep, AltSep}: - result = rawCreateDir(dir) + discard tryCreateDir(dir) proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", tags: [WriteIOEffect, ReadIOEffect], benign.} = diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index 0d726de567..a1ea60feb9 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -38,8 +38,6 @@ false false true true -true -false ''' """ # test os path creation, iteration, and deletion @@ -100,8 +98,3 @@ removeDir(dirs[0]) createDir(dname / "") echo dirExists(dname) # true removeDir(dname) - -# Check createDir return value -echo createDir(dname) # true -echo createDir(dname) # false -removeDir(dname) From 785e4022477248390565be20c52843b7b7c79222 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 22 Oct 2016 17:53:51 +0700 Subject: [PATCH 4/5] Make `createDir` more robust Should now properly work for root directories on Windows (after isAbsolute is fixed) --- lib/pure/os.nim | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 88b1bd2808..83d98a3fe7 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1064,8 +1064,10 @@ proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} = omitNext = false else: discard tryCreateDir(substr(dir, 0, i-1)) + # The loop does not create the dir itself if it doesn't end in separator - if dir[^1] notin {DirSep, AltSep}: + if dir.len > 0 and not omitNext and + dir[^1] notin {DirSep, AltSep}: discard tryCreateDir(dir) proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", From 1cd4799b0137926dbeef9b3eb8076aa7df7a7fc5 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 22 Oct 2016 18:36:44 +0700 Subject: [PATCH 5/5] Improve as previously discussed Better name for exposed primitive function, checks for pre-existing files --- lib/pure/os.nim | 32 +++++++++++++++++++++----------- tests/stdlib/tos.nim | 10 ++++++++++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 83d98a3fe7..110831d955 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1009,13 +1009,10 @@ proc removeDir*(dir: string) {.rtl, extern: "nos$1", tags: [ of pcDir: removeDir(path) rawRemoveDir(dir) -proc tryCreateDir*(dir: string): bool = - ## Try to create a `directory`:idx: `dir`. - ## - ## Does not create parent directories (fails if parent does not exist). - ## Returns `true` if a new directory was created, `false` if *path* - ## (not necessarily a directory) `dir` already exists. - +proc rawCreateDir(dir: string): bool = + # Try to create one directory (not the whole path). + # returns `true` for success, `false` if the path has previously existed + # # This is a thin wrapper over mkDir (or alternatives on other systems), # so in case of a pre-existing path we don't check that it is a directory. when defined(solaris): @@ -1048,12 +1045,25 @@ proc tryCreateDir*(dir: string): bool = else: raiseOSError(osLastError()) -proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} = +proc existsOrCreateDir*(dir: string): bool = + ## Check if a `directory`:idx: `dir` exists, and create it otherwise. + ## + ## Does not create parent directories (fails if parent does not exist). + ## Returns `true` if the directory already exists, and `false` + ## otherwise. + result = not rawCreateDir(dir) + if result: + # path already exists - need to check that it is indeed a directory + if not existsDir(dir): + raise newException(IOError, "Failed to create the directory") + +proc createDir*(dir: string) {.rtl, extern: "nos$1", + tags: [WriteDirEffect, ReadDirEffect].} = ## Creates the `directory`:idx: `dir`. ## ## The directory may contain several subdirectories that do not exist yet. ## The full path is created. If this fails, `OSError` is raised. It does **not** - ## fail if the path already exists because for most usages this does not + ## fail if the directory already exists because for most usages this does not ## indicate an error. var omitNext = false when doslike: @@ -1063,12 +1073,12 @@ proc createDir*(dir: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect].} = if omitNext: omitNext = false else: - discard tryCreateDir(substr(dir, 0, i-1)) + 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 tryCreateDir(dir) + discard existsOrCreateDir(dir) proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", tags: [WriteIOEffect, ReadIOEffect], benign.} = diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index a1ea60feb9..1ef02f97e2 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -38,6 +38,7 @@ false false true true +Raises ''' """ # test os path creation, iteration, and deletion @@ -98,3 +99,12 @@ removeDir(dirs[0]) createDir(dname / "") echo dirExists(dname) # true removeDir(dname) + +# createDir should raise IOError if the path exists +# and is not a directory +open(dname, fmWrite).close +try: + createDir(dname) +except IOError: + echo "Raises" +removeFile(dname)