Implement path_rename
#195
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements the path_rename WASI function to support file and directory renaming operations. The implementation adds cross-directory rename support using the renameat system call and includes special handling for trailing slashes in paths.
- Implements the
path_renamefunction inWASIBridgeToHostwith proper directory validation - Adds
renamemethod toWASIDirprotocol andDirEntryimplementation with trailing slash handling - Extends
FileDescriptorwithrenameatsystem call wrapper functions
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/WASITests/IntegrationTests.swift | Removes previously failing rename tests from exclusion list |
| Sources/WASI/WASI.swift | Implements path_rename function with directory validation |
| Sources/WASI/Platform/SandboxPrimitives/OpenParent.swift | Adds utility functions for handling trailing slashes in paths |
| Sources/WASI/Platform/Directory.swift | Implements rename method with cross-platform support and trailing slash handling |
| Sources/WASI/FileSystem.swift | Adds rename method to WASIDir protocol |
| Sources/SystemExtras/Syscalls.swift | Adds system wrapper for renameat syscall |
| Sources/SystemExtras/FileAtOperations.swift | Adds FileDescriptor extension methods for rename operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Re-append a slash if the original path had one | ||
| let finalSourceBasename = oldHasTrailingSlash ? sourceBasename + "/" : sourceBasename | ||
| let finalDestBasename = newHasTrailingSlash ? destBasename + "/" : destBasename |
There was a problem hiding this comment.
[nitpick] Consider extracting the slash re-appending logic into a helper function to reduce code duplication and improve readability.
| to newDirFd: FileDescriptor, | ||
| at newPath: UnsafePointer<CInterop.PlatformChar> | ||
| ) -> Result<(), Errno> { | ||
| #if os(Windows) |
There was a problem hiding this comment.
The Windows error code ERROR_NOT_SUPPORTED should be documented or explained why this specific error is chosen over other potential Windows error codes for unsupported operations.
| #if os(Windows) | |
| #if os(Windows) | |
| // The Windows API does not provide a direct equivalent to `renameat`. | |
| // ERROR_NOT_SUPPORTED is used to indicate that this operation is not supported on Windows. |
kateinoigakukun
force-pushed
the
katei/implement-path-rename
branch
from
67dc2db to
a1aa0e0
Compare
No description provided.