pex throws a resolution error when using cache, succeeds otherwise

[kwlzn]

when passing two requirements of the form (unpinned, pinned) for the same package (e.g. 'thrift thrift==0.9.1'), pex has different behavior in resolution depending on whether or not the cache is used. this also manifests in consumers of pex (i.e. pants) when dependencies land in a similar order.

repro:

[illuminati build]$ pex --version
pex 1.0.3
[illuminati build]$ rm -rf ~/.pex
[illuminati build]$ pex thrift thrift==0.9.1 -o test.pex
[illuminati build]$ pex thrift thrift==0.9.1 -o test.pex
Could not satisfy all requirements for thrift:
    thrift, thrift==0.9.1

this also repros under 1.1.0 and for different packages:

[illuminati build]$ pex --version
pex 1.1.0
[illuminati build]$ rm -rf ~/.pex
[illuminati build]$ pex psutil psutil==3.2.1 -o test.pex
[illuminati build]$ pex psutil psutil==3.2.1 -o test.pex
Could not satisfy all requirements for psutil:
    psutil, psutil==3.2.1

and is also affected by ordering:

[illuminati build]$ rm -rf ~/.pex
[illuminati build]$ pex psutil==3.2.1 psutil -o test.pex
[illuminati build]$ pex psutil==3.2.1 psutil -o test.pex
[illuminati build]$ pex psutil==3.2.1 psutil -o test.pex
[illuminati build]$ pex psutil psutil==3.2.1 -o test.pex
Could not satisfy all requirements for psutil:
    psutil, psutil==3.2.1

[jsirois]

Some of the research here is relevant: pantsbuild/pants#2533

[arnib]

I've been hitting this with our pants setup as well.
We have tests depending on moto, which in turn has a dependency on boto with an unpinned version.
The application has a dependency on boto with a pinned version.

This results in a error from pants/pex, which looks like it could be solved:
Exception message: Could not satisfy all requirements for boto==2.38.0: boto==2.38.0, boto>=2.20.0(from: moto==0.4.10)

[kwlzn]

@arnib do you have any data or a repro you can share that indicates these issues are related? afaict, #178 only happens when one of the deps is completely unpinned - however in your case it appears boto is pinned somewhere (boto==2.38.0).

fwiw:

$ pex boto==2.38.0 moto==0.4.10
Python 2.7.10 (default, Dec 16 2015, 14:09:45) 
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> import boto
>>> import moto
>>> boto.__file__, moto.__file__
('/private/var/folders/3t/xkwqrkld4xxgklk2s4n41jb80000gn/T/tmpul9NGo/.deps/boto-2.38.0-py2.py3-none-any.whl/boto/__init__.py', '/private/var/folders/3t/xkwqrkld4xxgklk2s4n41jb80000gn/T/tmpul9NGo/.deps/moto-0.4.10-py2.py3-none-any.whl/moto/__init__.py')

$ pex boto moto==0.4.10
Python 2.7.10 (default, Dec 16 2015, 14:09:45) 
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> 

[arnib]

Hey @kwlzn, thanks for checking this out.
I'm not able to reproduce the issue using the pex command line, so my error might not be related to this particular issue.

However the exception message still looks like something that should be resolvable.

[kwlzn]

related to pantsbuild/pants#3527

[butlern]

+1 Would like to see this fixed as well. We're currently seeing random pants failures due to this. 1.1.11 fails as well.

$ rm -rf ~/.pex/
$ pex --version
pex 1.1.11
$ pex thrift thrift==0.9.1 -o test.pex
$ pex thrift thrift==0.9.1 -o test.pex
Could not satisfy all requirements for thrift:
    thrift, thrift==0.9.1

[butlern]

So I dove into ze code and this is a multilayered problem. The trivial issue that causes this test to fail on the second go-around is because we're not passing --cache-ttl in the test. This works:

$ rm -rf ~/.pex
$ pex --version
pex 1.2.2
$ pex thrift thrift==0.9.1 -o test.pex
$ pex thrift thrift==0.9.1 -o test.pex
Could not satisfy all requirements for thrift:
    thrift, thrift==0.9.1
$ pex --cache-ttl=120 thrift thrift==0.9.1 -o test.pex
$

The reason it works with cache-ttl is because in https://github.com/pantsbuild/pex/blob/master/pex/resolver.py#L249 it resolves the packages found in the cache for the unbounded thrift resolvable if cache-ttl is set.

Note that in https://github.com/pantsbuild/pex/blob/master/pex/resolver.py#L246 if the resolvable is exact, i.e. thrift==0.9.1 and it finds it in the cache, it always uses the cache.

So, if cache-ttl is unset, for the unbounded thrift resolvable, it finds a bunch of SourcePackages on pypi, for the thrift==0.9.0 resolvable, if it finds 0.9.0 in cache, it ALWAYS returns the cached versions and then in https://github.com/pantsbuild/pex/blob/master/pex/resolver.py#L58 it does a set intersection and finds no common packages and returns the empty set.

So, in the case of --cache-ttl being unset, you can either:

1. Set --cache-tll, and this will work until --cache-ttl expires, and then it won't work until you remove the cached 0.9.0 version
2. Remove the behavior of always returning the cached artifact if the resolvable is exact. I would favor this, because if --cache-ttl is set and the ttl of the artifact is within the ttl, it will still return the cached artifacts for both the unbounded and the exact resolvables.

However, I still think we have an identity crisis. Ultimately, I think it's a problem that this:

SourcePackage(u'https://pypi.python.org/packages/e7/9e/a1e13b5c3cb24f160fc34f589baf874fcb53951ec54895a7aa101cc39056/thrift-0.9.0.tar.gz#md5=600ad4d76bc87d3e0c8570557d60ae65')

Is not considered the same as this:

SourcePackage('file:///Users/nate/.pex/build/thrift-0.9.0.tar.gz')

Potential Solutions:

1. Hash on the name and version in the Package class

diff --git a/pex/package.py b/pex/package.py
index f786729..3520725 100644
--- a/pex/package.py
+++ b/pex/package.py
@@ -55,6 +55,12 @@ class Package(Link):
       cls._HREF_TO_PACKAGE_CACHE.store(href, package)
     return package

+  def __hash__(self):
+      return hash((self.name, self.version))
+
+  def __eq__(self, other):
+      return hash(self) == hash(other)
+
   @property
   def name(self):
     return NotImplementedError

The problem with this approach is that it's now order dependent, the set intersection can return the remote SourcePackage or the local SourcePackage depending upon which side of the intersection you're on.

2. Don't use intersection, create a comparitor based on name, value and maybe package type (prefer Wheels/Eggs over SourcePackages and Local over Remote) and loop through the two sets applying it.

Ultimately this bites us in pants land, we have no control over transitive 3rdparty dependencies and are often bit at build time by this which creates non-deterministic builds. We could disable cache but that would slow our build times unacceptably.

In the meantime I'm going to probably go for option 1, patch our pex with it in our cheeseshop and have it run on our build environment for a time to test it out.

[butlern]

Actually, I ran into another edge case, this happens when an unbounded or inexact requirement in a resolvable is passed and --cache-ttl is used. In this case, if the cached version fits within the bounds of the resolvable requirement, and is less than ttl, only the cached version is returned. But if another resolvable has a requirement that doesn't match that cached version, then it fails.

For example:

$ pex --cache-ttl=86400 'thrift' 'thrift<0.9.1' -o test.pex
$ pex --cache-ttl=86400 'thrift' 'thrift==0.9.1 -o test.pex
Could not satisfy all requirements for thrift:
    thrift, thrift==0.9.1

In this case, the first pex run pulls down thrift==0.9.0 into the cache. On the second run, it finds the unbounded dependency and pulls the thrift==0.9.0 from cache. It then finds the exact dependency for 0.9.1, grabs that from pypi, but since the first resolvable only pulls from cache, the intersection results in an empty set and Unsatisfiable is raised.

This patch addresses that case by returning both the cached version found and the pypi results. In fact, this patch should mean we don't need to deal with package identity at all.

diff --git i/pex/resolver.py w/pex/resolver.py
index e7669c4..bc512ed 100644
--- i/pex/resolver.py
+++ w/pex/resolver.py
@@ -241,8 +241,11 @@ class CachingResolver(Resolver):

       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)

This doesn't solve the case where --cache-ttl is not used, but I think as far as pants goes, it passes a cache-ttl by default so this shouldn't affect it.

I'll add some unit tests after further testing in our build environment and submit a PR soon.

[kwlzn]

hmm, I still seem to be hitting this after the fix in #362:

$ pex --version
pex 1.2.3
$ pex psutil psutil==3.2.1 -o test.pex
$ pex psutil psutil==3.2.1 -o test.pex
Could not satisfy all requirements for psutil:
    psutil, psutil==3.2.1
$ pex psutil psutil==3.2.1 -o test.pex --disable-cache
$

[butlern]

Yah, 731d124 fixes a related problem where --cache-ttl is set and still raises Unsatisfiable. If there's no --cache-ttl then you can still hit this because it returns 3.2.1 from cache because it's exact and then searches pypi for the unbounded and takes the latest and those don't match in the resolvable_set.merge(). Since pants always sets --cache-ttl I fixed it for that case.

[butlern]

Actually, now that I think about it, there is still the problem where you pass --cache-ttl, have an exact resolvable, an inexact resolvable and an expired cache. Probably exact resolvables shouldn't return from cache either if the cache has expired. But by default cache-ttl is set to None.

[kwlzn]

makes sense - was mainly just highlighting my bad assumption that the core repro in this issue was repaired by #362.

and fwiw, I have pantsbuild/pants#4277 out to consume this change in pants.

[butlern]

Cool, I'll work on a fix and submit a new PR w/unit tests for this specific issue.

[kwlzn]

verified this is fixed in master. huge shoutout to @butlern for the bugfix(es)!