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

Add support for ucrt binaries on Windows #196

Merged
merged 2 commits into from
Jul 22, 2016
Merged

Conversation

mhils
Copy link
Contributor

@mhils mhils commented Jun 28, 2016

Hi,

This PR is a first big step towards resolving #1326096. I went through the pain to recompile libiconv, libxml2 and libxslt with Visual Studio 2015/ucrt to have binaries that can be used to build a Python 3.5 wheel.

This PR makes sure that the ucrt binaries are downloaded when we are on Python 3.5. I documented the actual compilation of the binaries in a reproducible manner at https://github.com/mhils/libxml2-win-binaries. After merging this PR and installing the Visual C++ Build Tools (or Visual Studio), a Python 3.5 x86 Windows wheel can be build as follows:

> "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"
> python3 setup.py bdist_wheel --static-deps

I sucessfully tested the resulting wheel on a clean Win7 VM which worked fine.

If I could ask for a favor, it would be great if you could upload a Python 3.5 Windows wheel to PyPi as soon as possible (feel free to take the wheel linked above or compile your own). We're currently migrating @mitmproxy to Python 3 and lxml is currently the only dependency that holds a pip3 install mitmproxy back.

Thanks!
Max

@scoder
Copy link
Member

scoder commented Jun 28, 2016

Are you planning to maintain these library builds in the future?

@mhils
Copy link
Contributor Author

mhils commented Jun 28, 2016

I'm happy to maintain them for now.
(Besides, the last update to the zlatkovic binaries was in 2011)

@scoder
Copy link
Member

scoder commented Jun 28, 2016

Ok, then we'd need a way to let the download script find the latest versions on your site.

@mhils
Copy link
Contributor Author

mhils commented Jun 28, 2016

Is there any issue with pinning the version? I'm happy to update this here whenever I update the binaries.

@scoder
Copy link
Member

scoder commented Jun 28, 2016

That means I have to merge your pull request and release a new version each time you update the library binaries. It also means that users can't just rebuild their existing packages to update their library versions but have to change the setup script first, or upgrade their lxml version or whatever.

That's way too much manual interaction on all sides.

@mhils
Copy link
Contributor Author

mhils commented Jun 28, 2016

I am surprised that you don't prefer pinned/deterministic builds, but ok. I updated the PR. 😃

@mhils
Copy link
Contributor Author

mhils commented Jun 30, 2016

Is there anything else you want to discuss/change before this can be merged? ☺️

@pfmoore
Copy link

pfmoore commented Jul 4, 2016

Are your library builds 32-bit only? I tried to build on 64-bit Python 3.5, and got a lot of unresolved external errors...

@mhils
Copy link
Contributor Author

mhils commented Jul 4, 2016

@pfmoore: I only built x86 binaries so far to get the common case working. Adjusting https://github.com/mhils/libxml2-win-binaries/blob/master/build.ps1 to build x64 binaries should be relatively straightforward though.

@pfmoore
Copy link

pfmoore commented Jul 4, 2016

@mhils Cool. I did have a quick try at doing a 64-bit build, but got some odd results and didn't have time to dig into what was wrong (a $lib variable was null when it was expected to have a value).

@pfmoore
Copy link

pfmoore commented Jul 4, 2016

I assume that this PR will need to be updated, whether (ideally) to handle 64-bits or at least to give a better error report explaining what's wrong when run with a 64-bit Python.

@mhils
Copy link
Contributor Author

mhils commented Jul 4, 2016

@pfmoore: You are most likely seeing errors because the dist folders are different for x64 - not much of a big deal, just makes the build script a bit messy.

As I understand it, the current version of download_and_extract_zlatkovic_binaries does not work for x64 either. I can add some checks if @scoder wants that, I just wanted this to be as minimally invasive as possible in the first place.

@pfmoore
Copy link

pfmoore commented Jul 4, 2016

OK, that's fair. As I only use 64-bit Python, I guess I'll wait for now (maybe I'll get some time to spend on this and then I'll look into 64-bit if you haven't already).

benoit-pierre pushed a commit to benoit-pierre/lxml that referenced this pull request Jul 19, 2016
benoit-pierre pushed a commit to benoit-pierre/lxml that referenced this pull request Jul 19, 2016
@scoder
Copy link
Member

scoder commented Jul 22, 2016

Thanks! The 64 bits issue doesn't seem worse than what we have.

@scoder scoder merged commit 37da3dc into lxml:master Jul 22, 2016
@mhils mhils deleted the py35-wheels branch July 22, 2016 20:47
@mhils
Copy link
Contributor Author

mhils commented Jul 22, 2016

Thanks! 😊

I just updated libxslt to 1.1.29 as per mhils/libxml2-win-binaries#1, so everything should be up-to-date now. Can we have lxml-3.6.0-cp35-cp35m-win32.whl (sig, feel free to build yourself if you prefer that) on PyPI?

@scoder
Copy link
Member

scoder commented Jul 24, 2016

Looks like you provided an HMAC signature. PyPI wants a PGP signature instead.

@mhils
Copy link
Contributor Author

mhils commented Jul 24, 2016

Signatures on PyPI are not verified by pip, so there's very little value in that. This was for you to personally blame me if there should be any issues with the binary 😉

@rancyd
Copy link

rancyd commented Dec 27, 2016

@mhils Thank you for this.

Is there any more work to be done? It seems 64bit build still doesn't work. I would be more than willing to help out.

@mhils
Copy link
Contributor Author

mhils commented Dec 27, 2016

@rancyd: #207 introduced 64 bit support and there's a 64 bit wheel for Python 3.5 on PyPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants