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

Android Things 1.06 with Termux changes #3312

Merged
merged 4 commits into from
Nov 23, 2018
Merged

Android Things 1.06 with Termux changes #3312

merged 4 commits into from
Nov 23, 2018

Conversation

brubakerjeff
Copy link
Contributor

Please consider the following changes to the configuration scripts. After making the changes below ( macro to check for libatomic and enhanced verification for tipc[I don't have a tipc unit to verify with but inserted the line that was causing an error on this configuration into configure.ac]) I was able to compile with termux under Android 1.06 and succesfully use test cases

[still had to manually change configure script to use bash in this context but think that may normally be encapsulated in termux build scripts]
./configure --prefix=/data/data/com.termux/files/usr

This effort was necessitated to recompile libzmq from scratch in support of addressing this issue zeromq/pyzmq#1240

configure.ac Outdated
@@ -89,6 +89,8 @@ fi
# Check whether to build a with debug symbols
LIBZMQ_CHECK_ENABLE_DEBUG

AC_CHECK_LIB([atomic],[__atomic_load_4],[LIBS="${LIBS} -latomic"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always enable this linking, which is not right. There's a section that checks the host os, and looks for android, you should add it there instead with the appropriate checks for "android things"

@bluca
Copy link
Member

bluca commented Nov 21, 2018

thanks - please send a separate PR with a relicensing grant, see bottom of PR template: https://github.com/zeromq/libzmq/blob/master/.github/PULL_REQUEST_TEMPLATE.md

@bluca
Copy link
Member

bluca commented Nov 21, 2018

actually I double checked, and there's already an m4 test to see if linking to latomic is needed, so check the log instead and see why it is not triggering: https://github.com/zeromq/libzmq/pull/3083/files

@brubakerjeff
Copy link
Contributor Author

brubakerjeff commented Nov 21, 2018 via email

@bluca
Copy link
Member

bluca commented Nov 21, 2018

the tipc change is fine, it's the latomic one that is not as it overwrites the work done by the linked pr in acinclude.m4

@brubakerjeff brubakerjeff reopened this Nov 21, 2018
@brubakerjeff
Copy link
Contributor Author

from configure
"checking for __atomic_load_4 in -latomic... yes"

which means this should be triggered.

[AC_MSG_RESULT(yes) ; libzmq_cv_has_atomic_instrisics="yes" LIBS="-latomic" ; $1],

My understanding is in the current version LIBS= was factored into the test case in the new change?

https://github.com/PIDriverAssist/libzmq/blob/4f48c5615ecbbd70423e163151a3448e75570f99/acinclude.m4#L694
[AC_MSG_RESULT(yes) ; libzmq_cv_has_atomic_instrisics="yes" ; $1],

changed at
https://github.com/PIDriverAssist/libzmq/commit/6de5f18be3baeccd2c7c4b095a1c09dbecb779a4#diff-a5522b90382504dc530177f63ced5209
Note is "with -latomic if needed however using LDFLAGS doesn't work when statically linking because LDFLAGS is added before LIBS"

If you can confirm my perception, a thought is to reinstate setting LIBS under condition that linking is not static?

@bluca
Copy link
Member

bluca commented Nov 21, 2018

LIBS is already been set (or rather, it's not reset back if it's needed), so the problem must be something else

@bluca
Copy link
Member

bluca commented Nov 21, 2018

if you build with make V=1 what's the actual content of *FLAGS?

@brubakerjeff
Copy link
Contributor Author

brubakerjeff commented Nov 21, 2018

  1. from configure..
    checking whether compiler supports __atomic_Xxx intrinsics... no

  2. running make
    $ make V=1
    Making all in doc
    make[1]: Entering directory '/data/data/com.termux/files/home/libzmq-3/libzmq/doc'
    make[1]: Nothing to be done for 'all'.
    make[1]: Leaving directory '/data/data/com.termux/files/home/libzmq-3/libzmq/doc'
    make[1]: Entering directory '/data/data/com.termux/files/home/libzmq-3/libzmq'
    /bin/sh ./libtool --tag=CXX --mode=link g++ -pedantic -Werror -Wall -Wno-long-long -g -O2 -Wno-tautological-constant-compare -o tools/curve_keygen tools/curve_keygen.o src/libzmq.la -lrt -lpthread
    libtool: link: g++ -pedantic -Werror -Wall -Wno-long-long -g -O2 -Wno-tautological-constant-compare -o tools/.libs/curve_keygen tools/curve_keygen.o src/.libs/libzmq.so -lrt -lpthread -Wl,-rpath -Wl,/usr/local/lib
    /data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: src/.libs/libzmq.so: undefined reference to __atomic_fetch_sub_4' /data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: src/.libs/libzmq.so: undefined reference to __atomic_load_4'
    /data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: src/.libs/libzmq.so: undefined reference to __atomic_exchange_4' /data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: src/.libs/libzmq.so: undefined reference to __atomic_compare_exchange_4'
    /data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: src/.libs/libzmq.so: undefined reference to __atomic_fetch_add_4' /data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: src/.libs/libzmq.so: undefined reference to __atomic_store_4'

  3. I'm reading this and seeing anywhere a state change would/could occur for LIBS to be concatenated with latomic. Per your statement it would occur previously in execution?
    AC_DEFUN([LIBZMQ_CHECK_ATOMIC_INTRINSICS], [{
    AC_MSG_CHECKING(whether compiler supports __atomic_Xxx intrinsics)
    AC_LINK_IFELSE([AC_LANG_SOURCE([
    /* atomic intrinsics test */
    int v = 0;
    int main (int, char **)
    {
    int t = __atomic_add_fetch (&v, 1, __ATOMIC_ACQ_REL);
    return t;
    }
    ])],
    [AC_MSG_RESULT(yes) ; GCC_ATOMIC_BUILTINS_SUPPORTED=1 libzmq_cv_has_atomic_instrisics="yes" ; $1])

    if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" != x1; then
    save_LIBS=$LIBS
    LIBS="$LIBS -latomic"
    AC_LINK_IFELSE([AC_LANG_SOURCE([
    /* atomic intrinsics test */
    int v = 0;
    int main (int, char **)
    {
    int t = __atomic_add_fetch (&v, 1, __ATOMIC_ACQ_REL);
    return t;
    }
    ])],
    [AC_MSG_RESULT(yes) ; libzmq_cv_has_atomic_instrisics="yes" ; $1],
    [AC_MSG_RESULT(no) ; libzmq_cv_has_atomic_instrisics="no" LIBS=$save_LIBS ; $2])
    fi
    }])

  4. The host_os is reporting as linux-gnueabi not android for Android Things 1.06 worth considering depending upon directory for libc

@bluca
Copy link
Member

bluca commented Nov 21, 2018

are you passing all the cross compilation flags for autoconf?

@brubakerjeff
Copy link
Contributor Author

brubakerjeff commented Nov 21, 2018

I'm compiling on device. Would they be relevant? (Termux on android)

@bluca bluca merged commit d983251 into zeromq:master Nov 23, 2018
@bluca
Copy link
Member

bluca commented Nov 23, 2018

I'm not sure. I don't have the device to try it, so I'll merge as-is.

@brubakerjeff
Copy link
Contributor Author

Thank you. I think we will need another iteration as we understand this more. I'm thinking the branches should condition upon presence of atomic intrinsic and whether or not libzmq is statically compiled [based upon the notes] (none of this logic is encapsulated by lower level tools?) but I will read more to confirm this.

@bluca
Copy link
Member

bluca commented Nov 24, 2018

Ok, feel free to send more pull requests

MohammadAlTurany pushed a commit to FairRootGroup/libzmq that referenced this pull request Jan 28, 2019
* include atomic when needed

* update for tipc

* moved check under android

* added license
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants