st_file_attributes #6353
st_file_attributes #6353
Conversation
WalkthroughA new public field Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
youknowone
force-pushed
the
st_file_attributes
branch
from
c8f86e6 to
5a5944b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/vm/src/stdlib/os.rs (1)
818-822: Windows/non-Windows initialization pattern is consistent but check desired API semanticsThe cfg-gated initialization:
- Uses
stat.st_file_attributeson Windows.- Falls back to
0on non-Windows.This mirrors the existing
st_reparse_taghandling and is compile-time safe. The only open design question is whether you want the attribute to exist (with value0) on non-Windows, or to follow CPython’s pattern of only exposing some stat-related items on platforms that support them. If closer CPython parity is desired, you might consider gating the attribute itself behind#[cfg(windows)]instead of defaulting to0elsewhere (similar to how platform-specific constants are conditionally exposed instat.rs). Based on learnings, this is more of a design/alignment choice than a correctness issue.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Lib/test/test_os.pyis excluded by!Lib/**Lib/test/test_site.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/stdlib/os.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/stdlib/os.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Applied to files:
crates/vm/src/stdlib/os.rs
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
Applied to files:
crates/vm/src/stdlib/os.rs
🧬 Code graph analysis (1)
crates/vm/src/stdlib/os.rs (2)
crates/derive-impl/src/lib.rs (1)
pystruct_sequence(61-63)crates/derive/src/lib.rs (1)
pystruct_sequence(235-239)
🔇 Additional comments (2)
crates/vm/src/stdlib/os.rs (2)
781-783: StatResultData gains st_file_attributes with appropriate typing and defaultsAdding
pub st_file_attributes: u32with#[pyarg(any, default)]and#[pystruct_sequence(skip)]is consistent with howst_reparse_tagis modeled, and theu32type matches the underlying Win32 file-attribute representation. I don’t see issues with this field definition.
841-842: Struct initialization correctly threads through st_file_attributesIncluding
st_file_attributesin theSelf { ... }initializer ensures the new field is always populated from the cfg-gated local variable. This keepsStatResultData::from_statself-contained and in sync with the struct definition.
1 participant
Summary by CodeRabbit
Release Notes
os.stat()to expose file attributes information. Windows systems now provide detailed file metadata through stat results, while other platforms include default values to maintain consistency.✏️ Tip: You can customize this high-level summary in your review settings.