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

Building wheels takes a long time with .tox directories present #13

Open
nedbat opened this issue Apr 5, 2017 · 6 comments
Open

Building wheels takes a long time with .tox directories present #13

nedbat opened this issue Apr 5, 2017 · 6 comments

Comments

@nedbat
Copy link

nedbat commented Apr 5, 2017

Building the manylinux wheels for coverage.py took 1hour 45minutes. Once I removed all my .tox directories, it took 3 minutes.

The problem is that "pip wheel" copies the entire tree somewhere else first. If we use "python setup.py bdist_wheel", it skips that step. If I do that, then even with .tox directories, building wheels takes only 2 minutes.

Here's an IRC exchange about it:

[13:18:39] nedbat	I have two different ways to build wheels:  "pip wheel . -w wheelhouse" and "python setup.py bdist_wheel".  They both produce the same name wheel, with the same size.  But the first command takes 3min42sec, the second command takes 11sec.
[13:19:11] nedbat	The time for the first one seems to depends on how many files are in the entire tree, including .tox directories... What's the difference between these commands, and is there a reason to prefer the first one?
[13:25:42] dstufft	nedbat: the first command copies the files to a temporary directory, does some gross stuff to ensure you're using setuptools in your setup.py, and then invokes setup.py bdist_wheel
[13:26:13] nedbat	dstufft: any idea why it seems take forever when there are many irrelevant files in the tree?
[13:26:18] Wooble	(doesn't pip wheel also recursively build wheels for all of your deps, and bdist_wheel... not?)
[13:26:31] nedbat	Wooble: the results are the same size (though not the same sha1)
[13:26:33] dstufft	nedbat: presumably because we're copying the entire tree to a temporary directory
[13:26:42] nedbat	dstufft: oh, yikes.
[13:27:50] xafer	(cf https://github.com/pypa/pip/issues/2195)
[13:27:59] dstufft	and yea we also build wheels for all dependencies
[13:28:06] nedbat	dstufft: is there a reason i should use "pip wheel" if the other is working for me?
[13:28:07] xafer	(install/wheel, potato/potato)
[13:28:14] dstufft	but those will be seperate .wl files
[13:28:50] dstufft	nedbat: Not particularly, ``pip wheel`` is typically less used by the maintainer of Foo, and more for consumers of Foo-That-Doesnt-Have-A-Wheel-Already
[13:29:14] nedbat	dstufft: ok, good to know.
@nedbat
Copy link
Author

nedbat commented Apr 5, 2017

coverage.py has no dependencies, so the "also builds dependent wheels" part doesn't into play for me, but I would expect that people using this repo are building wheels for uploading to PyPI, so they don't want dependent wheels either.

@rmcgibbo
Copy link
Member

rmcgibbo commented Apr 5, 2017

That stinks. Can you think of any downside of just changing https://github.com/pypa/python-manylinux-demo/blob/master/travis/build-wheels.sh#L10 to use python setup.py bdist_wheel?

@nedbat
Copy link
Author

nedbat commented Apr 5, 2017

I changed my own version of this script, and it seems fine (and fast!), but I haven't distributed those wheels yet. I found that the wheels made both ways had identical names and sizes, though the sha1 sums were different, maybe just because of timestamps.

@rmcgibbo
Copy link
Member

rmcgibbo commented Apr 5, 2017

Sounds good. I'd be happy to merge a PR to that effect, if you think it's warranted.

@njsmith
Copy link
Member

njsmith commented Apr 5, 2017

In the long run we want to move towards pip wheel or similar as the standard because that will support non-setup.py-based build systems. But clearly pip will need some work to optimize it for this use case, and in the mean time you should definitely use bdist_wheel directly if it works better.

@nedbat
Copy link
Author

nedbat commented Aug 8, 2017

I'm forgetting the details of why I had to change directories, but this is how I changed this script in the coverage.py repo: nedbat/coveragepy@f2517c9

The clean step is needed or one build's files will be packaged into the next build.

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

No branches or pull requests

3 participants