Skip to content

SWBUtil: use FileManager for more operations #189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 4 additions & 150 deletions Sources/SWBUtil/FSProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@ class LocalFS: FSProxy, @unchecked Sendable {
/// Check whether a given path is a symlink.
/// - parameter destinationExists: If the path is a symlink, then this `inout` parameter will be set to `true` if the destination exists. Otherwise it will be set to `false`.
func isSymlink(_ path: Path, _ destinationExists: inout Bool) -> Bool {
#if os(Windows)
do {
let destination = try fileManager.destinationOfSymbolicLink(atPath: path.str)
destinationExists = exists((path.isAbsolute ? path.dirname : Path.currentDirectory).join(destination))
Expand All @@ -371,22 +370,6 @@ class LocalFS: FSProxy, @unchecked Sendable {
destinationExists = false
return false
}
#else
destinationExists = false
var statBuf = stat()
if lstat(path.str, &statBuf) < 0 {
return false
}
guard createFileInfo(statBuf).isSymlink else {
return false
}
statBuf = stat()
if stat(path.str, &statBuf) < 0 {
return true
}
destinationExists = true
return true
#endif
}

func listdir(_ path: Path) throws -> [String] {
Expand All @@ -397,70 +380,11 @@ class LocalFS: FSProxy, @unchecked Sendable {
/// - parameter recursive: If `false`, then the parent directory at `path` must already exist in order to create the directory. If it doesn't, then it will return without creating the directory (it will not throw an exception). If `true`, then the directory hierarchy of `path` will be created if possible.
func createDirectory(_ path: Path, recursive: Bool) throws {
// Try to create the directory.
#if os(Windows)
do {
return try fileManager.createDirectory(atPath: path.str, withIntermediateDirectories: recursive)
} catch {
throw StubError.error("Could not create directory at path '\(path.str)': \(error)")
}
#else
let result = mkdir(path.str, S_IRWXU | S_IRWXG | S_IRWXO)

// If it succeeded, we are done.
if result == 0 {
return
}

// If the failure was because something exists at this path, then we examine it to see whether it means we're okay.
if errno == EEXIST {
var destinationExists = false
if isDirectory(path) {
// If the item at the path is a directory, then we're good. This includes if it's a symlink which points to a directory.
return
}
else if isSymlink(path, &destinationExists) {
// If the item at the path is a symlink, then we check whether it's a broken symlink or points to something that is not a directory.
if destinationExists {
// The destination does exist, so it's not a directory.
throw StubError.error("File is a symbolic link which references a path which is not a directory: \(path.str)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test expectations need to be updated to accommodate these specific error strings going away.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't see a good way to handle the check. There is no guarantee that the message is going to be identical across platforms and Foundation implementations :(

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine if we just check that it does fail in the expected cases without checking the specific error string.

}
else {
// The destination does not exist - throw an exception because we have a broken symlink.
throw StubError.error("File is a broken symbolic link: \(path.str)")
}
}
else {
/// The path exists but is not a directory
throw StubError.error("File exists but is not a directory: \(path.str)")
}
}

// If we are recursive and not the root path, then...
if recursive && !path.isRoot {
// If it failed due to ENOENT (e.g., a missing parent), then attempt to create the parent and retry.
if errno == ENOENT {
// Attempt to create the parent.
guard path.isAbsolute else {
throw StubError.error("Cannot recursively create directory at non-absolute path: \(path.str)")
}
try createDirectory(path.dirname, recursive: true)

// Re-attempt creation, non-recursively.
try createDirectory(path)

// We are done.
return
}

// If our parent is not a directory, then report that.
if !isDirectory(path.dirname) {
throw StubError.error("File exists but is not a directory: \(path.dirname.str)")
}
}

// Otherwise, we failed due to some other error. Report it.
throw POSIXError(errno, context: "mkdir", path.str, "S_IRWXU | S_IRWXG | S_IRWXO")
#endif
}

func createTemporaryDirectory(parent: Path) throws -> Path {
Expand Down Expand Up @@ -562,24 +486,12 @@ class LocalFS: FSProxy, @unchecked Sendable {
}

func remove(_ path: Path) throws {
guard unlink(path.str) == 0 else {
throw POSIXError(errno, context: "unlink", path.str)
}
try fileManager.removeItem(atPath: path.str)
}

func removeDirectory(_ path: Path) throws {
if isDirectory(path) {
#if os(Windows)
try fileManager.removeItem(atPath: path.str)
#else
var paths = [path]
try traverse(path) { paths.append($0) }
for path in paths.reversed() {
guard SWBLibc.remove(path.str) == 0 else {
throw POSIXError(errno, context: "remove", path.str)
}
}
#endif
}
}

Expand Down Expand Up @@ -608,69 +520,11 @@ class LocalFS: FSProxy, @unchecked Sendable {
}

func touch(_ path: Path) throws {
#if os(Windows)
let handle: HANDLE = path.withPlatformString {
CreateFileW($0, DWORD(GENERIC_WRITE), DWORD(FILE_SHARE_READ), nil,
DWORD(OPEN_EXISTING), DWORD(FILE_FLAG_BACKUP_SEMANTICS), nil)
}
if handle == INVALID_HANDLE_VALUE {
throw StubError.error("Failed to update file time")
}
try handle.closeAfter {
var ft = FILETIME()
var st = SYSTEMTIME()
GetSystemTime(&st)
SystemTimeToFileTime(&st, &ft)
if !SetFileTime(handle, nil, &ft, &ft) {
throw StubError.error("Failed to update file time")
}
}
#else
try eintrLoop {
guard utimensat(AT_FDCWD, path.str, nil, 0) == 0 else {
throw POSIXError(errno, context: "utimensat", "AT_FDCWD", path.str)
}
}
#endif
try fileManager.setAttributes([.modificationDate:Date()], ofItemAtPath: path.str)
}

func setFileTimestamp(_ path: Path, timestamp: Int) throws {
#if os(Windows)
let handle: HANDLE = path.withPlatformString {
CreateFileW($0, DWORD(GENERIC_WRITE), DWORD(FILE_SHARE_READ), nil,
DWORD(OPEN_EXISTING), DWORD(FILE_FLAG_BACKUP_SEMANTICS), nil)
}
if handle == INVALID_HANDLE_VALUE {
throw StubError.error("Failed to update file time")
}
try handle.closeAfter {
// Number of 100ns intervals between 1601 and 1970 epochs
let delta = 116444736000000000

let ll = UInt64((timestamp * 10000000) + delta)

var timeInt = ULARGE_INTEGER()
timeInt.QuadPart = ll

var ft = FILETIME()
ft.dwLowDateTime = timeInt.LowPart
ft.dwHighDateTime = timeInt.HighPart
if !SetFileTime(handle, nil, &ft, &ft) {
throw StubError.error("Failed to update file time")
}
}
#else
try eintrLoop {
#if os(Linux) || os(Android)
let UTIME_OMIT = 1073741822
#endif
let atime = timespec(tv_sec: 0, tv_nsec: Int(UTIME_OMIT))
let mtime = timespec(tv_sec: timestamp, tv_nsec: 0)
guard utimensat(AT_FDCWD, path.str, [atime, mtime], 0) == 0 else {
throw POSIXError(errno, context: "utimensat", "AT_FDCWD", path.str, String(timestamp))
}
}
#endif
try fileManager.setAttributes([.modificationDate:Date(timeIntervalSince1970: Double(timestamp))], ofItemAtPath: path.str)
}

func getFileInfo(_ path: Path) throws -> FileInfo {
Expand Down Expand Up @@ -711,7 +565,7 @@ class LocalFS: FSProxy, @unchecked Sendable {
#if os(Windows)
try eintrLoop {
guard stat(path.str, &buf) == 0 else {
throw POSIXError(errno, context: "lstat", path.str)
throw POSIXError(errno, context: "stat", path.str)
}
}

Expand Down
40 changes: 16 additions & 24 deletions Tests/SWBUtilTests/FSProxyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,8 @@ import SWBTestSupport
var didThrow = false
do {
try localFS.createDirectory(filePath)
}
catch {
} catch {
didThrow = true
#expect(error.localizedDescription == "File exists but is not a directory: \(filePath.str)")
}
#expect(didThrow)

Expand All @@ -108,10 +106,8 @@ import SWBTestSupport
didThrow = false
do {
try localFS.createDirectory(dirPath, recursive: true)
}
catch {
} catch {
didThrow = true
#expect(error.localizedDescription == "File exists but is not a directory: \(filePath.str)")
}
#expect(didThrow)
}
Expand All @@ -127,10 +123,8 @@ import SWBTestSupport
var didThrow = false
do {
try localFS.createDirectory(symlinkPath)
}
catch {
} catch {
didThrow = true
#expect(error.localizedDescription == "File is a broken symbolic link: \(symlinkPath.str)")
}
#expect(didThrow)

Expand All @@ -139,10 +133,8 @@ import SWBTestSupport
didThrow = false
do {
try localFS.createDirectory(dirPath, recursive: true)
}
catch {
} catch {
didThrow = true
#expect(error.localizedDescription == "File is a broken symbolic link: \(symlinkPath.str)")
}
#expect(didThrow)
}
Expand All @@ -164,10 +156,8 @@ import SWBTestSupport
var didThrow = false
do {
try localFS.createDirectory(symlinkPath)
}
catch {
} catch {
didThrow = true
#expect(error.localizedDescription == "File is a symbolic link which references a path which is not a directory: \(symlinkPath.str)")
}
#expect(didThrow)

Expand All @@ -176,32 +166,34 @@ import SWBTestSupport
didThrow = false
do {
try localFS.createDirectory(dirPath, recursive: true)
}
catch {
} catch {
didThrow = true
#expect(error.localizedDescription == "File exists but is not a directory: \(symlinkPath.str)")
}
#expect(didThrow)
}

// Test that trying to recursively create a directory with an empty path fails.
do {
let filePath = Path("")
#expect {
var didThrow = false
do {
try localFS.createDirectory(filePath, recursive: true)
} throws: { error in
error.localizedDescription == "Cannot recursively create directory at non-absolute path: "
} catch {
didThrow = true
}
#expect(didThrow)
}

// Test that trying to recursively create a directory with a relative path fails.
do {
let filePath = Path("foo/bar/baz")
#expect {
var didThrow = false
do {
try localFS.createDirectory(filePath, recursive: true)
} throws: { error in
error.localizedDescription == "Cannot recursively create directory at non-absolute path: foo/bar/baz"
} catch {
didThrow = true
}
#expect(didThrow)
}
}
}
Expand Down