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

Introduce a multi os travis build that builds OSX wheels #247

Closed
wants to merge 5 commits into from

Conversation

Bachmann1234
Copy link
Contributor

@Bachmann1234 Bachmann1234 commented Apr 30, 2017

Improvements could be made to build the manylinux wheels as well but
this is a big change on its own. When creating a tag the wheel
will pushed as a github release

So this is unlikely ready to merge in its current state but I think its at a point where some feedback would be helpful.

Example build
Example "release"

Here is me sanity testing the 2.7 wheel similarly to how the makefile does

 I   ² venv  ~/Downloads  pip install lxml-3.7.3-cp27-cp27m-macosx_10_11_x86_64.whl
Processing ./lxml-3.7.3-cp27-cp27m-macosx_10_11_x86_64.whl
Installing collected packages: lxml
Successfully installed lxml-3.7.3
 I   ² venv  ~/Downloads  python                                1014ms  Sun Apr 30 08:14:12 2017
Python 2.7.13 (default, Apr  5 2017, 22:17:22)
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import lxml.etree
>>> import lxml.objectify
>>>

and 2.6

 I   ² venv  ~/Downloads  pip install lxml-3.7.3-cp26-cp26m-macosx_10_11_x86_64.whl
DEPRECATION: Python 2.6 is no longer supported by the Python core team, please upgrade your Python. A future version of pip will drop support for Python 2.6
Processing ./lxml-3.7.3-cp26-cp26m-macosx_10_11_x86_64.whl
Installing collected packages: lxml
Successfully installed lxml-3.7.3
 I   ² venv  ~/Downloads  python                                 728ms  Sun Apr 30 08:25:55 2017
Python 2.6.9 (unknown, Apr 30 2017, 08:22:20)
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import lxml.etree
>>> import lxml.objectify
>>>

Some notes:

  • I dont bother building all the python3 builds for OSX as they will all fail the same way the allowed_failure I do build does. Filed bug here I can help with that bug but not sure what to do next on it.
  • The linux wheels that get built are likely not useful as they are not manywheels builds. I experimented with doing the manywheels here and while I think its possible I abandoned it (for now) when I hit an FTP error I had no idea how to deal with. here is a Failing build and the abandoned branch

.travis.yml Outdated
cache:
directories:
- $HOME/.cache/pip
- "$HOME/.cache/pip"
deploy:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section would have to be changed in master. It currently sends stuff to my repo which obviously wont work if merged.

Basically you update the encrypted api_key to yours (travis can encrypts with the command line client)

If you are feeling brave you can also have travis push to pypi

python -u setup.py clean
CFLAGS="-O0 -g" python -u setup.py build_ext --inplace
CFLAGS="-O0 -g" PYTHONUNBUFFERED=x make test
python setup.py bdist_wheel
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can update this to only build the wheels if the OS is osx if that makes more sense.

@@ -1 +1 @@
Cython>=0.20
Cython==0.25.2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pin is likely not strictly required. I can roll it back if desired.

@scoder
Copy link
Member

scoder commented May 1, 2017

Thanks! I'll look through it and give it a try as soon as I can.

The FTP failure is probably this one:
https://bugs.python.org/issue27973
See this commit: d0125dc

cache:
directories:
- $HOME/.cache/pip
- "$HOME/.cache/pip"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: probably a good idea to add "$HOME/.pyenv" to this

@Bachmann1234
Copy link
Contributor Author

Think I realized what I was doing wrong for manywheels. Ill update this PR assuming I did it right.

https://travis-ci.org/Bachmann1234/lxml/builds/227574774

https://github.com/Bachmann1234/lxml/tree/linux_manywheels

@Bachmann1234
Copy link
Contributor Author

Bah! So close. Gets all the way to "show wheels" and explodes due to a missing directory. I'll have that a deeper look tomorrow

@anthrotype
Copy link

I suggest you also have a look at https://github.com/matthew-brett/multibuild

It makes it super easy to build manylinux1 and osx wheels with Travis CI.

@Bachmann1234
Copy link
Contributor Author

I'll check it out. Thanks!

@Bachmann1234
Copy link
Contributor Author

I went ahead and added the linux wheels since I have a working build

https://travis-ci.org/Bachmann1234/lxml/builds/227923232

Still working out a kink... the log shows the wheels being made the the linux wheels have not uploaded.

@Bachmann1234
Copy link
Contributor Author

Solved the last problem during the commute https://github.com/Bachmann1234/lxml/releases/tag/20170502-5

I'll rebase later

@Bachmann1234
Copy link
Contributor Author

as you may have noticed im still iterating on this. Im trying to enable static osx wheels. Im doing this by fixing the download code which fails in the osx environment. Ill post the tag if the build passes (I think it will but im wrong a whole bunch)

Bachmann1234 and others added 4 commits May 4, 2017 08:21
Improvements could be made to build the manylinux wheels as well but
this is a big change on its own. When creating a tag the wheel
will pushed as a github release
I realized io is just docker linking to the repo
@Bachmann1234
Copy link
Contributor Author

Ugh, Spoke too soon. One of the downloads timed out. Ill rebase and push to see if thats a blip. Will also resolve the conflicts

@Bachmann1234
Copy link
Contributor Author

Bachmann1234 commented May 4, 2017

That time it worked
https://travis-ci.org/Bachmann1234/lxml/builds/228720176
https://github.com/Bachmann1234/lxml/releases/tag/20170504-1

Think im done fooling with this PR until you get a chance to look at it. I managed to make a python3 wheel on osx but that wheel fails the tests with "Unsupported encoding UCS-4LE"

Copy link
Member

@scoder scoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous use of ftplib was intentionally removed here: a882fd9
There were, admittedly, some issues with that, but proxy usage is a nice feature by itself.

