Skip to content

Conversation

@Koichi98
Copy link
Contributor

@Koichi98 Koichi98 commented Jun 26, 2025

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.

  • String's invariant that it must always contain valid UTF-8. Rust does not permit this state to occur in safe code.

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.

@Koichi98 Koichi98 self-assigned this Jun 26, 2025
@Koichi98 Koichi98 changed the base branch from main to fix/Vfserror_for_nostd June 28, 2025 10:01
@Koichi98 Koichi98 changed the base branch from fix/Vfserror_for_nostd to fix/asyncfs_ioerror June 28, 2025 14:40
Base automatically changed from fix/asyncfs_ioerror to main July 1, 2025 07:58
Koichi98 added 7 commits July 1, 2025 17:25
Signed-off-by: Koichi Imai <[email protected]>
Signed-off-by: Koichi Imai <[email protected]>
Signed-off-by: Koichi Imai <[email protected]>
Signed-off-by: Koichi Imai <[email protected]>
Signed-off-by: Koichi Imai <[email protected]>
@Koichi98 Koichi98 changed the base branch from main to fix/introduce_vfsioerror July 2, 2025 11:48
Koichi98 and others added 5 commits July 2, 2025 20:51
Base automatically changed from fix/introduce_vfsioerror to main July 3, 2025 01:04
Koichi98 added 8 commits July 3, 2025 13:18
Signed-off-by: Koichi <[email protected]>
Signed-off-by: Koichi <[email protected]>
Signed-off-by: Koichi <[email protected]>
Signed-off-by: Koichi <[email protected]>
Signed-off-by: Koichi <[email protected]>
@Koichi98 Koichi98 requested review from Copilot, veqcc and ytakano and removed request for Copilot July 3, 2025 06:55
@Koichi98 Koichi98 marked this pull request as ready for review July 3, 2025 06:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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_lib VFS path API to use alloc and Time and exposed PathLike publicly.
  • Introduced an async VFS layer in awkernel_async_lib with updated file-system traits, AsyncVfsPath, and a new async_copy helper.
  • 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, and copy_file will improve discoverability and maintainability.
//! Async virtual filesystem path

awkernel_async_lib/src/file/filesystem.rs:113

  • Adding a Debug bound to AsyncFileSystem is 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::new from a generic parameter to a Box<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_copy function 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 Vec import does not appear to be used in this file after trimming its contents; consider removing it to avoid an unused import.
};

Koichi98 and others added 2 commits July 3, 2025 16:01
@ytakano ytakano merged commit 5907dd3 into main Jul 3, 2025
1 check passed
@ytakano ytakano deleted the feat/async_vfs_path branch July 3, 2025 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants