Merge pull request #5606 from cjerdonek/vcs-parse-url-once

Change VersionControl to parse the URL only once inside get_url_rev().
This commit is contained in:
Pradyun Gedam 2018-07-22 23:37:01 +05:30 committed by GitHub
commit a2968978c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 165 additions and 108 deletions

View File

@ -213,10 +213,26 @@ class VersionControl(object):
"""
raise NotImplementedError
def get_url_rev(self, url):
def get_netloc_and_auth(self, netloc):
"""
Returns the correct repository URL and revision by parsing the given
repository URL
Parse the repository URL's netloc, and return the new netloc to use
along with auth information.
This is mainly for the Subversion class to override, so that auth
information can be provided via the --username and --password options
instead of through the URL. For other subclasses like Git without
such an option, auth information must stay in the URL.
Returns: (netloc, (username, password)).
"""
return netloc, (None, None)
def get_url_rev_and_auth(self, url):
"""
Parse the repository URL to use, and return the URL, revision,
and auth info to use.
Returns: (url, rev, (username, password)).
"""
error_message = (
"Sorry, '%s' is a malformed VCS url. "
@ -226,26 +242,27 @@ class VersionControl(object):
assert '+' in url, error_message % url
url = url.split('+', 1)[1]
scheme, netloc, path, query, frag = urllib_parse.urlsplit(url)
netloc, user_pass = self.get_netloc_and_auth(netloc)
rev = None
if '@' in path:
path, rev = path.rsplit('@', 1)
url = urllib_parse.urlunsplit((scheme, netloc, path, query, ''))
return url, rev
return url, rev, user_pass
def get_url_rev_args(self, url):
def make_rev_args(self, username, password):
"""
Return the URL and RevOptions "extra arguments" to use in obtain(),
as a tuple (url, extra_args).
Return the RevOptions "extra arguments" to use in obtain().
"""
return url, []
return []
def get_url_rev_options(self, url):
"""
Return the URL and RevOptions object to use in obtain() and in
some cases export(), as a tuple (url, rev_options).
"""
url, rev = self.get_url_rev(url)
url, extra_args = self.get_url_rev_args(url)
url, rev, user_pass = self.get_url_rev_and_auth(url)
username, password = user_pass
extra_args = self.make_rev_args(username, password)
rev_options = self.make_rev_options(rev, extra_args=extra_args)
return url, rev_options

View File

@ -66,12 +66,12 @@ class Bazaar(VersionControl):
cmd_args = ['pull', '-q'] + rev_options.to_args()
self.run_command(cmd_args, cwd=dest)
def get_url_rev(self, url):
def get_url_rev_and_auth(self, url):
# hotfix the URL scheme after removing bzr+ from bzr+ssh:// readd it
url, rev = super(Bazaar, self).get_url_rev(url)
url, rev, user_pass = super(Bazaar, self).get_url_rev_and_auth(url)
if url.startswith('ssh://'):
url = 'bzr+' + url
return url, rev
return url, rev, user_pass
def get_url(self, location):
urls = self.run_command(['info'], show_stdout=False, cwd=location)

View File

@ -265,7 +265,7 @@ class Git(VersionControl):
req += '&subdirectory=' + subdirectory
return req
def get_url_rev(self, url):
def get_url_rev_and_auth(self, url):
"""
Prefixes stub URLs like 'user@hostname:user/repo.git' with 'ssh://'.
That's required because although they use SSH they sometimes don't
@ -275,12 +275,12 @@ class Git(VersionControl):
if '://' not in url:
assert 'file:' not in url
url = url.replace('git+', 'git+ssh://')
url, rev = super(Git, self).get_url_rev(url)
url, rev, user_pass = super(Git, self).get_url_rev_and_auth(url)
url = url.replace('ssh://', '')
else:
url, rev = super(Git, self).get_url_rev(url)
url, rev, user_pass = super(Git, self).get_url_rev_and_auth(url)
return url, rev
return url, rev, user_pass
def update_submodules(self, location):
if not os.path.exists(os.path.join(location, '.gitmodules')):

View File

@ -4,11 +4,9 @@ import logging
import os
import re
from pip._vendor.six.moves.urllib import parse as urllib_parse
from pip._internal.index import Link
from pip._internal.utils.logging import indent_log
from pip._internal.utils.misc import display_path, remove_auth_from_url, rmtree
from pip._internal.utils.misc import display_path, rmtree
from pip._internal.vcs import VersionControl, vcs
_svn_xml_url_re = re.compile('url="([^"]+)"')
@ -102,18 +100,45 @@ class Subversion(VersionControl):
revision = max(revision, localrev)
return revision
def get_url_rev(self, url):
def get_netloc_and_auth(self, netloc):
"""
Parse out and remove from the netloc the auth information.
This allows the auth information to be provided via the --username
and --password options instead of via the URL.
"""
if '@' not in netloc:
return netloc, (None, None)
# Split from the right because that's how urllib.parse.urlsplit()
# behaves if more than one @ is present (by checking the password
# attribute of urlsplit()'s return value).
auth, netloc = netloc.rsplit('@', 1)
if ':' in auth:
# Split from the left because that's how urllib.parse.urlsplit()
# behaves if more than one : is present (again by checking the
# password attribute of the return value)
user_pass = tuple(auth.split(':', 1))
else:
user_pass = auth, None
return netloc, user_pass
def get_url_rev_and_auth(self, url):
# hotfix the URL scheme after removing svn+ from svn+ssh:// readd it
url, rev = super(Subversion, self).get_url_rev(url)
url, rev, user_pass = super(Subversion, self).get_url_rev_and_auth(url)
if url.startswith('ssh://'):
url = 'svn+' + url
return url, rev
return url, rev, user_pass
def get_url_rev_args(self, url):
extra_args = get_rev_options_args(url)
url = remove_auth_from_url(url)
def make_rev_args(self, username, password):
extra_args = []
if username:
extra_args += ['--username', username]
if password:
extra_args += ['--password', password]
return url, extra_args
return extra_args
def get_url(self, location):
# In cases where the source is in a subdirectory, not alongside
@ -193,32 +218,4 @@ class Subversion(VersionControl):
return False
def get_rev_options_args(url):
"""
Return the extra arguments to pass to RevOptions.
"""
r = urllib_parse.urlsplit(url)
if hasattr(r, 'username'):
# >= Python-2.5
username, password = r.username, r.password
else:
netloc = r[1]
if '@' in netloc:
auth = netloc.split('@')[0]
if ':' in auth:
username, password = auth.split(':', 1)
else:
username, password = auth, None
else:
username, password = None, None
extra_args = []
if username:
extra_args += ['--username', username]
if password:
extra_args += ['--password', password]
return extra_args
vcs.register(Subversion)

View File

