Skip to content

Commit

Permalink
Fix handling of pre-release option.
Browse files Browse the repository at this point in the history
Pex takes a `--pre` option from the command line, but it does not pass
it correctly during build. We need to propagate it to resolver to get
the correct behavior.

Fixes #423
  • Loading branch information
zvezdan committed Oct 11, 2017
1 parent b8a3734 commit cbb9e57
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 2 deletions.
3 changes: 2 additions & 1 deletion pex/bin/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ def build_pex(args, options, resolver_option_builder):
interpreters=interpreters,
platforms=options.platform,
cache=options.cache_dir,
cache_ttl=options.cache_ttl)
cache_ttl=options.cache_ttl,
allow_prereleases=resolver_option_builder.prereleases_allowed)

for dist in resolveds:
log(' %s' % dist, v=options.verbosity)
Expand Down
25 changes: 25 additions & 0 deletions pex/resolver_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,31 @@ def no_allow_builds(self):
[precedent for precedent in self._precedence if precedent is not SourcePackage])
return self

# TODO: Make this whole interface more Pythonic.
#
# This method would be better defined as a property allow_prereleases.
# Unfortunately, the existing method below already usurps the name allow_prereleases.
# It is an existing API that returns self as if it was written in an attempt to allow
# Java style chaining of method calls.
# Due to that return type, it cannot be used as a Python property setter.
# It's currently used in this manner:
#
# builder.allow_prereleases(True)
#
# and we cannot change it into @allow_prereleases.setter and use in this manner:
#
# builder.allow_prereleases = True
#
# without affecting the existing API calls.
#
# The code review shows that, for this particular method (allow_prereleases),
# the return value (self) is never used in the current API calls.
# It would be worth examining if the API change for this and some other methods here
# would be a good idea.
@property
def prereleases_allowed(self):
return self._allow_prereleases

def allow_prereleases(self, allowed):
self._allow_prereleases = allowed
return self
Expand Down
72 changes: 71 additions & 1 deletion tests/test_pex_binary.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os
from contextlib import contextmanager
from optparse import OptionParser
from tempfile import NamedTemporaryFile

from twitter.common.contextutil import temporary_dir

from pex.bin.pex import build_pex, configure_clp, configure_clp_pex_resolution
from pex.common import safe_copy
from pex.compatibility import to_bytes
from pex.fetcher import PyPIFetcher
from pex.fetcher import Fetcher, PyPIFetcher
from pex.package import SourcePackage, WheelPackage
from pex.resolver_options import ResolverOptionsBuilder
from pex.sorter import Sorter
from pex.testing import make_sdist

try:
from unittest import mock
except ImportError:
import mock


@contextmanager
Expand Down Expand Up @@ -115,3 +125,63 @@ def test_clp_prereleases():

options, _ = parser.parse_args(args=['--pre'])
assert builder._allow_prereleases


def test_clp_prereleases_resolver():
prerelease_dep = make_sdist(name='dep', version='1.2.3b1')
with temporary_dir() as td:
safe_copy(prerelease_dep, os.path.join(td, os.path.basename(prerelease_dep)))
fetcher = Fetcher([td])

# When no specific options are specified, allow_prereleases is None
parser, resolver_options_builder = configure_clp()
assert resolver_options_builder._allow_prereleases is None

# When we specify `--pre`, allow_prereleases is True
options, reqs = parser.parse_args(args=['--pre', 'dep==1.2.3b1', 'dep'])
assert resolver_options_builder._allow_prereleases
# We need to use our own fetcher instead of PyPI
resolver_options_builder._fetchers.insert(0, fetcher)

#####
# The resolver created during processing of command line options (configure_clp)
# is not actually passed into the API call (resolve_multi) from build_pex().
# Instead, resolve_multi() calls resolve() where a new ResolverOptionsBuilder instance
# is created. The only way to supply our own fetcher to that new instance is to patch it
# here in the test so that it can fetch our test package (dep-1.2.3b1). Hence, this class
# below and the change in the `pex.resolver` module where the patched object resides.
#
import pex.resolver

class BuilderWithFetcher(ResolverOptionsBuilder):
def __init__(self,
fetchers=None,
allow_all_external=False,
allow_external=None,
allow_unverified=None,
allow_prereleases=None,
precedence=None,
context=None
):
super(BuilderWithFetcher, self).__init__(fetchers=fetchers,
allow_all_external=allow_all_external,
allow_external=allow_external,
allow_unverified=allow_unverified,
allow_prereleases=allow_prereleases,
precedence=precedence,
context=context)
self._fetchers.insert(0, fetcher)
# end stub
#####

# Without a corresponding fix in pex.py, this test failed for a dependency requirement of
# dep==1.2.3b1 from one package and just dep (any version accepted) from another package.
# The failure was an exit from build_pex() with the message:
#
# Could not satisfy all requirements for dep==1.2.3b1:
# dep==1.2.3b1, dep
#
# With a correct behavior the assert line is reached and pex_builder object created.
with mock.patch.object(pex.resolver, 'ResolverOptionsBuilder', BuilderWithFetcher):
pex_builder = build_pex(reqs, options, resolver_options_builder)
assert pex_builder is not None
27 changes: 27 additions & 0 deletions tests/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,33 @@ def assert_resolve(dep, expected_version, **resolve_kwargs):
assert_resolve('dep>=1.rc1,<4', '3.0.0rc3', fetchers=[])


def test_resolve_prereleases_and_no_version():
prerelease_dep = make_sdist(name='dep', version='3.0.0rc3')

with temporary_dir() as td:
safe_copy(prerelease_dep, os.path.join(td, os.path.basename(prerelease_dep)))
fetchers = [Fetcher([td])]

def assert_resolve(deps, expected_version, **resolve_kwargs):
dists = list(
resolve_multi(deps, fetchers=fetchers, **resolve_kwargs)
)
assert 1 == len(dists)
dist = dists[0]
assert expected_version == dist.version

# When allow_prereleases is specified, the requirement (from two dependencies)
# for a specific pre-release version and no version specified, accepts the pre-release
# version correctly.
assert_resolve(['dep==3.0.0rc3', 'dep'], '3.0.0rc3', allow_prereleases=True)

# Without allow_prereleases set, the pre-release version is rejected.
# This used to be an issue when a command-line use did not pass the `--pre` option
# correctly into the API call for resolve_multi() from build_pex() in pex.py.
with pytest.raises(Unsatisfiable):
assert_resolve(['dep==3.0.0rc3', 'dep'], '3.0.0rc3')


def test_resolve_prereleases_multiple_set():
stable_dep = make_sdist(name='dep', version='2.0.0')
prerelease_dep1 = make_sdist(name='dep', version='3.0.0rc3')
Expand Down

0 comments on commit cbb9e57

Please sign in to comment.