upgrade shutil by youknowone · Pull Request #6558 · RustPython/RustPython

youknowone

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Relocates unlink to platform-specific modules (Windows and POSIX), adds Windows fd-based chmod support (fchmod_impl) with HANDLE-based attribute updates, and optimizes DirEntry::is_dir/is_file to use cached file_type short-circuits.

Changes

Cohort / File(s)	Summary
Windows stdlib changes `crates/vm/src/stdlib/nt.rs`	Added `remove` (exposed as `unlink`) that chooses `RemoveDirectoryW` vs `DeleteFileW`; introduced fd-aware chmod: `ChmodArgs<'a>` uses `OsPathOrFd<'a>`, added `fchmod_impl` and `fchmod`, S_IWRITE constant, and updated `chmod` to route fd paths to `fchmod_impl`. Error mapping to Python exceptions added.
POSIX unlink additions `crates/vm/src/stdlib/posix.rs`, `crates/vm/src/stdlib/posix_compat.rs`	Added `remove` (exposed as `unlink`) implementations that call `fs::remove_file`, validate empty `dir_fd`, and map IO errors to Python `OSError`. Both modules expose `unlink` alias.
Core os module adjustments `crates/vm/src/stdlib/os.rs`	Removed the cross-platform Python-facing `unlink` wrapper. Modified `DirEntry::is_dir` and `DirEntry::is_file` to early-return using cached `file_type` when present (with adjusted NotFound => false behavior and related comments).

Sequence Diagram(s)

sequenceDiagram
  participant Py as Python caller
  participant VM as Rust VM (nt.rs)
  participant WinAPI as Windows API / Kernel

  Note left of Py: call os.chmod(path_or_fd, mode, follow_symlinks?)
  Py->>VM: chmod(args)
  alt path argument
    VM->>VM: resolve path, preserve existing path-based flow
    VM->>WinAPI: GetFileAttributesExW / SetFileAttributesW (existing flow)
    WinAPI-->>VM: status
    VM-->>Py: map to Ok or OSError
  else fd argument
    VM->>VM: match OsPathOrFd::Fd(fd)
    VM->>WinAPI: _get_osfhandle(fd) → HANDLE
    WinAPI-->>VM: HANDLE
    VM->>WinAPI: GetFileInformationByHandleEx / SetFileInformationByHandle (toggle read-only)
    WinAPI-->>VM: status
    VM-->>Py: map to Ok or OSError / ValueError (if follow_symlinks unsupported)
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

nt junction #6407 — Overlaps OsPathOrFd introduction and Windows fd/path handling in nt.rs, directly related to fd-aware chmod/unlink work.
Fix os.remove #6352 — Modifies Windows unlink/remove implementations and touches the same removal code paths.
Rework crt_fd to be more aligned with io-safety #5789 — Related refactors around fd/handle safety (crt_fd/OsPathOrFd) that intersect with these changes.

Poem

🐰
Files hop to platform homes with delight,
Windows handles toggle read-only right.
POSIX removes with a skip and a cheer,
Cached DirEntry makes lookups clear,
A rabbit nods — unlink found its sight. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'upgrade shutil' is vague and generic, failing to describe the specific changes made to the codebase.	Consider a more descriptive title such as 'Add unlink and fchmod support across os modules' or 'Implement file removal and chmod for file descriptors in Windows/POSIX modules' to clarify the main changes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 308ac1d and 3f89316.

⛔ Files ignored due to path filters (2)

Lib/shutil.py is excluded by !Lib/**
Lib/test/test_shutil.py is excluded by !Lib/**

📒 Files selected for processing (4)

crates/vm/src/stdlib/nt.rs
crates/vm/src/stdlib/os.rs
crates/vm/src/stdlib/posix.rs
crates/vm/src/stdlib/posix_compat.rs

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}