Conversation
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
…feat/async_vfs_path
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
There was a problem hiding this comment.
Pull Request Overview
This PR adapts the synchronous and asynchronous virtual filesystem paths for no_std environments, replacing SystemTime with a custom Time type and restructuring modules to separate sync (awkernel_lib) and async (awkernel_async_lib) code. Key changes include:
- Migrated
awkernel_libVFS path API to useallocandTimeand exposedPathLikepublicly. - Introduced an async VFS layer in
awkernel_async_libwith updated file-system traits,AsyncVfsPath, and a newasync_copyhelper. - Updated module exports (
pub mod path/file) and added new dependencies (async-trait,async-recursion).
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 |
|---|---|
| awkernel_lib/src/file/vfs/path.rs | Switched imports to alloc, replaced SystemTime with Time, made PathLike public |
| awkernel_lib/src/file/vfs.rs | Re-exported path module |
| awkernel_async_lib/src/lib.rs | Added top-level file module |
| awkernel_async_lib/src/file/path.rs | Rewrote async path logic, updated method signatures, added async_copy, removed many docs |
| awkernel_async_lib/src/file/filesystem.rs | Extended AsyncSeekAndRead with read_to_string, added Debug to AsyncFileSystem |
| awkernel_async_lib/src/file.rs | Introduced filesystem and path modules |
| awkernel_async_lib/Cargo.toml | Added async-trait and async-recursion dependencies |
Comments suppressed due to low confidence (5)
awkernel_async_lib/src/file/path.rs:1
- [nitpick] Most public methods have lost their doc comments; adding concise documentation (including examples) for key APIs like
join,read_dir, andcopy_filewill improve discoverability and maintainability.
//! Async virtual filesystem path
awkernel_async_lib/src/file/filesystem.rs:113
- Adding a
Debugbound toAsyncFileSystemis a breaking change for existing implementations; consider documenting this change or making the bound optional to preserve backward compatibility.
pub trait AsyncFileSystem: Sync + Send + Debug {
awkernel_async_lib/src/file/path.rs:58
- [nitpick] Changing
AsyncVfsPath::newfrom a generic parameter to aBox<dyn AsyncFileSystem>reduces API flexibility; consider providing a generic constructor or helper to allow callers to pass concrete types directly.
pub fn new(filesystem: Box<dyn AsyncFileSystem + Send + Sync>) -> Self {
awkernel_async_lib/src/file/path.rs:613
- The new
async_copyfunction does not have any accompanying tests; adding unit tests (including error paths) will help ensure correctness and prevent regressions.
pub async fn async_copy<R, W>(reader: &mut R, writer: &mut W) -> VfsResult<u64>
awkernel_lib/src/file/vfs/path.rs:11
- The
Vecimport does not appear to be used in this file after trimming its contents; consider removing it to avoid an unused import.
};
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
veqcc
approved these changes
Jul 3, 2025
ytakano
approved these changes
Jul 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR ports AsyncVfsPath for no_std environments. File systems that implement AsyncFileSystem, which was introduced in a previous PR, will be able to perform various file operations using this AsyncVfsPath.
This pull request primarily involves changes due to VfsError becoming VfsError(PR #548), switching from std::time::SystemTime to awkernel_lib::time::Time, and removing unnecessary comments.
std::io::Read read_to_string() is used in read_to_string() method of this AsyncVfsPath. Therefore, I've implemented it myself in this PR, since we didn't have read_to_string() method in our AsyncSeekAndRead trait. It seems like it contains two unnecessary copies, but this is to follow the rule below.
Related links
#548 (delete type parameter E from VfsError)
#539 (introducing AsyncFilesystem)
#511 (main PR)
How was this PR tested?
Notes for reviewers
As a result of these recent deletions, awkernel_lib/src/file/vfs/path.rs is now almost empty, containing only helper functions necessary for AsyncVfsPath and the definition of VfsMetadata. It's highly likely that this content will eventually be moved to awkernel_async_lib/src/file/path.rs, but I'm holding off on that for this PR to avoid making it overly complex.