Skip to content
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

new Android azure build scripts need soname fixes #897

Closed
atsushieno opened this issue May 27, 2021 · 12 comments · Fixed by #906
Closed

new Android azure build scripts need soname fixes #897

atsushieno opened this issue May 27, 2021 · 12 comments · Fixed by #906
Labels

Comments

@atsushieno
Copy link
Contributor

FluidSynth version

Confirmed build artifacts from: https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=5294&view=results

Reference Cerbero builds: https://app.circleci.com/pipelines/github/FluidSynth/fluidsynth/1391/workflows/c05fb732-8b6c-405e-90f7-c73a480ed659/jobs/1377

Describe the bug

The Android build artifacts from new build scripts cause run-time failures due to inconsistent shared library symbolic name, like this:

2021-05-27 17:16:44.138 5972-5972/? E/lilv: lilv_lib_open(): Failed to open library /lv2/aap-fluidsynth.lv2/libaap-fluidsynth.so (dlopen failed: library "libintl.so.8" not found: needed by /data/app/~~9cu6aDbjCS1Rmi_C3EiV-A==/org.androidaudioplugin.aap_fluidsynth-jp8iHYCsIs0dB2vTX4n8_Q==/base.apk!/lib/x86/libfluidsynth.so in namespace classloader-namespace)

