-
-
Notifications
You must be signed in to change notification settings - Fork 291
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
Add blacklist handling for skipping requirements in pex resolver #457
Changes from 1 commit
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 |
---|---|---|
|
@@ -181,7 +181,7 @@ def build(self, package, options): | |
'Could not get distribution for %s on platform %s.' % (package, self._platform)) | ||
return dist | ||
|
||
def resolvable_is_blacklisted(self, resolvable_name): | ||
def _resolvable_is_blacklisted(self, resolvable_name): | ||
return ( | ||
resolvable_name in self._blacklist and | ||
self._interpreter.identity.matches(self._blacklist[resolvable_name]) | ||
|
@@ -203,7 +203,7 @@ def resolve(self, resolvables, resolvable_set=None): | |
|
||
# TODO: Remove blacklist strategy in favor of smart requirement handling | ||
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. this TODO is good, but it'd be great to make the temporary nature of this change more evident in the docstring(s) below where developers are more likely to look. Would be good to include the ticket in there as well. |
||
# https://github.com/pantsbuild/pex/issues/456 | ||
if not self.resolvable_is_blacklisted(resolvable.name): | ||
if not self._resolvable_is_blacklisted(resolvable.name): | ||
resolvable_set.merge(resolvable, packages, parent) | ||
processed_resolvables.add(resolvable) | ||
|
||
|
@@ -347,6 +347,8 @@ def resolve(requirements, | |
that universal requirement resolves for a target interpreter version do not error out on | ||
interpreter specific requirements such as backport libs like `functools32`. | ||
For example, a valid blacklist is {'functools32': 'CPython>3'}. | ||
NOTE: this keyword is a temporary fix and will be reverted in favor of a long term solution | ||
tracked by: https://github.com/pantsbuild/pex/issues/456 | ||
:returns: List of :class:`pkg_resources.Distribution` instances meeting ``requirements``. | ||
:raises Unsatisfiable: If ``requirements`` is not transitively satisfiable. | ||
:raises Untranslateable: If no compatible distributions could be acquired for | ||
|
@@ -442,6 +444,8 @@ def resolve_multi(requirements, | |
that universal requirement resolves for a target interpreter version do not error out on | ||
interpreter specific requirements such as backport libs like `functools32`. | ||
For example, a valid blacklist is {'functools32': 'CPython>3'}. | ||
NOTE: this keyword is a temporary fix and will be reverted in favor of a long term solution | ||
tracked by: https://github.com/pantsbuild/pex/issues/456 | ||
:yields: All :class:`pkg_resources.Distribution` instances meeting ``requirements``. | ||
:raises Unsatisfiable: If ``requirements`` is not transitively satisfiable. | ||
:raises Untranslateable: If no compatible distributions could be acquired for | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -353,31 +353,25 @@ def test_resolvable_set_built(): | |
assert updated_rs.get('foo') == set([binary_pkg]) | ||
assert updated_rs.packages() == [(rq, set([binary_pkg]), None, False)] | ||
|
||
|
||
def test_resolver_blacklist(): | ||
project = make_sdist(name='project', version='1.0.0') | ||
other_project = make_sdist(name='other_project', version='1.1.0') | ||
if PY2: | ||
blacklist = {'project2': '<3'} | ||
required_project = "project2;python_version>'3'" | ||
else: | ||
blacklist = {'project2': '>3'} | ||
required_project = "project2;python_version<'3'" | ||
|
||
project1 = make_sdist(name='project1', version='1.0.0', install_reqs=["project2;python_version<'3'"]) | ||
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.
|
||
project2 = make_sdist(name='project2', version='1.1.0') | ||
|
||
with temporary_dir() as td: | ||
safe_copy(project, os.path.join(td, os.path.basename(project))) | ||
safe_copy(other_project, os.path.join(td, os.path.basename(other_project))) | ||
safe_copy(project1, os.path.join(td, os.path.basename(project1))) | ||
safe_copy(project2, os.path.join(td, os.path.basename(project2))) | ||
fetchers = [Fetcher([td])] | ||
|
||
if PY2: | ||
blacklist = {'project': '<3'} | ||
else: | ||
blacklist = {'project': '>3'} | ||
|
||
dists = resolve(['project'], fetchers=fetchers, pkg_blacklist=blacklist) | ||
assert len(dists) == 0 | ||
|
||
dists = resolve(['other_project'], fetchers=fetchers, pkg_blacklist=blacklist) | ||
assert len(dists) == 1 | ||
assert '1.1.0' == dists[0].version | ||
|
||
dists = list(resolve_multi(['project'], fetchers=fetchers, pkg_blacklist=blacklist)) | ||
assert len(dists) == 0 | ||
dists = resolve(['project1'], fetchers=fetchers) | ||
assert len(dists) == 2 | ||
|
||
dists = list(resolve_multi(['project', 'other_project'], fetchers=fetchers, | ||
pkg_blacklist=blacklist)) | ||
dists = resolve(['project1'], fetchers=fetchers, pkg_blacklist=blacklist) | ||
assert len(dists) == 1 | ||
assert '1.1.0' == dists[0].version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
seems cleaner to me.