From 4ea79372648d087a61580e342ddcad754ea4437c Mon Sep 17 00:00:00 2001 From: Xavier Fernandez Date: Sun, 7 Dec 2014 23:49:26 +0100 Subject: [PATCH 1/4] parse_editable: clarify output extras and editable_options are two different things --- pip/req/req_install.py | 21 +++++++++++++++------ tests/unit/test_req.py | 11 +++++++---- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/pip/req/req_install.py b/pip/req/req_install.py index 16247baf1ae..a0f878e2373 100644 --- a/pip/req/req_install.py +++ b/pip/req/req_install.py @@ -89,7 +89,8 @@ def __init__(self, req, comes_from, source_dir=None, editable=False, @classmethod def from_editable(cls, editable_req, comes_from=None, default_vcs=None, isolated=False): - name, url, extras_override = parse_editable(editable_req, default_vcs) + name, url, extras_override, editable_options = parse_editable( + editable_req, default_vcs) if url.startswith('file:'): source_dir = url_to_path(url) else: @@ -98,7 +99,7 @@ def from_editable(cls, editable_req, comes_from=None, default_vcs=None, res = cls(name, comes_from, source_dir=source_dir, editable=True, url=url, - editable_options=extras_override, + editable_options=editable_options, isolated=isolated) if extras_override is not None: @@ -1034,8 +1035,15 @@ def _build_editable_options(req): def parse_editable(editable_req, default_vcs=None): - """Parses svn+http://blahblah@rev#egg=Foobar into a requirement - (Foobar) and a URL""" + """Parses an editable requirement into: + - a requirement name + - an URL + - extras + - editable options + Accepted requirements: + svn+http://blahblah@rev#egg=Foobar[baz]&subdirectory=version_subdir + .[some_extra] + """ url = editable_req extras = None @@ -1065,9 +1073,10 @@ def parse_editable(editable_req, default_vcs=None): pkg_resources.Requirement.parse( '__placeholder__' + extras ).extras, + {}, ) else: - return None, url_no_extras, None + return None, url_no_extras, None, {} for version_control in vcs: if url.lower().startswith('%s:' % version_control): @@ -1109,4 +1118,4 @@ def parse_editable(editable_req, default_vcs=None): req = options['egg'] package = _strip_postfix(req) - return package, url, options + return package, url, None, options diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index f4e7e675d06..7f885bfdcb7 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -204,10 +204,10 @@ def test_parse_editable_local( exists_mock.return_value = isdir_mock.return_value = True # mocks needed to support path operations on windows tests normcase_mock.return_value = getcwd_mock.return_value = "/some/path" - assert parse_editable('.', 'git') == (None, 'file:///some/path', None) + assert parse_editable('.', 'git') == (None, 'file:///some/path', None, {}) normcase_mock.return_value = "/some/path/foo" assert parse_editable('foo', 'git') == ( - None, 'file:///some/path/foo', None, + None, 'file:///some/path/foo', None, {}, ) @@ -215,6 +215,7 @@ def test_parse_editable_default_vcs(): assert parse_editable('https://foo#egg=foo', 'git') == ( 'foo', 'git+https://foo#egg=foo', + None, {'egg': 'foo'}, ) @@ -223,6 +224,7 @@ def test_parse_editable_explicit_vcs(): assert parse_editable('svn+https://foo#egg=foo', 'git') == ( 'foo', 'svn+https://foo#egg=foo', + None, {'egg': 'foo'}, ) @@ -231,6 +233,7 @@ def test_parse_editable_vcs_extras(): assert parse_editable('svn+https://foo#egg=foo[extras]', 'git') == ( 'foo[extras]', 'svn+https://foo#egg=foo[extras]', + None, {'egg': 'foo[extras]'}, ) @@ -244,11 +247,11 @@ def test_parse_editable_local_extras( exists_mock.return_value = isdir_mock.return_value = True normcase_mock.return_value = getcwd_mock.return_value = "/some/path" assert parse_editable('.[extras]', 'git') == ( - None, 'file://' + "/some/path", ('extras',), + None, 'file://' + "/some/path", ('extras',), {}, ) normcase_mock.return_value = "/some/path/foo" assert parse_editable('foo[bar,baz]', 'git') == ( - None, 'file:///some/path/foo', ('bar', 'baz'), + None, 'file:///some/path/foo', ('bar', 'baz'), {}, ) From 06da6c07ec5c5b15f6ffbbbd712e0e4d16f9b2e4 Mon Sep 17 00:00:00 2001 From: Xavier Fernandez Date: Sun, 7 Dec 2014 23:51:46 +0100 Subject: [PATCH 2/4] Fix environment extras installation for sdist use pkg_resources.Distribution.requires instead of Requirements.requirements to have environment markers parsing for free It also unifies a little the process for wheel and non-wheel installs closes #2174 --- pip/req/req_install.py | 28 +++---- pip/req/req_set.py | 79 +++++++------------ .../packages/LocalEnvironMarker/.gitignore | 1 + .../localenvironmarker/__init__.py | 0 .../data/packages/LocalEnvironMarker/setup.py | 29 +++++++ tests/unit/test_req.py | 17 ++++ 6 files changed, 86 insertions(+), 68 deletions(-) create mode 100644 tests/data/packages/LocalEnvironMarker/.gitignore create mode 100644 tests/data/packages/LocalEnvironMarker/localenvironmarker/__init__.py create mode 100644 tests/data/packages/LocalEnvironMarker/setup.py diff --git a/pip/req/req_install.py b/pip/req/req_install.py index a0f878e2373..a72981b9320 100644 --- a/pip/req/req_install.py +++ b/pip/req/req_install.py @@ -491,23 +491,6 @@ def dependency_links(self): _requirements_section_re = re.compile(r'\[(.*?)\]') - def requirements(self, extras=()): - if self.satisfied_by: - for r in self.satisfied_by.requires(extras): - yield str(r) - return - in_extra = None - for line in self.egg_info_lines('requires.txt'): - match = self._requirements_section_re.match(line.lower()) - if match: - in_extra = match.group(1) - continue - if in_extra and in_extra not in extras: - logger.debug('skipping extra %s', in_extra) - # Skip requirement for an extra we aren't requiring - continue - yield line - @property def installed_version(self): if self.is_wheel: @@ -989,6 +972,17 @@ def move_wheel_files(self, wheeldir, root=None): isolated=self.isolated, ) + def get_dist(self): + """Return a pkg_resources.Distribution built from self.egg_info_path""" + egg_info = self.egg_info_path('') + base_dir = os.path.dirname(egg_info) + metadata = pkg_resources.PathMetadata(base_dir, egg_info) + dist_name = os.path.splitext(os.path.basename(egg_info))[0] + return pkg_resources.Distribution( + os.path.dirname(egg_info), + project_name=dist_name, + metadata=metadata) + def _strip_postfix(req): """ diff --git a/pip/req/req_set.py b/pip/req/req_set.py index 9e42f829053..8c287f5dfbf 100644 --- a/pip/req/req_set.py +++ b/pip/req/req_set.py @@ -410,67 +410,44 @@ def prepare_files(self, finder): # ###################### # # # parse dependencies # # # ###################### # + if (req_to_install.extras): + logger.info( + "Installing extra requirements: %r", + ','.join(req_to_install.extras), + ) if is_wheel: dist = list( pkg_resources.find_distributions(location) )[0] - if not req_to_install.req: - req_to_install.req = dist.as_requirement() - self.add_requirement(req_to_install) - if not self.ignore_dependencies: - for subreq in dist.requires( - req_to_install.extras): - if self.has_requirement( - subreq.project_name): - continue - subreq = InstallRequirement( - str(subreq), - req_to_install, - isolated=self.isolated, - ) - reqs.append(subreq) - self.add_requirement(subreq) - - # sdists - else: + else: # sdists # FIXME: shouldn't be globally added: finder.add_dependency_links( req_to_install.dependency_links ) - if (req_to_install.extras): - logger.info( - "Installing extra requirements: %r", - ','.join(req_to_install.extras), + if req_to_install.satisfied_by: + dist = req_to_install.satisfied_by + else: + dist = req_to_install.get_dist() + + if not self.ignore_dependencies: + for subreq in dist.requires( + req_to_install.extras): + if self.has_requirement( + subreq.project_name): + # FIXME: check for conflict + continue + subreq = InstallRequirement( + str(subreq), + req_to_install, + isolated=self.isolated, ) - if not self.ignore_dependencies: - for req in req_to_install.requirements( - req_to_install.extras): - try: - name = pkg_resources.Requirement.parse( - req - ).project_name - except ValueError as exc: - # FIXME: proper warning - logger.error( - 'Invalid requirement: %r (%s) in ' - 'requirement %s', - req, exc, req_to_install, - ) - continue - if self.has_requirement(name): - # FIXME: check for conflict - continue - subreq = InstallRequirement( - req, - req_to_install, - isolated=self.isolated, - ) - reqs.append(subreq) - self.add_requirement(subreq) - if not self.has_requirement(req_to_install.name): - # 'unnamed' requirements will get added here - self.add_requirement(req_to_install) + reqs.append(subreq) + self.add_requirement(subreq) + + if not self.has_requirement(req_to_install.name): + # 'unnamed' requirements will get added here + self.add_requirement(req_to_install) # cleanup tmp src if (self.is_download or diff --git a/tests/data/packages/LocalEnvironMarker/.gitignore b/tests/data/packages/LocalEnvironMarker/.gitignore new file mode 100644 index 00000000000..02fa6ede3a2 --- /dev/null +++ b/tests/data/packages/LocalEnvironMarker/.gitignore @@ -0,0 +1 @@ +/LocalEnvironMarker.egg-info diff --git a/tests/data/packages/LocalEnvironMarker/localenvironmarker/__init__.py b/tests/data/packages/LocalEnvironMarker/localenvironmarker/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/packages/LocalEnvironMarker/setup.py b/tests/data/packages/LocalEnvironMarker/setup.py new file mode 100644 index 00000000000..3ba33bf358e --- /dev/null +++ b/tests/data/packages/LocalEnvironMarker/setup.py @@ -0,0 +1,29 @@ +import os +from setuptools import setup, find_packages + + +def path_to_url(path): + """ + Convert a path to URI. The path will be made absolute and + will not have quoted path parts. + """ + path = os.path.normpath(os.path.abspath(path)) + drive, path = os.path.splitdrive(path) + filepath = path.split(os.path.sep) + url = '/'.join(filepath) + if drive: + return 'file:///' + drive + url + return 'file://' +url + + +HERE = os.path.dirname(__file__) +DEP_PATH = os.path.join(HERE, '..', '..', 'indexes', 'simple', 'simple') +DEP_URL = path_to_url(DEP_PATH) + +setup( + name='LocalEnvironMarker', + version='0.0.1', + packages=find_packages(), + extras_require={":python_version == '2.7' or python_version == '3.4'": ['simple'] }, + dependency_links=[DEP_URL] +) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 7f885bfdcb7..51dec531ffe 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -55,6 +55,23 @@ def test_no_reuse_existing_build_dir(self, data): finder, ) + def test_environment_marker_extras(self, data): + """ + Test that the environment marker extras are used with + non-wheel installs. + """ + reqset = self.basic_reqset() + req = InstallRequirement.from_editable( + data.packages.join("LocalEnvironMarker")) + reqset.add_requirement(req) + finder = PackageFinder([data.find_links], [], session=PipSession()) + reqset.prepare_files(finder) + # This is hacky but does test both case in py2 and py3 + if sys.version_info[:2] in ((2, 7), (3, 4)): + assert reqset.has_requirement('simple') + else: + assert not reqset.has_requirement('simple') + @pytest.mark.parametrize(('file_contents', 'expected'), [ (b'\xf6\x80', b'\xc3\xb6\xe2\x82\xac'), # cp1252 From de7e9c5266534314668925ebfe45b16860efdfb7 Mon Sep 17 00:00:00 2001 From: Xavier Fernandez Date: Wed, 17 Dec 2014 20:39:43 +0100 Subject: [PATCH 3/4] drop egg_info_lines and use pkg_resources.Distribution instead --- pip/req/req_install.py | 16 ---------------- pip/req/req_set.py | 9 +++++---- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/pip/req/req_install.py b/pip/req/req_install.py index a72981b9320..5a23d25b3d9 100644 --- a/pip/req/req_install.py +++ b/pip/req/req_install.py @@ -462,18 +462,6 @@ def egg_info_path(self, filename): self._egg_info_path = os.path.join(base, filenames[0]) return os.path.join(self._egg_info_path, filename) - def egg_info_lines(self, filename): - data = self.egg_info_data(filename) - if not data: - return [] - result = [] - for line in data.splitlines(): - line = line.strip() - if not line or line.startswith('#'): - continue - result.append(line) - return result - def pkg_info(self): p = FeedParser() data = self.egg_info_data('PKG-INFO') @@ -485,10 +473,6 @@ def pkg_info(self): p.feed(data or '') return p.close() - @property - def dependency_links(self): - return self.egg_info_lines('dependency_links.txt') - _requirements_section_re = re.compile(r'\[(.*?)\]') @property diff --git a/pip/req/req_set.py b/pip/req/req_set.py index 8c287f5dfbf..08450323f07 100644 --- a/pip/req/req_set.py +++ b/pip/req/req_set.py @@ -421,14 +421,15 @@ def prepare_files(self, finder): pkg_resources.find_distributions(location) )[0] else: # sdists - # FIXME: shouldn't be globally added: - finder.add_dependency_links( - req_to_install.dependency_links - ) if req_to_install.satisfied_by: dist = req_to_install.satisfied_by else: dist = req_to_install.get_dist() + # FIXME: shouldn't be globally added: + if dist.has_metadata('dependency_links.txt'): + finder.add_dependency_links( + dist.get_metadata_lines('dependency_links.txt') + ) if not self.ignore_dependencies: for subreq in dist.requires( From 1921ad1611287282c0ebf8e7b0232416706aec76 Mon Sep 17 00:00:00 2001 From: Xavier Fernandez Date: Fri, 19 Dec 2014 19:13:04 +0100 Subject: [PATCH 4/4] switch extra requirements log to debug --- pip/req/req_set.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/req/req_set.py b/pip/req/req_set.py index 08450323f07..cf39045f2eb 100644 --- a/pip/req/req_set.py +++ b/pip/req/req_set.py @@ -411,7 +411,7 @@ def prepare_files(self, finder): # # parse dependencies # # # ###################### # if (req_to_install.extras): - logger.info( + logger.debug( "Installing extra requirements: %r", ','.join(req_to_install.extras), )