@Bachmann1234
Copy link
Contributor Author

@scoder aw jeez. OK I guess ill have to fallback to ftplib if the http call fails

@scoder
Copy link
Member

scoder commented May 7, 2017

Looks cool. Thank you for going through all this. Let's see what happens when I merge it. :)

@Bachmann1234
Copy link
Contributor Author

No problem! Just remember to redo the depoly section with your repo. Travis has a client that should do it for you

@anthrotype
Copy link

anthrotype commented May 7, 2017

@Bachmann1234 thanks for working on this!
I see one problem though. You are using a python built with pyenv for building the osx wheels. This is not good, because since that python has been built with pyenv for that target machine (10.11 sdk, 64 bit architecture), the wheels generated from it are also going to work only on that platform (or above). It is recommended when building wheels for mac os x, to use the Python.org installers for OSX which are compiled as "fat" binaries (both 32 and 64 bit) and using the 10.6 as minimum deployment target. This ensures that the produced wheels will work anywere, and not only on latest OS versions, or with 64-bit-only python builds.

The multibuild tools that I recommended above uses the Python.org's builds:
https://github.com/matthew-brett/multibuild/blob/master/osx_utils.sh

All the osx wheels for the python scientific packages are being built using multibuild now, e.g. see https://github.com/MacPython

@Bachmann1234
Copy link
Contributor Author

@anthrotype Alrighty, ill take a look sometime next week most likey!

@anthrotype
Copy link

Oh, I just found this: https://github.com/joerick/cibuildwheel
I haven't tried, but looks even easier than multibuild to set up.
It seems to be related to multibuild, as they mention its author Matthew Brett in the credits section.

@Bachmann1234
Copy link
Contributor Author

Hmm. I've been hesitant to touch the windows stuff cuse the scope of this work had already grown. But what the heck, I'll give it a shot. If this works the whole system will be simpler

@Bachmann1234
Copy link
Contributor Author

That will be a different branch/pr

@Bachmann1234
Copy link
Contributor Author

Welp. My laptop decided to give out on me so I won't be able to look at that fancy build until I get that sorted. Probably at least a week

@Bachmann1234
Copy link
Contributor Author

Ugh, looks like its gonna be more like 2 weeks.

@Bachmann1234
Copy link
Contributor Author

Ok, im back. Let me take a look at this thing @anthrotype posted

@Bachmann1234
Copy link
Contributor Author

Ok, I played with both I think using multibuild is worth exploring. I'm trying that out here https://github.com/Bachmann1234/lxml-wheels

Its not anywhere near done. But ill keep playing with it.

@scoder is this an approach you would be ok with? As soon as I got this working I could hand the repo over. The other option would be to keep hacking away at this one and fix the issues @anthrotype mentioned.

@scoder
Copy link
Member

scoder commented May 19, 2017

Seems much better, if only because it does not depend on specific CPython version numbers. The git subrepo also seems (not great but) ok. Updating the sub-repo link for each release seems doable. And I like the fact that it's a separate repo with a separate travis.yml file, so that the normal lxml repo can continue to do simple CI test builds.

@Bachmann1234
Copy link
Contributor Author

Ok, think I got this working.

https://github.com/Bachmann1234/lxml-wheels/releases/tag/wheel-test-4
https://travis-ci.org/Bachmann1234/lxml-wheels/builds/234419129

This should be fairly easy to use. We just update the BUILD_COMMIT for each release in .travis.yml

Some notes:

  • I have not done windows yet but that should be fairly easy (and you probably dont need it since you have that automated... but eh, one repo for all the wheels could be nice)
  • Im building python 3 wheels of OSX even though https://bugs.launchpad.net/lxml/+bug/1687236 suggest these wheels wont quite work (though neither will pulling from pypi and building it yourself so I guess its still an improvement)
  • No python 2.6, 3.3 mac wheels. I filed https://github.com/matthew-brett/multibuild/issues/30

@Bachmann1234
Copy link
Contributor Author

Should I close this and we continue talking in the bug form? What are next steps?

@Bachmann1234
Copy link
Contributor Author

Bachmann1234 commented May 31, 2017

Just an update. I have 3.3 mac wheels. But I wont be doing 2.6 mac wheels since to do so I would have to avoid ssl validation by downgrading pip (pypa/pip#829) which I don't see as worth it (security concern for an ancient python version and users who really want that can build their own wheel)

I dont think I have anything else to do. Just poke me if there is a next step. Repo is here. https://github.com/Bachmann1234/lxml-wheels

if there is interest in using this repo ill write some notes in the readme

@scoder
Copy link
Member

scoder commented Jun 1, 2017

Sorry for being slow in responding and thank you for your continued investment into this. I'll give it another try this weekend. Still have to get the travis upload set up on my side.

@scoder
Copy link
Member

scoder commented Jun 12, 2017

I forked your repo into the lxml github organisatiion.
https://github.com/lxml/lxml-wheels
It nicely builds your lxml branch on travis:
https://travis-ci.org/lxml/lxml-wheels/builds/241921367
But master fails for 2.7:
https://travis-ci.org/lxml/lxml-wheels/builds/241999467
So I think it's missing that FTP fix. Could you maybe extract that as a separate pull request?
I uploaded the wheels it built, let's hope they work for everyone.
https://pypi.python.org/pypi/lxml/3.8.0
Thank you for your efforts!

@Bachmann1234
Copy link
Contributor Author

Bachmann1234 commented Jun 12, 2017 via email

@scoder
Copy link
Member

scoder commented Jun 12, 2017

Ah, sorry, had forgotten about that one. There's always a next release ;)

@scoder scoder closed this Jun 12, 2017
@Bachmann1234
Copy link
Contributor Author

👍 Thanks. Let me know if there are any issues :-D

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