Fix build for case-insensitive FS and add CI test for OSX

opacam commented 4 years ago

It turns out that the generated binary for MacOS and Cygwin is not python...it's python.exe...this leads to not set correctly the ctx.hostpython variable for Mac OS. This rename of the python binary for MacOS is to, probably, avoid issues with the Mac OS filesystem which could be Case Insensitive and it would conflict with the build folder named Python).

I hope that this solve the Mac OS build issue :crossed_fingers:

Note: this should be tested by some mac user, unfortunately I could not test it because I don't have a mac...so I recommend to merge after someone test this, but all seems indicate that this is the problem of p4a with Mac OS...thanks to this comment we got the solution :smile:,Thanks @TheSin-

Closes: #1817 Closes: #1800 Closes: #1682 Closes: #1647

TheSin- commented 4 years ago

No problem I'm happy to help, I'm very very new to python so I'm just fumbling around.

I'll build a new docker with this patch and test it, hopefully it won't take me too long to test it.

ghost commented 4 years ago

~~I am wondering, don't we sometimes call "python2" or "python3" directly in other places? E.g. I do that here: https://github.com/kivy/python-for-android/pull/1937~~

~~It seems weird that this call wouldn't work, after all it also does on Windows, actually. And wouldn't it also for an actual Cygwin-compiled program too?~~

~~Why is Cygwin involved here anyway? Shouldn't we target macOS without Cygwin? This situation confuses me a little~~

Nevermind, I thought the actual python on macOS was somehow installed as python.exe but this applies only to the one we build internally. I get it now, sorry for my temporary confusion :joy:

inclement commented 4 years ago

I'd quite like to get this into the next release (sometime in the next few days), or at the very least: we could add a simple log that confirms the python exe name to give confidence in the change. It would then be easy to pull the result from any user's log.

TheSin- commented 4 years ago

I’m hopefully going to run a test after dinner tonight.

running it right now, I'll repost back once it gets to that part, I removed all my work arounds

TheSin- commented 4 years ago

did not break, but also did not fix it. I'm trying to dig deeper but I'm pretty new to docker and even newer to python-for-android...

be sooo much easier if it didn't capture the output :) I can't see where or why it fails, I'm going to attach and run it by hand see if I can figure it out. When it builds python3-libffi-openssl-sqlite3 what make command is used or where do I find the receipt for it?

TheSin- commented 4 years ago

does this help

/opt/android-ndk-r17c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -target armv7a-none-linux-androideabi -gcc-toolchain /opt/android-ndk-r17c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 -shared --sysroot=/opt/android-ndk-r17c/platforms/android-21/arch-arm -L/opt/android-ndk-r17c/platforms/android-21/arch-arm/usr/lib -L/opt/android-ndk-r17c/platforms/android-21/arch-arm/usr/lib -L/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/sqlite3/armeabi-v7a__ndk_target_21/sqlite3/obj/local/armeabi-v7a -L/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/libffi/armeabi-v7a__ndk_target_21/libffi/.libs -L/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/openssl/armeabi-v7a__ndk_target_21/openssl1.1 -march=armv7-a -Wl,--fix-cortex-a8  -Wl,--no-as-needed -o libpython3.so -Wl,-hlibpython3.so libpython3.7m.so
/opt/android-ndk-r17c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -target armv7a-none-linux-androideabi -gcc-toolchain /opt/android-ndk-r17c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 --sysroot=/opt/android-ndk-r17c/platforms/android-21/arch-arm -L/opt/android-ndk-r17c/platforms/android-21/arch-arm/usr/lib -L/opt/android-ndk-r17c/platforms/android-21/arch-arm/usr/lib -L/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/sqlite3/armeabi-v7a__ndk_target_21/sqlite3/obj/local/armeabi-v7a -L/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/libffi/armeabi-v7a__ndk_target_21/libffi/.libs -L/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/openssl/armeabi-v7a__ndk_target_21/openssl1.1 -march=armv7-a -Wl,--fix-cortex-a8  -pie -Xlinker -export-dynamic -o python.exe Programs/python.o -L. -lpython3.7m -ldl  -lsqlite3 -lffi -lcrypto1.1 -lssl1.1   -lm  
_PYTHON_PROJECT_BASE=/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3/android-build _PYTHON_HOST_PLATFORM=linux-arm PYTHONPATH=/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3/Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata_m_linux_ python -S -m sysconfig --generate-posix-vars ;\
if test $? -ne 0 ; then \
    echo "generate-posix-vars failed" ; \
    rm -f ./pybuilddir.txt ; \
    exit 1 ; \