@ -129,23 +129,60 @@ def test_translate_egg_surname():
assert vc.translate_egg_surname("foo/1.2.3") == "foo_1.2.3"
# The non-SVN backends all use the same get_netloc_and_auth(), so only test
# Git as a representative.
@pytest.mark.parametrize('netloc, expected', [
# Test a basic case.
('example.com', ('example.com', (None, None))),
# Test with username and password.
('user:pass@example.com', ('user:pass@example.com', (None, None))),
])
def test_git__get_netloc_and_auth(netloc, expected):
"""
Test VersionControl.get_netloc_and_auth().
"""
actual = Git().get_netloc_and_auth(netloc)
assert actual == expected
@pytest.mark.parametrize('netloc, expected', [
# Test a basic case.
('example.com', ('example.com', (None, None))),
# Test with username and no password.
('user@example.com', ('example.com', ('user', None))),
# Test with username and password.
('user:pass@example.com', ('example.com', ('user', 'pass'))),
# Test the password containing an @ symbol.
('user:pass@word@example.com', ('example.com', ('user', 'pass@word'))),
# Test the password containing a : symbol.
('user:pass:word@example.com', ('example.com', ('user', 'pass:word'))),
])
def test_subversion__get_netloc_and_auth(netloc, expected):
"""
Test Subversion.get_netloc_and_auth().
"""
actual = Subversion().get_netloc_and_auth(netloc)
assert actual == expected
def test_git__get_url_rev__idempotent():
"""
Check that Git.get_url_rev() is idempotent for what the code calls
Check that Git.get_url_rev_and_auth() is idempotent for what the code calls
"stub URLs" (i.e. URLs that don't contain "://").
Also check that it doesn't change self.url.
"""
url = 'git+git@git.example.com:MyProject#egg=MyProject'
vcs = Git(url)
result1 = vcs.get_url_rev(url)
result1 = vcs.get_url_rev_and_auth(url)
assert vcs.url == url
result2 = vcs.get_url_rev(url)
assert result1 == ('git@git.example.com:MyProject', None)
assert result2 == ('git@git.example.com:MyProject', None)
result2 = vcs.get_url_rev_and_auth(url)
expected = ('git@git.example.com:MyProject', None, (None, None))
assert result1 == expected
assert result2 == expected
def test_bazaar__get_url_rev():
def test_bazaar__get_url_rev_and_auth():
"""
Test bzr url support.
@ -170,61 +207,67 @@ def test_bazaar__get_url_rev():
url='bzr+lp:MyLaunchpadProject#egg=MyLaunchpadProject'
)
assert http_bzr_repo.get_url_rev(http_bzr_repo.url) == (
'http://bzr.myproject.org/MyProject/trunk/', None,
assert http_bzr_repo.get_url_rev_and_auth(http_bzr_repo.url) == (
'http://bzr.myproject.org/MyProject/trunk/', None, (None, None),
)
assert https_bzr_repo.get_url_rev(https_bzr_repo.url) == (
'https://bzr.myproject.org/MyProject/trunk/', None,
assert https_bzr_repo.get_url_rev_and_auth(https_bzr_repo.url) == (
'https://bzr.myproject.org/MyProject/trunk/', None, (None, None),
)
assert ssh_bzr_repo.get_url_rev(ssh_bzr_repo.url) == (
'bzr+ssh://bzr.myproject.org/MyProject/trunk/', None,
assert ssh_bzr_repo.get_url_rev_and_auth(ssh_bzr_repo.url) == (
'bzr+ssh://bzr.myproject.org/MyProject/trunk/', None, (None, None),
)
assert ftp_bzr_repo.get_url_rev(ftp_bzr_repo.url) == (
'ftp://bzr.myproject.org/MyProject/trunk/', None,
assert ftp_bzr_repo.get_url_rev_and_auth(ftp_bzr_repo.url) == (
'ftp://bzr.myproject.org/MyProject/trunk/', None, (None, None),
)
assert sftp_bzr_repo.get_url_rev(sftp_bzr_repo.url) == (
'sftp://bzr.myproject.org/MyProject/trunk/', None,
assert sftp_bzr_repo.get_url_rev_and_auth(sftp_bzr_repo.url) == (
'sftp://bzr.myproject.org/MyProject/trunk/', None, (None, None),
)
assert launchpad_bzr_repo.get_url_rev(launchpad_bzr_repo.url) == (
'lp:MyLaunchpadProject', None,
assert launchpad_bzr_repo.get_url_rev_and_auth(launchpad_bzr_repo.url) == (
'lp:MyLaunchpadProject', None, (None, None),
)
# The non-SVN backends all use the same make_rev_args(), so only test
# Git as a representative.
@pytest.mark.parametrize('username, password, expected', [
(None, None, []),
('user', None, []),
('user', 'pass', []),
])
def test_git__make_rev_args(username, password, expected):
"""
Test VersionControl.make_rev_args().
"""
actual = Git().make_rev_args(username, password)
assert actual == expected
@pytest.mark.parametrize('username, password, expected', [
(None, None, []),
('user', None, ['--username', 'user']),
('user', 'pass', ['--username', 'user', '--password', 'pass']),
])
def test_subversion__make_rev_args(username, password, expected):
"""
Test Subversion.make_rev_args().
"""
actual = Subversion().make_rev_args(username, password)
assert actual == expected
def test_subversion__get_url_rev_options():
"""
Test Subversion.get_url_rev_options().
"""
url = 'svn+https://user:pass@svn.example.com/MyProject@v1.0#egg=MyProject'
url, rev_options = Subversion().get_url_rev_options(url)
assert url == 'https://svn.example.com/MyProject'
assert rev_options.rev == 'v1.0'
assert rev_options.extra_args == (
['--username', 'user', '--password', 'pass']
)
def test_get_git_version():
git_version = Git().get_git_version()
assert git_version >= parse_version('1.0.0')
# The non-SVN backends all have the same get_url_rev_args() implementation,
# so test with Git as a representative.
@pytest.mark.parametrize('url, expected', [
# Test a basic case.
('git+https://git.example.com/MyProject#egg=MyProject',
('git+https://git.example.com/MyProject#egg=MyProject', [])),
# Test with username and password.
('git+https://user:pass@git.example.com/MyProject#egg=MyProject',
('git+https://user:pass@git.example.com/MyProject#egg=MyProject', [])),
])
def test_git__get_url_rev_args(url, expected):
"""
Test Git.get_url_rev_args().
"""
actual = Git().get_url_rev_args(url)
assert actual == expected
@pytest.mark.parametrize('url, expected', [
# Test a basic case.
('svn+https://svn.example.com/MyProject#egg=MyProject',
('svn+https://svn.example.com/MyProject#egg=MyProject', [])),
# Test with username and password.
('svn+https://user:pass@svn.example.com/MyProject#egg=MyProject',
('svn+https://svn.example.com/MyProject#egg=MyProject',
['--username', 'user', '--password', 'pass'])),
])
def test_subversion__get_url_rev_args(url, expected):
"""
Test Subversion.get_url_rev_args().
"""
actual = Subversion().get_url_rev_args(url)
assert actual == expected