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

Normalize all names in ResolvableSet. Fixes #147. #148

Merged
merged 1 commit into from
Aug 12, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 8 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
CHANGES
=======

----------
1.1.0.dev0
----------

* Bug fix: We did not normalize package names in ``ResolvableSet``, so it was possible to depend on
``sphinx`` and ``Sphinx-1.4a0.tar.gz`` and get two versions build and included into the pex.
`#147 <https://github.com/pantsbuild/pex/issues/147>`_.

-----
1.0.3
-----
Expand Down
16 changes: 12 additions & 4 deletions pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import time
from collections import namedtuple

from pkg_resources import safe_name

from .common import safe_mkdir
from .fetcher import Fetcher
from .interpreter import PythonInterpreter
Expand Down Expand Up @@ -56,6 +58,10 @@ def merge(self, other):


class _ResolvableSet(object):
@classmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just generally always use classmethods instead of staticmethods to allow for future subclassing/testing. For example, we could write an implementation like so:

class BuggyResolvableSet(_ResolvableSet):
  @classmethod
  def normalize(cls, name):
    return name

and then assert that it fails when running the same test suite as the regular implementation.

As it stands, we're subclassing it nowhere and it's an internal class so it doesn't really give us any leverage over a staticmethod but why be inconsistent.

def normalize(cls, name):
return safe_name(name).lower()

def __init__(self, tuples=None):
# A list of _ResolvedPackages
self.__tuples = tuples or []
Expand All @@ -71,7 +77,7 @@ def _collapse(self):
# adversely but could be the source of subtle resolution quirks.
resolvables = {}
for resolved_packages in self.__tuples:
key = resolved_packages.resolvable.name
key = self.normalize(resolved_packages.resolvable.name)
previous = resolvables.get(key, _ResolvedPackages.empty())
if previous.resolvable is None:
resolvables[key] = resolved_packages
Expand All @@ -86,7 +92,7 @@ def render_resolvable(resolved_packages):
'(from: %s)' % resolved_packages.parent if resolved_packages.parent else '')
return ', '.join(
render_resolvable(resolved_packages) for resolved_packages in self.__tuples
if resolved_packages.resolvable.name == name)
if self.normalize(resolved_packages.resolvable.name) == self.normalize(name))

def _check(self):
# Check whether or not the resolvables in this set are satisfiable, raise an exception if not.
Expand All @@ -102,7 +108,8 @@ def merge(self, resolvable, packages, parent=None):

def get(self, name):
"""Get the set of compatible packages given a resolvable name."""
resolvable, packages, parent = self._collapse().get(name, _ResolvedPackages.empty())
resolvable, packages, parent = self._collapse().get(
self.normalize(name), _ResolvedPackages.empty())
return packages

def packages(self):
Expand All @@ -111,7 +118,8 @@ def packages(self):

def extras(self, name):
return set.union(
*[set(tup.resolvable.extras()) for tup in self.__tuples if tup.resolvable.name == name])
*[set(tup.resolvable.extras()) for tup in self.__tuples
if self.normalize(tup.resolvable.name) == self.normalize(name)])

def replace_built(self, built_packages):
"""Return a copy of this resolvable set but with built packages.
Expand Down
2 changes: 1 addition & 1 deletion pex/version.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

__version__ = '1.0.3'
__version__ = '1.1.0.dev0'

SETUPTOOLS_REQUIREMENT = 'setuptools>=2.2,<16'
WHEEL_REQUIREMENT = 'wheel>=0.24.0,<0.25.0'
10 changes: 5 additions & 5 deletions tests/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,21 @@ def test_resolvable_set():
rs = _ResolvableSet()
rq = ResolvableRequirement.from_string('foo[ext]', builder)
source_pkg = SourcePackage.from_href('foo-2.3.4.tar.gz')
binary_pkg = EggPackage.from_href('foo-2.3.4-py3.4.egg')
binary_pkg = EggPackage.from_href('Foo-2.3.4-py3.4.egg')

rs.merge(rq, [source_pkg, binary_pkg])
assert rs.get('foo') == set([source_pkg, binary_pkg])
assert rs.get(source_pkg.name) == set([source_pkg, binary_pkg])
assert rs.get(binary_pkg.name) == set([source_pkg, binary_pkg])
assert rs.packages() == [(rq, set([source_pkg, binary_pkg]), None)]

# test methods
assert rs.extras('foo') == set(['ext'])
assert rs.extras('Foo') == set(['ext'])

# test filtering
rs.merge(rq, [source_pkg])
assert rs.get('foo') == set([source_pkg])
assert rs.get('Foo') == set([source_pkg])

with pytest.raises(Unsatisfiable):
rs.merge(rq, [binary_pkg])
Expand All @@ -88,6 +91,3 @@ def test_resolvable_set_built():
updated_rs.merge(rq, [binary_pkg])
assert updated_rs.get('foo') == set([binary_pkg])
assert updated_rs.packages() == [(rq, set([binary_pkg]), None)]


# TODO(wickman) Write more than simple resolver test.