fi
/bin/sh: 1: python: not found
generate-posix-vars failed
Makefile:595: recipe for target 'pybuilddir.txt' failed
make: *** [pybuilddir.txt] Error 1

looks like here it switches from python.exe to python, this might be the case insensitive issue, I'll dig further back and see

TheSin- commented 4 years ago

so this is the line in the python3 Makefile

PYTHON_FOR_BUILD=_PYTHON_PROJECT_BASE=$(abs_builddir) _PYTHON_HOST_PLATFORM=$(_PYTHON_HOST_PLATFORM) PYTHONPATH=$(shell test -f pybuilddir.txt && echo $(abs_builddir)/`cat pybuilddir.txt`:)$(srcdir)/Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata_$(ABIFLAGS)_$(MACHDEP)_$(MULTIARCH) python

so it can be modified, or make an alias could be made so that python calls actually call python.exe?

TheSin- commented 4 years ago

So I'm continuing to investigate, I know I'm posting lots but I keep hoping that I'll post the right thing for someone that understands and knows the build process.

oddly enough I do not have python, but the docker image does have python3 and python3.6 interpreters. But the python3 configure script seems to find python.

if test "$cross_compiling" = yes; then
    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for python interpreter for cross build" >&5
$as_echo_n "checking for python interpreter for cross build... " >&6; }
    if test -z "$PYTHON_FOR_BUILD"; then
        for interp in python$PACKAGE_VERSION python3 python; do
            which $interp >/dev/null 2>&1 || continue
            if $interp -c "import sys;sys.exit(not '.'.join(str(n) for n in sys.version_info[:2]) == '$PACKAGE_VERSION')"; then
                break
            fi
            interp=
        done
        if test x$interp = x; then
            as_fn_error $? "python$PACKAGE_VERSION interpreter not found" "$LINENO" 5
        fi
        { $as_echo "$as_me:${as_lineno-$LINENO}: result: $interp" >&5
$as_echo "$interp" >&6; }
        PYTHON_FOR_BUILD='_PYTHON_PROJECT_BASE=$(abs_builddir) _PYTHON_HOST_PLATFORM=$(_PYTHON_HOST_PLATFORM) PYTHONPATH=$(shell test -f pybuilddir.txt && echo $(abs_builddir)/`cat pybuilddir.txt`:)$(srcdir)/Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata_$(ABIFLAGS)_$(MACHDEP)_$(MULTIARCH) '$interp
    fi

for some reason this code int eh configure returns python

configure:2917: checking for python interpreter for cross build
configure:2930: result: python

lds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3$ echo $PATH
/home/buildozer/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
buildozer@7fad8d7bf18a:~/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3$ which python3
/usr/bin/python3
buildozer@7fad8d7bf18a:~/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3$ which python
buildozer@7fad8d7bf18a:~/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3$

and there is no option for it to use the hostpython3 which is built just before it, which would then be ../../../../hostpython3/desktop/hostpython3/native-build/python.exe

but patching for that would be insane, any other ideas? I still have no idea how it's finding python vs python3 :. Maybe it's being passed in via configure param or ENV?

TheSin- commented 4 years ago

found the issue, it's the in the python configure script.

for interp in python$PACKAGE_VERSION python3 python; do
     which $interp >/dev/null 2>&1 || continue

it continues cause it's not found. BUT interp is already set, so by default it will finish the loop with interp='python' even if it's not found.

So no matter what it'll think it found python when in reality it didn't find anything cause it needs to look for python.exe

TheSin- commented 4 years ago

Testing with this setup, no idea if I'm out to lunch here but figured I'd help try to solve it, I still have the original PR on as well as this patch.

--- a/pythonforandroid/recipes/python3/__init__.py  2019-08-06 02:39:19.595367919 +0000
+++ b/pythonforandroid/recipes/python3/__init__.py  2019-08-06 02:38:58.098543863 +0000
@@ -1,6 +1,7 @@
 import sh
 from pythonforandroid.python import GuestPythonRecipe
 from pythonforandroid.recipe import Recipe
+from pythonforandroid.patching import is_darwin

 class Python3Recipe(GuestPythonRecipe):
@@ -27,6 +28,9 @@
     if sh.which('lld') is not None:
         patches = patches + ["patches/remove-fix-cortex-a8.patch"]

+    if is_darwin():
+        patches = patches + ["patches/fix-interpreter-for-darwin.patch"]
+
     depends = ['hostpython3', 'sqlite3', 'openssl', 'libffi']
     conflicts = ['python3crystax', 'python2']

fix-interpreter-for-darwin.patch

diff --git a/configure b/configure
--- a/configure 2019-08-06 02:35:17.000000000 +0000
+++ b/configure 2019-08-06 02:36:22.000000000 +0000
@@ -2868,7 +2868,7 @@
 # pybuilddir.txt will be created by --generate-posix-vars in the Makefile
 rm -f pybuilddir.txt

-for ac_prog in python$PACKAGE_VERSION python3 python
+for ac_prog in python$PACKAGE_VERSION python3 python.exe python
 do
   # Extract the first word of "$ac_prog", so it can be a program name with args.
 set dummy $ac_prog; ac_word=$2
@@ -2917,7 +2917,7 @@
     { $as_echo "$as_me:${as_lineno-$LINENO}: checking for python interpreter for cross build" >&5
 $as_echo_n "checking for python interpreter for cross build... " >&6; }
     if test -z "$PYTHON_FOR_BUILD"; then
-        for interp in python$PACKAGE_VERSION python3 python; do
+        for interp in python$PACKAGE_VERSION python3 python.exe python; do
        which $interp >/dev/null 2>&1 || continue
        if $interp -c "import sys;sys.exit(not '.'.join(str(n) for n in sys.version_info[:2]) == '$PACKAGE_VERSION')"; then
            break

TheSin- commented 4 years ago

okay bigger issue, because I'm running this in a docker on Darwin, is_darwin() does not return true but docker is still bleeding enough through to get python.exe and case insensitivity. Maybe host python defaults to python.exe cause it detect the fs being case insensitive?

I'm testing that without using is_darwin() right now to make sure then I'll diagnose hostpython3 and see if I can figure out why it builds python.exe vs python.

FOUND IT

hostpython3, configure

$as_echo_n "checking for case-insensitive build directory... " >&6; }
if test ! -d CaseSensitiveTestDir; then
mkdir CaseSensitiveTestDir
fi

if test -d casesensitivetestdir
then
    { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
$as_echo "yes" >&6; }
    BUILDEXEEXT=.exe
