Skip to content

Conversation

@drodriguez
Copy link
Contributor

Executable tests cannot currently run in the Android CI machines, and
must be marked as such to be skipped.

/cc @pschuh: The test still fails to work on Android, but it might not be the test itself, but something else. I want to at least quickly fix the test in CI.

Executable tests cannot currently run in the Android CI machines, and
must be marked as such to be skipped.
@drodriguez
Copy link
Contributor Author

@swift-ci please smoke test

@pschuh
Copy link
Contributor

pschuh commented Jul 10, 2019

It doesn't actually execute anything, but fixing the CI is good.

@drodriguez
Copy link
Contributor Author

Yes. I was too fast to say it was executing something. I will not merge this one for the moment. I might mark it as XFAIL in a moment instead.

The problem is that LibcShims.h header can be included from C in Android because there's an implicit declaration of the function malloc_usable_size, but that's not possible in C++. The problem is that the Clang Importer do not pass the __ANDROID_API__, which is used in that header to define the function. I will try to figure out what can we do.

Sorry for the noise.

@drodriguez drodriguez closed this Jul 10, 2019
@pschuh
Copy link
Contributor

pschuh commented Jul 10, 2019

There is no fallback, and if malloc_usable_size really is undefined before 17, swift would just not compile on older versions. Is the right thing to do to assert that __ANDROID_API__ >= 17? That way, it would also fail properly in c rather than succeeding silently? Maybe that #if should be removed temporarily?

@drodriguez
Copy link
Contributor Author

I'm pretty sure that the code was written before the minimum API level for Android was set in 21 (the documentation mentions 21), so checking for 17 might have been correct at some point. I think #26046 is the right way of handling this.

@drodriguez drodriguez deleted the android-fix-cxx-interop branch July 10, 2019 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants