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

Multiple updates, mostly for realpath() #98

Merged
merged 9 commits into from
Nov 7, 2024
Merged

Conversation

fhgwright
Copy link
Contributor

@fhgwright fhgwright commented Nov 7, 2024

See the individual commit messages.

Highlights:

  • Rewrites realpath() wrapper to handle its variants more correctly.
  • Expands realpath() tests to cover all variants.

The recently added test_clean target was initially misisng a couple
of items, and became much more deficient with the addition of
the new test targets.  This fixes all that, as well as removing
some redundant entries from the main clean target.

TESTED:
All build products from test_all are now removed by test_clean.
This provides a tool for investigating the behavior of the different
versions of realpath() with specified paths.
Makes failures more informative, and minimizes output in success
case unless -v option is given.  Also includes a test for the
"traditional" realpath().

TESTED:
Provides better information, while still demonstrating the failure on
10.4 x86_64.
The former implementation didn't necessarily provide all versions of
realpath() that might be invoked, and didn't necessarily invoke the
proper version.  That led to possible crashes and possible unnecessary
semantic differences.  This fixes all that, though it doesn't retrofit
the 10.5+ semantic changes to 10.4 (and neither did the earlier
implementation).

The former implementation also failed to return any internally
allocated buffer on failures, causing a memory leak.  That is now
fixed as well.

TESTED:
Builds and passes all tests (including expanded tests) on all
platforms, with all applicable SDKs.
This ensures that all three versions of realpath() (two in 64-bit
builds) have test coverage.  The test for the impossible 64-bit
non-POSIX case is a dummy.

The 10.15 non-POSIX realpath() has a bug (unrelated to
legacy-support) causing the relevant test to fail.  A subsequent
commit excludes that case from the test.

This test uncovered real bugs in the former wrapper implementation.

TESTED:
Without the realpath() fix, fails on 10.5 and on 10.4 with later SDK.
With the fix, passes on all platforms (except as noted above).
This excludes testing the non-POSIX version on 10.15, which has a bug
(Apple's).

Also reverses order of nonposix cases, for consistency, and expands
the comment.

TESTED:
Now passes on all platforms.
This tests the macports_legacy_realpath() entry point, which is no
longer used by new builds but possibly referenced by old builds.

TESTED:
Passes on all platforms; fails when the legacy entry is disabled.
The special treatment for 10.4 in splitandfilterandmergemultiarch was
not being applied appropriately to x86_64, causing a build failure
for fdopendir.

Fixing this bug prior to fixing the realpath() wrapper resulted in
successfully building code that didn't work correctly, though the
realpath() bug wasn't limited to this one configuration.

Note that at the time of this writing, when this case is built as a
port, the architecture is coerced to i386.  Until that is fixed, this
fix won't affect the actual port.

TESTED:
With the realpath() fix, builds successfully on all platforms,
including 10.4 x86_64.
@fhgwright
Copy link
Contributor Author

@mascguy
Here's another set of changes. I'm planning to add a fix for the 10.6 non-POSIX bug, but I ran out of time to get this out before becoming mostly unavailable for about a week. As is, it's fully tested, including via a temporary Portfile.

@mascguy mascguy merged commit 8dfd627 into macports:master Nov 7, 2024
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