-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Allow PEP 508 URL spec as editable #9471
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
Changes from all commits
9e97705
9b11a41
9defcc0
0fcde33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add support to install a PEP 508 URL-specified requirement as editable. |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -70,11 +70,24 @@ def parse_editable(editable_req): | |||||||||
- a requirement name | ||||||||||
- an URL | ||||||||||
- extras | ||||||||||
- editable options | ||||||||||
Accepted requirements: | ||||||||||
svn+http://blahblah@rev#egg=Foobar[baz]&subdirectory=version_subdir | ||||||||||
.[some_extra] | ||||||||||
- svn+http://blahblah@rev#egg=Foobar[baz]&subdirectory=version_subdir | ||||||||||
- local_path[some_extra] | ||||||||||
- Foobar[extra] @ svn+http://blahblah@rev#subdirectory=subdir ; markers | ||||||||||
""" | ||||||||||
try: | ||||||||||
req = Requirement(editable_req) | ||||||||||
except InvalidRequirement: | ||||||||||
pass | ||||||||||
else: | ||||||||||
if req.url and "://" in req.url: | ||||||||||
# Join the marker back into the name part. This will be parsed out | ||||||||||
# later into a Requirement again. | ||||||||||
if req.marker: | ||||||||||
name = f"{req.name} ; {req.marker}" | ||||||||||
else: | ||||||||||
name = req.name | ||||||||||
return (name, req.url, req.extras) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, on second look... this will let non VCS URLs go through ? So we need to merge #9436 first and see how we can do the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good point, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing we ca do here is to add an extra check, and only treat the string as PEP 508 if the URL part contains There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sbidoul I added an additional check to prevent pseudo-URLs from going through here. Do you think it’d help? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @uranusjr I have not tested (I whish I had more time for pip...) but this version would accept any URLs ? Editables URLs must be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. There are already checks in a later stage to error out on invalid URL values, so I think it’s fine to pass them through here. pip/src/pip/_internal/req/constructors.py Lines 116 to 119 in baaf66f
|
||||||||||
|
||||||||||
url = editable_req | ||||||||||
|
||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.