Fix python/platform constraints not being picked up for subdependencies

7ae7e2b7 · Sébastien Eustace · d3491fe7 · 7ae7e2b7 · 7ae7e2b7 · 7ae7e2b7
Commit 7ae7e2b7 authored Mar 14, 2018 by Sébastien Eustace
13 changed files
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@
 - Fixed handling of post releases.
 - Fixed python restricted dependencies not being checked agaisnt virtualenv version.
+- Fixed python/platform constraint not being picked up for subdependencies.
 ## [0.4.2] - 2018-03-10

--- a/poetry/console/commands/lock.py
+++ b/poetry/console/commands/lock.py
 from poetry.installation import Installer
-from poetry.repositories.pypi_repository import PyPiRepository
 from .command import Command
@@ -9,7 +8,6 @@ class LockCommand(Command):
    Locks the project dependencies.
    lock
-        { --no-dev : Do not install dev dependencies. }
    """
    help = """The <info>lock</info> command reads the <comment>poetry.toml</> file from

--- a/poetry/installation/installer.py
+++ b/poetry/installation/installer.py
@@ -5,6 +5,7 @@ from typing import List
 from poetry.packages import Dependency
 from poetry.packages import Locker
 from poetry.packages import Package
+from poetry.packages.constraints.platform_constraint import PlatformConstraint
 from poetry.puzzle import Solver
 from poetry.puzzle.operations import Install
 from poetry.puzzle.operations import Uninstall
@@ -114,7 +115,7 @@ class Installer:
    def _do_install(self, local_repo):
        locked_repository = Repository()
        # initialize locked repo if we are installing from lock
-        if not self._update or self._locker.is_locked():
+        if not self._update:
            locked_repository = self._locker.locked_repository(True)
        if self._update:
@@ -131,6 +132,9 @@ class Installer:
            if self._whitelist:
                # collect packages to fixate from root requirements
                candidates = []
+                if self._locker.is_locked():
+                    locked_repository = self._locker.locked_repository(True)
                for package in locked_repository.packages:
                    candidates.append(package)
@@ -146,6 +150,8 @@ class Installer:
                            Dependency(candidate.name, candidate.version)
                        )
+            locked_repository = Repository()
            solver = Solver(
                self._package,
                self._pool,
@@ -389,6 +395,17 @@ class Installer:
                    op.skip('Not needed for the current python version')
                    continue
+            if 'platform' in package.requirements:
+                platform_constraint = PlatformConstraint.parse(
+                    package.requirements['platform']
+                )
+                if not platform_constraint.matches(
+                        PlatformConstraint('=', sys.platform)
+                ):
+                    # Incompatible systems
+                    op.skip('Not need for the current platform')
+                    continue
            if self._update:
                extras = {}
                for extra, deps in self._package.extras.items():

--- a/poetry/packages/constraints/__init__.py
+++ b/poetry/packages/constraints/__init__.py
--- a/poetry/packages/constraints/platform_constraint.py
+++ b/poetry/packages/constraints/platform_constraint.py
+import operator
+import re
+from poetry.semver.constraints import EmptyConstraint
+from poetry.semver.constraints import MultiConstraint
+from poetry.semver.constraints.base_constraint import BaseConstraint
+class PlatformConstraint(BaseConstraint):
+    OP_EQ = operator.eq
+    OP_NE = operator.ne
+    _trans_op_str = {
+        '=': OP_EQ,
+        '==': OP_EQ,
+        '!=': OP_NE
+    }
+    _trans_op_int = {
+        OP_EQ: '==',
+        OP_NE: '!='
+    }
+    def __init__(self, operator, platform):
+        if operator not in self._trans_op_str:
+            raise ValueError(
+                f'Invalid operator "{operator}" given, '
+                f'expected one of: {", ".join(self.supported_operators)}'
+            )
+        self._operator = self._trans_op_str[operator]
+        self._string_operator = operator
+        self._platform = platform
+    @property
+    def supported_operators(self) -> list:
+        return list(self._trans_op_str.keys())
+    @property
+    def operator(self):
+        return self._operator
+    @property
+    def string_operator(self):
+        return self._string_operator
+    @property
+    def platform(self) -> str:
+        return self._platform
+    def matches(self, provider):
+        if not isinstance(provider, PlatformConstraint):
+            raise ValueError(
+                'Platform constraints can only be compared with each other'
+            )
+        is_equal_op = self.OP_EQ is self._operator
+        is_non_equal_op = self.OP_NE is self._operator
+        is_provider_equal_op = self.OP_EQ is provider.operator
+        is_provider_non_equal_op = self.OP_NE is provider.operator
+        if (
+            is_equal_op and is_provider_equal_op
+            or is_non_equal_op and is_provider_non_equal_op
+        ):
+            return self._platform == provider.platform
+        if (
+            is_equal_op and is_provider_non_equal_op
+            or is_non_equal_op and is_provider_equal_op
+        ):
+            return self._platform != provider.platform
+        return False
+    @classmethod
+    def parse(cls, constraints):
+        """
+        Parses a constraint string into
+        MultiConstraint and/or PlatformConstraint objects.
+        """
+        pretty_constraint = constraints
+        or_constraints = re.split('\s*\|\|?\s*', constraints.strip())
+        or_groups = []
+        for constraints in or_constraints:
+            and_constraints = re.split(
+                '(?<!^)(?<![ ,]) *(?<!-)[, ](?!-) *(?!,|$)',
+                constraints
+            )
+            if len(and_constraints) > 1:
+                constraint_objects = []
+                for constraint in and_constraints:
+                    for parsed_constraint in cls._parse_constraint(constraint):
+                        constraint_objects.append(parsed_constraint)
+            else:
+                constraint_objects = cls._parse_constraint(and_constraints[0])
+            if len(constraint_objects) == 1:
+                constraint = constraint_objects[0]
+            else:
+                constraint = MultiConstraint(constraint_objects)
+            or_groups.append(constraint)
+        if len(or_groups) == 1:
+            constraint = or_groups[0]
+        else:
+            constraint = MultiConstraint(or_groups, False)
+        constraint.pretty_string = pretty_constraint
+        return constraint
+    @classmethod
+    def _parse_constraint(cls, constraint):
+        m = re.match('(?i)^v?[xX*](\.[xX*])*$', constraint)
+        if m:
+            return EmptyConstraint(),
+        # Basic Comparators
+        m = re.match('^(!=|==?)?\s*(.*)', constraint)
+        if m:
+            return PlatformConstraint(m.group(1) or '=', m.group(2)),
+        raise ValueError(
+            'Could not parse platform constraint: {}'.format(constraint)
+        )
+    def __str__(self):
+        op = self._trans_op_int[self._operator]
+        if op == '==':
+            op = ''
+        else:
+            op = op + ' '
+        return '{}{}'.format(
+            op,
+            self._platform
+        )
+    def __repr__(self):
+        return '<PlatformConstraint \'{}\'>'.format(str(self))
--- a/poetry/packages/dependency.py
+++ b/poetry/packages/dependency.py
 import poetry.packages
 from poetry.semver.constraints import Constraint
+from poetry.semver.constraints import EmptyConstraint
 from poetry.semver.constraints import MultiConstraint
-from poetry.semver.constraints.base_constraint import BaseConstraint
 from poetry.semver.version_parser import VersionParser
+from .constraints.platform_constraint import PlatformConstraint
 class Dependency:
@@ -31,7 +33,7 @@ class Dependency:
        self._python_versions = '*'
        self._python_constraint = self._parser.parse_constraints('*')
        self._platform = '*'
-        self._platform_constraint = self._parser.parse_constraints('*')
+        self._platform_constraint = EmptyConstraint()
        self._extras = []
@@ -75,6 +77,7 @@ class Dependency:
    @platform.setter
    def platform(self, value: str):
        self._platform = value
+        self._platform_constraint = PlatformConstraint.parse(value)
    @property
    def platform_constraint(self):
@@ -150,6 +153,12 @@ class Dependency:
        """
        self._optional = False
+    def deactivate(self):
+        """
+        Set the dependency as optional.
+        """
+        self._optional = True
    def __eq__(self, other):
        if not isinstance(other, Dependency):
            return NotImplemented

--- a/poetry/packages/locker.py
+++ b/poetry/packages/locker.py
@@ -95,6 +95,9 @@ class Locker:
            for dep_name, constraint in info.get('dependencies', {}).items():
                package.add_dependency(dep_name, constraint)
+            if 'requirements' in info:
+                package.requirements = info['requirements']
            if 'source' in info:
                package.source_type = info['source']['type']
                package.source_url = info['source']['url']

--- a/poetry/packages/package.py
+++ b/poetry/packages/package.py
@@ -2,10 +2,12 @@ import re
 from typing import Union
 from poetry.semver.constraints import Constraint
+from poetry.semver.constraints import EmptyConstraint
 from poetry.semver.helpers import parse_stability
 from poetry.semver.version_parser import VersionParser
 from poetry.version import parse as parse_version
+from.constraints.platform_constraint import PlatformConstraint
 from .dependency import Dependency
 from .vcs_dependency import VCSDependency
@@ -93,7 +95,7 @@ class Package:
        self._python_versions = '*'
        self._python_constraint = self._parser.parse_constraints('*')
        self._platform = '*'
-        self._platform_constraint = self._parser.parse_constraints('*')
+        self._platform_constraint = EmptyConstraint()
    @property
    def name(self):
@@ -180,7 +182,7 @@ class Package:
    @platform.setter
    def platform(self, value: str):
        self._platform = value
-        self._platform_constraint = self._parser.parse_constraints(value)
+        self._platform_constraint = PlatformConstraint.parse(value)
    @property
    def platform_constraint(self):

--- a/poetry/puzzle/solver.py
+++ b/poetry/puzzle/solver.py
@@ -24,7 +24,7 @@ class Solver:
        self._locked = locked
        self._io = io
-    def solve(self, requested, fixed=None, extras=None) -> List[Operation]:
+    def solve(self, requested, fixed=None) -> List[Operation]:
        resolver = Resolver(Provider(self._package, self._pool), UI(self._io))
        base = None
@@ -69,7 +69,7 @@ class Solver:
                        if current.matches(previous):
                            requirements['python'] = req
-                    if 'platform' in req:
+                    if req_name == 'platform':
                        if 'platform' not in requirements:
                            requirements['platform'] = req
                            continue
@@ -131,6 +131,14 @@ class Solver:
                    break
        else:
            for edge in vertex.incoming_edges:
+                for req in edge.origin.payload.requires:
+                    if req.name == vertex.payload.name:
+                        if req.python_versions != '*':
+                            tags['requirements']['python'].append(req.python_versions)
+                        if req.platform != '*':
+                            tags['requirements']['platform'].append(req.platform)
                sub_tags = self._get_tags_for_vertex(edge.origin, requested)
                tags['category'] += sub_tags['category']

--- a/poetry/repositories/pypi_repository.py
+++ b/poetry/repositories/pypi_repository.py
@@ -99,11 +99,9 @@ class PyPiRepository(Repository):
                dependency = Dependency(
                    name,
-                    version,
+                    version
-                    optional=req.markers
                )
-                is_extra = False
                if req.markers:
                    # Setting extra dependencies and requirements
                    requirements = self._convert_markers(
@@ -126,6 +124,9 @@ class PyPiRepository(Repository):
                        for or_ in requirements['sys_platform']:
                            ands = []
                            for op, platform in or_:
+                                if op == '==':
+                                    op = ''
                                ands.append(f'{op}{platform}')
                            ors.append(' '.join(ands))
@@ -133,7 +134,7 @@ class PyPiRepository(Repository):
                        dependency.platform = ' || '.join(ors)
                    if 'extra' in requirements:
-                        is_extra = True
+                        dependency.deactivate()
                        for _extras in requirements['extra']:
                            for _, extra in _extras:
                                if extra not in package.extras:
@@ -141,7 +142,7 @@ class PyPiRepository(Repository):
                                package.extras[extra].append(dependency)
-                if not is_extra:
+                if not dependency.is_optional():
                    package.requires.append(dependency)
            # Adding description

--- a/tests/installation/fixtures/with-platform-dependencies.test
+++ b/tests/installation/fixtures/with-platform-dependencies.test
+[[package]]
+name = "A"
+version = "1.0"
+description = ""
+category = "main"
+optional = true
+python-versions = "*"
+platform = "*"
+[[package]]
+name = "B"
+version = "1.1"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+platform = "*"
+[package.requirements]
+platform = "win32"
+[[package]]
+name = "C"
+version = "1.3"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+platform = "*"
+[package.dependencies]
+D = "^1.2"
+[package.requirements]
+platform = "darwin"
+[[package]]
+name = "D"
+version = "1.4"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+platform = "*"
+[package.requirements]
+platform = "darwin"
+[metadata]
+python-versions = "*"
+platform = "*"
+content-hash = "123456789"
+[metadata.hashes]
+A = []
+B = []
+C = []
+D = []
--- a/tests/installation/test_installer.py
+++ b/tests/installation/test_installer.py
@@ -62,14 +62,17 @@ class Locker(BaseLocker):
 @pytest.fixture(autouse=True)
 def setup():
-    # Mock python version to get reliable tests
+    # Mock python version and platform to get reliable tests
    original = sys.version_info
+    original_platform = sys.platform
    sys.version_info = (3, 6, 3, 'final', 0)
+    sys.platform = 'darwin'
    yield
    sys.version_info = original
+    sys.platform = original_platform
 @pytest.fixture()
@@ -305,6 +308,38 @@ def test_run_with_optional_and_python_restricted_dependencies(installer, locker,
    assert installer.installs[1].name == 'c'
+def test_run_with_optional_and_platform_restricted_dependencies(installer, locker, repo, package):
+    package_a = get_package('A', '1.0')
+    package_b = get_package('B', '1.1')
+    package_c12 = get_package('C', '1.2')
+    package_c13 = get_package('C', '1.3')
+    package_d = get_package('D', '1.4')
+    package_c13.add_dependency('D', '^1.2')
+    repo.add_package(package_a)
+    repo.add_package(package_b)
+    repo.add_package(package_c12)
+    repo.add_package(package_c13)
+    repo.add_package(package_d)
+    package.add_dependency('A', {'version': '~1.0', 'optional': True})
+    package.add_dependency('B', {'version': '^1.0', 'platform': 'win32'})
+    package.add_dependency('C', {'version': '^1.0', 'platform': 'darwin'})
+    installer.run()
+    expected = fixture('with-platform-dependencies')
+    assert locker.written_data == expected
+    installer = installer.installer
+    # We should only have 2 installs:
+    # C,D since the mocked python version is not compatible
+    # with B's python constraint and A is optional
+    assert len(installer.installs) == 2
+    assert installer.installs[0].name == 'd'
+    assert installer.installs[1].name == 'c'
 def test_run_with_dependencies_extras(installer, locker, repo, package):
    package_a = get_package('A', '1.0')
    package_b = get_package('B', '1.0')

--- a/tests/repositories/test_pypi_repository.py
+++ b/tests/repositories/test_pypi_repository.py
@@ -39,4 +39,4 @@ def test_package():
    win_inet = package.extras['socks'][0]
    assert win_inet.name == 'win-inet-pton'
    assert win_inet.python_versions == '==2.7 || ==2.6'
-    assert win_inet.platform == '==win32'
+    assert win_inet.platform == 'win32'