-
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
Do wheel installs from VCS/directories using ephemeral caching #4764
Conversation
Follow up from pypagh-4144 To allow build system abstractions, we want installation to go through wheels in more cases. In particular, installing packages from a local directory or a VCS URL currently runs 'setup.py install'. The aim of this PR is to have it build a wheel, which is stored in an ephemeral cache directory, used for installation, and then discarded. We can't cache it permanently based on the path/URL, because the code there might change, but our cache wouldn't be invalidated.
When there's a previous build directory, no_clean is set
@@ -102,13 +103,18 @@ def _link_for_candidate(self, link, candidate): | |||
|
|||
return index.Link(path_to_url(path)) | |||
|
|||
def cleanup(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhettinger What do you think about using context managers for cleanup? You're the person that I got this from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, I definitely think using a context manager is a good idea. But I agree with @pradyunsg that it's something we should defer until a followup PR. Let's get the functionality landed then address the cleanup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with that; I was just wondering what others thought in principle.
For some reason, my newer commits aren't showing up on GitHub's UI. :/ |
@pypa/pip-committers This PR does not depend on the current PEP 518 implementation; it's something we should probably be doing anyway. Can we merge this? |
+1 from me |
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
Okay. This should make this PR mergeable. @pypa/pip-committers Do we want to go ahead with this, regardless of PEP 517/518? |
+1 from me. At this point, I'm not sure if we'll need to do a pip 10 release without PEP 517/518, but I'd rather this be merged just in case we decide to. |
@pypa/pip-committers Ping! :) I still think this change can go in on it's own. Is there something we want to do before merging this? Aside: When merging this PR, please use plain merge instead of squash/rebase merges since I have other branches sitting on top of this one. |
OK, merged - hope I got the right type of merge for you. |
Yeps. Thanks! ^>^ |
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. |
Closes #4501
Closes #4562
Closes #4714
Closes #536
Quoting from #4501:
This is implemented by reusing the current
WheelCache
and wrapping what is basically 2 instances of it in a wrapper that implements everything aCache
should, with an extra helper for direct access to the ephemeral directory when caching wheels.I'm building on top of some awesome work by @takluyver, @alex and @xoviat.