From f18543bb906aa1f2b9c24cbbcd512629343bdfd5 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Fri, 17 Feb 2017 17:29:22 -0500 Subject: [PATCH 1/2] Fix corner case in cached dependency resolution Ref: https://github.com/pantsbuild/pex/issues/178 The error is as follows: 1. Two resolvables, one pinned, the other not. 2. First pex run with clean cache works. 3. Change the pinned version. 4. Second pex run fails with Unsatisfiable. The problem is that the non-pinned resolvable locates the cached dep and only returns it. Since that version doesn't match the pinned version, we fail. This fixes that behavior by returning both the cached version and all uncached pypi packages for unpinned resolvables. We change the pinned version to test the use case where you have a cached version of a dep and then you transitively pull in another requirement that either has a tighter bound or a pin on a version not found in the cache. --- pex/resolver.py | 8 ++++++-- tests/test_resolver.py | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pex/resolver.py b/pex/resolver.py index 99a412490..80105e607 100644 --- a/pex/resolver.py +++ b/pex/resolver.py @@ -248,8 +248,12 @@ def package_iterator(self, resolvable, existing=None): if self.__cache_ttl: packages = self.filter_packages_by_ttl(packages, self.__cache_ttl) - if packages: - return packages + # Return both cached packages and the pypi packages. The edge case is if a inexact + # resolvable finds a cached version, it needs to return that cached package AND all pypi + # packages because the cached version might not match another resolvable which has a tighter + # bound or an exact version. + return packages + super(CachingResolver, self).package_iterator(resolvable, + existing=existing) return super(CachingResolver, self).package_iterator(resolvable, existing=existing) diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 757191df3..a2dc8a07b 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -7,6 +7,7 @@ from twitter.common.contextutil import temporary_dir from pex.common import safe_copy +from pex.crawler import Crawler from pex.fetcher import Fetcher from pex.package import EggPackage, SourcePackage from pex.resolvable import ResolvableRequirement @@ -48,6 +49,29 @@ def test_diamond_local_resolve_cached(): assert len(dists) == 2 +def test_cached_dependency_pinned_unpinned_resolution_multi_run(): + # This exercises the issue described here: https://github.com/pantsbuild/pex/issues/178 + project1_0_0 = make_sdist(name='project', version='1.0.0') + project1_1_0 = make_sdist(name='project', version='1.1.0') + + with temporary_dir() as td: + for sdist in (project1_0_0, project1_1_0): + safe_copy(sdist, os.path.join(td, os.path.basename(sdist))) + fetchers = [Fetcher([td])] + with temporary_dir() as cd: + # First run, pinning 1.0.0 in the cache + dists = resolve(['project', 'project==1.0.0'], fetchers=fetchers, cache=cd, cache_ttl=1000) + assert len(dists) == 1 + assert dists[0].version == '1.0.0' + # This simulates separate invocations of pex but allows us to keep the same tmp cache dir + Crawler.reset_cache() + # Second, run, the unbounded 'project' req will find the 1.0.0 in the cache. But should also + # return SourcePackages found in td + dists = resolve(['project', 'project==1.1.0'], fetchers=fetchers, cache=cd, cache_ttl=1000) + assert len(dists) == 1 + assert dists[0].version == '1.1.0' + + def test_resolve_prereleases(): stable_dep = make_sdist(name='dep', version='2.0.0') prerelease_dep = make_sdist(name='dep', version='3.0.0rc3') From b8601a9c2c7d31ab86c398a27a673d0ea2403470 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Tue, 21 Feb 2017 20:33:19 -0500 Subject: [PATCH 2/2] fixup! Fix corner case in cached dependency resolution --- pex/resolver.py | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/pex/resolver.py b/pex/resolver.py index 80105e607..feb4980f4 100644 --- a/pex/resolver.py +++ b/pex/resolver.py @@ -3,6 +3,7 @@ from __future__ import print_function +import itertools import os import shutil import time @@ -238,24 +239,26 @@ def __init__(self, cache, cache_ttl, *args, **kw): # Short-circuiting package iterator. def package_iterator(self, resolvable, existing=None): - iterator = Iterator(fetchers=[Fetcher([self.__cache])]) - packages = self.filter_packages_by_interpreter( - resolvable.compatible(iterator), self._interpreter, self._platform) - - if packages: - if resolvable.exact: - return packages - - if self.__cache_ttl: - packages = self.filter_packages_by_ttl(packages, self.__cache_ttl) - # Return both cached packages and the pypi packages. The edge case is if a inexact - # resolvable finds a cached version, it needs to return that cached package AND all pypi - # packages because the cached version might not match another resolvable which has a tighter - # bound or an exact version. - return packages + super(CachingResolver, self).package_iterator(resolvable, - existing=existing) - - return super(CachingResolver, self).package_iterator(resolvable, existing=existing) + packages = [] + if self.__cache_ttl or resolvable.exact: + iterator = Iterator(fetchers=[Fetcher([self.__cache])]) + packages = self.filter_packages_by_interpreter( + resolvable.compatible(iterator), + self._interpreter, + self._platform + ) + + if packages: + if resolvable.exact: + return packages + + if self.__cache_ttl: + packages = self.filter_packages_by_ttl(packages, self.__cache_ttl) + + return itertools.chain( + packages, + super(CachingResolver, self).package_iterator(resolvable, existing=existing) + ) # Caching sandwich. def build(self, package, options):