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

Fix handling of pre-release option. #424

Merged
merged 1 commit into from
Oct 11, 2017
Merged
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
Fix handling of pre-release option.
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
zvezdan committed Oct 11, 2017
commit cbb9e5724b47eccdf63b69e2b48ede8230f5e6e8
3 changes: 2 additions & 1 deletion pex/bin/pex.py
Original file line number Diff line number Diff line change
@@ -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)
25 changes: 25 additions & 0 deletions pex/resolver_options.py
Original file line number Diff line number Diff line change
@@ -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
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
@@ -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
@@ -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')