-
Notifications
You must be signed in to change notification settings - Fork 503
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
Dependency specification and testing should be cleaned up. #1685
Comments
I have realised that Cython is actually not directly needed by Buildozer, but only by p4a and kivy-ios. In an ideal world, the requirement should be moved into them, and they should be pip installed, simplifying Buildozer's task. |
This would be great, a significant portion of user questions are new users who watch a YouTube video then in Buildozer crash and burn in some obscure way because they didn't look at the install instructions. Trapping these errors at the start would greatly improve the user experience. This has to be in p4a, as these are p4a dependencies not Buildozer dependencies. Tests in Buildozer will produce false results as the Buildozer version and the p4a version are not related. Also most of the dependencies are not pip dependencies, they are system libraries. These should ideally be tested without requiring another Python package (e.g. python-apt), adding this would break every existing installation. There is apparently some variability in system library names between Linux flavors. We don't document this, but any testing would have to take this into account. Automatic installation would require sudo, which is the opposite of what is required for an apk build on most platforms. That said, this would be great. |
@RobertFlatt: There is some discussion in #1686 that may be helpful, and a promise of further discussion. Note sure how adding
Ugh! That'll be a nightmare.
Sure. Hopefully we can limit it to just the apt-get/homebrew portions. [I am not aware if the warning against running as root is a sensible precaution against overwriting important files, or it leads to known issues. If the latter, we should put that in a comment in |
It would become a required dependency, which (presumably) nobody currently has installed.
Just some research that nobody has bothered to document. These projects are not famous for the quality of their documentation.
There are three reasons not to run as root.
|
One more for the list, Java version |
One more for the list: test cwd is not on an NTFS partition. I've just seen a new twist on this, with the latest version of git reporting this if the project is on an NTFS partition
Git presumably thinks it is on windows :( |
@RobertFlatt: I know that the Linux Android SDK doesn't like being installed on NTFS (symbolic links break) and now Git complains, so such a check makes sense if platform is linux/macos. However, if the platform is win32 (I know, I know; not yet supported - p4a is the big bottleneck), the Windows versions of the SDK and Git are fine with NTFS. |
@Julian-O The common case we see is WSL, and setting The issue with the latest Git on NTFS (and also with building some Python packages on NTFS) is they see they are on NTFS and choose to operate as if they are under the Windows OS, despite being executed in a Linux environment. This combination results in obscure fails. It not the currently supported host OS that is the issue, it is the formatting of the disk containing the directory that is the So for the current OS an NTFS test would be very helpful. Separately, when considering adding a Windows (not WSL) port of p4a, this is also an NTFS port. The question becomes what OS related issues will there be? I hope for your sake not many. In the specific case of Git, one could argue the port will be OK (it is the OS combination of a built app that is the issue)- we'll know when you test it. The recipes are a separate issue, and again we'll know when you test. Exactly what the issues for a Windows port of p4a will be would be foolish to predict. But it is normal to expect to have to address such issues when porting. Any need for the above NTFS test for a Windows (not WSL) port of p4a would imply the project is dead in the water, so the above test would not be applied. Which does raise the question how does one check for Windows OS without checking the disk format? I expect there is an answer somewhere. |
@RobertFlatt - short version: I completely agree - that is all consistent with my understanding.
Right. I think it would a great idea to reduce support requests to have a check that says:
I have been working to make Buildozer itself platform-independent, and reacted to this suggestion to make sure we don't go backwards.
Fully expecting this to be a nightmare. I have been focussed on Buildozer to date, but p4a is much scarier. Just did an info dump in #1651 if you want to know more about where I am at there.
|
@Julian-O |
This is a review of the way Buildozer's (pre-run) dependencies are set-up, and how that set-up is tested.
I hope to follow up with a PR shortly.
Ideally:
extras_require
. They will only be installed for users who want that functionality, and allow those users to install all dependencies with relative ease.The current system falls short of this ideal in the following ways:
Buildozer does not specify the version of Cython.
Cython
for now, as most of the recipes are incompatible withCython==3.x.x
#1637setup.py
, so anyone who doesn't pre-install Cython still gets the right version.Buildozer does not specify that it needs
pytest
to run the unit-tests in the/tests
directory.pytest
is currently named as a dependencytox.ini
, but you can run unit-tests without tox. The dependency should be added to extras_require for "tests" insetup.py
.coverage
is also installed by tox, but it is not required to run the tests, so need not be mentioned insetup.py
.The Android integration test installs
automake
. This is not required for Android. It should be removed to both save time and to show it is not required.The Android integration messes with
ssl
links on MacOs:This is not documented and not required. Notably, it doesn't appear in the ios version. It should be removed.
setup.py
unnecessarily requiresvirtualenv
.venv
has been part of Python since 3.3, and we no longer support earlier versions than that.virtualenv
, already usingpython3 -m venv
#1515.)pip install virtualenv
. That instruction should be removed.setup.py
unnecessarily requiressh
.kivy-ios
needs it. So, the dependency should be fromkivy-ios
, not Buildozer. Unexpectedly,kivy-ios
does listsh
in its requires. It turns out Buildozer's TargetIos attempts to installkivy-ios
from Github, but doesn't actually install it (with pip). It just copies the files across. The dependencies are never resolved.setup.py
. Replace it with anextras_require
for ios, that installskivy-ios
. That will install its dependencies, such as sh.kivy-ios
. That instruction should be replaced with installingbuildozer[ios]
, and let the system work out the individual dependencies.The ios integration test unnecessarily installs
cookiecutter
andpbxproject
.Buildozer does not specify that it needs
sphinx
to generate the documents in the/docs
directory.extra_requires
fordocs
.Tox doesn't need to special-case Python 3 any more.
The tests shouldn't install Buildozer as an "editable install" (i.e. the
-e
option onpip install
)The text was updated successfully, but these errors were encountered: