Repository navigation
Preserve signed filesystem timestamps in os.utime - #9047
Conversation
Extract the Python 3.14-compatible signed timestamp conversion from RustPython#8954, preserving floor rounding, float-or-index argument rules and signed divmod remainders. Reuse the existing Windows file-time helper rather than the mega patch range restriction, and keep the 3.14 argument diagnostics. Add three concise parser regressions to the existing os snippet without native filesystem range assumptions. Assisted-by: OpenAI Codex:model identifier unavailable
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable issue is established from the supplied evidence. The change is mergeable after normal checks. Pre-merge checks |
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac2e2fcd8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for atime in (-1, -0.25): | ||
| assert_raises(TypeError, os.utime, fname, (atime, object())) | ||
| assert_raises(TypeError, os.utime, fname, ns=(-1, object())) |
There was a problem hiding this comment.
Revert the forbidden test-logic additions
These new calls add assertions and test logic to extra_tests/snippets/stdlib_os.py, but the root AGENTS.md explicitly permits test-file edits only for adding or removing expected-failure markers and forbids modifying assertions or test logic. Revert this test-file hunk so the commit complies with the repository's scoped contribution rules.
AGENTS.md reference: AGENTS.md:L275-L279
Useful? React with 👍 / 👎.
| crate::host_env::os::set_file_times(&path, acc, modif) | ||
| .map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) | ||
| let wide_path = path.to_wide_cstring(vm)?; | ||
| host_nt::set_file_times( |
There was a problem hiding this comment.
Preserve Windows sharing when setting file times
In the Windows branch, when the target already has an open read or write handle, switching to host_nt::set_file_times makes os.utime(path, ...) fail with a sharing violation: that helper calls CreateFileW with dwShareMode = 0 (crates/host_env/src/nt.rs:406-412), whereas the previous OpenOptions path used the normal read/write/delete sharing flags. Preserve those share flags in the new helper so updating timestamps does not regress for open files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The access-mode change matters here: this helper requests only FILE_WRITE_ATTRIBUTES, whereas the old OpenOptions path requested data-write access. Windows excludes attribute-only access from its sharing-conflict checks, and CPython 3.14.7 uses the same FILE_WRITE_ATTRIBUTES/share-mode-zero call. Sources: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/8c0e3f4f-0729-49f4-a14d-7f7add593819 and https://github.com/python/cpython/blob/v3.14.7/Modules/posixmodule.c#L6269-L6274 . On that evidence, changing the sharing flags is not warranted. This is source verification; I have not run an open-handle Windows runtime test locally.
Summary
Extract signed filesystem timestamps for
os.utimefrom #8954 while preserving the Python 3.14.7 contract:The POSIX/WASI host-helper signatures now take signed FileTime fields rather than Duration. This is scoped to filesystem timestamps, not a general time ABI change.
Three concise calls in the existing os snippet verify negative integer/fractional/ns arguments reach an invalid second timestamp, without assuming filesystem date ranges or precision. No implementation guard or canonical assertion change.
Validation
Extraction, regression and validation were AI-assisted with OpenAI Codex. Draft pending hosted platform CI and review. README unchanged.
Summary by CodeRabbit
os.utimetimestamp handling for integer, floating-point, and nanosecond inputs, including negative fractional timestamps.