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

Fix stat64 handling on Intel macOS #1897

Merged
merged 1 commit into from Jan 25, 2022
Merged

Fix stat64 handling on Intel macOS #1897

merged 1 commit into from Jan 25, 2022

Conversation

carlocab
Copy link
Contributor

@carlocab carlocab commented Jan 25, 2022

ad87ced fixed compilation for ARM64 macOS, but it broke builds on Intel
macOS.

Let's fix it by gating the changes from ad87ced behind
_DARWIN_FEATURE_ONLY_64_BIT_INODE. This macro is defined whenever the
ARM64 macOS fix is appropriate, but only after we've included
sys/cdefs.h.

This does mean that we're still using a deprecated API on Intel macOS
10.6+. I looked into avoiding this, but it seems to require more
significant refactoring. It's not clear that more significant changes would be
worth it given that Apple is slowly phasing out their Intel machines.

ad87ced fixed compilation for ARM64 macOS, but it broke builds on Intel
macOS.

Let's fix it by gating the changes from ad87ced behind
`_DARWIN_FEATURE_ONLY_64_BIT_INODE`. This macro is defined whenever the
ARM64 macOS fix is appropriate, but only after we've included
`sys/cdefs.h`.

This does mean that we're still using a deprecated API on Intel macOS
10.6+. I looked into avoiding this, but it seems to require more
significant refactoring. It's not clear that these changes would be
worth it given that Apple is slowly phasing out their Intel machines.
Comment on lines +34 to +37
/* Needed for _DARWIN_FEATURE_ONLY_64_BIT_INODE check below. */
#if defined(__APPLE__)
#include <sys/cdefs.h>
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can avoid this #include by checking for __aarch64__ instead of _DARWIN_FEATURE_ONLY_64_BIT_INODE, but an architecture check here seems a bit hacky. I'm willing to change it though if desired.

@carlocab
Copy link
Contributor Author

carlocab commented Jan 25, 2022

Build tested on Intel macOS 10.15-12, and ARM macOS 11-12 at Homebrew/homebrew-core#93683 (see Homebrew/homebrew-core@e7c88bf). Didn't run the test suite, though.

@ffesti ffesti merged commit d5d743d into rpm-software-management:master Jan 25, 2022
@ffesti
Copy link
Contributor

ffesti commented Jan 25, 2022

Thanks for the quick fix!

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.

None yet

2 participants