Skip to content

Call libc::utimensat directly rather than using filetimes crate#10063

Closed
iburaky2 wants to merge 5 commits into
uutils:mainfrom
iburaky2:fix-touch-dev-full
Closed

Call libc::utimensat directly rather than using filetimes crate#10063
iburaky2 wants to merge 5 commits into
uutils:mainfrom
iburaky2:fix-touch-dev-full

Conversation

@iburaky2
Copy link
Copy Markdown
Contributor

@iburaky2 iburaky2 commented Jan 5, 2026

#9870 coincidentally works on linux but does not work on platforms where UTIME_NOW which is originally a long doesn't fit into u32, such as freebsd.

It's not possible to use UTIME_NOW in utimensat with the filetimes crate on these platforms. I copied the relevant piece of code being called from filetimes so I can call libc::utimensat directly. Providing a nullptr as the times argument as the same effect as both tv_nsec fields being UTIME_NOW.

For the tests, OpenBSD doesn't have a /dev/full but touch /dev/null essentially tests the same permission issue.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 5, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jan 5, 2026

CodSpeed Performance Report

Merging #10063 will improve performance by 3.57%

Comparing iburaky2:fix-touch-dev-full (22b3425) with main (666c6df)

Summary

⚡ 1 improvement
✅ 138 untouched
⏩ 37 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Efficiency
tsort_input_parsing_heavy[5000] 73 ms 70.5 ms +3.57%

Footnotes

  1. 37 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Comment thread src/uu/touch/src/touch.rs
set_file_times(path, atime, mtime)
#[cfg(unix)]
{
if opts.source == Source::Now
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment explaining why you are doing this


#[test]
#[cfg(target_os = "linux")]
#[cfg(all(unix, not(target_os = "openbsd")))]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not openbsd ?

Copy link
Copy Markdown
Contributor Author

@iburaky2 iburaky2 Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openbsd doesn't have /dev/full
https://man.openbsd.org/MAKEDEV.8

@iburaky2 iburaky2 closed this Mar 17, 2026
@xtqqczze
Copy link
Copy Markdown
Contributor

@iburaky2 Would you be interesting in continuing?

@iburaky2
Copy link
Copy Markdown
Contributor Author

I remember having trouble testing on all the different platforms. Also I wasn't really happy with moving so much code from the filetime crate to here, I think a better solution would be maybe maintain filetime functionality inside uucore.

I'm a bit busy with work atm, feel free to continue from where I left off.

@xtqqczze
Copy link
Copy Markdown
Contributor

It may be worth someone picking this up again, given that filetime appears to have breaking changes in v0.2.28 (see also #12201).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants