touch: use write-open + futimens on Unix to trigger IN_CLOSE_WRITE#9813
touch: use write-open + futimens on Unix to trigger IN_CLOSE_WRITE#9813mattsu2020 wants to merge 1 commit intouutils:mainfrom
Conversation
|
is it possible to add tests ? thanks |
add |
|
sorry, it needs to be rebased |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Merging this PR will not alter performance
Comparing Footnotes
|
|
GNU testsuite comparison: |
src/uu/touch/src/touch.rs
Outdated
There was a problem hiding this comment.
he conversion from nix::Error to std::io::Error is incorrect
| #[cfg(unix)] | ||
| { | ||
| // Open write-only and use futimens to trigger IN_CLOSE_WRITE on Linux. | ||
| if !is_stdout && try_futimens_via_write_fd(path, atime, mtime).is_ok() { |
There was a problem hiding this comment.
Silent error swallowing with is_ok() loses information about why futimens failed.
src/uu/touch/src/touch.rs
Outdated
There was a problem hiding this comment.
Unnecessary metadata check before open. Opens file twice (metadata + open). Should check metadata after opening via file.metadata() to avoid TOCTOU and reduce syscalls.
src/uu/touch/src/touch.rs
Outdated
src/uu/touch/src/touch.rs
Outdated
There was a problem hiding this comment.
Unsafe type cast 'as _' for nanoseconds. Should explicitly cast to i32: atime.nanoseconds() as i32
|
GNU testsuite comparison: |
src/uu/touch/src/touch.rs
Outdated
There was a problem hiding this comment.
Redundant import - std::fs is already imported unconditionally at line 26
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
On Unix, try futimens through a write-opened file descriptor to trigger IN_CLOSE_WRITE semantics, and fall back to set_file_times when needed. Also adds a unit test for the futimens path and wires unix-only dependencies.
d741ba8 to
8e88196
Compare
|
GNU testsuite comparison: |
Summary
Make
touchopen regular files for write and callfutimenson the fd (Unix only). This matches GNU touch behavior and ensures inotify emitsIN_CLOSE_WRITEwhen timestamps are updated.Motivation
Some inotify-based reloaders watch only
IN_CLOSE_WRITE. With the current path-based timestamp update,touchemits onlyIN_ATTRIB, so those watchers miss the change. This caused infinite loops in downstream tests (e.g., 0 A.D. hotload tests on Ubuntu Questing).Changes
OpenOptions::new().write(true)and callfutimenson the fd for regular files.set_file_timespath-based update when open/futimens fails or the target is not a regular file.Testing
cargo test -p uu_touchNotes
related
#9812