-
Notifications
You must be signed in to change notification settings - Fork 3k
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
pip install of a directory is super slow #2195
Comments
It is making a copy of the entire directory, including |
|
I tried passing 3 |
Stepping through it with pdb, things slow down when I get to:
|
And yep, @tomprince is right - it slows down when it does a copy of the whole tree:
|
|
Some discussion in #2196 |
by ignoring .tox, .git, .hg, .bzr, and .svn when doing `shutil.copytree` in unpack_file_url in pip/download.py. Fixes: pypaGH-2195
It's much faster now that #2196 is merged. |
This should be reopened since #2196 was reverted. I'd like to come with an alternative PR that builds an sdist instead of using heuristics to figure out what to copy. See the comments on that PR for details. |
Yikes, almost 3 minutes. Probably mostly due to this:
The |
Right now it's just a pretty simple `shutil.copytree`, but ideally we want it to do something more complex, involving building an sdist. Plus, this makes `unpack_file_url` fit on a single screen without scrolling for me. :-) See pypa#2195
E.g.: `pip install /path/to/dir` by building an sdist and then unpacking it instead of doing `shutil.copytree`. `shutil.copytree` may copy files that aren't included in the sdist, which means that: 1. If those files are large, a `pip install` could take much longer than it should. 2. Folks that are using `python setup.py install` to test their package are not fully testing it, because it may copy over files that wouldn't be there if they built and sdist and installed that. So this method building an sdist is both more accurate and faster. Fixes pypa#2195
Please see #2535, which speeds up |
This issue should be reopened, because the merged PR did nothing (see gh-3219). |
This is a follow-up to pypagh-2535, which added the code to copy via (sdist + unpack) instead of shutil.copytree, but forgot to actually call that function. Fixes pypagh-2195 (slow pip install of a dir).
This works around a problem with the new sdist-based "pip install .": * when creating the sdist, we don't run a literal "setup.py sdist" * instead, sys.argv[0] is a complicated shim that injects setuptools even into distutils-based projects * as a result, distutils.command.sdist.add_defaults() doesn't realize that "setup.py" is the name of its setup script (it gets confused because sys.argv[0] is not a real file). * so add_defaults() doesn't include setup.py in the generated tarball. (projects could add "include setup.py" to their MANIFEST.in, but this is not common practice because usually it's automatic) * so the unpacked sdist (from which pip will make a wheel) lacks the critical setup.py This copies the setup.py from source tree to unpacked target tree. The patch also removes a performance comment that was obsoleted by switching to _copy_dist_from_dir(). refs pypa#2195, pypa#2535, pypa#3176
This should be resolved by #7882 (build local directories in place). |
We have now (per #7951) published a beta release of pip, pip 20.1b1. This release includes #7882, which implemented a solution for this issue. I hope participants in this issue will help us by testing the beta and checking for new bugs. We'd like to identify and iron out any potential issues before the main 20.1 release on Tuesday. I also welcome positive feedback along the lines of "yay, it works better now!" as well, since the issue tracker is usually full of "issues". :) |
I will say that it is considerably better. Old: New: |
Works great/faster for me! 👍 |
down from ~2 minutes to 4 seconds, thank you so much! |
Thank you for the positive reports @PythonCoderAS @astrofrog @klamann! :) Unfortunately, there have been a number of issues with the implementation of in-place builds (which are being tracked under #7555) which means that for now, we need to revert #7882. As a result, this issue will become a problem again, and we'll therefore be reopening it. Longer-term, we hope to have a solution that addresses the issues that in-place builds solved, but without the impact on other workflows that the current solution had. Sorry for the disruption that this will cause. |
@pradyunsg thanks for the update. Some feedback on terminology (please feel free to ignore, just FYI): this sentence, as well as gh-7555, confused me because pip does not do in-place builds. What in-place builds has always meant is Here you changed the meaning to: "build without copying to a tmpdir". Extension modules still don't end up in-place, they end up in a |
That was originally my wording. Sorry for any confusion, I wasn't aware that setuptools used the term "in place" to mean something different (and I'm still not really sure how that terminology applies outside of setuptools). We'll see if we can find a more neutral term in future (although offhand, I'm not sure what - suggestions gratefully accepted 😉) |
No worries at all, thanks @pfmoore. I just thought I'd point it out, since confusion about terminology can sometimes result in talking past each other.
For tools like CMake and scikit-build I think it means the same thing: actually in-place, binaries land next to sources. "editable installs" on the other hand is (I believe) invented here, and kinda means "in-place that pip is aware of".
maybe just "local build" (vs. the current "copy to tmpdir and build")? |
We recently had a long discussion on what editable install means, and I think we actually landed in a place that is more along the lines of |
Could try «in-tree build» (similar to «in-tree PEP 517 backend») or «build in source dir» |
My question is, why can't the feature be optional, so it does not cause problems but can be enabled by an argument or something similar? |
I'm trying to wrap my head around the workarounds for this, where an editable install isn't an option. Is there any? |
A workaround could be to build a wheel (using your build backend directly) then point pip to install it |
It can. The reason for reverting the change was that we didn't have any opt-outs or a period for getting feedback on the change. We do have new flags to help facilitate that (--use-feature and --deprecated-feature), but someone has to reimplement/reintroduce the functionality in this context now. Broadly, I think what we want to do here is:
|
I was thinking without an extra build step. But I guess I should have never thought Python could get away without Makefile equivalents from the beginning. |
FYI for those who are running into this issue -- A workaround is to replace
|
setup.py was changed in 200625-8ea3fd9 to explicitly specify the packages to be installed rather then using find_packages() in an attempt to diagnose what turned out to be a setuptools problem with excessively slow installs (pypa/pip#2195). Didn't realize subpackages also have to be named explicitly.
Now that |
Yes. |
See #2195 (comment), for a summary of this issue.
I am dubious of why pip needs 17 seconds to process a local directory that is not on NFS (in fact, it's on an SSD drive) for pip, which has no dependencies, since everything is vendored.
It should probably at least be logging whatever is taking that long, but maybe it shouldn't even be doing whatever it's doing.
Note that the "Processing" line appears right away and pretty much the whole delay seems to be between that line and the next one.
The text was updated successfully, but these errors were encountered: