From 137d9fe21468ed6c3913cf0b1dddb479d0e59cab Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 21 Jan 2021 18:02:28 +0200 Subject: [PATCH 1/3] stdlib/os: handle symlinks in copy/move functions - Added optional `options` argument to `copyFile`, `copyFileToDir`, and `copyFileWithPermissions`. By default, symlinks are followed (copy files symlinks point to). - `copyDir` and `copyDirWithPermissions` copy symlinks as symlinks (instead of skipping them as it was before). - `moveFile` and `moveDir` move symlinks as symlinks (instead of skipping them sometimes as it was before). - Added optional `followSymlinks` argument to `setFilePermissions`. See also: https://github.com/nim-lang/RFCs/issues/319 Co-authored-by: Timothee Cour --- changelog.md | 10 ++ lib/posix/posix.nim | 2 + lib/posix/posix_haiku.nim | 1 + lib/posix/posix_linux_amd64.nim | 1 + lib/posix/posix_macos_amd64.nim | 1 + lib/posix/posix_nintendoswitch.nim | 1 + lib/posix/posix_openbsd_amd64.nim | 1 + lib/posix/posix_other.nim | 2 + lib/pure/os.nim | 256 ++++++++++++++++++----------- lib/windows/winlean.nim | 15 ++ tests/stdlib/tos.nim | 108 ++++++++++++ tests/test_nimscript.nims | 26 +++ 12 files changed, 326 insertions(+), 98 deletions(-) diff --git a/changelog.md b/changelog.md index b07186eed6073..e9cc3a35f2ea1 100644 --- a/changelog.md +++ b/changelog.md @@ -112,6 +112,16 @@ with other backends. see #9125. Use `-d:nimLegacyJsRound` for previous behavior. - Removed the optional `longestMatch` parameter of the `critbits._WithPrefix` iterators (it never worked reliably) + +- Added optional `options` argument to `copyFile`, `copyFileToDir`, and + `copyFileWithPermissions`. By default, symlinks are followed (copy files + symlinks point to). +- `copyDir` and `copyDirWithPermissions` copy symlinks as symlinks (instead of + skipping them as it was before). +- `moveFile` and `moveDir` move symlinks as they are (instead of skipping them + sometimes as it was before). +- Added optional `followSymlinks` argument to `setFilePermissions`. + ## Language changes - `nimscript` now handles `except Exception as e`. diff --git a/lib/posix/posix.nim b/lib/posix/posix.nim index c7a324cde4f1d..db5e2d681abb3 100644 --- a/lib/posix/posix.nim +++ b/lib/posix/posix.nim @@ -586,6 +586,8 @@ proc fstatvfs*(a1: cint, a2: var Statvfs): cint {. importc, header: "".} proc chmod*(a1: cstring, a2: Mode): cint {.importc, header: "", sideEffect.} +when hasLchmod: + proc lchmod*(a1: cstring, a2: Mode): cint {.importc, header: "", sideEffect.} proc fchmod*(a1: cint, a2: Mode): cint {.importc, header: "", sideEffect.} proc fstat*(a1: cint, a2: var Stat): cint {.importc, header: "", sideEffect.} proc lstat*(a1: cstring, a2: var Stat): cint {.importc, header: "", sideEffect.} diff --git a/lib/posix/posix_haiku.nim b/lib/posix/posix_haiku.nim index eaf2cfb857f50..ab977527d6072 100644 --- a/lib/posix/posix_haiku.nim +++ b/lib/posix/posix_haiku.nim @@ -13,6 +13,7 @@ when defined(nimHasStyleChecks): const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = defined(linux) + hasLchmod* = false when defined(linux) and not defined(android): # On Linux: diff --git a/lib/posix/posix_linux_amd64.nim b/lib/posix/posix_linux_amd64.nim index 7f6a589f08501..9e7fe192082bc 100644 --- a/lib/posix/posix_linux_amd64.nim +++ b/lib/posix/posix_linux_amd64.nim @@ -15,6 +15,7 @@ const hasSpawnH = not defined(haiku) # should exist for every Posix system nowadays hasAioH = defined(linux) + hasLchmod* = false # On Linux: # timer_{create,delete,settime,gettime}, diff --git a/lib/posix/posix_macos_amd64.nim b/lib/posix/posix_macos_amd64.nim index 94c08acad468f..f408c93399d9e 100644 --- a/lib/posix/posix_macos_amd64.nim +++ b/lib/posix/posix_macos_amd64.nim @@ -13,6 +13,7 @@ when defined(nimHasStyleChecks): const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = false + hasLchmod* = true type DIR* {.importc: "DIR", header: "", diff --git a/lib/posix/posix_nintendoswitch.nim b/lib/posix/posix_nintendoswitch.nim index 44e4314379950..19ce37a22dc0b 100644 --- a/lib/posix/posix_nintendoswitch.nim +++ b/lib/posix/posix_nintendoswitch.nim @@ -12,6 +12,7 @@ const hasSpawnH = true hasAioH = false + hasLchmod* = false type DIR* {.importc: "DIR", header: "", diff --git a/lib/posix/posix_openbsd_amd64.nim b/lib/posix/posix_openbsd_amd64.nim index 584d0650116f0..d342f50bc0210 100644 --- a/lib/posix/posix_openbsd_amd64.nim +++ b/lib/posix/posix_openbsd_amd64.nim @@ -13,6 +13,7 @@ when defined(nimHasStyleChecks): const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = false + hasLchmod* = false type DIR* {.importc: "DIR", header: "", diff --git a/lib/posix/posix_other.nim b/lib/posix/posix_other.nim index a5eb32d2267fa..1492f57d75c34 100644 --- a/lib/posix/posix_other.nim +++ b/lib/posix/posix_other.nim @@ -14,10 +14,12 @@ when defined(freertos): const hasSpawnH = false # should exist for every Posix system nowadays hasAioH = false + hasLchmod* = false else: const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = defined(linux) + hasLchmod* = false when defined(linux) and not defined(android): # On Linux: diff --git a/lib/pure/os.nim b/lib/pure/os.nim index d642e5242a4e7..5e6f0df577134 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1592,10 +1592,17 @@ proc getFilePermissions*(filename: string): set[FilePermission] {. else: result = {fpUserExec..fpOthersRead} -proc setFilePermissions*(filename: string, permissions: set[FilePermission]) {. - rtl, extern: "nos$1", tags: [WriteDirEffect], noWeirdTarget.} = +proc setFilePermissions*(filename: string, permissions: set[FilePermission], + followSymlinks = true) + {.rtl, extern: "nos$1", tags: [ReadDirEffect, WriteDirEffect], + noWeirdTarget.} = ## Sets the file permissions for `filename`. ## + ## If ``followSymlinks`` set to true (default) and ``filename`` points to a + ## symlink, permissions are set to the file symlink points to. + ## ``followSymlinks`` set to false do not affect on Windows and some POSIX + ## systems (including Linux) which do not have ``lchmod`` available. + ## ## `OSError` is raised in case of an error. ## On Windows, only the ``readonly`` flag is changed, depending on ## ``fpUserWrite`` permission. @@ -1617,7 +1624,13 @@ proc setFilePermissions*(filename: string, permissions: set[FilePermission]) {. if fpOthersWrite in permissions: p = p or S_IWOTH.Mode if fpOthersExec in permissions: p = p or S_IXOTH.Mode - if chmod(filename, cast[Mode](p)) != 0: raiseOSError(osLastError(), $(filename, permissions)) + if not followSymlinks and filename.symlinkExists: + when hasLchmod: + if lchmod(filename, cast[Mode](p)) != 0: + raiseOSError(osLastError(), $(filename, permissions)) + else: + if chmod(filename, cast[Mode](p)) != 0: + raiseOSError(osLastError(), $(filename, permissions)) else: when useWinUnicode: wrapUnary(res, getFileAttributesW, filename) @@ -1634,6 +1647,53 @@ proc setFilePermissions*(filename: string, permissions: set[FilePermission]) {. var res2 = setFileAttributesA(filename, res) if res2 == - 1'i32: raiseOSError(osLastError(), $(filename, permissions)) +proc createSymlink*(src, dest: string) {.noWeirdTarget.} = + ## Create a symbolic link at `dest` which points to the item specified + ## by `src`. On most operating systems, will fail if a link already exists. + ## + ## **Warning**: + ## Some OS's (such as Microsoft Windows) restrict the creation + ## of symlinks to root users (administrators). + ## + ## See also: + ## * `createHardlink proc <#createHardlink,string,string>`_ + ## * `expandSymlink proc <#expandSymlink,string>`_ + + when defined(Windows): + # 2 is the SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE. This allows + # anyone with developer mode on to create a link + let flag = dirExists(src).int32 or 2 + when useWinUnicode: + var wSrc = newWideCString(src) + var wDst = newWideCString(dest) + if createSymbolicLinkW(wDst, wSrc, flag) == 0 or getLastError() != 0: + raiseOSError(osLastError(), $(src, dest)) + else: + if createSymbolicLinkA(dest, src, flag) == 0 or getLastError() != 0: + raiseOSError(osLastError(), $(src, dest)) + else: + if symlink(src, dest) != 0: + raiseOSError(osLastError(), $(src, dest)) + +proc expandSymlink*(symlinkPath: string): string {.noWeirdTarget.} = + ## Returns a string representing the path to which the symbolic link points. + ## + ## On Windows this is a noop, ``symlinkPath`` is simply returned. + ## + ## See also: + ## * `createSymlink proc <#createSymlink,string,string>`_ + when defined(windows): + result = symlinkPath + else: + result = newString(maxSymlinkLen) + var len = readlink(symlinkPath, result, maxSymlinkLen) + if len < 0: + raiseOSError(osLastError(), symlinkPath) + if len > maxSymlinkLen: + result = newString(len+1) + len = readlink(symlinkPath, result, len) + setLen(result, len) + const hasCCopyfile = defined(osx) and not defined(nimLegacyCopyFile) # xxx instead of `nimLegacyCopyFile`, support something like: `when osxVersion >= (10, 5)` @@ -1652,10 +1712,21 @@ when hasCCopyfile: COPYFILE_XATTR {.nodecl.}: copyfile_flags_t {.pop.} -proc copyFile*(source, dest: string) {.rtl, extern: "nos$1", - tags: [ReadIOEffect, WriteIOEffect], noWeirdTarget.} = +type CopyFlag* = enum ## Copy options. + cfSymlinkAsIs, ## Copy symlinks as symlinks + cfSymlinkFollow, ## Copy the files symlinks point to + cfSymlinkIgnore ## Ignore symlinks + +const copyFlagSymlink = {cfSymlinkAsIs, cfSymlinkFollow, cfSymlinkIgnore} + +proc copyFile*(source, dest: string, options = {cfSymlinkFollow}) {.rtl, + extern: "nos$1", tags: [ReadDirEffect, ReadIOEffect, WriteIOEffect], + noWeirdTarget.} = ## Copies a file from `source` to `dest`, where `dest.parentDir` must exist. ## + ## `options` specify the way file is copied; by default, if `source` is a + ## symlink, copies the file symlink points to. + ## ## If this fails, `OSError` is raised. ## ## On the Windows platform this proc will @@ -1663,7 +1734,8 @@ proc copyFile*(source, dest: string) {.rtl, extern: "nos$1", ## ## On other platforms you need ## to use `getFilePermissions <#getFilePermissions,string>`_ and - ## `setFilePermissions <#setFilePermissions,string,set[FilePermission]>`_ procs + ## `setFilePermissions <#setFilePermissions,string,set[FilePermission]>`_ + ## procs ## to copy them by hand (or use the convenience `copyFileWithPermissions ## proc <#copyFileWithPermissions,string,string>`_), ## otherwise `dest` will inherit the default permissions of a newly @@ -1676,19 +1748,29 @@ proc copyFile*(source, dest: string) {.rtl, extern: "nos$1", ## `-d:nimLegacyCopyFile` is used. ## ## See also: + ## * `CopyFlag enum <#CopyFlag>`_ ## * `copyDir proc <#copyDir,string,string>`_ ## * `copyFileWithPermissions proc <#copyFileWithPermissions,string,string>`_ ## * `tryRemoveFile proc <#tryRemoveFile,string>`_ ## * `removeFile proc <#removeFile,string>`_ ## * `moveFile proc <#moveFile,string,string>`_ + doAssert card(copyFlagSymlink * options) == 1, "There should be exactly " & + "one cfSymlink* in options" + let isSymlink = source.symlinkExists + if isSymlink and cfSymlinkIgnore in options: + return when defined(Windows): + let dwCopyFlags = if cfSymlinkAsIs in options: COPY_FILE_COPY_SYMLINK else: 0'i32 + var pbCancel = 0'i32 when useWinUnicode: let s = newWideCString(source) let d = newWideCString(dest) - if copyFileW(s, d, 0'i32) == 0'i32: raiseOSError(osLastError(), $(source, dest)) + if copyFileExW(s, d, nil, nil, addr pbCancel, dwCopyFlags) == 0'i32: + raiseOSError(osLastError(), $(source, dest, options)) else: - if copyFileA(source, dest, 0'i32) == 0'i32: raiseOSError(osLastError(), $(source, dest)) + if copyFileExA(source, dest, nil, nil, addr pbCancel, dwCopyFlags) == 0'i32: + raiseOSError(osLastError(), $(source, dest, options)) elif hasCCopyfile: let state = copyfile_state_alloc() # xxx `COPYFILE_STAT` could be used for one-shot `copyFileWithPermissions`. @@ -1700,34 +1782,45 @@ proc copyFile*(source, dest: string) {.rtl, extern: "nos$1", let status2 = copyfile_state_free(state) if status2 != 0: raiseOSError(osLastError(), $(source, dest)) else: - # generic version of copyFile which works for any platform: - const bufSize = 8000 # better for memory manager - var d, s: File - if not open(s, source): raiseOSError(osLastError(), source) - if not open(d, dest, fmWrite): + if isSymlink and cfSymlinkAsIs in options: + createSymlink(expandSymlink(source), dest) + else: + # generic version of copyFile which works for any platform: + const bufSize = 8000 # better for memory manager + var d, s: File + if not open(s, source): raiseOSError(osLastError(), source) + if not open(d, dest, fmWrite): + close(s) + raiseOSError(osLastError(), dest) + var buf = alloc(bufSize) + while true: + var bytesread = readBuffer(s, buf, bufSize) + if bytesread > 0: + var byteswritten = writeBuffer(d, buf, bytesread) + if bytesread != byteswritten: + dealloc(buf) + close(s) + close(d) + raiseOSError(osLastError(), dest) + if bytesread != bufSize: break + dealloc(buf) close(s) - raiseOSError(osLastError(), dest) - var buf = alloc(bufSize) - while true: - var bytesread = readBuffer(s, buf, bufSize) - if bytesread > 0: - var byteswritten = writeBuffer(d, buf, bytesread) - if bytesread != byteswritten: - dealloc(buf) - close(s) - close(d) - raiseOSError(osLastError(), dest) - if bytesread != bufSize: break - dealloc(buf) - close(s) - flushFile(d) - close(d) - -proc copyFileToDir*(source, dir: string) {.noWeirdTarget, since: (1,3,7).} = + flushFile(d) + close(d) + +proc copyFileToDir*(source, dir: string, options = {cfSymlinkFollow}) + {.noWeirdTarget, since: (1,3,7).} = ## Copies a file `source` into directory `dir`, which must exist. + ## + ## `options` specify the way file is copied; by default, if `source` is a + ## symlink, copies the file symlink points to. + ## + ## See also: + ## * `CopyFlag enum <#CopyFlag>`_ + ## * `copyFile proc <#copyDir,string,string>`_ if dir.len == 0: # treating "" as "." is error prone raise newException(ValueError, "dest is empty") - copyFile(source, dir / source.lastPathPart) + copyFile(source, dir / source.lastPathPart, options) when not declared(ENOENT) and not defined(Windows): when NoFakeVars: @@ -1821,9 +1914,12 @@ proc tryMoveFSObject(source, dest: string): bool {.noWeirdTarget.} = return true proc moveFile*(source, dest: string) {.rtl, extern: "nos$1", - tags: [ReadIOEffect, WriteIOEffect], noWeirdTarget.} = + tags: [ReadDirEffect, ReadIOEffect, WriteIOEffect], noWeirdTarget.} = ## Moves a file from `source` to `dest`. ## + ## Symlinks are not followed: if `source` is a symlink, it is itself moved, + ## not its target. + ## ## If this fails, `OSError` is raised. ## If `dest` already exists, it will be overwritten. ## @@ -1839,7 +1935,7 @@ proc moveFile*(source, dest: string) {.rtl, extern: "nos$1", if not tryMoveFSObject(source, dest): when not defined(windows): # Fallback to copy & del - copyFile(source, dest) + copyFile(source, dest, {cfSymlinkAsIs}) try: removeFile(source) except: @@ -2349,9 +2445,11 @@ proc createDir*(dir: string) {.rtl, extern: "nos$1", discard existsOrCreateDir(dir) proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", - tags: [WriteIOEffect, ReadIOEffect], benign, noWeirdTarget.} = + tags: [ReadDirEffect, WriteIOEffect, ReadIOEffect], benign, noWeirdTarget.} = ## Copies a directory from `source` to `dest`. ## + ## Symlinks are copied as symlinks. + ## ## If this fails, `OSError` is raised. ## ## On the Windows platform this proc will copy the attributes from @@ -2373,16 +2471,17 @@ proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", createDir(dest) for kind, path in walkDir(source): var noSource = splitPath(path).tail - case kind - of pcFile: - copyFile(path, dest / noSource) - of pcDir: + if kind == pcDir: copyDir(path, dest / noSource) - else: discard + else: + copyFile(path, dest / noSource, {cfSymlinkAsIs}) proc moveDir*(source, dest: string) {.tags: [ReadIOEffect, WriteIOEffect], noWeirdTarget.} = ## Moves a directory from `source` to `dest`. ## + ## Symlinks are not followed: if `source` contains symlinks, they itself are + ## moved, not theirs target. + ## ## If this fails, `OSError` is raised. ## ## See also: @@ -2398,34 +2497,6 @@ proc moveDir*(source, dest: string) {.tags: [ReadIOEffect, WriteIOEffect], noWei copyDir(source, dest) removeDir(source) -proc createSymlink*(src, dest: string) {.noWeirdTarget.} = - ## Create a symbolic link at `dest` which points to the item specified - ## by `src`. On most operating systems, will fail if a link already exists. - ## - ## **Warning**: - ## Some OS's (such as Microsoft Windows) restrict the creation - ## of symlinks to root users (administrators). - ## - ## See also: - ## * `createHardlink proc <#createHardlink,string,string>`_ - ## * `expandSymlink proc <#expandSymlink,string>`_ - - when defined(Windows): - # 2 is the SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE. This allows - # anyone with developer mode on to create a link - let flag = dirExists(src).int32 or 2 - when useWinUnicode: - var wSrc = newWideCString(src) - var wDst = newWideCString(dest) - if createSymbolicLinkW(wDst, wSrc, flag) == 0 or getLastError() != 0: - raiseOSError(osLastError(), $(src, dest)) - else: - if createSymbolicLinkA(dest, src, flag) == 0 or getLastError() != 0: - raiseOSError(osLastError(), $(src, dest)) - else: - if symlink(src, dest) != 0: - raiseOSError(osLastError(), $(src, dest)) - proc createHardlink*(src, dest: string) {.noWeirdTarget.} = ## Create a hard link at `dest` which points to the item specified ## by `src`. @@ -2449,9 +2520,13 @@ proc createHardlink*(src, dest: string) {.noWeirdTarget.} = raiseOSError(osLastError(), $(src, dest)) proc copyFileWithPermissions*(source, dest: string, - ignorePermissionErrors = true) {.noWeirdTarget.} = + ignorePermissionErrors = true, + options = {cfSymlinkFollow}) {.noWeirdTarget.} = ## Copies a file from `source` to `dest` preserving file permissions. ## + ## `options` specify the way file is copied; by default, if `source` is a + ## symlink, copies the file symlink points to. + ## ## This is a wrapper proc around `copyFile <#copyFile,string,string>`_, ## `getFilePermissions <#getFilePermissions,string>`_ and ## `setFilePermissions<#setFilePermissions,string,set[FilePermission]>`_ @@ -2467,25 +2542,30 @@ proc copyFileWithPermissions*(source, dest: string, ## `OSError`. ## ## See also: + ## * `CopyFlag enum <#CopyFlag>`_ ## * `copyFile proc <#copyFile,string,string>`_ ## * `copyDir proc <#copyDir,string,string>`_ ## * `tryRemoveFile proc <#tryRemoveFile,string>`_ ## * `removeFile proc <#removeFile,string>`_ ## * `moveFile proc <#moveFile,string,string>`_ ## * `copyDirWithPermissions proc <#copyDirWithPermissions,string,string>`_ - copyFile(source, dest) + copyFile(source, dest, options) when not defined(Windows): try: - setFilePermissions(dest, getFilePermissions(source)) + setFilePermissions(dest, getFilePermissions(source), followSymlinks = + (cfSymlinkFollow in options)) except: if not ignorePermissionErrors: raise proc copyDirWithPermissions*(source, dest: string, - ignorePermissionErrors = true) {.rtl, extern: "nos$1", - tags: [WriteIOEffect, ReadIOEffect], benign, noWeirdTarget.} = + ignorePermissionErrors = true) + {.rtl, extern: "nos$1", tags: [ReadDirEffect, WriteIOEffect, ReadIOEffect], + benign, noWeirdTarget.} = ## Copies a directory from `source` to `dest` preserving file permissions. ## + ## Symlinks are copied as symlinks. + ## ## If this fails, `OSError` is raised. This is a wrapper proc around `copyDir ## <#copyDir,string,string>`_ and `copyFileWithPermissions ## <#copyFileWithPermissions,string,string>`_ procs @@ -2511,18 +2591,17 @@ proc copyDirWithPermissions*(source, dest: string, createDir(dest) when not defined(Windows): try: - setFilePermissions(dest, getFilePermissions(source)) + setFilePermissions(dest, getFilePermissions(source), followSymlinks = + false) except: if not ignorePermissionErrors: raise for kind, path in walkDir(source): var noSource = splitPath(path).tail - case kind - of pcFile: - copyFileWithPermissions(path, dest / noSource, ignorePermissionErrors) - of pcDir: + if kind == pcDir: copyDirWithPermissions(path, dest / noSource, ignorePermissionErrors) - else: discard + else: + copyFileWithPermissions(path, dest / noSource, ignorePermissionErrors, {cfSymlinkAsIs}) proc inclFilePermissions*(filename: string, permissions: set[FilePermission]) {. @@ -2542,25 +2621,6 @@ proc exclFilePermissions*(filename: string, ## setFilePermissions(filename, getFilePermissions(filename)-permissions) setFilePermissions(filename, getFilePermissions(filename)-permissions) -proc expandSymlink*(symlinkPath: string): string {.noWeirdTarget.} = - ## Returns a string representing the path to which the symbolic link points. - ## - ## On Windows this is a noop, ``symlinkPath`` is simply returned. - ## - ## See also: - ## * `createSymlink proc <#createSymlink,string,string>`_ - when defined(windows): - result = symlinkPath - else: - result = newString(maxSymlinkLen) - var len = readlink(symlinkPath, result, maxSymlinkLen) - if len < 0: - raiseOSError(osLastError(), symlinkPath) - if len > maxSymlinkLen: - result = newString(len+1) - len = readlink(symlinkPath, result, len) - setLen(result, len) - proc parseCmdLine*(c: string): seq[string] {. noSideEffect, rtl, extern: "nos$1".} = ## Splits a `command line`:idx: into several components. diff --git a/lib/windows/winlean.nim b/lib/windows/winlean.nim index 1334a85d549df..8b8c42d651cf6 100644 --- a/lib/windows/winlean.nim +++ b/lib/windows/winlean.nim @@ -350,6 +350,13 @@ else: proc findClose*(hFindFile: Handle) {.stdcall, dynlib: "kernel32", importc: "FindClose".} +type LPPROGRESS_ROUTINE* = proc(TotalFileSize, TotalBytesTransferred, + StreamSize, StreamBytesTransferred: int64, dwStreamNumber, + dwCallbackReason: DWORD, hSourceFile, hDestinationFile: Handle, + lpData: pointer): void {.stdcall.} + +const COPY_FILE_COPY_SYMLINK* = 0x00000800'i32 + when useWinUnicode: proc getFullPathNameW*(lpFileName: WideCString, nBufferLength: int32, lpBuffer: WideCString, @@ -366,6 +373,10 @@ when useWinUnicode: proc copyFileW*(lpExistingFileName, lpNewFileName: WideCString, bFailIfExists: WINBOOL): WINBOOL {. importc: "CopyFileW", stdcall, dynlib: "kernel32", sideEffect.} + proc copyFileExW*(lpExistingFileName, lpNewFileName: WideCString, + lpProgressRoutine: LPPROGRESS_ROUTINE, lpData: pointer, + pbCancel: ptr WINBOOL, dwCopyFlags: DWORD): WINBOOL {. + importc: "CopyFileExW", stdcall, dynlib: "kernel32", sideEffect.} proc moveFileW*(lpExistingFileName, lpNewFileName: WideCString): WINBOOL {. importc: "MoveFileW", stdcall, dynlib: "kernel32", sideEffect.} @@ -396,6 +407,10 @@ else: proc copyFileA*(lpExistingFileName, lpNewFileName: cstring, bFailIfExists: cint): cint {. importc: "CopyFileA", stdcall, dynlib: "kernel32", sideEffect.} + proc copyFileExA*(lpExistingFileName, lpNewFileName: cstring, + lpProgressRoutine: LPPROGRESS_ROUTINE, lpData: pointer, + pbCancel: ptr WINBOOL, dwCopyFlags: DWORD): WINBOOL {. + importc: "CopyFileExA", stdcall, dynlib: "kernel32", sideEffect.} proc moveFileA*(lpExistingFileName, lpNewFileName: cstring): WINBOOL {. importc: "MoveFileA", stdcall, dynlib: "kernel32", sideEffect.} diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index af3606a4a1d04..dac06e5127590 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -154,6 +154,114 @@ block fileOperations: doAssert fileExists("../dest/a/file.txt") removeDir("../dest") + # Symlink handling in `copyFile`, `copyFileWithPermissions`, `copyFileToDir`, + # `copyDir`, `copyDirWithPermissions`, `moveFile`, and `moveDir`. + block: + when not defined(windows): + const checkExpandSymlink = true + else: + const checkExpandSymlink = false + + const buildDir = currentSourcePath.parentDir.parentDir/"build" + const dname = buildDir/"D20210116T140629" + const subDir = dname/"sub" + const subDir2 = dname/"sub2" + const brokenSymlinkName = "D20210101T191320_BROKEN_SYMLINK" + const brokenSymlink = dname/brokenSymlinkName + const brokenSymlinkSrc = "D20210101T191320_I_DO_NOT_EXIST" + const brokenSymlinkCopy = brokenSymlink & "_COPY" + const brokenSymlinkInSubDir = subDir/brokenSymlinkName + const brokenSymlinkInSubDir2 = subDir2/brokenSymlinkName + + createDir(subDir) + createSymlink(brokenSymlinkSrc, brokenSymlink) + + # Test copyFile + doAssertRaises(OSError): + copyFile(brokenSymlink, brokenSymlinkCopy) + doAssertRaises(OSError): + copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkFollow}) + copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkIgnore}) + doAssert not fileExists(brokenSymlinkCopy) + copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkAsIs}) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkCopy) + removeFile(brokenSymlinkCopy) + doAssertRaises(AssertionDefect): + copyFile(brokenSymlink, brokenSymlinkCopy, + {cfSymlinkAsIs, cfSymlinkFollow}) + + # Test copyFileWithPermissions + doAssertRaises(OSError): + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy) + doAssertRaises(OSError): + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, + options = {cfSymlinkFollow}) + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, + options = {cfSymlinkIgnore}) + doAssert not fileExists(brokenSymlinkCopy) + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, + options = {cfSymlinkAsIs}) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkCopy) + removeFile(brokenSymlinkCopy) + doAssertRaises(AssertionDefect): + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, + options = {cfSymlinkAsIs, cfSymlinkFollow}) + + # Test copyFileToDir + doAssertRaises(OSError): + copyFileToDir(brokenSymlink, subDir) + doAssertRaises(OSError): + copyFileToDir(brokenSymlink, subDir, {cfSymlinkFollow}) + copyFileToDir(brokenSymlink, subDir, {cfSymlinkIgnore}) + doAssert not fileExists(brokenSymlinkInSubDir) + copyFileToDir(brokenSymlink, subDir, {cfSymlinkAsIs}) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkInSubDir) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkInSubDir) + removeFile(brokenSymlinkInSubDir) + + createSymlink(brokenSymlinkSrc, brokenSymlinkInSubDir) + + # Test copyDir + copyDir(subDir, subDir2) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkInSubDir2) + removeDir(subDir2) + + # Test copyDirWithPermissions + copyDirWithPermissions(subDir, subDir2) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkInSubDir2) + removeDir(subDir2) + + # Test moveFile + moveFile(brokenSymlink, brokenSymlinkCopy) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkCopy) + removeFile(brokenSymlinkCopy) + + # Test moveDir + moveDir(subDir, subDir2) + when checkExpandSymlink: + doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc + else: + doAssert symlinkExists(brokenSymlinkInSubDir2) + + removeDir(dname) + import times block modificationTime: # Test get/set modification times diff --git a/tests/test_nimscript.nims b/tests/test_nimscript.nims index ea640cac69203..b492bb529c69e 100644 --- a/tests/test_nimscript.nims +++ b/tests/test_nimscript.nims @@ -87,3 +87,29 @@ block: try: doAssert false except Exception as e: discard + +block: # #16709 + const buildDir = currentSourcePath.parentDir.parentDir/"build" + const dname = buildDir/"D20210121T175016" + const subDir = dname/"sub" + const subDir2 = dname/"sub2" + const fpath = subDir/"f" + const fpath2 = subDir/"f2" + const fpath3 = subDir2/"f" + mkDir(subDir) + writeFile(fpath, "some text") + cpFile(fpath, fpath2) + doAssert fileExists(fpath2) + rmFile(fpath2) + cpDir(subDir, subDir2) + doAssert fileExists(fpath3) + rmDir(subDir2) + mvFile(fpath, fpath2) + doAssert not fileExists(fpath) + doAssert fileExists(fpath2) + mvFile(fpath2, fpath) + mvDir(subDir, subDir2) + doAssert not dirExists(subDir) + doAssert dirExists(subDir2) + mvDir(subDir2, subDir) + rmDir(dname) \ No newline at end of file From d3bd6dca1d2c9658f53e042cb14b795c4616dcef Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Sun, 31 Jan 2021 09:04:51 +0200 Subject: [PATCH 2/3] Address comments in #16709 Co-authored-by: Timothee Cour --- changelog.md | 2 +- lib/posix/posix.nim | 2 +- lib/posix/posix_haiku.nim | 1 - lib/posix/posix_linux_amd64.nim | 1 - lib/posix/posix_macos_amd64.nim | 1 - lib/posix/posix_nintendoswitch.nim | 1 - lib/posix/posix_openbsd_amd64.nim | 1 - lib/posix/posix_other.nim | 2 -- lib/pure/os.nim | 34 ++++++++++++++++++++---------- tests/stdlib/tos.nim | 4 ++-- tests/test_nimscript.nims | 5 +++-- 11 files changed, 30 insertions(+), 24 deletions(-) diff --git a/changelog.md b/changelog.md index e9cc3a35f2ea1..26c24f2158497 100644 --- a/changelog.md +++ b/changelog.md @@ -118,7 +118,7 @@ with other backends. see #9125. Use `-d:nimLegacyJsRound` for previous behavior. symlinks point to). - `copyDir` and `copyDirWithPermissions` copy symlinks as symlinks (instead of skipping them as it was before). -- `moveFile` and `moveDir` move symlinks as they are (instead of skipping them +- `moveFile` and `moveDir` move symlinks as symlinks (instead of skipping them sometimes as it was before). - Added optional `followSymlinks` argument to `setFilePermissions`. diff --git a/lib/posix/posix.nim b/lib/posix/posix.nim index db5e2d681abb3..e8ad786e90b5d 100644 --- a/lib/posix/posix.nim +++ b/lib/posix/posix.nim @@ -586,7 +586,7 @@ proc fstatvfs*(a1: cint, a2: var Statvfs): cint {. importc, header: "".} proc chmod*(a1: cstring, a2: Mode): cint {.importc, header: "", sideEffect.} -when hasLchmod: +when defined(osx) or defined(freebsd): proc lchmod*(a1: cstring, a2: Mode): cint {.importc, header: "", sideEffect.} proc fchmod*(a1: cint, a2: Mode): cint {.importc, header: "", sideEffect.} proc fstat*(a1: cint, a2: var Stat): cint {.importc, header: "", sideEffect.} diff --git a/lib/posix/posix_haiku.nim b/lib/posix/posix_haiku.nim index ab977527d6072..eaf2cfb857f50 100644 --- a/lib/posix/posix_haiku.nim +++ b/lib/posix/posix_haiku.nim @@ -13,7 +13,6 @@ when defined(nimHasStyleChecks): const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = defined(linux) - hasLchmod* = false when defined(linux) and not defined(android): # On Linux: diff --git a/lib/posix/posix_linux_amd64.nim b/lib/posix/posix_linux_amd64.nim index 9e7fe192082bc..7f6a589f08501 100644 --- a/lib/posix/posix_linux_amd64.nim +++ b/lib/posix/posix_linux_amd64.nim @@ -15,7 +15,6 @@ const hasSpawnH = not defined(haiku) # should exist for every Posix system nowadays hasAioH = defined(linux) - hasLchmod* = false # On Linux: # timer_{create,delete,settime,gettime}, diff --git a/lib/posix/posix_macos_amd64.nim b/lib/posix/posix_macos_amd64.nim index f408c93399d9e..94c08acad468f 100644 --- a/lib/posix/posix_macos_amd64.nim +++ b/lib/posix/posix_macos_amd64.nim @@ -13,7 +13,6 @@ when defined(nimHasStyleChecks): const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = false - hasLchmod* = true type DIR* {.importc: "DIR", header: "", diff --git a/lib/posix/posix_nintendoswitch.nim b/lib/posix/posix_nintendoswitch.nim index 19ce37a22dc0b..44e4314379950 100644 --- a/lib/posix/posix_nintendoswitch.nim +++ b/lib/posix/posix_nintendoswitch.nim @@ -12,7 +12,6 @@ const hasSpawnH = true hasAioH = false - hasLchmod* = false type DIR* {.importc: "DIR", header: "", diff --git a/lib/posix/posix_openbsd_amd64.nim b/lib/posix/posix_openbsd_amd64.nim index d342f50bc0210..584d0650116f0 100644 --- a/lib/posix/posix_openbsd_amd64.nim +++ b/lib/posix/posix_openbsd_amd64.nim @@ -13,7 +13,6 @@ when defined(nimHasStyleChecks): const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = false - hasLchmod* = false type DIR* {.importc: "DIR", header: "", diff --git a/lib/posix/posix_other.nim b/lib/posix/posix_other.nim index 1492f57d75c34..a5eb32d2267fa 100644 --- a/lib/posix/posix_other.nim +++ b/lib/posix/posix_other.nim @@ -14,12 +14,10 @@ when defined(freertos): const hasSpawnH = false # should exist for every Posix system nowadays hasAioH = false - hasLchmod* = false else: const hasSpawnH = true # should exist for every Posix system nowadays hasAioH = defined(linux) - hasLchmod* = false when defined(linux) and not defined(android): # On Linux: diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 5e6f0df577134..443cabb5bf637 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1598,10 +1598,11 @@ proc setFilePermissions*(filename: string, permissions: set[FilePermission], noWeirdTarget.} = ## Sets the file permissions for `filename`. ## - ## If ``followSymlinks`` set to true (default) and ``filename`` points to a + ## If `followSymlinks` set to true (default) and ``filename`` points to a ## symlink, permissions are set to the file symlink points to. - ## ``followSymlinks`` set to false do not affect on Windows and some POSIX - ## systems (including Linux) which do not have ``lchmod`` available. + ## `followSymlinks` set to false is a noop on Windows and some POSIX + ## systems (including Linux) on which `lchmod` is either unavailable or always + ## fails, given that symlinks permissions there are not observed. ## ## `OSError` is raised in case of an error. ## On Windows, only the ``readonly`` flag is changed, depending on @@ -1625,7 +1626,7 @@ proc setFilePermissions*(filename: string, permissions: set[FilePermission], if fpOthersExec in permissions: p = p or S_IXOTH.Mode if not followSymlinks and filename.symlinkExists: - when hasLchmod: + when declared(lchmod): if lchmod(filename, cast[Mode](p)) != 0: raiseOSError(osLastError(), $(filename, permissions)) else: @@ -1653,16 +1654,16 @@ proc createSymlink*(src, dest: string) {.noWeirdTarget.} = ## ## **Warning**: ## Some OS's (such as Microsoft Windows) restrict the creation - ## of symlinks to root users (administrators). + ## of symlinks to root users (administrators) or users with developper mode enabled. ## ## See also: ## * `createHardlink proc <#createHardlink,string,string>`_ ## * `expandSymlink proc <#expandSymlink,string>`_ when defined(Windows): - # 2 is the SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE. This allows - # anyone with developer mode on to create a link - let flag = dirExists(src).int32 or 2 + const SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE = 2 + # allows anyone with developer mode on to create a link + let flag = dirExists(src).int32 or SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE when useWinUnicode: var wSrc = newWideCString(src) var wDst = newWideCString(dest) @@ -1678,7 +1679,7 @@ proc createSymlink*(src, dest: string) {.noWeirdTarget.} = proc expandSymlink*(symlinkPath: string): string {.noWeirdTarget.} = ## Returns a string representing the path to which the symbolic link points. ## - ## On Windows this is a noop, ``symlinkPath`` is simply returned. + ## On Windows this is a noop, `symlinkPath` is simply returned. ## ## See also: ## * `createSymlink proc <#createSymlink,string,string>`_ @@ -1761,16 +1762,27 @@ proc copyFile*(source, dest: string, options = {cfSymlinkFollow}) {.rtl, if isSymlink and cfSymlinkIgnore in options: return when defined(Windows): + proc handleOSError = + const ERROR_PRIVILEGE_NOT_HELD = 1314 + let errCode = osLastError() + let context = $(source, dest, options) + if isSymlink and errCode.int32 == ERROR_PRIVILEGE_NOT_HELD: + stderr.write("Failed copy the symlink: error " & $errCode & ": " & + osErrorMsg(errCode) & "Additional info: " & context & + "\n") + else: + raiseOSError(errCode, context) + let dwCopyFlags = if cfSymlinkAsIs in options: COPY_FILE_COPY_SYMLINK else: 0'i32 var pbCancel = 0'i32 when useWinUnicode: let s = newWideCString(source) let d = newWideCString(dest) if copyFileExW(s, d, nil, nil, addr pbCancel, dwCopyFlags) == 0'i32: - raiseOSError(osLastError(), $(source, dest, options)) + handleOSError() else: if copyFileExA(source, dest, nil, nil, addr pbCancel, dwCopyFlags) == 0'i32: - raiseOSError(osLastError(), $(source, dest, options)) + handleOSError() elif hasCCopyfile: let state = copyfile_state_alloc() # xxx `COPYFILE_STAT` could be used for one-shot `copyFileWithPermissions`. diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index dac06e5127590..c22682561bd65 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -26,6 +26,7 @@ Raises # test os path creation, iteration, and deletion import os, strutils, pathnorm +from stdtest/specialpaths import buildDir block fileOperations: let files = @["these.txt", "are.x", "testing.r", "files.q"] @@ -162,13 +163,12 @@ block fileOperations: else: const checkExpandSymlink = false - const buildDir = currentSourcePath.parentDir.parentDir/"build" const dname = buildDir/"D20210116T140629" const subDir = dname/"sub" const subDir2 = dname/"sub2" const brokenSymlinkName = "D20210101T191320_BROKEN_SYMLINK" const brokenSymlink = dname/brokenSymlinkName - const brokenSymlinkSrc = "D20210101T191320_I_DO_NOT_EXIST" + const brokenSymlinkSrc = "D20210101T191320_nonexistant" const brokenSymlinkCopy = brokenSymlink & "_COPY" const brokenSymlinkInSubDir = subDir/brokenSymlinkName const brokenSymlinkInSubDir2 = subDir2/brokenSymlinkName diff --git a/tests/test_nimscript.nims b/tests/test_nimscript.nims index b492bb529c69e..9bfdff55ec805 100644 --- a/tests/test_nimscript.nims +++ b/tests/test_nimscript.nims @@ -3,6 +3,8 @@ {.warning[UnusedImport]: off.} +from stdtest/specialpaths import buildDir + import std/[ # Core: bitops, typetraits, lenientops, macros, volatile, @@ -88,8 +90,7 @@ block: except Exception as e: discard -block: # #16709 - const buildDir = currentSourcePath.parentDir.parentDir/"build" +block: # cpDir, cpFile, dirExists, fileExists, mkDir, mvDir, mvFile, rmDir, rmFile const dname = buildDir/"D20210121T175016" const subDir = dname/"sub" const subDir2 = dname/"sub2" From c1301f71958f1cbd820bb677a530ce926992dcd5 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 4 Feb 2021 15:16:58 +0200 Subject: [PATCH 3/3] Address comments in #16709 (second iteration) Skip symlinks on Windows. --- changelog.md | 14 ++--- lib/pure/os.nim | 113 +++++++++++++++++++--------------------- lib/windows/winlean.nim | 15 ------ tests/stdlib/tos.nim | 67 ++++++++++++------------ 4 files changed, 95 insertions(+), 114 deletions(-) diff --git a/changelog.md b/changelog.md index 26c24f2158497..e36a6ae3090ac 100644 --- a/changelog.md +++ b/changelog.md @@ -114,12 +114,14 @@ with other backends. see #9125. Use `-d:nimLegacyJsRound` for previous behavior. - Added optional `options` argument to `copyFile`, `copyFileToDir`, and - `copyFileWithPermissions`. By default, symlinks are followed (copy files - symlinks point to). -- `copyDir` and `copyDirWithPermissions` copy symlinks as symlinks (instead of - skipping them as it was before). -- `moveFile` and `moveDir` move symlinks as symlinks (instead of skipping them - sometimes as it was before). + `copyFileWithPermissions`. By default, on non-Windows OSes, symlinks are + followed (copy files symlinks point to); on Windows, `options` argument is + ignored and symlinks are skipped. +- On non-Windows OSes, `copyDir` and `copyDirWithPermissions` copy symlinks as + symlinks (instead of skipping them as it was before); on Windows symlinks are + skipped. +- On non-Windows OSes, `moveFile` and `moveDir` move symlinks as symlinks + (instead of skipping them sometimes as it was before). - Added optional `followSymlinks` argument to `setFilePermissions`. ## Language changes diff --git a/lib/pure/os.nim b/lib/pure/os.nim index 443cabb5bf637..3b5d2cf1aa630 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -1725,8 +1725,9 @@ proc copyFile*(source, dest: string, options = {cfSymlinkFollow}) {.rtl, noWeirdTarget.} = ## Copies a file from `source` to `dest`, where `dest.parentDir` must exist. ## - ## `options` specify the way file is copied; by default, if `source` is a - ## symlink, copies the file symlink points to. + ## On non-Windows OSes, `options` specify the way file is copied; by default, + ## if `source` is a symlink, copies the file symlink points to. `options` is + ## ignored on Windows: symlinks are skipped. ## ## If this fails, `OSError` is raised. ## @@ -1759,73 +1760,64 @@ proc copyFile*(source, dest: string, options = {cfSymlinkFollow}) {.rtl, doAssert card(copyFlagSymlink * options) == 1, "There should be exactly " & "one cfSymlink* in options" let isSymlink = source.symlinkExists - if isSymlink and cfSymlinkIgnore in options: + if isSymlink and (cfSymlinkIgnore in options or defined(windows)): return when defined(Windows): - proc handleOSError = - const ERROR_PRIVILEGE_NOT_HELD = 1314 - let errCode = osLastError() - let context = $(source, dest, options) - if isSymlink and errCode.int32 == ERROR_PRIVILEGE_NOT_HELD: - stderr.write("Failed copy the symlink: error " & $errCode & ": " & - osErrorMsg(errCode) & "Additional info: " & context & - "\n") - else: - raiseOSError(errCode, context) - - let dwCopyFlags = if cfSymlinkAsIs in options: COPY_FILE_COPY_SYMLINK else: 0'i32 - var pbCancel = 0'i32 when useWinUnicode: let s = newWideCString(source) let d = newWideCString(dest) - if copyFileExW(s, d, nil, nil, addr pbCancel, dwCopyFlags) == 0'i32: - handleOSError() + if copyFileW(s, d, 0'i32) == 0'i32: + raiseOSError(osLastError(), $(source, dest)) else: - if copyFileExA(source, dest, nil, nil, addr pbCancel, dwCopyFlags) == 0'i32: - handleOSError() - elif hasCCopyfile: - let state = copyfile_state_alloc() - # xxx `COPYFILE_STAT` could be used for one-shot `copyFileWithPermissions`. - let status = c_copyfile(source.cstring, dest.cstring, state, COPYFILE_DATA) - if status != 0: - let err = osLastError() - discard copyfile_state_free(state) - raiseOSError(err, $(source, dest)) - let status2 = copyfile_state_free(state) - if status2 != 0: raiseOSError(osLastError(), $(source, dest)) + if copyFileA(source, dest, 0'i32) == 0'i32: + raiseOSError(osLastError(), $(source, dest)) else: if isSymlink and cfSymlinkAsIs in options: createSymlink(expandSymlink(source), dest) else: - # generic version of copyFile which works for any platform: - const bufSize = 8000 # better for memory manager - var d, s: File - if not open(s, source): raiseOSError(osLastError(), source) - if not open(d, dest, fmWrite): + when hasCCopyfile: + let state = copyfile_state_alloc() + # xxx `COPYFILE_STAT` could be used for one-shot + # `copyFileWithPermissions`. + let status = c_copyfile(source.cstring, dest.cstring, state, + COPYFILE_DATA) + if status != 0: + let err = osLastError() + discard copyfile_state_free(state) + raiseOSError(err, $(source, dest)) + let status2 = copyfile_state_free(state) + if status2 != 0: raiseOSError(osLastError(), $(source, dest)) + else: + # generic version of copyFile which works for any platform: + const bufSize = 8000 # better for memory manager + var d, s: File + if not open(s, source):raiseOSError(osLastError(), source) + if not open(d, dest, fmWrite): + close(s) + raiseOSError(osLastError(), dest) + var buf = alloc(bufSize) + while true: + var bytesread = readBuffer(s, buf, bufSize) + if bytesread > 0: + var byteswritten = writeBuffer(d, buf, bytesread) + if bytesread != byteswritten: + dealloc(buf) + close(s) + close(d) + raiseOSError(osLastError(), dest) + if bytesread != bufSize: break + dealloc(buf) close(s) - raiseOSError(osLastError(), dest) - var buf = alloc(bufSize) - while true: - var bytesread = readBuffer(s, buf, bufSize) - if bytesread > 0: - var byteswritten = writeBuffer(d, buf, bytesread) - if bytesread != byteswritten: - dealloc(buf) - close(s) - close(d) - raiseOSError(osLastError(), dest) - if bytesread != bufSize: break - dealloc(buf) - close(s) - flushFile(d) - close(d) + flushFile(d) + close(d) proc copyFileToDir*(source, dir: string, options = {cfSymlinkFollow}) {.noWeirdTarget, since: (1,3,7).} = ## Copies a file `source` into directory `dir`, which must exist. ## - ## `options` specify the way file is copied; by default, if `source` is a - ## symlink, copies the file symlink points to. + ## On non-Windows OSes, `options` specify the way file is copied; by default, + ## if `source` is a symlink, copies the file symlink points to. `options` is + ## ignored on Windows: symlinks are skipped. ## ## See also: ## * `CopyFlag enum <#CopyFlag>`_ @@ -2460,7 +2452,8 @@ proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", tags: [ReadDirEffect, WriteIOEffect, ReadIOEffect], benign, noWeirdTarget.} = ## Copies a directory from `source` to `dest`. ## - ## Symlinks are copied as symlinks. + ## On non-Windows OSes, symlinks are copied as symlinks. On Windows, symlinks + ## are skipped. ## ## If this fails, `OSError` is raised. ## @@ -2491,8 +2484,8 @@ proc copyDir*(source, dest: string) {.rtl, extern: "nos$1", proc moveDir*(source, dest: string) {.tags: [ReadIOEffect, WriteIOEffect], noWeirdTarget.} = ## Moves a directory from `source` to `dest`. ## - ## Symlinks are not followed: if `source` contains symlinks, they itself are - ## moved, not theirs target. + ## Symlinks are not followed: if `source` contains symlinks, they themself are + ## moved, not their target. ## ## If this fails, `OSError` is raised. ## @@ -2536,8 +2529,9 @@ proc copyFileWithPermissions*(source, dest: string, options = {cfSymlinkFollow}) {.noWeirdTarget.} = ## Copies a file from `source` to `dest` preserving file permissions. ## - ## `options` specify the way file is copied; by default, if `source` is a - ## symlink, copies the file symlink points to. + ## On non-Windows OSes, `options` specify the way file is copied; by default, + ## if `source` is a symlink, copies the file symlink points to. `options` is + ## ignored on Windows: symlinks are skipped. ## ## This is a wrapper proc around `copyFile <#copyFile,string,string>`_, ## `getFilePermissions <#getFilePermissions,string>`_ and @@ -2576,7 +2570,8 @@ proc copyDirWithPermissions*(source, dest: string, benign, noWeirdTarget.} = ## Copies a directory from `source` to `dest` preserving file permissions. ## - ## Symlinks are copied as symlinks. + ## On non-Windows OSes, symlinks are copied as symlinks. On Windows, symlinks + ## are skipped. ## ## If this fails, `OSError` is raised. This is a wrapper proc around `copyDir ## <#copyDir,string,string>`_ and `copyFileWithPermissions diff --git a/lib/windows/winlean.nim b/lib/windows/winlean.nim index 8b8c42d651cf6..1334a85d549df 100644 --- a/lib/windows/winlean.nim +++ b/lib/windows/winlean.nim @@ -350,13 +350,6 @@ else: proc findClose*(hFindFile: Handle) {.stdcall, dynlib: "kernel32", importc: "FindClose".} -type LPPROGRESS_ROUTINE* = proc(TotalFileSize, TotalBytesTransferred, - StreamSize, StreamBytesTransferred: int64, dwStreamNumber, - dwCallbackReason: DWORD, hSourceFile, hDestinationFile: Handle, - lpData: pointer): void {.stdcall.} - -const COPY_FILE_COPY_SYMLINK* = 0x00000800'i32 - when useWinUnicode: proc getFullPathNameW*(lpFileName: WideCString, nBufferLength: int32, lpBuffer: WideCString, @@ -373,10 +366,6 @@ when useWinUnicode: proc copyFileW*(lpExistingFileName, lpNewFileName: WideCString, bFailIfExists: WINBOOL): WINBOOL {. importc: "CopyFileW", stdcall, dynlib: "kernel32", sideEffect.} - proc copyFileExW*(lpExistingFileName, lpNewFileName: WideCString, - lpProgressRoutine: LPPROGRESS_ROUTINE, lpData: pointer, - pbCancel: ptr WINBOOL, dwCopyFlags: DWORD): WINBOOL {. - importc: "CopyFileExW", stdcall, dynlib: "kernel32", sideEffect.} proc moveFileW*(lpExistingFileName, lpNewFileName: WideCString): WINBOOL {. importc: "MoveFileW", stdcall, dynlib: "kernel32", sideEffect.} @@ -407,10 +396,6 @@ else: proc copyFileA*(lpExistingFileName, lpNewFileName: cstring, bFailIfExists: cint): cint {. importc: "CopyFileA", stdcall, dynlib: "kernel32", sideEffect.} - proc copyFileExA*(lpExistingFileName, lpNewFileName: cstring, - lpProgressRoutine: LPPROGRESS_ROUTINE, lpData: pointer, - pbCancel: ptr WINBOOL, dwCopyFlags: DWORD): WINBOOL {. - importc: "CopyFileExA", stdcall, dynlib: "kernel32", sideEffect.} proc moveFileA*(lpExistingFileName, lpNewFileName: cstring): WINBOOL {. importc: "MoveFileA", stdcall, dynlib: "kernel32", sideEffect.} diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index c22682561bd65..b47412a62798e 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -158,11 +158,7 @@ block fileOperations: # Symlink handling in `copyFile`, `copyFileWithPermissions`, `copyFileToDir`, # `copyDir`, `copyDirWithPermissions`, `moveFile`, and `moveDir`. block: - when not defined(windows): - const checkExpandSymlink = true - else: - const checkExpandSymlink = false - + const symlinksAreHandled = not defined(windows) const dname = buildDir/"D20210116T140629" const subDir = dname/"sub" const subDir2 = dname/"sub2" @@ -177,77 +173,80 @@ block fileOperations: createSymlink(brokenSymlinkSrc, brokenSymlink) # Test copyFile - doAssertRaises(OSError): - copyFile(brokenSymlink, brokenSymlinkCopy) - doAssertRaises(OSError): - copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkFollow}) + when symlinksAreHandled: + doAssertRaises(OSError): + copyFile(brokenSymlink, brokenSymlinkCopy) + doAssertRaises(OSError): + copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkFollow}) copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkIgnore}) doAssert not fileExists(brokenSymlinkCopy) copyFile(brokenSymlink, brokenSymlinkCopy, {cfSymlinkAsIs}) - when checkExpandSymlink: + when symlinksAreHandled: doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc + removeFile(brokenSymlinkCopy) else: - doAssert symlinkExists(brokenSymlinkCopy) - removeFile(brokenSymlinkCopy) + doAssert not fileExists(brokenSymlinkCopy) doAssertRaises(AssertionDefect): copyFile(brokenSymlink, brokenSymlinkCopy, - {cfSymlinkAsIs, cfSymlinkFollow}) + {cfSymlinkAsIs, cfSymlinkFollow}) # Test copyFileWithPermissions - doAssertRaises(OSError): - copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy) - doAssertRaises(OSError): - copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, - options = {cfSymlinkFollow}) + when symlinksAreHandled: + doAssertRaises(OSError): + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy) + doAssertRaises(OSError): + copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, + options = {cfSymlinkFollow}) copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, options = {cfSymlinkIgnore}) doAssert not fileExists(brokenSymlinkCopy) copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, options = {cfSymlinkAsIs}) - when checkExpandSymlink: + when symlinksAreHandled: doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc + removeFile(brokenSymlinkCopy) else: - doAssert symlinkExists(brokenSymlinkCopy) - removeFile(brokenSymlinkCopy) + doAssert not fileExists(brokenSymlinkCopy) doAssertRaises(AssertionDefect): copyFileWithPermissions(brokenSymlink, brokenSymlinkCopy, options = {cfSymlinkAsIs, cfSymlinkFollow}) # Test copyFileToDir - doAssertRaises(OSError): - copyFileToDir(brokenSymlink, subDir) - doAssertRaises(OSError): - copyFileToDir(brokenSymlink, subDir, {cfSymlinkFollow}) + when symlinksAreHandled: + doAssertRaises(OSError): + copyFileToDir(brokenSymlink, subDir) + doAssertRaises(OSError): + copyFileToDir(brokenSymlink, subDir, {cfSymlinkFollow}) copyFileToDir(brokenSymlink, subDir, {cfSymlinkIgnore}) doAssert not fileExists(brokenSymlinkInSubDir) copyFileToDir(brokenSymlink, subDir, {cfSymlinkAsIs}) - when checkExpandSymlink: + when symlinksAreHandled: doAssert expandSymlink(brokenSymlinkInSubDir) == brokenSymlinkSrc + removeFile(brokenSymlinkInSubDir) else: - doAssert symlinkExists(brokenSymlinkInSubDir) - removeFile(brokenSymlinkInSubDir) + doAssert not fileExists(brokenSymlinkInSubDir) createSymlink(brokenSymlinkSrc, brokenSymlinkInSubDir) # Test copyDir copyDir(subDir, subDir2) - when checkExpandSymlink: + when symlinksAreHandled: doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc else: - doAssert symlinkExists(brokenSymlinkInSubDir2) + doAssert not fileExists(brokenSymlinkInSubDir2) removeDir(subDir2) # Test copyDirWithPermissions copyDirWithPermissions(subDir, subDir2) - when checkExpandSymlink: + when symlinksAreHandled: doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc else: - doAssert symlinkExists(brokenSymlinkInSubDir2) + doAssert not fileExists(brokenSymlinkInSubDir2) removeDir(subDir2) # Test moveFile moveFile(brokenSymlink, brokenSymlinkCopy) - when checkExpandSymlink: + when not defined(windows): doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc else: doAssert symlinkExists(brokenSymlinkCopy) @@ -255,7 +254,7 @@ block fileOperations: # Test moveDir moveDir(subDir, subDir2) - when checkExpandSymlink: + when not defined(windows): doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc else: doAssert symlinkExists(brokenSymlinkInSubDir2)