Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nanosecond precision to File.utime on Unix #15335

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Jan 10, 2025

Reopens #9392 with fixes to the android and solaris bindings, and #system_utime has already been updated to support nanosecond precision when available.


Crystal::System::File.utime uses LibC.utimensat when available, otherwise falls back to utimes.

The utimensat syscall is specified in POSIX.1-2008 and was introduced in the following OS releases:

  • Linux 2.6.22, glibc 2.6
  • macOS 10.14
  • FreeBSD 10.3
  • OpenBSD 5.0

To keep some backward compatibility, the x86_64-darwin bindings do not
declare it. We can however assume the syscalls to be present when
compiling for aarch64-darwin.

@straight-shoota straight-shoota added this to the 1.16.0 milestone Jan 10, 2025
@straight-shoota
Copy link
Member

AT_FDCWD seems to be missing for aarch64-linux-android.

@Sija
Copy link
Contributor

Sija commented Jan 10, 2025

I hope that unlike #9392, this won't drop support for macOS < 10.13, High Sierra

@straight-shoota
Copy link
Member

straight-shoota commented Jan 10, 2025

I suppose it would, assuming utimensat isn't available on MacOS < 10.13. The implementation is pretty much identical to #9392.

What's your concern with MacOS < 10.13? It has been EOL since 2019 so I don't see much reason to keep up support.
Also we might already not support such old versions anyway: #9392 (comment)

It might be possible to do something similar to bytecodealliance/rustix#275 to keep old versions supported, but I doubt the effort would be justified.

The `utimensat` and `futimens` syscalls only appeared in macOS 10.14. To
keep some backward compatibility, the x86_64 bindings are fixed to not
declare them. We can however assume the syscalls to be present when
compiling for aarch64.

`Crystal::System::File.utime` now falls back to `utimes` when `futimens`
isn't defined.

Adds the missing `AT_FDCWD` constant to aarch64-android bindings. Fixes
the DragonflyBSD binding.
@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Jan 10, 2025

I kept the legacy syscalls for macOS x86_64 builds to keep support with macOS < 10.14, but use the new ones for aarch64 targets that's only supported since macOS 11+ (hence always fine).

I also fixed the aarch64-android and dragonflybsd bindings.

@straight-shoota straight-shoota changed the title Add nanosecond precision to File.utime (UNIX) Add nanosecond precision to File.utime on Unix Jan 14, 2025
@straight-shoota
Copy link
Member

@ysbaddaden I updated the OP to include a full description of the change that can be used in the commit message. Please update it if you see any need. I'll wait for your go to merge this.

@ysbaddaden
Copy link
Contributor Author

@straight-shoota Perfect!

@straight-shoota straight-shoota merged commit 2458e35 into crystal-lang:master Jan 14, 2025
69 checks passed
straight-shoota added a commit to crystal-lang/crystal-book that referenced this pull request Jan 14, 2025
@Sija
Copy link
Contributor

Sija commented Jan 16, 2025

@ysbaddaden Would it be possible to keep the legacy ones on aarch64 as well? I'm still using macOS 10.13 on my M1 machine.

@ysbaddaden ysbaddaden deleted the refactor/file-utime-nanosecond-precision-on-unix branch January 16, 2025 11:07
@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Jan 16, 2025

What? But the minimum macOS version for the oldest M1 machine (Apple macbook Air 2020) is macOS 11.x 😕

All aarch64-apple-darwin targets necessarily have these syscalls.

Do not confuse 10.13 (high sierra) with 13 (ventura).

@straight-shoota
Copy link
Member

straight-shoota commented Jan 16, 2025

I suppose we could consider making this configurable somehow, in order to opt-in to the modern API on x86_64-darwin as well.

@Sija
Copy link
Contributor

Sija commented Jan 16, 2025

@ysbaddaden Ah, you're right of course. I'm using macOS 12 (Monterey), so that's not an issue, I guess.

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

Successfully merging this pull request may close these issues.

3 participants