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

crystaxpython with openssl support #707

Closed
wants to merge 1 commit into from

Conversation

emillynge
Copy link

Fix to #705

This PR makes crystax-python recompile with _ssl.so module if openssl is in the build order.

tested for arm64-v8a

There is a bug that I don't know how to fix for armeabi-v7a similar to this issue:
openframeworks/openFrameworks#4171

Building openssl shared lobraries with the crystax ndk script build-target-openssl.sh however, instead of copying over the ones from the openssl recipe, solves this problem. I'm not nearly good enough at examing make files to know what the difference between the produced .so files are. But crystax produces slightly smaller .so files, and uses make install rather than make build_libs.

Maybe the recipe should ignore recompilation if armeabi-v7a and throw a warning that ssl.py will not work until such time that this bug is fixed.

I could also make the openssl recipe use the crystax build script, but then it's hard to have proper bookeeping on what has been built, and what is missing :-/
Additionally I have no idea whether the crystax script produces openssl libs that will work on the phone, or whether they just serve to be linked against by the python compilation...

NB: I had to implement a realpath function as os.path.realpath is buggy. maybe that func should go into utils...

Regards
Emil

@inclement
Copy link
Member

Nice. I've a small concern about the build directories - where are python and openssl built? It would be best to do it in p4a's build dirs.

I'm also not sure about some of the code structure, but if it works then it will be great to make some changes and merge it. Maybe the openssl build should really go in the openssl recipe (or a new one, but it can just use an if statement) and the python recipe should have a more generic python build option rather than just copying the crystax libs.

@emillynge
Copy link
Author

emillynge commented Apr 15, 2016

In this implementation the crystax makes it's own temporary builddir, and relevant files are extracted into p4a after build is complete.

When using the crystax build scripts, it is possible to specify a build dir. This could be pointed to the p4a build dir I guess. This would be a rewrite of openssl recipe which would be very different if the crystax ndk is used.

...python recipe should have a more generic python build option rather than just copying the crystax libs

Not sure what you mean?

@inclement
Copy link
Member

When using the crystax build scripts, it is possible to specify a build dir.

If it uses a relative directory by default, the command can just be called from with current_directory(self.get_build_dir(...)).

This would be a rewrite of openssl recipe which would be very different if the crystax ndk is used.

This would be an alternative, not a replacement of the existing recipe.

Not sure what you mean?

The python3crystax recipe should have the option to compile python, regardless of any of these concerns.

@emillynge
Copy link
Author

Quite a lot of the dirs and options are set via the dev-defaults.sh script which makes it very hard to tweak anything other than the options that is explicitly set at input arguments :/

The program description states:

Rebuild python libraries for the CrystaX NDK.

This requires a temporary NDK installation containing
toolchain binaries for all target architectures.

By default, this will try with the current NDK directory, unless
you use the --ndk-dir= option.

The output will be placed in appropriate sub-directories of
/$PYTHON_SUBDIR, but you can override this with the --out-dir=
option.

So i think it's easiest just to use the "--out-dir" option

Thing is, that I cannot make the build scripts look for the openssl libraries any other place than in the NDK itself :(
As seen by this snippet from build-target-python.sh

OPENSSL_HOME=''
if [ -n "$DEFAULT_OPENSSL_VERSION" ]; then
echo "version found: $DEFAULT_OPENSSL_VERSION"
if [ -f "$NDK_DIR/$OPENSSL_SUBDIR/$DEFAULT_OPENSSL_VERSION/Android.mk"
-a -f "$NDK_DIR/$OPENSSL_SUBDIR/$DEFAULT_OPENSSL_VERSION/include/openssl/opensslconf.h" ]; then
echo "home found"
OPENSSL_HOME="openssl/$DEFAULT_OPENSSL_VERSION"
fi
fi

DEFAULT_OPENSSL_VERSION and OPENSSL_SUBDIR is set by dev-defaults.sh

This would be an alternative, not a replacement of the existing recipe.

Ok, so i can just make an openssl-crystax recipe? Although I guess some dependencies should be fixed to accept either (openssl, openssl-crystax)

The python3crystax recipe should have the option to compile python, regardless of any of these concerns.

Can I tap into the --force-build argument from the recipe methods?

@inclement
Copy link
Member

I see what you mean about the crystax build scripts, but let's just not worry about it.

Feel free to leave it for now, I'll take a proper look at the changes later.

As an aside, an alternative to all of this would be to build openssl manually (ignoring the crystax scripts), as is done with python2. This would avoid all these problems, but would be more complexI'm not suggesting you should do it, just mentioning the possibility.

Feel free to leave it for now, I'll take a proper look at the changes later.

@FeralBytes
Copy link
Contributor

So when I first run this with a clean P4A; I get a traceback; because it appears to have a dependency on the original Python3Crystax recipe having been run once.

Traceback (most recent call last):
  File "setup_SDL2_crystax_py3.py", line 82, in <module>
    package_data=package_data
  File "/usr/lib/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/usr/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/bdistapk.py", line 83, in run
    main()
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/toolchain.py", line 900, in main
    ToolchainCL()
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/toolchain.py", line 508, in __init__
    getattr(self, args.subparser_name.replace('-', '_'))(args)
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/toolchain.py", line 147, in wrapper_func
    build_dist_from_args(ctx, dist, args)
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/toolchain.py", line 190, in build_dist_from_args
    build_recipes(build_order, python_modules, ctx)
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/build.py", line 550, in build_recipes
    recipe.prepare_build_dir(arch.arch)
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/recipe.py", line 550, in prepare_build_dir
    self.unpack(arch)
  File "/usr/local/lib/python2.7/dist-packages/pythonforandroid/recipe.py", line 472, in unpack
    .format(extraction_filename))
Exception: Given path is neither a file nor a directory: /home/wolfrage/.local/share/python-for-android/packages/python3crystax/master.tar.gz

@professor-python
Copy link

Some info about this feature please. Tell me if I right when using p4a with buildozer. CrystaxNdk is downloaded as binaries (10.3.2 now). All works without ssl. And now we just set p4a.source_dir to p4a source with fixed python3crystax recipe (?). Need I download CrystaxNdk sources for this recipe if it will try to compile it?

@inclement
Copy link
Member

@professor-python I'm not sure I understand the question - are you trying to test this PR?

@professor-python
Copy link

@inclement yes, I'm trying to add openssl support with crystax by this PR

@ecdsa
Copy link

ecdsa commented Aug 25, 2017

@emillynge could you please update your PR?

@emillynge
Copy link
Author

@ecdsa update how? as in rebase?

I am no longer actively using this project, so I'n not sure whether I am at all capable of rebasing without causing problems :/ I can barely remember what I did at all.

@ecdsa
Copy link

ecdsa commented Aug 30, 2017

yes, make it compatible with current p4a
I had to copy .so files manually in order to compile an APK with python3 and SSL

@fyookball
Copy link

I am also having the same problem as @ecdsa. I see that @frmdstryr put some changes on their branch but I get the same error.

@ghost
Copy link

ghost commented Jan 13, 2019

@AndreMiras since I assume crystax will be left behind now, maybe it would make sense to close this?

@emillynge thanks so much for your work! We got openssl/TLS working on p4a master with "python3" (no crystax!) now so this is probably no longer required, but it helped a lot as an intermediate solution

@tshirtman tshirtman closed this Jan 13, 2019
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.

7 participants