-
Notifications
You must be signed in to change notification settings - Fork 460
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
Android support #162
Android support #162
Conversation
@@ -0,0 +1 @@ | |||
#import <sys/syscall.h> |
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.
please just have
#ifdef __ANDROID__
#include <sys/syscall.h>
#endif
in the right place instead of these overlaying of headers
@@ -64,10 +65,15 @@ DISPATCH_CFLAGS=-Wall $(VISIBILITY_FLAGS) $(OMIT_LEAF_FP_FLAGS) \ | |||
if DISPATCH_ENABLE_ASSERTS | |||
DISPATCH_CFLAGS+=-DDISPATCH_DEBUG=1 | |||
endif | |||
|
|||
if ANDROID | |||
ANDROID_LIBS= -L$(top_srcdir)/src/shims/android |
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.
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.
you should do pull requests for both these projects to be merged in their upstreams and update the submodules here instead.
these .a's are really gross IMNSHO.
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.
Trust me, I'm not proud about them. The submodules are not in the apple
organization, so I wasn't sure how that may affect the chances + delay to include this. I'll start working on that tho.
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.
@frankeh is a committer on libpwq, so processing a pull request there should be fast. We usually get good time turn around time from libkqueue (hours to days). So, it's worth trying to do the right thing and maybe dropping this hunk from the PR
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.
Great! I'll send a PR to both projects then. Thanks @dgrove-oss :)
I think the .a issue should be resolved with other upstreams. |
The autoconf/automake looks plausible to me. I tried the disable-tests argument to configure and a vanilla build on linux and both work fine. I assume the big android block in configure.ac does what you want (didn't check it). |
Ok, dependency PRs created. @frankeh would you mind to take a look? Thanks. |
c558f62
to
e3573e4
Compare
@MadCoder @dgrove-oss Changes in Cheers |
the patch looks reasonnable, please squash in a single commit with a good message indeed |
We realized yesterday that the libpwq and libkqueue change actually broke the build on linux because the default for bionic-libc was backwards. Fix has been merged to libpwq and is pending for libkqueue. We'll need to wait for the libkqueue fix to be merged, then update the submodule heads again before this can be merged. |
@dgrove-oss I just seen it, so sorry about that. Thanks for submitting the patch. I'll wait until it is merged to update the heads and squash everything. |
2391cbe
to
cde5131
Compare
@dgrove-oss @MadCoder Squashed :) |
@dgrove-oss can you check it doesn't break linux? I'm not familiar with the CI env. I'm ok with the change otherwise |
@swift-ci please test |
This is the error the CI is reporting, however I don't see any relation with the changes in this PR. Is it possible that this is a failure somewhere else? Let me know if I need to do anything else. Thanks! |
cde5131
to
5d624a8
Compare
Rebased. Is there any change I should work on? Please let me know. @modocache make sense to re-trigger CI test now? |
@swift-ci please test |
Sorry I didn't see this sooner, @gonzalolarralde! I can't wait to see this in! :) |
Looks like an assertion failure in libkqueue. You could try the tests again perhaps? This is unlikely to be to do with this Android port which is largely to do with Makefiles and headers. Test Case 'TestURLSession.test_dataTaskWithURL' started at 12:48:08.279 |
@swift-ci please test |
constraints to the options added in `swift` to add Android Support. This will allow initially those features to be just passed from `utils/build-script` without any change until a full strategy for cross-compilation in all the involved projects is defined. This is an example call to build for Android with Swift support: ``` env \ CC="${swift_android_path}/build/Ninja-ReleaseAssert/llvm-linux-x86_64/bi n/clang" \ CXX="${swift_android_path}/build/Ninja-ReleaseAssert/llvm-linux-x86_64/b in/clang++" \ SWIFTC="${swift_android_path}/build/Ninja-ReleaseAssert/swift-linux-x86_ 64/bin/swiftc" \ ${swift_android_path}/swift-corelibs-libdispatch/configure \ --with-swift-toolchain=“${swift_android_path}/build/Ninja-ReleaseAssert/ swift-linux-x86_64/" \ --with-build-variant=release \ --enable-android \ --host=arm-linux-androideabi \ --with-android-ndk=${ndk_path} \ --with-android-api-level=21 \ --disable-build-tests ```
5d624a8
to
deaa565
Compare
Rebased. |
@gonzalolarralde Do you have a companion PR for integrating with build-script in the swift project, or does this have to be done manually outside of the greater build? If it is separate, is it possible to build a dispatch-aware foundation using this work? By the way, this is fantastic! Thanks for doing all this!! |
Hi @hpux735, I don't have a companion PR yet, however I've defined a set of options similar to the options needed to call build-script when compiling for Android. However, I've been working on a dev environment that is compiling all the libraries, and enabling dispatch in foundation. The project is here: https://github.com/gonzalolarralde/swifty-robot-environment/ The specific compilation script for foundation with all the dependencies + dispatch here: https://github.com/gonzalolarralde/swifty-robot-environment/blob/master/util/prepare_environment/070_build_corelibs_foundation.sh Some shortcuts have been taken in this scripts. Sorry about that :) I'll try to put some time to add this to build-script and build-script-impl (not sure if the maintainers are accepting changes in this last one due to the Python refactor). Cheers! |
😁👌 Thanks!! |
Android support Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
This changeset adds support for Android cross-compilation with the same restrictions applied to
swift
project.Options
--disable-build-tests
: tests are failing to be built due to the lack ofspawn
. Until this is fixed, the flag will be useful to complete compilation--with-android-ndk
: Android NDK path to be used as a base for cross toolchain, SDK and support files.--with-android-ndk-gcc-version
: NDK GCC version to be used (defaults to4.9
)--with-android-api-level
: API version to be used for compilation--enable-android
: Enabled Android cross-compilation. In this implementation, as it is being done right now inswift
, this is enforcingarm-linux-androideabi
armeabi-v7a
.Issues
There are a few things in this changeset that I'd like to iterate, assuming there are better or more generic solution that can be applied:
src/shims/android
includes: In this directory I createdsys/sysctl.h
+syscall.h
to redirect headers to their locations in Android. -- update: moved to compiler conditionals instead of include overlay. commitsrc/shims/android
libraries:libpthread.a
+librt.a
are dummy libraries because of the difference described here. To avoid this bothlibkqueue
andlibpwq
should be modified to avoid asking libtool to link against them.configure.ac
andsrc/Makefile.am
: This the first time I deal withautotools
, so I think there should be a better way to implement this.(C|CXX|LD)FLAGS
params I think will be needed explicitly declared somewhere to enable cross compilation, but maybe inAM_$0_FLAGS
? I think there are some overwrites in theam
files that were causing troubles, but they can be fixed if this is the right direction. Removing flags fromac_configure_args
like that withsed
seems like a terrible idea, maybe bothlibkqueue
andlibpwq
need to be extended somehow to support this in a better way?Build
This is an example call to compile
libdispatch
for Android. This is similarly formatted to the call that build script inswift
does to compilelibdispatch
.Side note:
va_list
issue mentioned in #155 is affecting the build process. There's no patch for that issues here, to avoid mixing problems that may be addressed in different ways.Outcome
This compilation process creates the shared library with the following ELF header:
and dependencies