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

midnight-commander: remove build workaround for macOS 10.13 (High Sie… #173306

Merged
merged 2 commits into from
May 31, 2024

Conversation

zyv
Copy link
Contributor

@zyv zyv commented May 31, 2024

Hello,

I'm the upstream maintainer of mc and now wondering how to proceed with nanosecond timestamps. The background is as follows:

We introduced support for nanosecond timestamps with utimensat years ago and only enabled it if OS supported it.

This "worked" fine util macOS 10.13 High Sierra, when Apple introduced utimensat in a half-assed POSIX-incompatible way (apparently the name of the the st structure members were like st_atimespec instead of st_atim etc.). So the function was there, but our code couldn't use it and builds failed.

We neither had macOS, nor the kernel was published, so we couldn't fix it and the workaround here was to disable utimensat by force even if it was available.

I have now checked the builds on the latest version of macOS and they work, and also the nanosecond timestamps work, because apparently Apple fixed the POSIX compatibility somewhen along the way.

The current workaround is now causing data loss (file timestamps are always truncated to microseconds) for the users.

The problem is, I don't know when Apple fixed this. If the formula builds on Monterey and Ventura, then it will also work, but I don't have these systems. High Sierra is not supported for years.

Can we just remove this workaround?

@zyv
Copy link
Contributor Author

zyv commented May 31, 2024

Historical discussions upstream are here: MidnightCommander/mc#130

@zyv
Copy link
Contributor Author

zyv commented May 31, 2024

Originally introduced here: #17109

Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Yes, happy to get rid of it wherever it builds. If someone on an older system comes along and complains about build failure we can scope this workaround to their OS version.

Thanks for the PR!

Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label May 31, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue May 31, 2024
Merged via the queue into Homebrew:master with commit 002feea May 31, 2024
14 checks passed
@zyv zyv deleted the patch-1 branch May 31, 2024 07:52
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 7, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants