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

boost: Update from 1.65.1 to 1.67.0 #2384

Merged
merged 3 commits into from
May 14, 2018
Merged

boost: Update from 1.65.1 to 1.67.0 #2384

merged 3 commits into from
May 14, 2018

Conversation

tomty89
Copy link
Contributor

@tomty89 tomty89 commented Apr 29, 2018

No description provided.

@tomty89
Copy link
Contributor Author

tomty89 commented Apr 29, 2018

Unsplit libboost-python as the dependency or linkage to (lib)python was never necessary.

@its-pointless
Copy link
Contributor

It is necesary to link with python because of the android linker. Opencv wouldh't work properly without doing that.

@its-pointless
Copy link
Contributor

It would literally error out can;t find python symbol etc. Without linking with python it simply won't import lib without error.
So yes it was like that for a reason.

Copy link
Contributor

@its-pointless its-pointless left a comment

Choose a reason for hiding this comment

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

this bit
dl -lpython3.6m ;
is neccesary otherwise the boost python module cant be imported into python. This means for instance opencv can't work automatically.
On normal linux its not required but the android linker is not the same thing.

@tomty89
Copy link
Contributor Author

tomty89 commented Apr 30, 2018

I am not seeing the sense in this. Even on desktop distros libboost_python isn't linked (at the very least not dynamically) against libpython. Where comes the problem with opencv and the android linker?

Btw there's a chance that the libboost_python that we have been shipping was built for python2, if it was built on Ubuntu but not Arch, as python is python2 on Ubuntu AFAIK. The bootstrap script by default does version detection with the python executable and hence can only rely on the host python. Worsestill it was forced to use the python3 headers of the Termux build.

Even my last commit was not alright, because apparently ./b2 clean doesn't force a rebuilt, and instead I need to use -a. Will repush an updated version of the last two commit soon. Would be great if you can help testing @its-pointless

libboost_python was never supposed to be linked to libpython. Hence
the patch, and in turn the dependency to the python package, and in
turn the package split, were never necessary.
@tomty89
Copy link
Contributor Author

tomty89 commented Apr 30, 2018

Btw it's probably not a must to build libboost_python against Termux python. In fact I am not sure if either way is better or safer. Apparently the Travis CI image doesn't have python-dev and python2-dev installed though, so build test will fail if I use host python builds instead.

One good thing about using host python might be that it may allow us to build numpy bindings, if they are desired, since on Termux we rely on pip which can't really be used in the build system.

@Grimler91
Copy link
Member

Even on desktop distros libboost_python isn't linked (at the very least not dynamically) against libpython. Where comes the problem with opencv and the android linker?

See comments in #1992 for background why ˋ-lpython3.6mˋ was needed.

See android/ndk#201 for discussion about the differences in the linkers.

@tomty89
Copy link
Contributor Author

tomty89 commented Apr 30, 2018

@Grimler91 Well I failed to see that any libraries in opencv (including the cv2 ones for python) actually make use of any of boost libraries in any way, including but not limited to libboost_python. I have no idea how you have come to the conclusion that one should forcefully link opencv against libboost_python (and in turn forcefully link libboost_python against libpython).

Apparently you thought that PyFloat_Type is something specific to libboost_python, because you bumped into a bug report that mentioned it (where the error is not even the same), of a project that makes use of libboost_python; but it is not:

$ readelf -Ws ../usr/lib/libpython3.6m.so.1.0 | grep PyFloat_Type
   192: 00000000001f35a8   400 OBJECT  GLOBAL DEFAULT   16 PyFloat_Type
$

The OP never even return to report that -lboost_python actually helped in his case. To be honest, even if it does help magically, it still doesn't mean it's the right way, coz it really doesn't make any sense.

@its-pointless
Copy link
Contributor

Symbol visibility when opening shared libraries using dlopen() works differently. On a normal linker, when an executable linking against a shared library libA dlopen():s another shared library libB, the symbols of libA are exposed to libB without libB needing to link against libA explicitly. This does not work with the Android linker, which can break plug-in systems where the main executable dlopen():s a plug-in which doesn't explicitly link against some shared libraries already linked to by the executable. See the relevant NDK issue for more information.

@Grimler91
Copy link
Member

Looking back, it seems a bit weird that libboost_python is needed so you might be right here @tomty89.

I tried recompiling without -lboost_python and with -lpython3.6m but got the same error as before: dlopen failed: cannot locate symbol "PyFloat_Type" referenced by "/data/data/com.termux/usr/lib/python3.6/site-packages/cv2.cpython-36m.so so just setting LDFLAGS=-lpython3.6m doesn't work but linking against boost_python works for some reason.

Sidenote for @its-pointless : I also received an error first that header numpy/ndarrayobject.h couldn't be found, I solved it by symlinking site-packages/numpy-1.14-......./numpy/core/include/numpy/ to $PREFIX/include.

@tomty89
Copy link
Contributor Author

tomty89 commented May 3, 2018

@its-pointless The difference / issue of the Android linker isn't relevant here. As I've said, the cv2 library isn't even written with boost in mind, but only python and numpy. The decision that libboost_python should be linked against libpython was based on something that isn't even sensical (linking opencv against libboost_python, that is).

