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

Remove deprecated support for VCS pseudo URLs #9436

Merged
merged 5 commits into from
Jan 19, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Construct valid backends list for error message.
  • Loading branch information
sbidoul committed Jan 18, 2021
commit 5385d8a6cde85d28ea39ab55089cc391ea23743c
7 changes: 4 additions & 3 deletions src/pip/_internal/req/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@ def parse_editable(editable_req):
break

if '+' not in url:
backends = ", ".join([backend.name + '+' for backend in vcs.backends])
sbidoul marked this conversation as resolved.
Show resolved Hide resolved
raise InstallationError(
'{} is not a valid editable requirement. '
'It should either be a path to a local project or a VCS URL '
'(beginning with svn+, git+, hg+, or bzr+).'.format(editable_req)
f'{editable_req} is not a valid editable requirement. '
f'It should either be a path to a local project or a VCS URL '
f'(beginning with {backends}).'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should not simply show vcs.all_schemes directly.
Otherwise, we might crash on someone using git+ftp (or something else) and tell him to use a VCS URL beginning with git+ which is already the case.
Yes, it seems far fetched ^^

Copy link
Member Author

@sbidoul sbidoul Jan 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two possible issues with that are
1/ the list of all schemes is very long
2/ it still contains git, svn, etc (without +)

Regarding 2/, VCS schemes without + such as git:// are supported in editable mode only (see a few lines above), and I think it's not something we want to encourage. I suspect we could remove the schemes without + from all_schemes without changing behaviour, but I'm not 100% sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xavfernandez I've double checked and I don't see a reason for having the schemes without + in the list of all VCS schemes. So I removed them and did as you suggested to show the full list of supported schemes in the error message.

)

vc_type = url.split('+', 1)[0].lower()
Expand Down