Skip to content

Remove python 2-specific hardcoded hacks from lxml recipe #1446

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

Merged
merged 1 commit into from Nov 5, 2018
Merged

Remove python 2-specific hardcoded hacks from lxml recipe #1446

merged 1 commit into from Nov 5, 2018

Conversation

ghost
Copy link

@ghost ghost commented Nov 5, 2018

This pull request removes the python 2-specific hacks from the lxml-recipe that break Python 3 support. I expect that this possibly will break the Python 2 lxml recipe build. However, to fix that, the recipe shouldn't get any Python 2 specific flags hardcoded. Instead, either the core should be fixed if possible, or if that really isn't doable, then at least the Python 2 changes should be added as conditionals.

I therefore suggest that someone with Python 2 interest (I don't have any legacy Python 2 code) checks the state of this, and that these Python 2-specific hardcoded things get removed as in this pull request.

What was tested: I applied this patch and successfully tested both lxml install, python-docx install (which uses lxml), and generating DOCX documents from scratch directly on Android. Afterwards, I opened the document with the android libreoffice viewer to verify that it looks as expected. All of this was tested with python3crystax.

@ghost ghost mentioned this pull request Nov 5, 2018
@AndreMiras
Copy link
Member

Thanks CI looks happy.
However something weird happened with the CI (https://api.travis-ci.org/v3/job/450779313/log.txt):

[INFO]:    Found multiple valid dependency orders:
[INFO]:        ['hostpython2', 'libxml2', 'libxslt', 'python2', u'lxml']
[INFO]:        ['hostpython3crystax', 'libxml2', 'libxslt', 'python3crystax', u'lxml']
[INFO]:    Using the first of these: ['hostpython2', 'libxml2', 'libxslt', 'python2', u'lxml']
[INFO]:    Trying to find a bootstrap that matches the given recipes.
...
[INFO]:    Found 4 acceptable bootstraps: ['webview', 'service_only', 'sdl2_gradle', 'pygame']
[INFO]:    Using the first of these: webview
[INFO]:    Found a single valid recipe set: ['hostpython2', 'libxml2', 'libxslt', 'python2', u'lxml']
[INFO]:    -> directory context testapps/
('recipes modified:', set([u'lxml']))
('recipes to build:', set([u'lxml']))
incompatible with python3crystax
falling back to python2
('recipes to build (no broken):', set([u'lxml']))
('requirements:', u'lxml,python2')

So well the good news is that it doesn't seem to break the build on Python2 (ping @jerome-poisson).
But the funny thing is the CI seems to get mislead in thinking it's still not python3 compatible. I'm not too sure why.

Pro-tip for next PR, you can add a fixes #1445 so it gets closed automatically on merge.

@AndreMiras AndreMiras merged commit b64b9ec into kivy:master Nov 5, 2018
@ghost ghost deleted the lxmlfix branch November 6, 2018 18:58
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.

1 participant