This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Add deployment knobs to tests (for Apple platforms)
ClosedPublic

Authored by mehdi_amini on Feb 19 2016, 5:59 PM.

Details

Summary

The tests for libc++ specify -target on the command-line to the
compiler, but this is problematic for a few reasons.

Firstly, the -target option isn't supported on Apple platforms. Parts
of the triple get dropped and ignored. Instead, software should be
compiled with a combination of the -arch and -m<name>-version-min
options.

Secondly, the generic "darwin" target references a kernel version
instead of a platform version. Each platform has its own independent
versions (with different versions of libc++.1.dylib), independent of the
version of the Darwin kernel.

This commit adds support to the LIT infrastructure for testing against
Apple platforms using -arch and -platform options.

  • If the host is not on OS X, or the compiler type is not clang or apple-clang, then this commit has NFC.
  • If the host is on OS X and --param=target_triple=... is specified, then a warning is emitted to use arch and platform instead. Besides the warning, there's NFC.
  • If the host is on OS X and *no* target-triple is specified, then use the new deployment target logic. This uses two new lit parameters, --param=arch=<arch> and --param=platform=<platform>. <platform> has the form <name>[<version>].
    • By default, arch is auto-detected from clang -dumpmachine, and platform is "macosx".
    • If the platform doesn't have a version:
      • For "macosx", the version is auto-detected from the host system using sw_vers. This may give a different version than the SDK, since new SDKs can be installed on older hosts.
      • Otherwise, the version is auto-detected from the SDK version using xcrun --show-sdk-path.
    • -arch <arch> -m<name>-version-min=<version> is added to the compiler flags.
    • The target triple is computed as <arch>-apple-<platform>. It is *not* passed to clang, but it is available for XFAIL and UNSUPPORTED (as is with_system_cxx_lib=<target>).
    • For convenience, apple-darwin and <arch>-apple-darwin are added to the set of available features.

There were a number of tests marked to XFAIL on x86_64-apple-darwin11
and x86_64-apple-darwin12. I updated these to
x86_64-apple-macosx10.7 and x86_64-apple-macosx10.8.

Event Timeline

dexonsmith updated this revision to Diff 48565.Feb 19 2016, 5:59 PM
dexonsmith retitled this revision from to [libcxx] Add deployment knobs to tests (for Apple platforms).
dexonsmith updated this object.
dexonsmith added reviewers: EricWF, mclow.lists.
dexonsmith added a subscriber: cfe-commits.

Does anyone have a problem with this direction? I have commits to
follow that get tests green when run against various releases of OSX
but I need this in place first.

mclow.lists edited edge metadata.Apr 13 2016, 9:04 AM

This direction looks fine to me. All the test changes look fine to me.

EricWF accepted this revision.Apr 15 2016, 5:41 PM
EricWF edited edge metadata.

LGTM.

test/libcxx/test/config.py
66 ↗(On Diff #48565)

Was the omission of self.use_deployment here intentional? Python linters typically get upset by this.

This revision is now accepted and ready to land.Apr 15 2016, 5:41 PM

Looks like patch was not committed.

It's fine. I just walk through all accepted revisions to find forgotten ones.

Mehdi, are you interested in rebasing this?

mehdi_amini commandeered this revision.Mar 6 2017, 6:25 PM
mehdi_amini added a reviewer: dexonsmith.
mehdi_amini updated this revision to Diff 90773.Mar 6 2017, 6:25 PM
mehdi_amini edited the summary of this revision. (Show Details)

Rebase

mehdi_amini updated this revision to Diff 90779.Mar 6 2017, 7:02 PM

Add back the python changes (files were moved, I lost them in the rebase)

test/libcxx/test/config.py
66 ↗(On Diff #48565)

I can see how it would be useful not to add it here: we'd want an exception if it is used without being set since we don't expect it to happen.

mehdi_amini added inline comments.Mar 6 2017, 7:50 PM
test/libcxx/test/config.py
66 ↗(On Diff #48565)

But if it is not considered "pythonic", I'm fine adding it as well :)

Update: we set deployment by default even when not testing the system library.
It is intentional because a REQUIRE in a test might be because an issue with libc for example.
So for now the default is the OS on which the test runs.

mehdi_amini closed this revision.Mar 29 2017, 10:01 PM

Was committed a while back in r297798