else
        { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
        BUILDEXEEXT=$EXEEXT
fi
rmdir CaseSensitiveTestDir

so if it's Case Intensive is is python.exe, hence is_darwin() won't help need to test the fs

I'm going to end here cause it's late and I'm SURE there are more experienced ppl that can fix it from here. But I'm willing to test once a patch is available.

my ideas are, built I'm not sure which is best, I pout them in order of effort not what I think is best. 1) we check for python.exe and rename it to python3 after the hostpython3 build and just let it run 2) we add a patch to the hostpython3 configure that touches a file on Case Sensitive systems that we can use to patch python3 build using to patch I posted 3) we make a proper python function to detect caseSensitivity and call it instead of is_darwin()

Sorry for the spam everyone, was trying to work my way through it.

inclement commented 4 years ago

@TheSin- That's great debugging, thanks!

TheSin- commented 4 years ago

Thanks I just hope it helps. It's an odd issue but it'll be nice to have it solved I think.

There are 2 other solutions at least for macOS

build things on a USB drive that is formatted to be case sensitive (or install on case sensitive) or just run a VM which is what I did to get my build done. But native would be nice in the long run, and it seems like it should be easy enough to solve so long as that is the only issue since I didn't get to finish testing cause I wasn't sure on the direction the team would want to take to solve it.

opacam commented 4 years ago

@inclement, so sorry for the delay in the response in here, I was on vacations until yesterday, later I will force push the changes which will fix this issue once and for all (I hope...)

@TheSin-, so many thanks for your debugging session and for the pyton3's patch :+1: (I will include that in the next force push I will make).Honestly, I don't know how the hell you managed to test this OSX build with docker (as far as I know it's not possible to do so, unless you use qemu inside a docker image...but I may be wrong, so the darwin patch not applied as you commented because the kernel isn't the one expected nor the android's NDK/SDK are the Mac OS ones)...nevertheless... your conclusions are right and has been crucial to fix this :smile:

TheSin- commented 4 years ago

yeah it took me a bit to figure it all out since I'm so new to all this, docker and python-for-android, but it's a great tool so I wanted to help fix it for everyone. I'm glad I was able to help and once the push is available I'll give it all an other shot and let you know. I still have it all setup and ready.

opacam commented 4 years ago

@inclement, I force pushed some changes and added a new commit, now I'm pretty sure that the build success, because I created a CI test for Mac OS X which will force the Python 3 armeabi-v7a test to be performed with Mac OSX instead of linux. Honestly, I'm not sure if we must do this test (I don't like to dance with the apple's rhytm...)...we should probably decide this with the team (@AndreMiras, you are the missing piece since @Jtc0de is already in here... and I suspect that you will want to say something about this OSX test for CI)...so guys...let me know what do you think about this...

Anyway, at least now we are sure that the build for Mac OS X succeeds :smile:...it will work the generated apk? ...I guess that we will find out...:crossed_fingers:

@TheSin-, I think with the added CI test it may not be needed to test it as you did, but I will be happy to read and comment your reflexions :smile: if you test it...we may found other issues related with Mac OSX build...

note: I patched python2 for Mac OSX as @TheSin- did, but didn't test the build...teorically it should work...

TheSin- commented 4 years ago

here is the results of my current tests, but I need to revisit a bit of it. Other then what I already commented on.

_PYTHON_PROJECT_BASE=/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3/android-build _PYTHON_HOST_PLATFORM=linux-arm PYTHONPATH=/home/buildozer/build/.buildozer/android/platform/build/build/other_builds/python3-libffi-openssl-sqlite3/armeabi-v7a__ndk_target_21/python3/Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata_m_linux_ python -S -m sysconfig --generate-posix-vars ;\
if test $? -ne 0 ; then \
    echo "generate-posix-vars failed" ; \
    rm -f ./pybuilddir.txt ; \
    exit 1 ; \
fi
/bin/sh: 1: python: Permission denied

$ ls python3/android-build/
Makefile      Objects   config.log     libpython3.7m.a   python-config
Makefile.pre  Parser    config.status  libpython3.7m.so  python-config.py
Misc          Programs  crtbegin_so.o  libpython3.so     python.exe
Modules       Python    crtend_so.o    pyconfig.h        python.exe-gdb.py

so it did build it, but it's trying to run python instead of python.exe and causing the build to fail. I'm hading home but I'll diagnose more once I'm there.

OptimusGREEN commented 4 years ago

Hi guys,

Tested this (first time in months) and it built ok. my app is however crashing out at startup with no code changes so wondering if its something here.

here's my logcat

https://pastebin.com/raw/nX6VfmAC

looks like somthing to do with lib python so

opacam commented 4 years ago

aaa...that's good news @OptimusGREEN , we have the build...

but I still not convinced of the work I made in here, I'm testing in another branch a different approach (considering the last comments of @TheSin- ) and it will simplify the whole stuff...I will push it later...

regarding the runtime error...what kivy version do you use...master?

if that is the case I would recommend you to stick to p4a version...there was some conversation on discord (yesterday) and the error I see it's pretty similar (hard to tell with this kind of libc errors...)

If not...well...we have another problem...

OptimusGREEN commented 4 years ago

yeah its kivy master and my p4a fork rebased at this commit as didn't think it had been merged to p4a proper.

opacam commented 4 years ago

Ok , guys, travis seems happy with all python versions and OS tested (darwin and linux). So we should have a successful build with Mac OSX.

The thing works by making a copy of our compiled hostpython binary (@TheSin-, your first choice...thanks again :smile:) in such a way that will reflect the major version (because the python's configure scripts considers that possibility...so we avoid the patching, except for python2's recipe, which haves a wrong version of python :disappointed:).

So...@TheSin- , no more is_darwin :wink:...let see how it goes your docker tests now :grimacing: ...could you do that please?

@OptimusGREEN, it would be great that you could test it now, but with kivy's version 1.11.1 (the p4a's default)...because we know that version is working...the issue you got at runtime it maybe kivy's related (I'm not sure of this...haven't had time to test it)

TheSin- commented 4 years ago

I'm on it, I'm just securing a new shelving unit to my wall and I'll get a build started, thanks for all your hard work on this I'm very excited to be able to do my full development natively!

EDIT: it's up and running, report back shortly

EDIT 2: It BUILT!!!!! I'm just running my dogs and I'll come back to test it. w00t, great job everyone!!

TheSin- commented 4 years ago

100% Thumbs up from me, built and ran perfectly!!! Great job!

OptimusGREEN commented 4 years ago

I used Kivy 1.11.1 this time and I seemed to have the same issue (it maybe isn't but looks like it to me) so maybe my issue isn't related to this? It would be strange if it wasn't though as I haven't changed any code since I last successfully built on a linux vm a few months back.

here's my logcat https://pastebin.com/raw/qbDkhJXf

opacam commented 4 years ago

100% Thumbs up from me, built and ran perfectly!!! Great job!

@TheSin- , ¡¡¡thanks for all your work!!! in here :smile:, I consider this a collaboration work :+1:!!!...in matter fact...you did the hard work in here.

here's my logcat

@OptimusGREEN, the log you posted isn't useful (at least to me...). This libc error at runtime would probably require create a gdb debugable build...but this will require another PR which isn't merged yet. But before that...this error could also be caused due a previous p4a builds you got in your system (we have a problem in there...we are working on that also). If you didn't clean up the build environment before the last attempt you made, then do it and try it again...so...did you cleaned up your build environment before your tests? (using buildozer: buildozer android clean).

Note: If after the clean up and rebuilt still doesn't work, I would try to isolate the problem by building a simple apk (I would recommend to use one of our testapps: setup_testapp_python3_sqlite_openssl.py. If that apk doesn't work and fails with the same error you got...well...we probably have a bug with Mac OS X, but if that apk works, the problem may be in some recipe do you use or in your app's code...

As a side notes:

@OptimusGREEN, it would be useful to have the debug's log of the build...with buildozer it will be: buildozer android debug --debug
BTW, if you install the last buildozer version from master branch, you can easily test any p4a's PR by editing your buildozer.spec file, so for this one, you would use:
```
p4a.fork = opacam
p4a.branch = hotfix-macos-build
```
just remember to comment or eddit those lines again once you are done or this merged, since I supose that you don't want to get stuck in here :wink:

OptimusGREEN commented 4 years ago

yeah i had removed my .buildozer directory altogether.

Nice tips with the p4a.fork thing. i didn't know bout that, really handy.

I'll give it another try

OptimusGREEN commented 4 years ago

Ok so tried again and build was successful as previous but again the app crashes on launch with the same libpython37.so error.

output from build https://paste.ee/p/W9xz8

I also tried building on linux and everything was fine and app ran ok so there is defo still a mac issue. It might be a library or something but def only happening on my mac.

my requirements:

requirements = python3,kivy==1.11.1,pyopenssl,openssl,urllib3,android,pyjnius,dropbox,requests==2.20.0,chardet,certifi,idna

AndreMiras commented 4 years ago

Impressive work! I've been busy lately and will still be for a little while. From a quick look, well I like that we introduce CI tests if Apple users is a significant portion of our user base. What hurts me however is to see the .travis.yml file growing that much overtime. And I do mean "to see the .travis.yml file". I mean probably moving some of the logic to dedicated bash or Python script would probably peace my mind. I feel like it could help by keeping things a bit more isolated, but I'm not sure if it makes sense and if it doesn't introduce more overhead. Thanks again for the effort, I'll try to take a deeper look throughout the week.

opacam commented 4 years ago

@AndreMiras and @inclement, thanks for your reviews :smile:

...I was hoping that you guys don't want this Mac OS X test :laughing:. I already expressed my feelings about mac, so the main goal was only to be sure that the thing worked (at least at build time). But since there is interest on have this test, I agree with you guys, the Mac OS X logic has to be moved somewhere else, isolating the android part (so we can refactor as well the android part of our docker files). I liked the @AndreMiras's idea of Makefiles so I've been working on that, later I will force push the changes :wink:

kivy / python-for-android

Fix build for case-insensitive FS and add CI test for OSX #1951