-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
Many updates - primarily header-related #88
Conversation
Also adds some that were missing. Also removes some leading blank lines.
This provides an internal header to set up conditionals related to the SDK version. No version flags are included yet, but these will be added as needed. TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.4-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
@mascguy I just force-pushed a fix for a minor naming inconsistency (that wouldn't have mattered in practice). |
@mascguy |
No worries. I'll try to review everything over the next few days, if possible. |
TESTED: Targets mantest_* work as expected.
This simply reports the values of some macros related to the SDK version. No version-threshold flags are included yet, but these will be added as they are added to sdkversion.h. TESTED: Reports definitions as intended.
Commit e9cf5ab moved the renameat() declaration from sys/stdio.h to stdio.h, but this is incompatible with the normal header setup. This moves it back to restore compatibility, but requires knowledge of the SDK version to cope with the absence of sys/stdio.h in pre-10.10 SDKs. This is handled via the new sdkversion.h. Re: https://trac.macports.org/ticket/69867 TESTED: Builds and passes tests on all platforms, including the new test for this issue.
TESTED: Fails with old, incompatible legacy-support header layout, and works with new, compatible layout.
Prior to this, missing and deficient Tiger headers were handled by installing extra headers on 10.4. This was incompatible with builds using any non-Tiger SDK. The new approach uses wrapper headers with appropriate SDK-based conditionals, and is compatible with all SDKs. Special handling of header installs on 10.4 is no longer required. This also avoids the need for an extra include path when building the legacy-support components themselves on 10.4. Re: https://trac.macports.org/ticket/69867 TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
Instead of disabling the tests in the no-check case, it now explicitly tests for non-failure of the failure tests in that case, as well as unconditionally checking for non-failure of the non-failure case, but only in the "static" case, since the "auto" check seems to happen even when the checks are nominally disabled. This doesn't apply to the disabled runtime check tests. Also adds a check for the explicitly disabled case. TESTED: Passes normally in all cases. Now fails when tweaked to treat an enabled case as a disabled case.
This is done purely for comparison purposes, by testing security wrappers for strncpy(), which is *not* provided by legacy-support. Since the functions are identical except for the return value, which is ignored by these tests, this is a near-verbatim copy of the strncpy_chk tests, except for the name. TESTED: Behaves as expected, including failing some 10.4 with 10.5+ SDK cases, prior to the fix.
This allows rerunning tests with different SDKs without rebuilding the library each time. TESTED: All clean targets work as expected.
The previous fix for the 10.5 compiler issue only applied when the wrapper is being provided by the legacy-support headers. But the problem also exists when the wrapper is applied by a 10.7+ SDK when building for 10.5. This moves the hack outside the check for the prefdefined wrapper. In the 10.7+ SDK case, the inline was defined before the macro hack, so we use a different name for the inline here. To make this effective, we unconditionally define the macro here, overriding the SDK. Re: https://trac.macports.org/ticket/69867 TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
The comment for including the main MacportsLegacySupport.h header was not actually followed by the include, though it was using __MP_LEGACY_SUPPORT_COSSIN__. It's unclear how it ever worked, though the tests pass both before and after the fix. This appears to have been true ever since the code was moved here from a header-based version, which also lacked the main include. TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
Builds with 10.9+ SDKs expect these functions as part of an improved internal interface, but they don't become available until 10.9. Builds targeting 10.6 or earlier don't use them, but we don't worry about that. We don't try to do anything fancy - this just creates a compatible interface to the two-call approach. Re: https://trac.macports.org/ticket/69867 TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
Security wrappers are unsupported on 10.4. They're never enabled when using the 10.4 SDK, but builds with later SDKs and the enable on (default in 10.6+, allowable in 10.5) result in build failures. This is not limited to legacy-support's stpncpy(), but affects native functions (e.g. strncpy()) as well. We avoid this by #undefing _FORTIFY_SOURCE in any 10.4 build. Re: https://trac.macports.org/ticket/69867 TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
Beginning with the 10.14 SDK, AvailabilityInternal.h uses the __has_include() feature without checking whether it's available. So we add the usual default definition when it's not. Re: https://trac.macports.org/ticket/69867 TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
Beginning with the 14.x SDK, AvailabilityInternal.h uses the __has_builtin() operator. Although it includes a defined() condition, that doesn't actually work because the operator needs to be parseable before evaluating the boolean. So we provide a default (always false) definition when the compiler doesn't have it. Re: https://trac.macports.org/ticket/69867 TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
This replaces the ugly OS version check with logic obtained from the 10.9+ secure/_string.h, which was initially missed because it wasn't intoduced until two versions after stpncpy() was introduced in 10.7. See the comment for more detail. TESTED: Tested on 10.4-10.5 ppc, 10.5-10.6 ppc (i386 Rosetta), 10.4-10.6 i386, 10.5-12.x x86_64, and 11.x-14.x arm64. Tested against all 10.4-14.x SDKs, using the headerinfo manual test (from a subsequent commit). All target/SDK combinations where the SDK supports the CPU architecture work correctly, including all SDKs on Intel architectures. Only 10.4-10.6 SDKs support ppc, and only 11.x+ SDKs support arm64.
This allows handling Tiger-specific installs directly in the Makefile. Currently this is solely the replacement 'which' program, a slightly tweaked version of the original csh script. Also adds a dummy tiger-bins target, to allow for possible future use. TESTED: Works as expected in 10.4; unused otherwise.
@mascguy In doing this, I realized that I'd forgotten to declare the new clean targets as .PHONY. It's not that important in this case, but worth doing for consistency. Since it makes sense to do that in the same commits as the ones that added the targets, I amended the existing commits accordingly, which of course required a forced push. The overall result is 13 lines added to the Makefile, 12 of which are from the new commit. No other changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to review as much of the detail as possible, within the confines of an hour.
Looks great to me.
See the individual commit descriptions for more details. Note that the first commit is purely cosmetic and accounts for 12 of the changed files.
Tested on:
This requires removing most of the Tiger-specific hackery in the Portfile. Originally I thought that that would be optional, but the copying gets an error when there's nothing to copy. It's easier to remove the affected code than to fix it. The corresponding -devel port will need to fork the Tiger-specific code into two versions, to be reunified in a simpler form as of the next release version.