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

Fixes for copyfile(), *statx() and TargetConditionals #108

Merged
merged 16 commits into from
Jan 11, 2025

Conversation

fhgwright
Copy link
Contributor

Tested, both from repo and as a temporary Portfile, on:

Mac OS X 10.4.11 8S165, PPC, Xcode 2.5 8M2558
Mac OS X 10.4.11 8S2167, i386, Xcode 2.5 8M2558
Mac OS X 10.4.11 8S2167, x86_64, Xcode 2.5 8M2558
Mac OS X 10.5.8 9L31a, PPC, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, i386, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, x86_64, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, PPC (i386 Rosetta), Xcode 3.1.4 9M2809
Mac OS X 10.6.8 10K549, i386, Xcode 3.2.6 10M2518
Mac OS X 10.6.8 10K549, x86_64, Xcode 3.2.6 10M2518
Mac OS X 10.6.8 10K549, PPC (i386 Rosetta), Xcode 3.2.6 10M2518
Mac OS X 10.7.5 11G63, x86_64, Xcode 4.6.3 4H1503
OS X 10.8.5 12F2560, x86_64, Xcode 5.1.1 5B1008
OS X 10.9.5 13F1911, x86_64, Xcode 6.2 6C131e
OS X 10.10.5 14F2511, x86_64, Xcode 7.2 7C68
OS X 10.11.6 15G22010, x86_64, Xcode 8.1 8B62
macOS 10.12.6 16G2136, x86_64, Xcode 9.2 9C40b
macOS 10.13.6 17G14042, x86_64, Xcode 10.1 10B61
macOS 10.14.6 18G9323, x86_64, Xcode 11.3.1 11C505
macOS 10.15.7 19H15, x86_64, Xcode 12.4 12D4e
macOS 11.7.10 20G1427, x86_64, Xcode 13.2.1 13C100
macOS 11.7.10 20G1427, arm64, Xcode 13.2.1 13C100
macOS 12.7.6 21H1320, x86_64, Xcode 14.2 14C18
macOS 12.7.6 21H1320, arm64, Xcode 14.2 14C18
macOS 13.7.2 22H313, arm64, Xcode 15.2 15C500b
macOS 14.7.2 23H311, arm64, Xcode 16.2 16C5032a
macOS 15.2 24C101, arm64, Xcode 16.2 16C5032a

This creates a directory under /tmp which tests can use to write any
needed files, passing its location to test builds.

TESTED:
Directory is created and removed as expected.
This reworks the arrangement of the copyfile-related headers for 10.4,
where the SDK provided none.  In the new arrangement:

1) The added header now represents what a 10.4 copyfile.h would have
looked like if it existed.

2) The additions for 10.4->10.5 copyfile are now in the wrapper
header.

3) A new pair of feature flags is added for the 10.4 case.

TESTED:
Library still builds.
Test builds, and passes or fails as before.
After applying many fixes to the old copyfile wrapper, it was still
hopeless on 10.4 due to a fatal internal bug in Apple's code.  This
change throws all that out and imports the entire copyfile
implementation from 10.6.  This should provide the full 10.6 copyfile
on 10.5, and a mostly full implementation on 10.4, after additional
10.4 fixes in the next commit.

This retains the feature of Apple's copyfile that allows it to be
built as a standalone program for easier testing.

TESTED:
Builds and passes the test on 10.5+.
Doesn't yet build on 10.4 (fixed in subsequent commit).
Since the O_SYMLINK definition is missing from the 10.4 fcntl.h, we
provide a default here.  It's the correct value, but whether or not it
actually works correctly in 10.4 is unknown.

Since 10.4 lacks quarantine support, we provide dummy macros for the
quarantine definitions referenced here.

With these changes, 10.4 should now have the full 10.6 copyfile,
except that quarantine-related operations almost certainly don't work,
and special handling of symlinks may not work.

TESTED:
Builds and passes the test on all OS versions, including 10.4.
This is a simple test of copyfile(), along with a couple of
copyfile_state_set()/copyfile_state_get() options.

