Android compile flags leaking into dependencies

[edkimmel]

I wasn't sure how to name this one.

Environment: Compiling Android libraries on OSX

harfbuzz 1.4.4-ft has a dependency on ragel-6.9 on the HOST system.

CXXFLAGS and such are exported in the main mason.sh file. They retain their values when we begin compiling the dependency in mason_prepare_compile.

The android specific compile flags are being sent through as I try to compile ragel for OSX, which causes configure to fail.

I think there's also going to be a future issue here; any changes made to CXXFLAGS while building the dependencies will pollute the original library.

Having this be the order of events might fix it mason_prepare_compile export CXXFLAGS=... mason_compile

If mason_prepare_compile needs those flags in some cases, maybe dependency resolution needs to be an explicit step beforehand. mason_prepare_dependencies export CXXFLAGS=... mason_prepare_compile mason_compile

[springmeyer]

Good observation. I've not tried to get the harfbuzz package working yet on android, but planned to soon. So I would have hit this. Two initial thoughts:

1. ragel should be being installed via a binary. It may not be because some environment variable from the android environment setup is being used

2. I think we can work around these problems by clearing the environment like is done in the ICU package, which is known to work in a cross environment: https://github.com/mapbox/mason/blob/master/scripts/icu/58.1/script.sh#L30

Can you try the icu approach and see if that is viable? Happy to review a PR if you make progress.

[edkimmel]

If I just do mason install ragel 6.9, it does get further but has issues with code that just doesn't compile (narrowing issues, possibly legacy use of ostream)

Changing harfbuzz do follow icu, I do end up getting to the same compile error point as just compiling ragel alone. So it appears to work - it at least gets past configure.

First fails with common.cpp:37:70: error: constant expression evaluates to 18446744073709551615 which cannot be narrowed to type 'long long' [-Wc++11-narrowing]

So I add -Wno-narrowing to CXXFLAGS, and then failed with: rbxgoto.cpp:661:8: error: invalid operands to binary expression ('basic_ostream<char, std::__1::char_traits<char> >' and 'ostream' (aka 'basic_ostream<char>')) " " << rbxGoto(ret, "_out") << "\n"

Checking out that line, they are trying to call operator << on another ostream (Like, ostream << my_ostream).

If ragel continues to be a problem, I'm going to move forward with making it a system lib on osx, and installing it through brew.

[springmeyer]

Thanks for continuing to dig into this. I'm confused however with the issue of why ragel is being source compiled rather than installed via a binary. Can you share the exact commands you are using that trigger the original problem?

[edkimmel]

MASON_PLATFORM=android MASON_ANDROID_ABI=arm-v7 ./mason install harfbuzz 1.4.4-ft

That's the only command I need to run for this to happen. Harfbuzz has a requirement on ragel 6.9, and the script for ragel 6.9 builds from source.

[springmeyer]

Thanks @edkimmel - I'll take a closer look when I have a free moment next week. In the meantime I'll /cc @ChrisLoer @kkaefer who are also likely thinking about getting harfbuzz working on android and may have bandwidth to help get this working.

I saw you are also wanting to get protobuf working on android - Have you considered using protozero instead? It is header only and therefore easier to include in a project. We use protozero instead of the libprotobuf in mapbox-gl-native and therefore don't have a plan to port the mason protobuf package to android.

[edkimmel]

Protobuf on Android was one of those problems where finding what was wrong was 90% of the work.

if [ ${MASON_PLATFORM} = 'android' ]; then
  # protobuf does not compile with the -mthumb flag. 
  # Configure also fails to link aginst the android log library
  export CFLAGS=${CFLAGS//-mthumb /}
  export CXXFLAGS=${CXXFLAGS//-mthumb /}
  export LDFLAGS="${LDFLAGS:-} -llog"

  # When cross compiling, you need to specify the platform protoc for the tests to work.
  PROTOC="--with-protoc=protoc"
fi

I might have a PR in the near future, adding cross compiling support to a number of libraries.