@Grimler91 I am not sure how much support you can get from developers of opencv for this, but I do think it should be reported upstream (whether the reason is the android linker as @its-pointless said or not). AFAIR they do/did have at least some intention to support Android.

As for why the two forced linkage workaround the error, I have no idea. My guess is because libboost_python was malformed, and hence it somehow fits the taste of the broken cv2 library.

@tomty89
Copy link
Contributor Author

tomty89 commented May 3, 2018

From android/ndk#201 (comment):

On Android only the main executable and LD_PRELOADs are considered to be RTLD_GLOBAL, all the dependencies of the main executable remain RTLD_LOCAL.

ld-linux.so marks everything linked against main executable with RTLD_GLOBAL, this is why the example works on Linux.

Adding libshared.so dependency to libplugin.so should solve this problem

I guess the problem is, because we have libpython built on Termux as most distros do, the symbols of the Python C-API are in libpython and are marked with RTLD_LOCAL when loaded and are hence not available for python extensions as they are on desktop distros.

When I test with the example "spam" in https://docs.python.org/3.6/extending/index.html, the results are coherent to the quoted statements:

$ clang -shared -I ../usr/include/python3.6m spam.c -o spam.so
$ python -c 'import spam'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: dlopen failed: cannot locate symbol "PyArg_ParseTuple" referenced by "/data/data/com.termux/files/home/spam.so"...
$ clang -shared -I ../usr/include/python3.6m -lpython3.6m spam.c -o spam.so
$ python -c 'import spam'
$
$ clang -shared -I ../usr/include/python3.6m spam.c -o spam.so
$ python -c 'import spam'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: dlopen failed: cannot locate symbol "PyArg_ParseTuple" referenced by "/data/data/com.termux/files/home/spam.so"...
$ LD_PRELOAD=$PREFIX/lib/libpython3.6m.so python -c 'import spam'
$

So the only remaining question is why -lpython3.6m doesn't work for the cv2 library.

@Grimler91 Have you confirmed that the linkage has actually been applied to the cv2 library by checking it with ldd? (I mean, could -lpython3.6m but not -lboost_python somehow got filtered/ignored?) Also can you test if the LD_PRELOAD method works for the cv2 library?

P.S. I think it's clear enough that libboost_python is not relevant in the problem at all now.

@tomty89
Copy link
Contributor Author

tomty89 commented May 3, 2018

@its-pointless @Grimler91

$ cd opencv-3.4.1/build/lib/
$ ldd cv2.so | grep py
$ python2 -c 'import cv2'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: dlopen failed: cannot locate symbol "PyFloat_Type" referenced by "/data/data/com.termux/files/home/opencv-3.4.1/build/lib/cv2.so"...
$ LD_PRELOAD=$PREFIX/lib/libpython2.7.so python2 -c 'import cv2'
$ cd python3/
$ ldd cv2.cpython-36m.so | grep py
$ python -c 'import cv2'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: dlopen failed: cannot locate symbol "PyFloat_Type" referenced by "/data/data/com.termux/files/home/opencv-3.4.1/build/lib/python3/cv2.cpython-36m.so"...
$ LD_PRELOAD=$PREFIX/lib/libpython3.6m.so python -c 'import cv2'
$

With -DOPENCV_FORCE_PYTHON_LIBS=true:

$ cd opencv-3.4.1/build/lib/
$ ldd cv2.so | grep py
libpython2.7.so.1.0
$ python2 -c 'import cv2'
$ cd python3/
$ ldd cv2.cpython-36m.so | grep py
libpython3.6m.so.1.0
$ python -c 'import cv2'
$

@Grimler91
Copy link
Member

@tomty89 Awesome, so that's the proper fix..
Annoying that the normal LDFLAGS=... didn't work.

@tomty89
Copy link
Contributor Author

tomty89 commented May 3, 2018

@Grimler91 You don't want to use LDFLAGS anyway. Not only that it prevents you from building the cv2 library for both python2 and python3 in a sane manner, it also unnecessary link the main opencv libraries against libpython:

$ cd opencv-3.4.1/python/lib/
$ ldd libopencv_core.so | grep py
libpython3.6m.so.1.0
$ cd python3/
$ ldd cv2.cpython-36m.so | grep py
libpython3.6m.so.1.0
$ python -c 'import cv2'
$

However, as you can see, it also make the cv2 library importable on my phone, so I am not sure what's happening on yours.

@tomty89
Copy link
Contributor Author

tomty89 commented May 4, 2018

@fornwall With the last update this PR should be good to go. Thanks!

@its-pointless
Copy link
Contributor

you can enable coroutine and context but to do so we have to compile the assembler files. This needs
architecture=arm
abi=aapcs
binary-format=elf \

in the command line for arm and arm64 and for i686 and x86_64 its
architecture=x86
abi=sysv
binary-format=elf \

since for compabititlity all packages which are linked against c++ are going to be recompiled to work with new libc++_shared.so may as well update this as well since all packages that link against boost have to updated on boost update.

@its-pointless
Copy link
Contributor

@arkanoid87 arkanoid87 mentioned this pull request May 13, 2018
@fornwall fornwall merged commit ae4f2ea into termux:master May 14, 2018
@tomty89 tomty89 deleted the boost branch May 14, 2018 07:42
@fornwall
Copy link
Member

Nice work! The updated package is available (with the updates in #2425 to enable coroutine and context).

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.

4 participants