Moved to GitHub:

• https://github.com/llvm/llvm-project/pull/69271
• https://github.com/llvm/llvm-project/pull/69274

Moved to GitHub, https://github.com/llvm/llvm-project/pull/69273.

Abandoned in favor of GitHub PR, https://github.com/llvm/llvm-project/pull/69272.

Cache the LIBCXX_ABI_VERSION variable in AndroidNDK.cmake.

I can switch this review to a GitHub PR if needed.

Use stdin=subprocess.DEVNULL for adb_run.py subprocesses, except for the main adb shell invocation where we want to forward stdin. This change was necessary to fix stdin forwarding on a Pixel 6 (oriole) device. (However, the API 33 emulator tests passed even without this change. Not sure why.)

Rebase.

Rebase. Remove _ZNSt3__16locale2id6__initEv.

Rebase.

In D159292#4646096, @smeenai wrote:

Ping.

This change looks like an improvement to me. The NDK currently puts the libc++ headers into the sysroot, but putting it in the usual toolchain include location seems better.

further confirmation if android_get_device_api_level should work from ifunc_resolver

The last rebase accounted for about a month's worth of changes in LLVM. Fix the Android tests that were broken by the rebase:

Rebase.

Rebase.

Rebase.

Restore the buildkite-agent.cfg setting. I confirmed that the tags from this config file can be overridden using the BUILDKITE_AGENT_TAGS environment variable.

Address review comments.

Add the Android team's current Clang build to the libcxx-builder Dockerfile.

Disable flaky notify_one.pass.cpp test for Android, and remove the unnecessary adb socat workaround now that adb -a start-server works.

Mark some of the comments as Done.

Address some of the review comments.

Now that D155271 and D155341 are parent revisions of this revision, remove their edits from this revision. I expect this change to break the Android testing because the Android abilist files will be missing.

Fix indentation in libcxx/test/support/filesystem_test_helper.h to make git-clang-format happy.

I copied the two ABI lists into D155341. I think this patch is ready for review now.

These files are a part of D139147.

utils/adb_run.py: Print syncing command lines with -v.

In _job_limit_socket_thread, discard the address returned from accept.

This change was copied from the larger D139147 change.

I split off duplicated the Dockerfile changes to D155271.

rebase

Switch adb_run's job limiting from POSIX semaphore to a Unix domain socket. Instead of needing a SIGINT handler in adb_run.py to release the semaphore, we can instead rely on the OS to close the socket connection.

Run 'docker info' before 'docker image inspect ...' to make sure that Docker is working.

Switch Android testing to using the existing copies of Clang in ldionne/libcxx-builder. Add a recent Android sysroot and the platform-tools (e.g. adb) to libcxx-builder. Start an Android emulator in a sibling container to the libcxx-builder container. Use one Docker image per emulator OS image.

In D139147#4433086, @Mordante wrote:

In D139147#4416112, @rprichard wrote:

In D139147#4414313, @Mordante wrote:

The patch is quite large. I think it would be better to land it in smaller parts, preferable after we see these changes have a green Android CI.

Sure, I can split parts of it off. I think the XFAIL additions and the libcxx/lib/abi* files are a good place to start.

The ABI could remain, but I would like to see the XFAIL annotations and the Dockerfile changes in two separate commits.

In D139147#4414313, @Mordante wrote:

The patch is quite large. I think it would be better to land it in smaller parts, preferable after we see these changes have a green Android CI.

Switch the Android run-buildbot-container from --device /dev/kvm back to --privileged, but add an option to switch it back to --device /dev/kvm.

Rebase the commit.

In D139147#4412013, @Mordante wrote:

[...]

I'm not discouraged reviewing this, especially since it adds CI support. But several of us work as volunteer which means we have limited time to work on libc++.

For the way forward, I've a question. What is the intention regarding running the CI?

Are you going to host it yourself like AIX/FreeBSD do or is it the intention the run on our existing Linux runners?

Android apps aren't generally allowed to access /data/local/tmp, but they are able to access the app-specific cache dir. (/data/data/${APP_ID}/cache or /data/user/0/$APP_ID}/cache). New-enough versions of Android set TMPDIR to this cache dir for apps. I wonder if it'd make sense for libc++ to use the app cache as fallback for std::filesystem::temp_directory_path(), when running in an app context (app UID?). I suspect the answer is "no", because it's not feasible to determine the path to the app cache. Not sure though.

Rebase this commit.

In D139147#4386265, @rprichard wrote:

In D139147#4386100, @philnik wrote:

In D139147#4386090, @rprichard wrote:

I was wondering whether I can land this patch adding Android testing to buildkite-pipeline.yml while the Android libc++ tests are still failing, or if the buildbot isn't set up yet. e.g. I wouldn't want non-Android-related LLVM patches to be affected. (Maybe I should land all the Android-related work first, and only then add the entry to the yml file.)

I think the way to go is marking all the failing tests as XFAIL and go from there. This avoids regressions and makes it easier to verify that a patch actually does what it claims to do.

These are the currently-failing tests with just this patch, https://gist.github.com/rprichard/4a8e9b160e7adc472778699c62ae1fff.

I can mark these tests as XFAIL. Would I do that in this patch or could I do that in an earlier commit?

In D139147#4386100, @philnik wrote:

In D139147#4386090, @rprichard wrote:

I was wondering whether I can land this patch adding Android testing to buildkite-pipeline.yml while the Android libc++ tests are still failing, or if the buildbot isn't set up yet. e.g. I wouldn't want non-Android-related LLVM patches to be affected. (Maybe I should land all the Android-related work first, and only then add the entry to the yml file.)

I think the way to go is marking all the failing tests as XFAIL and go from there. This avoids regressions and makes it easier to verify that a patch actually does what it claims to do.

I reported a kernel bug related to this issue, https://bugzilla.kernel.org/show_bug.cgi?id=216834.

I was wondering whether I can land this patch adding Android testing to buildkite-pipeline.yml while the Android libc++ tests are still failing, or if the buildbot isn't set up yet. e.g. I wouldn't want non-Android-related LLVM patches to be affected. (Maybe I should land all the Android-related work first, and only then add the entry to the yml file.)

Add a comment explaning why we don't just check __ANDROID_API__.

I still haven't added anything to libcxx/utils/ci/buildkite-pipeline.yml. It's not clear to me whether that should be part of this patch, because there are several things that must happen first before CI tests will work:

• I need to create a new docker image to add the Android stuff to it, and host it somewhere.
• I need to setup a special buildbot on a VM somewhere that uses the new docker image.
• There are a bunch of patches I have uploaded to Phabricator currently that are required to get tests running cleanly.
• There are a few other fixes for tests that I don't have uploaded, because I'm not quite sure what we want to do (need to disable certain filesystem tests, and there are a couple of demangler problems involving floating-point).

Various updates:

• Use the std::__ndk1 namespace to match how the NDK libc++ is built.
• Use a POSIX semaphore to limit concurrent adb jobs (libcxx/utils/semaphore_tool.py, --job-limit-semaphore) more than host jobs are limited.
• Define android and LIBCXX-ANDROID-FIXME features in features.py.
• Add a android_libcxx_kind={ndk,platform} param to libcxx[abi]/test/configs/llvm-libc++[abi]-android.cfg.in, which allows for testing either the NDK build of libc++ or the platform (/system/lib[64]/libc++.so) build of libc++ for Android.

In D147849#4256641, @enh wrote:

In D147849#4256504, @rprichard wrote:

One issue I noticed earlier was that the compiler flag (-f[no-]emulated-tls) doesn't work with LTO:

• The flag is ignored when the compiler is only outputting LLVM IR, and
• The flag is also ignored at link-time when the IR is converted to emutls-or-efltls machine code.

This link-time LTO setting can be changed using -Wl,-plugin-opt,-emulated-tls={0,1}.

does https://reviews.llvm.org/D147834 fix this?

nit: In the description/commit message, the "when the API level is > 29" should be "when the API level is >= 29".

In D141184#4039656, @smeenai wrote:

In D141184#4033243, @philnik wrote:

LGTM assuming D139147 is being worked on.

I'm not involved with that effort directly (I work on Meta's Android compiler team, whereas @rprichard is on the Android NDK team at Google), but I understand wanting a buildbot to test this configuration. I can't speak on his behalf, but I trust that Ryan will work on that actively once holiday season is wrapped up.

In D140183#4009462, @ldionne wrote:

Would it be better to instead XFAIL it using Lit machinery? Right now you're disabling the test even when it would succeed, which means you're losing that coverage.

I'm fine with the patch, but I think you may want to do something less drastic to retain coverage on your platform.

In D137131#4009498, @ldionne wrote:

Requesting changes for the queue. Is this ready to be re-reviewed?

In D140183#4003855, @Mordante wrote:

Looks good from libc++'s point of view. I really like @danalbert review to see whether the comments are correct.

Reword a comment.

Add feature LIBCXX-ANDROID-FIXME.

Use XFAIL instead of UNSUPPORTED.

Bump BuildKite again.

No change.

I'm definitely not ruling out adding Android/Bionic support for modules later on. They might be needed to fix things like the user namespace pollution in D139380. At the moment, the goal is to update the libc++ used on Android, which is a few years out of date.