python-for-android icon indicating copy to clipboard operation
python-for-android copied to clipboard

Requirements parsing undefined/ambiguous behavior

Open UriyaHarpeness opened this issue 2 years ago • 7 comments

Checklist

  • [x] the issue is indeed a bug and not a support request
  • [x] issue doesn't already exist: https://github.com/kivy/python-for-android/issues
  • [x] I have a short, runnable example that reproduces the issue
  • [x] I reproduced the problem with the latest development version (p4a.branch = develop)
  • [x] I used the grave accent (aka backticks) to format code or logs when appropriated

Versions

  • Python: 3.11.5
  • OS: Ubuntu
  • Kivy: 2.2.1
  • Cython: 0.29.36
  • OpenJDK: 17

Description

I had an issue compiling my kivy application to .apk, after a lot of fiddling, I received the solution in the discord channel for the python-for-android project. (The content of the application does not matter, as the error happens for the basic application described in kivy's docs). When running the error on my device I received the error ImportError: dlopen failed: "/data/data/org.uriyaharpeness.neverfever/files/app/_python_bundle/site-packages/kivy/_clock.so" is for EM_X86_64 (62) instead of EM_AARCH64 (183), which was caused apparently because of the way I specified the requirements - using ~=.

I now know the reason for the failure, which can be seen here: https://github.com/kivy/python-for-android/blob/develop/pythonforandroid/toolchain.py#L654-L662 The parsing of the requirements supports only no constraints or exact version (==) of the packages, and not all the specifications pip supports, which I intuitively used for my requirements.

I have not seen any reference to that issue in the documentation, not a useful comment about this limitation in the buildozer.spec file. I would like to suggest a few mitigation steps, for all those theoretical poor souls who could run into the same issue:

  1. In the documentation of buildozer - specify that only no constrains or exact version are supported for the requirements.
  2. In the initial buildozer.spec file - include this limitation in the comment.
  3. In the parsing function, called upon compiling - raise an indicative exception if the above requirements are not met.

I would happily try to open a PR for all of these suggestions, but would first like to get some consensus.

"The first step in solving a problem is to recognize that it does exist." - Zig Ziglar

buildozer.spec

Command:

buildozer android debug

Spec file:

...
requirements = python3,kivy~=2.2.1
...

Logs

...
12-06 16:25:57.472 31566  8670 I python  : [ERROR  ] [Clock       ] Unable to import kivy._clock. Have you perhaps forgotten to compile kivy? Kivy contains Cython code which needs to be compiled. A missing kivy._clock often indicates the Cython code has not been compiled. Please follow the installation instructions and make sure to compile Kivy
12-06 16:25:57.473 31566  8670 I python  :  Traceback (most recent call last):
12-06 16:25:57.473 31566  8670 I python  :    File "/home/runner/work/myapp/myapp/.buildozer/android/app/main.py", line 1, in <module>
12-06 16:25:57.475 31566  8670 I python  :    File "/home/runner/work/myapp/myapp/.buildozer/android/platform/build-arm64-v8a_armeabi-v7a/build/python-installs/neverfever/arm64-v8a/kivy/clock.py", line 466, in <module>
12-06 16:25:57.476 31566  8670 I python  :  ImportError: dlopen failed: "/data/data/org.uriyaharpeness.neverfever/files/app/_python_bundle/site-packages/kivy/_clock.so" is for EM_X86_64 (62) instead of EM_AARCH64 (183)
12-06 16:25:57.476 31566  8670 I python  : Python for android ended.

UriyaHarpeness avatar Dec 06 '23 20:12 UriyaHarpeness

I think what you are proposing would be a good improvement.

What would be a better improvement would be to remove the limitations so they don't need to me documented - i.e. make p4a support ~=, <= and >= at a minimum.

(At the moment, version information is passed to the recipes via environment variables! Changing that to a more direct method might be a useful refactor too.)

Julian-O avatar Dec 06 '23 22:12 Julian-O

By supporting ~= ,<=, and >= - do you mean treating them all as ==? Or implementing a mechanism to see which versions available support the specification?

The first I think is not very good, as it doesn't behave as expected probably. And the latter would mean more complication, possibly slower runtime, and if each package has its own requirements with different version - a dependency hell like can happen in pip, resulting in spending a lot of time trying to get everything to play well together,

Also, in the recipes, I don't see a version for the packages each recipe depends on, in another sense it just says "all of these packages work with all of the other packages in the current (latest) state", or am I missing something?

UriyaHarpeness avatar Dec 06 '23 23:12 UriyaHarpeness

I definitely meant the latter. I hadn't considered the dependency hell issue - I guess I hoped it was delegated to pip to sort out.

[I pass on the second question - I don't know the answer.]

Julian-O avatar Dec 07 '23 03:12 Julian-O

Seeing as many recipes have a .patch file, which depends on the code's content (and thus, its version) - I'd assume that the statement in the second question is true.

It may not be defined that way, but if a package changes the patch may not work, so each package has theoretically one supported version at a time.

And if that's the case, perhaps even == is not truly valid...

This is getting a bit complicated I believe.

How do you suggest we take it forward @Julian-O?

On an unrelated note - do you know when my PR might be reviewed?

UriyaHarpeness avatar Dec 07 '23 06:12 UriyaHarpeness

@UriyaHarpeness : I wouldn't pin the success of this on my knowledge. I have never written a recipe.

I have idly thought before that recipes need to be totally refactored. They should be subclasses, not modules with weird, poorly documented assumptions. They should provide much more meta-data about what they are expecting, including versions that they are known to work with, versions they are assumed to work with and versions they are known not to work with.

They currently support two platforms (Darwin and Linux), but they should support more.

However, I am not expecting anything in that direction any time soon.

In the meantime, I think your original proposal would still be a step forward.

I have not got code review privileges (a state of affairs I think is very appropriate!) and I can't comment officially on when your PR might be reviewed. I know the core developer(s) are currently focussed on upcoming releases, so I would give it a couple of weeks before I started to ask for more attention.

Julian-O avatar Dec 07 '23 12:12 Julian-O

Just to be sure - original proposal meaning the 3 steps I wrote in the issue's description?

UriyaHarpeness avatar Dec 07 '23 20:12 UriyaHarpeness

Yes

If we can't handle the functionality a developer would expect, we should document it and (if we can) detect it and give a meaningful error.

Julian-O avatar Dec 08 '23 00:12 Julian-O