Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

Expose C and C++ build tools to crates #230

Closed
philip-alldredge opened this issue Aug 2, 2019 · 6 comments · Fixed by #232
Closed

Expose C and C++ build tools to crates #230

philip-alldredge opened this issue Aug 2, 2019 · 6 comments · Fixed by #232

Comments

@philip-alldredge
Copy link
Contributor

I would like to ask whether this is something that people felt should be part of cargo-apk before implementing this.

cc-rs and cmake-rs are commonly used for building C and C++ libraries via build scripts.

It is currently the user's responsibility to expose the appropriate build tools. I propose that cargo-apk do what is necessary to expose the NDK provided build tools to the build process. This will mostly involve setting environment variables based on the NDK location, ABI, and min_version_sdk.

While there may be aspects of this that can be used to improve cmake-rs, the build tools used should reflect the min_sdk_version in the Cargo.toml. As such, it is not practical for cc-rs and cmake-rs to determine the appropriate compiler.

Benefits:

  • Avoid additional user setup.
  • Select the appropriate build tools based on the min_sdk_version. This scales much better than having to manually configure the appropriate tool for each project.

I have experimented with doing this manually but it is non-trivial to manually set the environment variables in a way that would support multiple ABIs.

References:

@mb64
Copy link
Contributor

mb64 commented Aug 3, 2019

I think that this would be nice to have. However, many use cases for cc (such as openssl-sys are to link to system libraries, which Android probably won't have. A few things I can think of that would benefit from a working C/C++ toolchain setup are libz-sys (the NDK ships with zlib) and backtrace, which compiles and links libbacktrace statically.

@philip-alldredge
Copy link
Contributor Author

Agreed. I was thinking mostly about dependencies of a lot of gfx related libraries like shaderc.

@philip-alldredge
Copy link
Contributor Author

Libraries that used to link statically no longer work with the latest cargo-apk. This is another side affect of building a static library instead of a dynamic library. Since static libraries aren't linked, the native dependencies are not linked during the build process. Generating a rlib will embed that dependency info into the static library but there would need to be someway to get it from the rlib or other means and pass it to ndk-build.

@philip-alldredge
Copy link
Contributor Author

@mb64

As much as I liked the idea of using ndk-build to avoid getting entangled in the android toolchain, I am leaning towards not using it as being the cleanest solution. If we avoided ndk-make, we'd need to build the android_native_glue ourselves which should be straightforward anyways.

I suggest:

  • Exposing the NDK provided toolchain via environment variables.
  • Remove usage of ndk-build
  • Going ahead and vendoring the NDK version of android_native_glue source. Alternately, if Remove injected glue #228 is wrapped up before this, then we can just use that version.
  • Compile android_native_glue as part of the build process.
  • Direct rustc to build a dynamic library instead of a static library.
  • Use the appropriate rustc arguments to direct it to use the appropriate linker provided by the ndk add additional library paths based on the appropriate min_sdk_version.
  • retain the usage of temporary files in the build process as implemented in Fixes building of crates containing inner attributes. #231. This could be omitted if rustc build a bin crate as in the original build system but since we need the linker to produce a shared object, I think letting it do it naturally would be best.

@mb64 thoughts?

@mb64
Copy link
Contributor

mb64 commented Aug 4, 2019

This is another side affect of building a static library instead of a dynamic library. Since static libraries aren't linked, the native dependencies are not linked during the build process

This seemed weird to me, so I made a simple test case to see what the normal behavior was. I had a crate with a static native dependency that I compiled as a static lib, and it worked just fine; i.e. the static native dependency was included in the static library. This gives me hope that this is not an irresolvable issue.

thoughts?

The proposed plan looks OK to me. One thing that would simplify it, assuming the changes in #228 are agreed upon, is to have android_glue compile and link android_native_app_glue in a build.rs.

@philip-alldredge
Copy link
Contributor Author

Perhaps rustc will combine static libraries with ar. I haven't confirmed but it seems fairly reasonable. However, that wouldn't help the case when linking into shared libraries. I am certain that rustc does not execute the linker when building a static library.

I saw an issue on the rustc issue tracker discussing the need to be able to extract the list of dependencies somehow and an unstable command line option that might work. However, I don't think something like this warrants requiring the nightly toolchain.

I have experimenting with the proposed integration and things are working for the most part. It builds off #231 so things will need to be adjusted if that isn't merged. It currently includes tests which builds C and C++ libraries with cc-rs and a C library with cmake-rs. There is a good bit of cleanup that needs to be done before it'll be ready.

Caveats

  • When linking a C++ applications, it currently links with the shared C++ libc++. Unfortunately, due to dependency loading issues with older versions of android, it can crash dependeing on android version. Unsure of exact version.
  • Linking with the static C++ library is recommended by Google in cases where we are producing a single .so but there are linker issues. There are linker errors with the default linker. Using the lld linker, which google is encouraging developers to test with, fixes the issue on certain ABIs. However, it fails to build for some ABIs when using the windows toolchain. Google has fixed the issue and the fix should be included in R21 of the NDK. It is expected that lld will be the default linker in that release or in a near-term release.

Once the next revision of the build system is ready, I think it would be reasonable to go ahead and merge it along with a note about the C++ issues. Once R21 is released, we can switch to linking libc++ statically to fix compatibility with older versions.

The proposed plan looks OK to me. One thing that would simplify it, assuming the changes in #228 are agreed upon, is to have android_glue compile and link android_native_app_glue in a build.rs.

Building android_native_glue is fairly straightforward but I don't see why it couldn't be build by android_native_glue.

TLDR: I've got a working version but it'll be a week or so before it's ready and it's currently dependent on #231.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants