Fix extra installation

README.md

@@ -248,7 +248,7 @@ the `--no-dev` option.
 poetry install --no-dev
 ```
 
-You can also specify the extra features you want installed
+You can also specify the extras you want installed
 by passing the `--E|--extras` option (See [Extras](#extras) for more info)
 
 ```bash
@@ -259,7 +259,7 @@ poetry install -E mysql -E pgsql
 #### Options
 
 * `--no-dev`: Do not install dev dependencies.
-* `-f|--features`: Features to install (multiple values allowed).
+* `-E|--extras`: Features to install (multiple values allowed).
 
 ### update
 
@@ -635,7 +635,7 @@ name = "awesome"
 mandatory = "^1.0"
 
 # A list of all of the optional dependencies, some of which are included in the
-# above `features`. They can be opted into by apps.
+# below `extras`. They can be opted into by apps.
 psycopg2 = { version = "^2.7", optional = true }
 mysqlclient = { version = "^1.3", optional = true }
 
@@ -644,7 +644,7 @@ mysql = ["mysqlclient"]
 pgsql = ["psycopg2"]
 ```
 
-When installing packages, you can specify features by using the `-E|--extras` option:
+When installing packages, you can specify extras by using the `-E|--extras` option:
 
 ```bash
 poet install --extras "mysql pgsql"

poetry/installation/installer.py

@@ -183,7 +183,7 @@ class Installer:
         # We need to filter operations so that packages
         # not compatible with the current system,
         # or optional and not requested, are dropped
-        self._filter_operations(ops)
+        self._filter_operations(ops, local_repo)
 
         self._io.new_line()
 
@@ -336,12 +336,11 @@ class Installer:
                                   ) -> List[Operation]:
         installed_repo = InstalledRepository.load(self._io.venv)
         ops = []
-        extras = []
-        for extra_name, packages in self._locker.lock_data.get('extras').items():
-            if extra_name in self._extras:
-                for package in packages:
-                    extras.append(package.lower())
 
+        extra_packages = [
+            p.name
+            for p in self._get_extra_packages(locked_repository)
+        ]
         for locked in locked_repository.packages:
             is_installed = False
             for installed in installed_repo.packages:
@@ -349,7 +348,7 @@ class Installer:
                     is_installed = True
                     if locked.category == 'dev' and not self.is_dev_mode():
                         ops.append(Uninstall(locked))
-                    elif locked.is_optional() and locked.name not in extras:
+                    elif locked.optional and locked.name not in extra_packages:
                         # Installed but optional and not requested in extras
                         ops.append(Uninstall(locked))
                     elif locked.version != installed.version:
@@ -360,14 +359,16 @@ class Installer:
             if not is_installed:
                 # If it's optional and not in required extras
                 # we do not install
-                if locked.is_optional() and locked.name not in extras:
+                if locked.optional and locked.name not in extra_packages:
                     continue
 
                 ops.append(Install(locked))
 
         return ops
 
-    def _filter_operations(self, ops: List[Operation]):
+    def _filter_operations(self, ops: List[Operation], repo: Repository) -> None:
+        extra_packages = [p.name for p in
+                          self._get_extra_packages(repo)]
         for op in ops:
             if isinstance(op, Update):
                 package = op.target_package
@@ -394,20 +395,49 @@ class Installer:
                     extras[extra] = [dep.name for dep in deps]
             else:
                 extras = {}
-                for extra, deps in self._locker.lock_data.get('extras', {}):
+                for extra, deps in self._locker.lock_data.get('extras', {}).items():
                     extras[extra] = [dep.lower() for dep in deps]
 
             # If a package is optional and not requested
             # in any extra we skip it
             if package.optional:
-                drop = True
-                for extra in self._extras:
-                    if extra in extras and package.name in extras[extra]:
-                        drop = False
-                        continue
-
-                if drop:
+                if package.name not in extra_packages:
                     op.skip('Not required')
 
+    def _get_extra_packages(self, repo):
+        """
+        Returns all packages required by extras.
+
+        Maybe we just let the solver handle it?
+        """
+        if self._update:
+            extras = {
+                k: [d.name for d in v]
+                for k, v in self._package.extras.items()
+            }
+        else:
+            extras = self._locker.lock_data.get('extras', {})
+
+        extra_packages = []
+        for extra_name, packages in extras.items():
+            if extra_name not in self._extras:
+                continue
+
+            extra_packages += [Dependency(p, '*') for p in packages]
+
+        def _extra_packages(packages):
+            pkgs = []
+            for package in packages:
+                for pkg in repo.packages:
+                    if pkg.name == package.name:
+                        pkgs.append(package)
+                        pkgs += _extra_packages(pkg.requires)
+
+                        break
+
+            return pkgs
+
+        return _extra_packages(extra_packages)
+
     def _get_installer(self) -> BaseInstaller:
         return PipInstaller(self._io.venv, self._io)

poetry/poetry.py

 from pathlib import Path
 
 from .__version__ import __version__
+from .packages import Dependency
 from .packages import Locker
 from .packages import Package
 from .repositories import Pool
@@ -102,6 +103,12 @@ class Poetry:
             for name, constraint in local_config['dev-dependencies'].items():
                 package.add_dependency(name, constraint, category='dev')
 
+        if 'extras' in local_config:
+            for extra_name, requirements in local_config['extras'].items():
+                package.extras[extra_name] = [
+                    Dependency(req, '*') for req in requirements
+                ]
+
         locker = Locker(poetry_file.with_suffix('.lock'), local_config)
 
         return cls(poetry_file, local_config, package, locker)