Includes access to copyfile()'s secret debug mode.

TESTED:
Passes on all platforms, except 10.4 Rosetta.
The copyfile_state alloc/free functions were renamed between 10.4 and
10.5.  The 10.4 libSystem provides them under their old names, but
since the structure has changed between 10.4 and 10.6, using the old
functions would cause problems.  This provides wrappers for the new
functions under the old names (in 10.4 builds), so that any use of the
old names will properly use the new versions.

TESTED:
Builds, and new test for wrappers passes.
This verifies that the 10.4 alloc/free functions are actually using
the new versions, by checking the allocated size.

TESTED:
Passes on all platforms normally.
Fails as expected on 10.4 with wrappers disabled.
This is taken from:
https://github.com/apple-oss-distributions/copyfile/blob/copyfile-66.1/copyfile.3

Almost verbatim, but with additions to indicate that QUARANTINE
operations are unavailable on 10.4, and that ACL copying doesn't work
in 10.4 Rosetta.

Adds manpage directory creation to Makefile, including missing case
for the previously added 'which' manpage.

TESTED:
Manpages are installed as appropriate by the relevant targets.
Adds copyfile debugging targets to src/Makefile.

Fixes src/Makefile to honor CC.

Adds filesec_internal.h for debug builds of copyfile.
Due to a bug in fstatx_np() in 10.4 Rosetta, COPYFILE_ACL doesn't
work.  In that case, we disable that particular operation to avoid a
test failure.  The remaining three COPYFILE_ALL operations are still
tested.

There's also a bug in 10.4 Rosetta's chmodx_np(), which causes an
internal error when attempting to ensure that the destination is
writable, but that doesn't cause failures in the test case.

TESTED:
Passes on all platforms, including 10.4 Rosetta.
TESTED:
New tests for these functions build without prototype warnings.
TESTED:
Tests pass on all 10.5+ platforms.
Tests fail on 10.4 without new addition, and pass with it.
This expands the 10.4 64-bit inode support to include the *statx_np
functions.

TESTED:
Updated test_stat passes on all platforms with this change.
The fstatx_np() and fstatx64_np() functions don't work in 10.4
Rosetta.  This excludes their tests in that case.

TESTED:
Now passes on all platforms, including 10.4 Rosetta.
This replaces the old cherry-picked list with a more complete version
taken from the macOS 15 SDK.  In the process, it fixes a conflict with
a hack in the AvailabilityMacros.h wrapper.

Closes: https://trac.macports.org/ticket/70824

TESTED:
Passes updated test on all platforms, with all applicable SDKs.
TESTED:
Passes on all platforms, with all applicable SDKs.
@fhgwright
Copy link
Contributor Author

@mascguy
I'll make a -devel update once this is merged.

The copyfile() stuff turned out to be an even deeper rabbit hole than I originally thought. It actually never worked, except for allowing builds referencing COPYFILE_STATE_COPIED.

@mascguy
Copy link
Member

mascguy commented Jan 11, 2025

@mascguy
I'll make a -devel update once this is merged.

The copyfile() stuff turned out to be an even deeper rabbit hole than I originally thought. It actually never worked, except for allowing builds referencing COPYFILE_STATE_COPIED.

I was a bit worried at first, given the nearly 5,000 lines of changes. But thankfully some of that is for the man pages, which makes this a little less grueling to review.

Will try to finish the review today.

@mascguy mascguy merged commit f109400 into macports:master Jan 11, 2025
@fhgwright
Copy link
Contributor Author

@mascguy

I was a bit worried at first, given the nearly 5,000 lines of changes. But thankfully some of that is for the man pages, which makes this a little less grueling to review.

I suppose I should have warned you about that, given that a large part is the wholesale import of Apple's copyfile code from 10.6, with a few changes as noted in the comments.

Will try to finish the review today.

I see you did. Thanks. I'll create and test a -devel update today.

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

Successfully merging this pull request may close these issues.

2 participants