-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[android] Enable several C++ Interop and other tests #74883
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
Changes from all commits
15e1c73
3bd6c6b
fb7bb19
6ede1c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// UNSUPPORTED: OS=windows-msvc | ||
// static library is not well supported yet on Windows | ||
|
||
// XFAIL: OS=linux-android, OS=linux-androideabi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, I disabled this last year because it didn't work with clang 14 in NDK 25, but it works with clang 17 in NDK 26 now. |
||
|
||
// UNSUPPORTED: OS=xros | ||
|
||
// For LTO, the linker dlopen()'s the libLTO library, which is a scenario that | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -177,6 +177,10 @@ if platform.system() == 'Darwin': | |
# NOTE: this mirrors the kIsWindows from lit.lit.TestRunner in LLVM | ||
kIsWindows = platform.system() == 'Windows' | ||
|
||
# The global environment in Android sets ANDROID_DATA, so if that variable is | ||
# set, we are probably running in Android. | ||
kIsAndroid = 'ANDROID_DATA' in os.environ | ||
|
||
# testFormat: The test format to use to interpret tests. | ||
|
||
# Choose between lit's internal shell pipeline runner and a real shell. If | ||
|
@@ -423,6 +427,11 @@ if run_os.startswith('wasi'): | |
# Parse the host triple | ||
(host_cpu, host_vendor, host_os, host_vers) = re.match('([^-]+)-([^-]+)-([^0-9-]+)(.*)', config.host_triple).groups() | ||
|
||
# Manually set the host OS on Android, because the host OS from the triple says | ||
# `linux`. | ||
if kIsAndroid: | ||
host_os = 'android' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than overriding this just for the new |
||
|
||
if f"{host_cpu}-{host_vendor}-{host_os}" == f"{run_cpu}-{run_vendor}-{run_os}": | ||
# Ignore the version on purpose, to account for scenario | ||
# in which the compiler has a different deployment target | ||
|
@@ -1124,10 +1133,6 @@ elif platform.system() == 'Linux': | |
if swift_test_mode != 'only_non_executable': | ||
config.available_features.add('swift_interpreter') | ||
|
||
# The global environment in Android sets ANDROID_DATA, so if that variable is | ||
# set, we are probably running in Android. | ||
kIsAndroid = 'ANDROID_DATA' in os.environ | ||
|
||
# swift-remoteast-test requires the ability to compile and run code | ||
# for the system we compiled the swift-remoteast-test executable on. | ||
# This is potentially a stronger constraint than just "can we interpret", | ||
|
@@ -2465,8 +2470,12 @@ elif not kIsWindows and not run_os == 'wasi': | |
lit_config.note('Testing with the just-built libraries') | ||
|
||
lit_config.note('Library load path: {0}'.format(os.path.pathsep.join(target_stdlib_path))) | ||
env_path = "/usr/bin/env " | ||
# Android doesn't have /usr/bin/ | ||
if kIsAndroid: | ||
env_path = "/bin/env " | ||
config.target_run = ( | ||
"/usr/bin/env " + | ||
env_path + | ||
construct_library_path_env(target_stdlib_path) + | ||
config.target_run) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1393,7 +1393,7 @@ def create_argument_parser(): | |
|
||
option('--android-arch', store, | ||
choices=['armv7', 'aarch64', 'x86_64'], | ||
default='armv7', | ||
default='aarch64', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems unrelated? It's a welcome change, just seems not related to the test enablement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to the title, yes, but these are all Android changes, and I specifically mention these non-test changes in the commit message. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, but it still helps to keep changes focused :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, this should probably be a separate patch, since it isn't just a test-only change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It never says it's a test-only change, the label used is If you would like, I can split this into separate commits, but I see no reason to go farther. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR title doesn't mention any non-test changes, which makes it confusing. I initially assumed that this is a test-only change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I will split this commit up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
help='The target architecture when building for Android. ' | ||
'Currently, only armv7, aarch64, and x86_64 are supported. ' | ||
'%(default)s is the default.') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,7 @@ | |
'android_api_level': '21', | ||
'android_deploy_device_path': '/data/local/tmp', | ||
'android_ndk': None, | ||
'android_arch': 'armv7', | ||
'android_arch': 'aarch64', | ||
'assertions': True, | ||
'benchmark': False, | ||
'benchmark_num_o_iterations': 3, | ||
|
@@ -764,7 +764,7 @@ class BuildScriptImplOption(_BaseOption): | |
ChoicesOption('--swift-analyze-code-coverage', | ||
choices=['false', 'not-merged', 'merged']), | ||
ChoicesOption('--android-arch', | ||
choices=['armv7', 'aarch64']), | ||
choices=['armv7', 'aarch64', 'x86_64']), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This never triggered before because this just checks if it's a subset, so updating to all three arches supported now. |
||
|
||
StrOption('--android-api-level'), | ||
StrOption('--build-args'), | ||
|
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 disabled this last year, 12f20ec, because it was failing with the clang 14 in NDK 25 at that time, but it builds fine with clang 17 in NDK 26 now.