From 5b93c0919912efc548caa3e71df5e8bdf9706d2d Mon Sep 17 00:00:00 2001 From: Vinicyus Macedo <7549205+vinicyusmacedo@users.noreply.github.com> Date: Fri, 25 Jan 2019 10:24:14 -0200 Subject: [PATCH 1/2] Added test to fail pep508 --- news/6202.bugfix | 2 + src/pip/_internal/req/constructors.py | 79 ++++++++++++++++++++------- tests/unit/test_req.py | 27 +++++++++ tests/unit/test_req_file.py | 7 +++ 4 files changed, 94 insertions(+), 21 deletions(-) create mode 100644 news/6202.bugfix diff --git a/news/6202.bugfix b/news/6202.bugfix new file mode 100644 index 00000000000..03184fa8d93 --- /dev/null +++ b/news/6202.bugfix @@ -0,0 +1,2 @@ +Fix requirement line parser to correctly handle PEP 440 requirements with a URL +pointing to an archive file. diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 28cef933221..c4e59209f5e 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -20,7 +20,9 @@ from pip._vendor.packaging.specifiers import Specifier from pip._vendor.pkg_resources import RequirementParseError, parse_requirements -from pip._internal.download import is_archive_file, is_url, url_to_path +from pip._internal.download import ( + is_archive_file, is_url, path_to_url, url_to_path, +) from pip._internal.exceptions import InstallationError from pip._internal.models.index import PyPI, TestPyPI from pip._internal.models.link import Link @@ -201,6 +203,58 @@ def install_req_from_editable( ) +def _get_path_or_url(path, name): + # type: (str, str) -> str + """ + First, it checks whether a provided path is an installable directory + (e.g. it has a setup.py). If it is, returns the path. + + If false, check if the path is an archive file (such as a .whl). + The function checks if the path is a file. If false, if the path has + an @, it will treat it as a PEP 440 URL requirement and return the path. + """ + if os.path.isdir(path) and _looks_like_path(name): + if not is_installable_dir(path): + raise InstallationError( + "Directory %r is not installable. Neither 'setup.py' " + "nor 'pyproject.toml' found." % name + ) + return path_to_url(path) + elif is_archive_file(path): + if os.path.isfile(path): + return path_to_url(path) + else: + urlreq_parts = name.split('@', 1) + if len(urlreq_parts) < 2 or _looks_like_path(urlreq_parts[0]): + logger.warning( + 'Requirement %r looks like a filename, but the ' + 'file does not exist', + name + ) + return path_to_url(path) + # If the path contains '@' and the part before it does not look + # like a path, try to treat it as a PEP 440 URL req instead. + + +def _looks_like_path(name): + # type: (str) -> bool + """Checks whether the string "looks like" a path on the filesystem. + + This does not check whether the target actually exists, only judge from the + apperance. Returns true if any of the following is true: + + * A path separator is found (either os.path.sep or os.path.altsep). + * The string starts with "." (current directory). + """ + if os.path.sep in name: + return True + if os.path.altsep is not None and os.path.altsep in name: + return True + if name.startswith('.'): + return True + return False + + def install_req_from_line( name, # type: str comes_from=None, # type: Optional[Union[str, InstallRequirement]] @@ -241,26 +295,9 @@ def install_req_from_line( link = Link(name) else: p, extras_as_string = _strip_extras(path) - looks_like_dir = os.path.isdir(p) and ( - os.path.sep in name or - (os.path.altsep is not None and os.path.altsep in name) or - name.startswith('.') - ) - if looks_like_dir: - if not is_installable_dir(p): - raise InstallationError( - "Directory %r is not installable. Neither 'setup.py' " - "nor 'pyproject.toml' found." % name - ) - link = Link(path_to_url(p)) - elif is_archive_file(p): - if not os.path.isfile(p): - logger.warning( - 'Requirement %r looks like a filename, but the ' - 'file does not exist', - name - ) - link = Link(path_to_url(p)) + path_or_url = _get_path_or_url(p, name) + if path_or_url: + link = Link(path_or_url) # it's a local file, dir, or url if link: diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index ebd3e8f03bf..a85f5f70cfe 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -335,6 +335,33 @@ def test_url_with_query(self): req = install_req_from_line(url + fragment) assert req.link.url == url + fragment, req.link + def test_pep440_wheel_link_requirement(self): + url = 'https://whatever.com/test-0.4-py2.py3-bogus-any.whl' + line = 'test @ https://whatever.com/test-0.4-py2.py3-bogus-any.whl' + req = install_req_from_line(line) + parts = str(req.req).split('@', 1) + assert len(parts) == 2 + assert parts[0].strip() == 'test' + assert parts[1].strip() == url + + def test_pep440_url_link_requirement(self): + url = 'git+http://foo.com@ref#egg=foo' + line = 'foo @ git+http://foo.com@ref#egg=foo' + req = install_req_from_line(line) + parts = str(req.req).split('@', 1) + assert len(parts) == 2 + assert parts[0].strip() == 'foo' + assert parts[1].strip() == url + + def test_url_with_authentication_link_requirement(self): + url = 'https://what@whatever.com/test-0.4-py2.py3-bogus-any.whl' + line = 'https://what@whatever.com/test-0.4-py2.py3-bogus-any.whl' + req = install_req_from_line(line) + assert req.link is not None + assert req.link.is_wheel + assert req.link.scheme == "https" + assert req.link.url == url + def test_unsupported_wheel_link_requirement_raises(self): reqset = RequirementSet() req = install_req_from_line( diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py index 3ebb55d3cfc..3bc208f34bb 100644 --- a/tests/unit/test_req_file.py +++ b/tests/unit/test_req_file.py @@ -225,6 +225,13 @@ def test_yield_line_requirement(self): req = install_req_from_line(line, comes_from=comes_from) assert repr(list(process_line(line, filename, 1))[0]) == repr(req) + def test_yield_pep440_line_requirement(self): + line = 'SomeProject @ https://url/SomeProject-py2-py3-none-any.whl' + filename = 'filename' + comes_from = '-r %s (line %s)' % (filename, 1) + req = install_req_from_line(line, comes_from=comes_from) + assert repr(list(process_line(line, filename, 1))[0]) == repr(req) + def test_yield_line_constraint(self): line = 'SomeProject' filename = 'filename' From 16af35c61345866d582b43abfc649a0d79b16c9c Mon Sep 17 00:00:00 2001 From: Vinicyus Macedo <7549205+vinicyusmacedo@users.noreply.github.com> Date: Sat, 23 Feb 2019 15:10:34 -0300 Subject: [PATCH 2/2] Adding improvements to the _get_path_to_url function --- docs/html/reference/pip_install.rst | 18 ++++- src/pip/_internal/req/constructors.py | 86 ++++++++++++------------ tests/unit/test_req.py | 94 +++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 45 deletions(-) diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index f56dc9a0e9f..fa475c462c8 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -244,8 +244,7 @@ pip supports installing from a package index using a :term:`requirement specifier `. Generally speaking, a requirement specifier is composed of a project name followed by optional :term:`version specifiers `. :pep:`508` contains a full specification -of the format of a requirement (pip does not support the ``url_req`` form -of specifier at this time). +of the format of a requirement. Some examples: @@ -265,6 +264,13 @@ Since version 6.0, pip also supports specifiers containing `environment markers SomeProject ==5.4 ; python_version < '2.7' SomeProject; sys_platform == 'win32' +Since version 19.1, pip also supports `direct references +`__ like so: + + :: + + SomeProject @ file:///somewhere/... + Environment markers are supported in the command line and in requirements files. .. note:: @@ -880,6 +886,14 @@ Examples $ pip install http://my.package.repo/SomePackage-1.0.4.zip +#. Install a particular source archive file following :pep:`440` direct references. + + :: + + $ pip install SomeProject==1.0.4@http://my.package.repo//SomeProject-1.2.3-py33-none-any.whl + $ pip install "SomeProject==1.0.4 @ http://my.package.repo//SomeProject-1.2.3-py33-none-any.whl" + + #. Install from alternative package repositories. Install from a different index, and not `PyPI`_ :: diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index c4e59209f5e..cacf84bfc85 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -20,9 +20,7 @@ from pip._vendor.packaging.specifiers import Specifier from pip._vendor.pkg_resources import RequirementParseError, parse_requirements -from pip._internal.download import ( - is_archive_file, is_url, path_to_url, url_to_path, -) +from pip._internal.download import is_archive_file, is_url, url_to_path from pip._internal.exceptions import InstallationError from pip._internal.models.index import PyPI, TestPyPI from pip._internal.models.link import Link @@ -203,58 +201,60 @@ def install_req_from_editable( ) -def _get_path_or_url(path, name): - # type: (str, str) -> str - """ - First, it checks whether a provided path is an installable directory - (e.g. it has a setup.py). If it is, returns the path. - - If false, check if the path is an archive file (such as a .whl). - The function checks if the path is a file. If false, if the path has - an @, it will treat it as a PEP 440 URL requirement and return the path. - """ - if os.path.isdir(path) and _looks_like_path(name): - if not is_installable_dir(path): - raise InstallationError( - "Directory %r is not installable. Neither 'setup.py' " - "nor 'pyproject.toml' found." % name - ) - return path_to_url(path) - elif is_archive_file(path): - if os.path.isfile(path): - return path_to_url(path) - else: - urlreq_parts = name.split('@', 1) - if len(urlreq_parts) < 2 or _looks_like_path(urlreq_parts[0]): - logger.warning( - 'Requirement %r looks like a filename, but the ' - 'file does not exist', - name - ) - return path_to_url(path) - # If the path contains '@' and the part before it does not look - # like a path, try to treat it as a PEP 440 URL req instead. - - def _looks_like_path(name): # type: (str) -> bool """Checks whether the string "looks like" a path on the filesystem. This does not check whether the target actually exists, only judge from the - apperance. Returns true if any of the following is true: + appearance. - * A path separator is found (either os.path.sep or os.path.altsep). - * The string starts with "." (current directory). + Returns true if any of the following conditions is true: + * a path separator is found (either os.path.sep or os.path.altsep); + * a dot is found (which represents the current directory). """ if os.path.sep in name: return True if os.path.altsep is not None and os.path.altsep in name: return True - if name.startswith('.'): + if name.startswith("."): return True return False +def _get_url_from_path(path, name): + # type: (str, str) -> str + """ + First, it checks whether a provided path is an installable directory + (e.g. it has a setup.py). If it is, returns the path. + + If false, check if the path is an archive file (such as a .whl). + The function checks if the path is a file. If false, if the path has + an @, it will treat it as a PEP 440 URL requirement and return the path. + """ + if _looks_like_path(name) and os.path.isdir(path): + if is_installable_dir(path): + return path_to_url(path) + raise InstallationError( + "Directory %r is not installable. Neither 'setup.py' " + "nor 'pyproject.toml' found." % name + ) + if not is_archive_file(path): + return None + if os.path.isfile(path): + return path_to_url(path) + urlreq_parts = name.split('@', 1) + if len(urlreq_parts) >= 2 and not _looks_like_path(urlreq_parts[0]): + # If the path contains '@' and the part before it does not look + # like a path, try to treat it as a PEP 440 URL req instead. + return None + logger.warning( + 'Requirement %r looks like a filename, but the ' + 'file does not exist', + name + ) + return path_to_url(path) + + def install_req_from_line( name, # type: str comes_from=None, # type: Optional[Union[str, InstallRequirement]] @@ -295,9 +295,9 @@ def install_req_from_line( link = Link(name) else: p, extras_as_string = _strip_extras(path) - path_or_url = _get_path_or_url(p, name) - if path_or_url: - link = Link(path_or_url) + url = _get_url_from_path(p, name) + if url is not None: + link = Link(url) # it's a local file, dir, or url if link: diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index a85f5f70cfe..8151586e5e2 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -21,6 +21,8 @@ from pip._internal.operations.prepare import RequirementPreparer from pip._internal.req import InstallRequirement, RequirementSet from pip._internal.req.constructors import ( + _get_url_from_path, + _looks_like_path, install_req_from_editable, install_req_from_line, parse_editable, @@ -653,3 +655,95 @@ def test_mismatched_versions(caplog, tmpdir): 'Requested simplewheel==2.0, ' 'but installing version 1.0' ) + + +@pytest.mark.parametrize('args, expected', [ + # Test UNIX-like paths + (('/path/to/installable'), True), + # Test relative paths + (('./path/to/installable'), True), + # Test current path + (('.'), True), + # Test url paths + (('https://whatever.com/test-0.4-py2.py3-bogus-any.whl'), True), + # Test pep440 paths + (('test @ https://whatever.com/test-0.4-py2.py3-bogus-any.whl'), True), + # Test wheel + (('simple-0.1-py2.py3-none-any.whl'), False), +]) +def test_looks_like_path(args, expected): + assert _looks_like_path(args) == expected + + +@pytest.mark.skipif( + not sys.platform.startswith("win"), + reason='Test only available on Windows' +) +@pytest.mark.parametrize('args, expected', [ + # Test relative paths + (('.\\path\\to\\installable'), True), + (('relative\\path'), True), + # Test absolute paths + (('C:\\absolute\\path'), True), +]) +def test_looks_like_path_win(args, expected): + assert _looks_like_path(args) == expected + + +@pytest.mark.parametrize('args, mock_returns, expected', [ + # Test pep440 urls + (('/path/to/foo @ git+http://foo.com@ref#egg=foo', + 'foo @ git+http://foo.com@ref#egg=foo'), (False, False), None), + # Test pep440 urls without spaces + (('/path/to/foo@git+http://foo.com@ref#egg=foo', + 'foo @ git+http://foo.com@ref#egg=foo'), (False, False), None), + # Test pep440 wheel + (('/path/to/test @ https://whatever.com/test-0.4-py2.py3-bogus-any.whl', + 'test @ https://whatever.com/test-0.4-py2.py3-bogus-any.whl'), + (False, False), None), + # Test name is not a file + (('/path/to/simple==0.1', + 'simple==0.1'), + (False, False), None), +]) +@patch('pip._internal.req.req_install.os.path.isdir') +@patch('pip._internal.req.req_install.os.path.isfile') +def test_get_url_from_path( + isdir_mock, isfile_mock, args, mock_returns, expected +): + isdir_mock.return_value = mock_returns[0] + isfile_mock.return_value = mock_returns[1] + assert _get_url_from_path(*args) is expected + + +@patch('pip._internal.req.req_install.os.path.isdir') +@patch('pip._internal.req.req_install.os.path.isfile') +def test_get_url_from_path__archive_file(isdir_mock, isfile_mock): + isdir_mock.return_value = False + isfile_mock.return_value = True + name = 'simple-0.1-py2.py3-none-any.whl' + path = os.path.join('/path/to/' + name) + url = path_to_url(path) + assert _get_url_from_path(path, name) == url + + +@patch('pip._internal.req.req_install.os.path.isdir') +@patch('pip._internal.req.req_install.os.path.isfile') +def test_get_url_from_path__installable_dir(isdir_mock, isfile_mock): + isdir_mock.return_value = True + isfile_mock.return_value = True + name = 'some/setuptools/project' + path = os.path.join('/path/to/' + name) + url = path_to_url(path) + assert _get_url_from_path(path, name) == url + + +@patch('pip._internal.req.req_install.os.path.isdir') +def test_get_url_from_path__installable_error(isdir_mock): + isdir_mock.return_value = True + name = 'some/setuptools/project' + path = os.path.join('/path/to/' + name) + with pytest.raises(InstallationError) as e: + _get_url_from_path(path, name) + err_msg = e.value.args[0] + assert "Neither 'setup.py' nor 'pyproject.toml' found" in err_msg