It tries to load libintl.so.8 which we rename at copying files. But Android dlopen() does not seem to support soname differences like this (I'm not sure if it is POSIX-compliant or bionic specific, but it is how Android works).

After all, we have to provide "consistent" soname using -Wl,-soname,xxx compiler argument, or whatever equivalent.

This does not happen to Cerbero-based artifacts.

The Cerbero-based artifacts worked fine on x86 target (they show another issue on arm64 and x86_64 for my app, so it's hard to tell the precise difference). Most likely on armeabi-v7a too (unconfirmed). I believe the problem is ABI-agnostic.

When I was building fluidsynth for Android back in 2015, I had some ugly hackaround by retouching binaries directly: https://github.com/atsushieno/android-fluidsynth/blob/master/rewrite-binary.sh

Expected behavior

The resulting libraries do not contain libxxx.so.x.y as reference sonames.

Steps to reproduce

What I used to confirm was my Android app https://github.com/atsushieno/aap-lv2-fluidsynth/ but there is no intuitive alternative project so far...

@atsushieno atsushieno added the bug label May 27, 2021
@atsushieno
Copy link
Contributor Author

(This kinda tells why I didn't leave Cerbero while I don't prefer sticking to it...)

@atsushieno
Copy link
Contributor Author

atsushieno commented May 27, 2021

If you have trouble reproducing the issues locally, I had been working on locally runnable shell scripts. I put those files under doc/android/release-scripts directory and run bash build-all-archs.sh: https://gist.github.com/atsushieno/e1247a9b3e7bb235e9703cab18b69f95

@derselbst
Copy link
Member

https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=5294&view=results

This artifact is from vcpkg. I assume you meant: https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=5303&view=artifacts&pathAsName=false&type=publishedArtifacts

And what's the error when using the original, non-renamed libs? Like this artifact here: https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=5300&view=artifacts&pathAsName=false&type=publishedArtifacts

I've read on the web that this doesn't work (and you also told me), but I haven't found a meaningful error message. After all, the .so.x.y is just a filename. And the rest are just symlinks pointing to this file.

@atsushieno
Copy link
Contributor Author

atsushieno commented May 27, 2021

Oops, yes, I didn't mean vcpkg. I think I had no idea how to get to the right pipeline. fluidsynth-android24.zip was the one I downloaded to verify.

When I use the original (cerbero-based) builds, there is no error on x86. The errors on 64-bit environments (both intel and arm) are like this: atsushieno/aap-lv2-fluidsynth#1.

The soname semantic versions are just filename and not a problem IF the strongly-named version files are treated as shared libraries by the platform. Linux (I guess glibc) treats *.so.x.y as loadable and/or executable, but Android bionic doesn't.

@derselbst
Copy link
Member

Ok, I must admit I don't fully understand or comprehend the problem. I haven't found any official documentation about that either. I will just compile the four libs in question as static ones to avoid this problem. Not nice, but my best guess.

I've implemented static linking in this artifact. Feel free to try it. In the meantime, I'll try to get your lv2-fluidsynth app working with the android emulator.

@atsushieno
Copy link
Contributor Author

On .so not loading versioned files, I haven't seen any "official documentation" either, but Android is not a standard-based platform thus it wouldn't qualify the actual behavior. The problem has been commonly known among Android native app devs: https://stackoverflow.com/questions/11491065/linking-with-versioned-shared-library-in-android-ndk

Instead of my complicated app, I was trying to build test runner for the tests in test directory. Tests are not really runnable yet, but at least it is usable to check if libfluidsynth.so is loadable or not. I put it under my android-testing branch: https://github.com/atsushieno/fluidsynth-fork/tree/android-testing/test-android

@derselbst
Copy link
Member

I think I found the root cause of the incorrectly named .so files: Previously, I was passing linux target triplets to autotools. However, it looks like it must contain android to generate correctly named .so files, see 203f8ac.

When setting up the pipeline initially, I had already tried android targets, but gettext kept failing to find its library. It still does, see this build: https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=5354&view=results

And now I understand the problem for this one: the install command of gettext is broken, i.e. there is an extra character at the end of the file extension, but after a first look into gettext-tools/gnulib-lib/Makefile.in I couldn't find where this is comming from.

Apart from that I have been able to compile gradle build and gradle bundle your small test app locally.

@atsushieno
Copy link
Contributor Author

I'm having hard time getting successful builds from my scripts now, partly for the same reason as you had problem at gettext. It could be workarounded by skipping its installation, but I have another issue which stops build in the middle. Maybe the it is due to some mismatch from Azure build scripts and mine, but haven't figured out why. It is pushed on the same tree as I commented earlier, for public reference.

I was going to give up and tried to just use the binary builds, but since my attempt to build Android tests runner depends on internals (e.g. config.h) as well as dependencies (e.g. glib.h) so I need my build script working locally first. Thus, I'm quite stuck now.

Regarding shared library builds now I tried them from Azure artifacts with my own app (audio plugin), and confirmed that things haven't changed.

but after a first look into gettext-tools/gnulib-lib/Makefile.in I couldn't find where this is comming from.

I don't really understand libtool either, but it seems the *.soT extension seems quite common and I don't find it really an issue.

Apart from that I have been able to compile gradle build and gradle bundle your small test app locally.

Those gradle tasks don't really run tests on target Android. ./gradlew connectedCheck would do it. (Again, I cannot build it now and cannot confirm that it builds and run by myself.)

@derselbst
Copy link
Member

Try to git am this patch on top of your android-testing branch: 0001-fix-build.txt

In short: I wasn't able to figure out that *.soT issue. I just switched to static libraries.

In case it still doesn't work for you, I could simply publish the entire build-root as azure artifact. Just let me know.

@atsushieno
Copy link
Contributor Author

Thanks, the build fix worked like a charm! I thought you were going to build static libfluidsynth.a but it seems it's only for gettext etc. which would keep it simple for mere native library users. I confirmed those updated libraries work fine on my app too.

atsushieno added a commit to atsushieno/fluidsynth-fork that referenced this issue Jun 6, 2021
atsushieno added a commit to atsushieno/aap-lv2-fluidsynth that referenced this issue Jun 7, 2021
context: FluidSynth/fluidsynth#897
fixes: #1

This also removes uselss CI-SKIP scripting which never worked.

Now it points to my fluidsynth-fork, but the related scripts should be
brought upstream (once I sort out how to deal with the actual tests).
@derselbst derselbst linked a pull request Jun 7, 2021 that will close this issue
@atsushieno
Copy link
Contributor Author

Today I got a working "Android tests" running on x86 emulator, which I hope to make it runnable (my android-testing branch). It is with handful of disabled tests, but surprisingly not too many, It does not bring any changes for resolving sf files, so they are quite expected.

I have no idea whether the current CI settings are viable to get emulator up and running for every build though. The tests should be PR-ed with a separate issue probably.

@derselbst
Copy link
Member

Great, that looks very good already, thanks! I wouldn't mind about the few disabled tests. If you want I can deal with setting up CI properly.

atsushieno added a commit to atsushieno/fluidsynth-fork that referenced this issue Jun 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants