swift icon indicating copy to clipboard operation
swift copied to clipboard

[android] Enable several C++ Interop and other tests

Open finagolfin opened this issue 1 year ago • 3 comments

Also, update NDK and Ubuntu versions to the latest in the Android doc, enable inline bridging when building the compiler for Android, and update the default arch set by build-script to AArch64, as that is by far the most commonly used now.

These C++ Interop tests were all disabled over the years because of build errors with the Android NDK, but @hyp's recent pull #72161 adding a modularized Bionic module map fixed them and the inline bridging issue in the compiler itself. Alex, let me know if these tests all pass for you too.

@drodriguez, perhaps you'd like to take a look at this pull too.

finagolfin avatar Jul 02 '24 02:07 finagolfin

@swift-ci please smoke test

compnerd avatar Jul 03 '24 15:07 compnerd

Sigh, File "/home/build-user/swift/test/lit.cfg", line 618, in <module> toolchain_lib_dir = make_path(config.swift_lib_dir, 'swift', 'android' if kIsAndroid else host_os) NameError: name 'kIsAndroid' is not defined.

I had simply hacked in make_path(config.swift_lib_dir, 'swift', 'android') in my local testing and then cleaned it up with the kIsAndroid check before submitting, without checking if that variable had been defined yet.

I'll clean that up and run the validation suite natively on Android on the entirety of this pull before asking for another CI run.

finagolfin avatar Jul 03 '24 18:07 finagolfin

Alright, fixed the lit.cfg error and ran this entire pull through the compiler validation suite natively on Android, before splitting it up into narrower commits and rebasing it here.

This is ready for review and another CI run.

finagolfin avatar Jul 10 '24 12:07 finagolfin

@kateinoigakukun, need a CI run here.

@egorzhdan, maybe you can review.

finagolfin avatar Jul 12 '24 05:07 finagolfin

@rauhul, need a CI run here.

finagolfin avatar Jul 18 '24 08:07 finagolfin

@bnbarham, ready for a CI run here.

finagolfin avatar Jul 22 '24 21:07 finagolfin

@swift-ci please test

bnbarham avatar Jul 22 '24 21:07 bnbarham

Alright, passed CI, just need @egorzhdan or some one to review.

finagolfin avatar Jul 23 '24 13:07 finagolfin

@etcwilde, if you would review the few remaining Android changes, we can get this in.

finagolfin avatar Jul 23 '24 14:07 finagolfin

@bnbarham, ready for merge.

finagolfin avatar Jul 23 '24 15:07 finagolfin

@hyp, would you merge?

finagolfin avatar Jul 26 '24 12:07 finagolfin

Sorry, missed initial ping. Merged!

bnbarham avatar Jul 26 '24 18:07 bnbarham