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

Fix: Fix pkgconfig detection, and multiple subsequent build for Android #833

Merged

Conversation

lains
Copy link
Contributor

@lains lains commented Aug 17, 2019

This PR brings 2 changes for the build for Android platforms:

  1. It fixes rename of the filenames (have to be lowercase on Android) so builds work even if it is run multiple times (today, only the first build works, further builds fail because rename itself fails, as renamed lowercase files still exist in the previous build working directories)

  2. It changes the way the build process uses pkgconfig.
    Indeed glib and ezxml dependencies are checked by default against the build host, not against the toolchain Android libs.
    It seems that this started from 4e5b35a.
    This issue does not seem to occur on CircleCI builds, but seem to occur when manually building on a host where glib, gmodule, dbusglib and/or ezxml headers are present.

    Symptom of the wrong libs picked up is an error during compilation:

  In file included from ../../../../../support/wordexp/wordexp.c:15:
  In file included from /usr/include/glib-2.0/glib.h:30:
  In file included from /usr/include/glib-2.0/glib/galloca.h:32:
  /usr/include/glib-2.0/glib/gtypes.h:423:3: error: '_GStaticAssertCompileTimeAssertion_0' declared as an array with a negative size
    G_STATIC_ASSERT(sizeof (unsigned long long) == sizeof (guint64));
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/include/glib-2.0/glib/gmacros.h:232:103: note: expanded from macro 'G_STATIC_ASSERT'
  #define G_STATIC_ASSERT(expr) typedef char G_PASTE (_GStaticAssertCompileTimeAssertion_, __COUNTER__)[(expr) ? 1 : -1] G_GNUC_UNUSED
                                                                                                        ^~~~~~~~~~~~~~~
  1 error generated.

The source can then be spotted inside the -I options of the compiler's command line.
During compilation for ../../../../../support/wordexp/wordexp.c, the compiler is called with
-I/usr/include/glib-2.0, which uses the locally installed glib. The issue is fixed if this argument is replaced by: -I../../../../../support/glib

@lains lains self-assigned this Aug 17, 2019
jandegr added a commit to jandegr/navit that referenced this pull request Aug 17, 2019
try to confuse cmake , see navit-gps#833
@jandegr
Copy link
Contributor

jandegr commented Aug 17, 2019

Hi,
I tried to confuse cmake on cirlceci by installing libdbus-glib-1-dev libgtk2.0-dev

and indeed, it goes wrong on circleci too, so it is not just some local issue in your setup

debug|armeabi-v7a :--->>> speech
debug|armeabi-v7a :Enabled android ( Android detected )
debug|armeabi-v7a :Disabled cmdline ( Android detected )
debug|armeabi-v7a :Enabled dbus ( dbus-glib-1 found )
debug|armeabi-v7a :Disabled espeak ( Default )
debug|armeabi-v7a :Disabled iphone ( Default )
debug|armeabi-v7a :Disabled qt5_espeak ( Qt5 multimedia not found )
debug|armeabi-v7a :Disabled speech_dispatcher ( speech_dispatcher lib not found )
debug|armeabi-v7a :--->>> support
debug|armeabi-v7a :Disabled espeak ( Default )
debug|armeabi-v7a :Disabled ezxml ( native Glib found )
debug|armeabi-v7a :Enabled gettext_intl ( native libintl missing )
debug|armeabi-v7a :Disabled glib ( native Glib found )
debug|armeabi-v7a :Disabled libpng ( Android detected )
debug|armeabi-v7a :Enabled shapefile ( Default )
debug|armeabi-v7a :Enabled wordexp ( native wordexp missing )
debug|armeabi-v7a :Disabled zlib ( native zlib found )
debug|armeabi-v7a :--->>> traffic
debug|armeabi-v7a :Enabled dummy ( Default )
debug|armeabi-v7a :Enabled null ( Default )
debug|armeabi-v7a :Enabled traff_android ( Android detected )
debug|armeabi-v7a :--->>> vehicle
debug|armeabi-v7a :Enabled android ( Android detected )
debug|armeabi-v7a :Enabled demo ( Default )
debug|armeabi-v7a :Disabled file ( Android detected )

However, the solution you propose might work (did not test that), but it removes some flexibility by hardcoding some choices.

context of the remark : for quite a while I build for windows in 2 flavors, classic with glib from support or I build a dll for glib first and then cmake detects it and skips the stuff from support. Just now I wanted to start to port that dual system over to android, but with your solution cmake won't ever detect a prebuilt glib for android and keep building the stuff from support, at least it is what I think will happen.

@jandegr
Copy link
Contributor

jandegr commented Aug 17, 2019

Hi,

adding
export PKG_CONFIG_LIBDIR="/opt" # here to prevent finding regular linux libs
to the buildscript fixes the confusion I introduced in circleci

maybe we could add
export PKG_CONFIG_PATH="" # here to prevent finding regular linux libs
to be on the safe side

If you build smth. for linux again on your box you will have to reinitialize those paths

I don't know enough about cmake to find the actual cause, maybe somone else can.

@lains
Copy link
Contributor Author

lains commented Aug 18, 2019

Hi @jandegr, I tried your proposal and it does the trick for me:
Adding export PKG_CONFIG_LIBDIR="" in scripts/build_android.sh just before the line that runs cmake.
Removing file CMakeCache.txt
Cleaning up the folder navit/android (to pristine state)
And running scripts/build_android.sh once more.

@lains lains changed the title Fix: Force glib and ezxml lib selection for Android builds Fix: Fix pkgconfig detection, and multiple subsequent build for Android Aug 20, 2019
@lains
Copy link
Contributor Author

lains commented Aug 20, 2019

@jandegr, I have implemented your proposal, as it works wellfor my setup and it would be a good thing that this is done automatically during build.
In case someone still wants to detect specific libs for the target, providing a non-empty value for PKG_CONFIG_LIBDIR should work.
Also, this is outside of cmake so it only affects building for Android using scripts/build_android.sh
I also included a fix for subsequent builds for Android.
The first build succeeds, but subsequent builds fail (except if a cleanup is done manually on files renamed for Android).
I fixed the call to rename so that the cleanup is done automatically before each build.

@pgrandin pgrandin merged commit e7a70e9 into navit-gps:trunk Aug 20, 2019
@lains
Copy link
Contributor Author

lains commented Aug 20, 2019

Oops, there was still code on CMakeLists.txt changes in this PR (these diffs were not needed anymore, the implementation has been done another way in the build_android.sh script).
Because this PR has been merged already, I am going to revert on trunk the extra changes that were not supposed to be merge.

lains added a commit to lains/navit that referenced this pull request Aug 20, 2019
lains added a commit to lains/navit that referenced this pull request Aug 20, 2019
@lains lains deleted the android-remove-build-host-lib-detection branch August 20, 2019 17:53
lains added a commit that referenced this pull request Aug 20, 2019
Reverting part of e7a70e9 (PR #833) that was not needed
@gefin gefin mentioned this pull request Aug 23, 2019
viktorgino pushed a commit that referenced this pull request Sep 22, 2020
…id (#833)

* Fixing lib detection based on build host for Android builds

* Fixing workaround according to jandegr's proposal

* Fixing subsequent Android builds (where rename fails because of pre-existing renamed files)
viktorgino pushed a commit that referenced this pull request Sep 22, 2020
Reverting part of e7a70e9 (PR #833) that was not needed
jkoan pushed a commit to jkoan/navit that referenced this pull request Jun 30, 2021
…id (navit-gps#833)

* Fixing lib detection based on build host for Android builds

* Fixing workaround according to jandegr's proposal

* Fixing subsequent Android builds (where rename fails because of pre-existing renamed files)
jkoan pushed a commit to jkoan/navit that referenced this pull request Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants