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

[CORE UPDATE - PART IV] Fix cryptography+cffi #1545

Merged
merged 5 commits into from
Jan 12, 2019

Conversation

opacam
Copy link
Member

@opacam opacam commented Dec 22, 2018

This is one more part for the pr #1537 (discussed previously in #1460 and #1534)

Here I fix recipes cryptography and cffi (and touch some others which those depend on) to make it work with the core-update pull request, which also contains a new version of the openssl libs created by @tito. I tried to update the most of the packages to latest version and grants compatibility for both versions of python.

Note: this has to be merged only when the core-update has been merged because it depends on the new python2 recipe and the openssl libs (1.1)

Since the recent openssl upgrade this recipe stopped to work. Here we adapt the recipe to the new build circumstances and we also fix the cffi recipe because it is a direct dependency of cryptography and had the wrong flags for the new python2 build system.
In order to not have troubles with python3 we update most of the dependant recipes to the latest available versions.
@ghost
Copy link

ghost commented Jan 2, 2019

This is another one that is quite fundamentally relevant for everyone, isn't it? Doesn't ctypes require cffi? Once this is in I can do the ctypes.util.find_library() patch on top, and finally all the core things will work properly out of the box! 😍

# https://github.com/kivy/python-for-android/pull/1250/files#diff-569e13021e33ced8b54385f55b49cbe6
env['CFLAGS'] += ' -I{}/include/python/'.format(ndk_dir_python)
env['LDFLAGS'] += ' -L{}'.format(self.ctx.python_recipe.link_root(arch.arch))
env['LDFLAGS'] += ' -lpython{}'.format(self.ctx.python_recipe.major_minor_version_string)
Copy link

Choose a reason for hiding this comment

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

Hm does libffi actually depend on python? The old recipe doesn't seem to have done that other than for crystax, it just leaves me wondering why it is required? Or is libffi a python-specific lib

(I'm just curious, not saying it's necessarily wrong or anything!)

Copy link

Choose a reason for hiding this comment

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

@opacam got any input on this particular question?

Copy link
Member

Choose a reason for hiding this comment

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

That's a pretty valid concern @Jonast and definitely deserves some investigations. In the meantime this PR is already improving the current state while not bringing any regression to me.
I think we could merge and eventually address that later

Copy link

Choose a reason for hiding this comment

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

👍

' -lcrypto' + r.version

openssl_recipe = Recipe.get_recipe('openssl', self.ctx)
env['CFLAGS'] += openssl_recipe.include_flags(arch)
Copy link

Choose a reason for hiding this comment

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

That seems clever, I like it! 👍

@ghost
Copy link

ghost commented Jan 10, 2019

Edit: I previously wrote here that it doesn't work for me but my env is probably wrong and it works for others, please ignore

@ghost
Copy link

ghost commented Jan 10, 2019

I tried adding libffi as depends for both python3 and setuptools, and bumping setuptools to 40.6.3, but nothing helped, sadly. Still stuck with the same error

@misl6
Copy link
Member

misl6 commented Jan 10, 2019

Worked for me. (on top of the current master).

Just added libffi as first requirement in buildozer.
requirements = libffi, openssl, python3, kivy==master, android, cython, pyjnius, pycryptodome

@ghost
Copy link

ghost commented Jan 10, 2019

Ok, I spotted this output: [DEBUG]: checking for --with-system-ffi... yes - but still no _ctypes module. I will try enabling openssl now and see if that is connected (which I think it shouldn't be)

@ghost
Copy link

ghost commented Jan 10, 2019

--requirements I used: libffi,openssl,python3,cffi,sdl2,pyjnius,android,Pillow,/my-app/.android-build/code-lib-copy/

In the hostpython configure output I'm spotting: [DEBUG]: checking for --with-system-ffi... yes

However, I still get above error and poked around in the build folder afterwards:

# /root/.local/share/python-for-android/build/other_builds/hostpython3/desktop/hostpython3/native-build/python
Python 3.7.1 (default, Jan 10 2019, 21:28:13) 
[GCC 7.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/.local/share/python-for-android/build/other_builds/hostpython3/desktop/hostpython3/Lib/ctypes/__init__.py", line 7, in <module>
    from _ctypes import Union, Structure, Array
ModuleNotFoundError: No module named '_ctypes'
>>> import ssl
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/.local/share/python-for-android/build/other_builds/hostpython3/desktop/hostpython3/Lib/ssl.py", line 98, in <module>
    import _ssl             # if we can't import it, let the error propagate
ModuleNotFoundError: No module named '_ssl'
>>>

I don't know the exact folder layout very well, but I'd also expect _ctypes.so in here:

# ls /root/.local/share/python-for-android/build/other_builds/hostpython3/desktop/hostpython3/native-build/Modules/
Setup            _collectionsmodule.o  _math.o      _threadmodule.o  bufferedio.o  errnomodule.o   getbuildinfo.o  itertoolsmodule.o  pwdmodule.o       textio.o
Setup.local      _functoolsmodule.o    _operator.o  _tracemalloc.o   bytesio.o     faulthandler.o  getpath.o       ld_so_aix          signalmodule.o    timemodule.o
_abc.o           _iomodule.o           _sre.o       _weakref.o       config.c      fileio.o        hashtable.o     main.o             stringio.o        xxsubtype.o
_codecsmodule.o  _localemodule.o       _stat.o      atexitmodule.o   config.o      gcmodule.o      iobase.o        posixmodule.o      symtablemodule.o  zipimport.o

... and there's nothing like that in the entire folder structure:

# find /root/.local/share/python-for-android/build/other_builds/hostpython3/ | grep 'ctypes.so'
#

@ghost
Copy link

ghost commented Jan 10, 2019

Ah lol. I don't have libffi-dev on the system itself, and given this is hostpython that's probably it, right? I'll install it and try again. 🤦‍♀️

@opacam
Copy link
Member Author

opacam commented Jan 10, 2019

Yes, you need libffi-dev for hostpython3

@ghost
Copy link

ghost commented Jan 10, 2019

@opacam installed it, still not working 🤷‍♀️ I think I'll give up for today, something is missing but I don't know how to find out what it is... (probably another dependency on the host) anyway, shouldn't be a fault of this pull request since it works for @misl6

@opacam
Copy link
Member Author

opacam commented Jan 10, 2019

Can you give me your list of requeriments, the one you pass to p4a to build your app?

@opacam
Copy link
Member Author

opacam commented Jan 10, 2019

Aaa...I see, it's above...

@ghost
Copy link

ghost commented Jan 10, 2019

I just realized I installed libffi-dev in the wrong environment 😂 hang on. I am testing too many things today I'm confusing myself 🤦‍♀️ lol

@opacam
Copy link
Member Author

opacam commented Jan 10, 2019

No problem, let me know if you need help, today I'm going to bed 😴
...when those things happen it's a signal 😂
🌛

@ghost
Copy link

ghost commented Jan 10, 2019

👍 things also built for me now (with python3/non-crystax recipe) so that's good!

@opacam opacam changed the title [WIP][CORE UPDATE - PART IV] Fix cryptography+cffi [CORE UPDATE - PART IV] Fix cryptography+cffi Jan 11, 2019
Because this is how you are supposed to do it, you must use LDFLAGS for linker flags and LDLIBS (or the equivalent LOADLIBES) for the libraries
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.

3 participants