poetry/puzzle/solver.py

@@ -24,7 +24,7 @@ class Solver:
         self._locked = locked
         self._io = io
 
-    def solve(self, requested, fixed=None) -> List[Operation]:
+    def solve(self, requested, fixed=None, extras=None) -> List[Operation]:
         resolver = Resolver(Provider(self._package, self._pool), UI(self._io))
 
         base = None
@@ -40,7 +40,7 @@ class Solver:
 
         packages = [v.payload for v in graph.vertices.values()]
 
-        # Setting categories
+        # Setting info
         for vertex in graph.vertices.values():
             tags = self._get_tags_for_vertex(vertex, requested)
             if 'main' in tags['category']:

tests/fixtures/sample_project/pyproject.toml

@@ -24,6 +24,12 @@ pendulum = { git = "https://github.com/sdispater/pendulum.git", branch = "2.0" }
 requests = { version = "^2.18", optional = true, extras=[ "security" ] }
 pathlib2 = { version = "^2.2", python = "~2.7" }
 
+orator = { version = "^0.9", optional = true}
+
+
+[tool.poetry.extras]
+db = [ "orator" ]
+
 [tool.poetry.dev-dependencies]
 pytest = "~3.4"
 

tests/installation/fixtures/extras-with-dependencies.test

+[[package]]
+name = "A"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+platform = "*"
+
+[[package]]
+name = "B"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+platform = "*"
+
+[[package]]
+name = "C"
+version = "1.0"
+description = ""
+category = "main"
+optional = true
+python-versions = "*"
+platform = "*"
+
+[package.dependencies]
+D = "^1.0"
+
+[[package]]
+name = "D"
+version = "1.1"
+description = ""
+category = "main"
+optional = true
+python-versions = "*"
+platform = "*"
+
+[extras]
+foo = ["C"]
+
+[metadata]
+python-versions = "*"
+platform = "*"
+content-hash = "123456789"
+
+[metadata.hashes]
+A = []
+B = []
+C = []
+D = []

tests/installation/test_installer.py

@@ -10,6 +10,7 @@ from poetry.io import NullIO
 from poetry.packages import Locker as BaseLocker
 from poetry.repositories import Pool
 from poetry.repositories import Repository
+from poetry.repositories.installed_repository import InstalledRepository
 
 from tests.helpers import get_dependency
 from tests.helpers import get_package
@@ -43,6 +44,9 @@ class Locker(BaseLocker):
     def is_locked(self) -> bool:
         return self._locked
 
+    def is_fresh(self) -> bool:
+        return True
+
     def _get_content_hash(self) -> str:
         return '123456789'
     
@@ -87,6 +91,16 @@ def pool(repo):
 
 
 @pytest.fixture()
+def installed():
+    original = InstalledRepository.load
+    InstalledRepository.load = lambda _: InstalledRepository()
+
+    yield
+
+    InstalledRepository.load = original
+
+
+@pytest.fixture()
 def locker():
     return Locker()
 
@@ -373,3 +387,65 @@ def test_run_installs_extras_if_requested(installer, locker, repo, package):
     installer = installer.installer
     assert len(installer.installs) == 4  # A, B, C, D
 
+
+def test_run_installs_extras_with_deps_if_requested(installer, locker, repo, package):
+    package.extras['foo'] = [
+        get_dependency('C')
+    ]
+    package_a = get_package('A', '1.0')
+    package_b = get_package('B', '1.0')
+    package_c = get_package('C', '1.0')
+    package_d = get_package('D', '1.1')
+
+    repo.add_package(package_a)
+    repo.add_package(package_b)
+    repo.add_package(package_c)
+    repo.add_package(package_d)
+
+    package.add_dependency('A', '^1.0')
+    package.add_dependency('B', '^1.0')
+    package.add_dependency('C', {'version': '^1.0', 'optional': True})
+
+    package_c.add_dependency('D', '^1.0')
+
+    installer.extras(['foo'])
+    installer.run()
+    expected = fixture('extras-with-dependencies')
+
+    # Extras are pinned in lock
+    assert locker.written_data == expected
+
+    # But should not be installed
+    installer = installer.installer
+    assert len(installer.installs) == 4  # A, B, C, D
+
+
+def test_run_installs_extras_with_deps_if_requested_locked(installer, locker, repo, package, installed):
+    locker.locked(True)
+    locker.mock_lock_data(fixture('extras-with-dependencies'))
+    package.extras['foo'] = [
+        get_dependency('C')
+    ]
+    package_a = get_package('A', '1.0')
+    package_b = get_package('B', '1.0')
+    package_c = get_package('C', '1.0')
+    package_d = get_package('D', '1.1')
+
+    repo.add_package(package_a)
+    repo.add_package(package_b)
+    repo.add_package(package_c)
+    repo.add_package(package_d)
+
+    package.add_dependency('A', '^1.0')
+    package.add_dependency('B', '^1.0')
+    package.add_dependency('C', {'version': '^1.0', 'optional': True})
+
+    package_c.add_dependency('D', '^1.0')
+
+    installer.extras(['foo'])
+    installer.run()
+
+    # But should not be installed
+    installer = installer.installer
+    assert len(installer.installs) == 4  # A, B, C, D
+

tests/test_poetry.py

@@ -50,3 +50,5 @@ def test_poetry():
     assert pathlib2.pretty_constraint == '^2.2'
     assert pathlib2.python_versions == '~2.7'
     assert not pathlib2.is_optional()
+
+    assert 'db' in package.extras