From 224b6b0be5df88cf8d37f4afb3d51b13b3ac252b Mon Sep 17 00:00:00 2001 From: Brian Wickman Date: Wed, 3 Dec 2014 13:58:15 -0800 Subject: [PATCH] Only get mtime on local links. Fixes #29 Under certain circumstances it is possible for a remote link to have os.path.getmtime called on it during resolve when using caching. Instead, consider all remote links as cache hits, and only filter local links using mtime. --- CHANGES.rst | 7 +++++++ pex/resolver.py | 9 ++++++--- pex/version.py | 2 +- tests/test_resolver.py | 4 ++++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6beaa1a05..7a6cf48df 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,13 @@ CHANGES ======= +----- +0.8.1 +----- + +* Bug fix: Fix issue where it'd be possible to ``os.path.getmtime`` on a remote ``Link`` object + `Issue #29 `_ + ----- 0.8.0 ----- diff --git a/pex/resolver.py b/pex/resolver.py index b374ed132..110534011 100644 --- a/pex/resolver.py +++ b/pex/resolver.py @@ -91,7 +91,8 @@ def packages_from_requirement_cached(local_iterator, ttl, iterator, requirement, # match with inexact requirement, consider if ttl supplied. if ttl: now = time.time() - packages = [package for package in packages if (now - os.path.getmtime(package.path)) < ttl] + packages = [package for package in packages if package.remote or package.local and + (now - os.path.getmtime(package.path)) < ttl] if packages: TRACER.log('Package cache hit (inexact): %s' % requirement, V=3) return packages @@ -187,13 +188,15 @@ def resolve( def requires(package, requirement): if not distributions.has(package): - local_package = Package.from_href(context.fetch(package, into=cache)) + with TRACER.timed('Fetching %s' % package.url, V=2): + local_package = Package.from_href(context.fetch(package, into=cache)) if package.remote: # this was a remote resolution -- so if we copy from remote to local but the # local already existed, update the mtime of the local so that it is correct # with respect to cache_ttl. os.utime(local_package.path, None) - dist = translator.translate(local_package, into=cache) + with TRACER.timed('Translating %s into distribution' % local_package.path, V=2): + dist = translator.translate(local_package, into=cache) if dist is None: raise Untranslateable('Package %s is not translateable.' % package) if not distribution_compatible(dist, interpreter, platform): diff --git a/pex/version.py b/pex/version.py index 32a90a3b9..ef72cc0f1 100644 --- a/pex/version.py +++ b/pex/version.py @@ -1 +1 @@ -__version__ = '0.8.0' +__version__ = '0.8.1' diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 6333d6907..e9565bfe0 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -25,3 +25,7 @@ def test_simple_local_resolve(): fetchers = [Fetcher([td])] dists = resolve(['project'], fetchers=fetchers) assert len(dists) == 1 + + +# TODO(wickman) Test resolve and cached resolve more directly than via +# integration.