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..0fbc9f3d0d533 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,30 +1760,17 @@ 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() + if copyFileA(source, dest, 0'i32) == 0'i32: + raiseOSError(osLastError(), $(source, dest)) elif hasCCopyfile: let state = copyfile_state_alloc() # xxx `COPYFILE_STAT` could be used for one-shot `copyFileWithPermissions`. @@ -1824,8 +1812,9 @@ 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 +2449,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 +2481,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 +2526,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 +2567,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..902814018c2dc 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,88 +173,91 @@ 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 symlinksAreHandled: doAssert expandSymlink(brokenSymlinkCopy) == brokenSymlinkSrc else: - doAssert symlinkExists(brokenSymlinkCopy) + doAssert not fileExists(brokenSymlinkCopy) removeFile(brokenSymlinkCopy) # Test moveDir moveDir(subDir, subDir2) - when checkExpandSymlink: + when symlinksAreHandled: doAssert expandSymlink(brokenSymlinkInSubDir2) == brokenSymlinkSrc else: - doAssert symlinkExists(brokenSymlinkInSubDir2) + doAssert not fileExists(brokenSymlinkInSubDir2) removeDir(dname)