Feature/pipignore #4900
Feature/pipignore #4900
Conversation
``` (ve) thinkie merzky ~/projects/pip [master *] $ rm -rf data/ (ve) thinkie merzky ~/projects/pip [master *] $ unset PIP_IGNORE (ve) thinkie merzky ~/projects/pip [master *] $ rm -f .pipignore (ve) thinkie merzky ~/projects/pip [master *] $ time pip install --no-deps --upgrade . Looking in indexes: https://pypi.python.org/simple, https://packagecloud.io/github/git-lfs/pypi/simple Processing /home/merzky/projects/pip Installing collected packages: pip Found existing installation: pip 10.0.0.dev0 Uninstalling pip-10.0.0.dev0: Successfully uninstalled pip-10.0.0.dev0 Running setup.py install for pip ... done Successfully installed pip-10.0.0.dev0 r:0m3.421s u:0m2.828s s:0m0.604s (ve) thinkie merzky ~/projects/pip [master *] $ mkdir data (ve) thinkie merzky ~/projects/pip [master *] $ dd if=/dev/urandom of=data/test.dat count=$((2048*1024)) 2097152+0 records in 2097152+0 records out 1073741824 bytes (1.1 GB, 1.0 GiB) copied, 15.17 s, 70.8 MB/s (ve) thinkie merzky ~/projects/pip [master *] $ time pip install --no-deps --upgrade . Looking in indexes: https://pypi.python.org/simple, https://packagecloud.io/github/git-lfs/pypi/simple Processing /home/merzky/projects/pip Installing collected packages: pip Found existing installation: pip 10.0.0.dev0 Uninstalling pip-10.0.0.dev0: Successfully uninstalled pip-10.0.0.dev0 Running setup.py install for pip ... done Successfully installed pip-10.0.0.dev0 r:0m41.687s u:0m3.420s s:0m3.320s (ve) thinkie merzky ~/projects/pip [master *] $ export PIP_IGNORE=.pipignore (ve) thinkie merzky ~/projects/pip [master *] $ cat > .pipignore # and this data .pipignore # because, why not (ve) thinkie merzky ~/projects/pip [master *] $ time pip install --no-deps --upgrade . Looking in indexes: https://pypi.python.org/simple, https://packagecloud.io/github/git-lfs/pypi/simple Processing /home/merzky/projects/pip Installing collected packages: pip Found existing installation: pip 10.0.0.dev0 Uninstalling pip-10.0.0.dev0: Successfully uninstalled pip-10.0.0.dev0 Running setup.py install for pip ... done Successfully installed pip-10.0.0.dev0 r:0m3.495s u:0m2.780s s:0m0.664s ```
|
Aargh. @pradyunsg Agreed #4764 is a better resolution. I'd mistakenly recalled that as being linked with the PEP 517 stuff, which it isn't. @andre-merzky Apologies for missing this, I may have caused you to waste some time here. Perhaps you could test #4764 and confirm it would resolve your issue? |
|
To be clear, I am not sure if #4764 does or does not resolve this but I think that general approach of building a wheel in-place might be what we want to do here. |
|
@pfmoore : no worries, I worked on this patch before you prompted me for it (realized that just complaining is no good... *blush*) If #4764 resolves this issue and gets merged, I'm a happy camper. I'll give it a spin with our modules... I am in no position to argue what approach is better, since I am not familiar with the pip code base etc., at all. @pradyunsg: I am not sure a disabling-flag would make sense, at this is disabled by default, ie. nothing happens if the env var is not set. Or are you asking for a flag to disable the code path completely? |
|
I might be missing something, but #4764 seems not to resolve the issue for me: thinkie merzky ~/projects/pypa.pip [master] $ git checkout pr/4764
[...]
thinkie merzky ~/projects/pypa.pip 130 [pr/4764] $ git h | head -n 1
* 9b6f1d78 7 hours ago Pradyun Gedam (HEAD -> pr/4764, origin/pr/4764) Merge branch 'master' into cache/ephem-wheel-cache
thinkie merzky ~/projects/pypa.pip [pr/4764] $ virtualenv ve
[...]
thinkie merzky ~/projects/pypa.pip 1 [pr/4764] $ source ve/bin/activate
(ve) thinkie merzky ~/projects/pypa.pip [pr/4764] $ pip -V
pip 9.0.1 from /home/merzky/projects/pypa.pip/ve/local/lib/python2.7/site-packages (python 2.7)
(ve) thinkie merzky ~/projects/pypa.pip [pr/4764] $ time pip install --no-deps --upgrade .
[...]
Successfully installed pip-10.0.0.dev0
r:0m5.062s u:0m3.452s s:0m0.980s
(ve) thinkie merzky ~/projects/pypa.pip [pr/4764] $ pip -V
pip 10.0.0.dev0 from /home/merzky/projects/pypa.pip/ve/local/lib/python2.7/site-packages/pip (python 2.7)
(ve) thinkie merzky ~/projects/pypa.pip [pr/4764] $ time pip install --no-deps --upgrade .
[...]
Successfully installed pip-10.0.0.dev0
r:0m8.556s u:0m5.376s s:0m1.468s
(ve) thinkie merzky ~/projects/pypa.pip [pr/4764] $ mv ../pip/data/ . ; du -sh data/
1.1G data/
(ve) thinkie merzky ~/projects/pypa.pip [pr/4764] $ time pip install --no-deps --upgrade .
[...]
Successfully installed pip-10.0.0.dev0
r:0m31.282s u:0m5.968s s:0m3.856sAny suggestion what I should be looking at? |
|
ping - any suggestion on where I should start looking? Thanks! |
Aha. Turns out I tripped on the same wire twice. @pfmoore and I have had this exact same discussion on #4764 and @pfmoore had pointed out that this is not the case because we're still copying to the temporary build directory. I am honestly not sure what the approach should be. The things that come to mind are this idea and allowing/doing in-place builds for directories (which avoid copying)... I'm genuinely not sure which one I dislike less than the other. :P |
As I mentioned, I'm not sure how to move forward here. There's no obvious path so, I'd like some inputs from other @pypa/pip-committers. If it comes to that, I currently think that in-place builds are sorta fine for this. |
pradyunsg
added
state: needs discussion
|
This is fixed by PEP 517, essentially by letting the backend choose whether to do in-place builds or builds from sdist. The discussion on this point was pretty extensive, but the final conclusion was fundamentally that it's not the frontend's place to decide how the wheel gets built, but we do expect backends to handle it correctly (i.e., without issues like #2195). The immediate problem here is in pip's code that essentially implements a setuptools backend, and that code will be either completely ripped out or significantly rewritten as part of implementing PEP 517. As for the pipignore feature, while it's pretty simple, I'm not sure it'll have any meaning in a post-PEP 517 world, so my impression is that it'll end up being something that gets added and then taken away, probably all before we get a pip release. If the feature could be described (and implemented) in a way that made sense in the context of PEP 517, I'd be happier with it. As it stands, assuming this PR gets proper documentation and tests, I'd see it as something we should put on hold with the intention of rejecting it when PEP 517 is implemented, or merging it as a workaround if we decide to do a release without PEP 517 included. I'm not sure whether @andre-merzky would be keen on putting extra work into a PR that's likely not to be implemented, and I fully understand that. The uncertainty about PEP 517 and the next release puts us in a less than ideal place at the moment, which I can only apologise for... My personal feeling is that people have lived with #2195 for long enough now that waiting a little longer for PEP 517 should be fine. |
Let me turn this around: if there happens to emerge real interest in that approach, I would not mind picking this up and add the remaining pieces - just ping. |
|
Brilliant - thanks for your understanding, and assistance with this. |
|
@andre-merzky I'll close this PR, since even if we decide to go this way, we'll likely need a new PR anyway. :) |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
lock
bot
added
the
auto-locked
This addresses #2195.
/request-review