| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: b52aead3-702d-4304-b5fa-87220018cf62 📥 CommitsReviewing files that changed from the base of the PR and between 1c7759c and 29ee814. 📒 Files selected for processing (6)
📝 Walkthrough WalkthroughThe change replaces non-Windows libc stat operations with Rustix, moves stat_path to the Unix-like implementation, updates VM path-stat wiring, and normalizes timestamp and filesystem block fields. ChangesFilesystem stat migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: youknowone 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@crates/host_env/src/fileutils.rs`: - Line 5: Update the NetBSD branch in the OS stdlib implementation to access StatStruct fields st_atime_nsec, st_mtime_nsec, and st_ctime_nsec, replacing the legacy st_atimensec, st_mtimensec, and st_ctimensec names while preserving the existing timestamp behavior.
Fix all unresolved CodeRabbit comments on this PR:
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 30728473-f83e-44b2-88e7-d046b6050213
📥 CommitsReviewing files that changed from the base of the PR and between 1fa76a3 and b08a96b.
📒 Files selected for processing (6)
Sorry, something went wrong.
There was a problem hiding this comment.
as always, great finding!
ty<3
Sorry, something went wrong.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@crates/vm/src/stdlib/os.rs`: - Around line 1237-1243: Fix the conditional compilation around the timestamp assignments in the enclosing function by making the NetBSD condition consistent across both adjacent cfg attributes. Remove NetBSD from the exclusion in the second cfg so atime, mtime, and ctime are assigned for NetBSD while retaining the existing WASI exclusion.
Fix all unresolved CodeRabbit comments on this PR:
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5da13ae1-5f7a-4177-8be0-3eaf2ee864fc
📥 CommitsReviewing files that changed from the base of the PR and between 1c7759c and 71d5994.
📒 Files selected for processing (6)
Sorry, something went wrong.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Sorry, something went wrong.
According to POSIX, `time_t` should be 64 bits. `musl` changed its `time_t` to an `i64` over five years ago. `glibc` provides compatibility features that declare `time_t` as either `i32` or `i64`. Rustix uses the raw Linux syscall for stat which returns an `i64`. For our purposes, an `i64` makes sense because it's modern and avoids the year 2038 problem. It also reduces our dependency on what `libc` defines. Sources: * https://www.man7.org/linux/man-pages/man3/time_t.3type.html * https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/sys_types.h.html
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Sorry, something went wrong.
|
Cargo shear failed randomly. 🤔 |
Sorry, something went wrong.
|
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks, I wish we didn't lose CPython compatible behavior by this
Sorry, something went wrong.
According to POSIX, `time_t` should be 64 bits. `musl` changed its `time_t` to an `i64` over five years ago. `glibc` provides compatibility features that declare `time_t` as either `i32` or `i64`. Rustix uses the raw Linux syscall for stat which returns an `i64`. For our purposes, an `i64` makes sense because it's modern and avoids the year 2038 problem. It also reduces our dependency on what `libc` defines. Sources: * https://www.man7.org/linux/man-pages/man3/time_t.3type.html * https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/sys_types.h.html
| Back | FazBrowse Home | New Git URL |
According to POSIX, time_t should be 64 bits. musl changed its time_t to an i64 over five years ago. glibc provides compatibility features that declare time_t as either i32 or i64. Rustix uses the raw Linux syscall for stat which returns an i64. For our purposes, an i64 makes sense because it's modern and avoids the year 2038 problem. It also reduces our dependency on what libc defines.
Sources:
Summary
Summary by CodeRabbit
Bug Fixes
Refactor