diff --git a/poetry/installation/executor.py b/poetry/installation/executor.py index 9effaf3ebeb..8e1dea6e5ff 100644 --- a/poetry/installation/executor.py +++ b/poetry/installation/executor.py @@ -103,29 +103,48 @@ def execute(self, operations): # type: (Operation) -> int self._display_summary(operations) # We group operations by priority - groups = itertools.groupby(operations, key=lambda o: -o.priority) + unique = 0 + + def groupkey(op): + if op.job_type == "uninstall": + # See Github issue https://github.com/python-poetry/poetry/issues/2658 + # Running uninstalls in parallel is not safe. Therefore, we run each + # uninstall in its separate group. + nonlocal unique + unique += 1 + return unique + else: + # We group the rest of the operations by priority. + return -op.priority + + groups = itertools.groupby(operations, key=groupkey) self._sections = OrderedDict() for _, group in groups: - tasks = [] - for operation in group: - if self._shutdown: - break - - tasks.append(self._executor.submit(self._execute_operation, operation)) + if self._execute_group(tuple(group)) == 1: + return 1 - try: - wait(tasks) - except KeyboardInterrupt: - self._shutdown = True + return 0 + def _execute_group(self, group): # type: (Operation) -> int + tasks = [] + for operation in group: if self._shutdown: - # Cancelling further tasks from being executed - [task.cancel() for task in tasks] - self._executor.shutdown(wait=True) - break - return 1 if self._shutdown else 0 + tasks.append(self._executor.submit(self._execute_operation, operation)) + + try: + wait(tasks) + except KeyboardInterrupt: + self._shutdown = True + + if self._shutdown: + # Cancelling further tasks from being executed + [task.cancel() for task in tasks] + self._executor.shutdown(wait=True) + return 1 + + return 0 def _write(self, operation, line): if not self.supports_fancy_output() or not self._should_write_operation( diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index bb659321d0f..ff81b4515c1 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -4,6 +4,8 @@ import re import shutil +from unittest import mock + import pytest from clikit.api.formatter.style import Style @@ -126,6 +128,29 @@ def test_execute_executes_a_batch_of_operations( assert 5 == len(env.executed) +def test_execute_uninstalls_serially(config, io): + executor = Executor(None, None, Config(), io) + executor._execute_group = mock.Mock() + + assert 0 == executor.execute( + [ + Uninstall(Package("pytest", "3.5.2")), + Uninstall(Package("attrs", "17.4.0")), + Uninstall(Package("requests", "2.18.3")), + Uninstall(Package("clikit", "0.2.3")), + ] + ) + + executed_groups = [] + for call in executor._execute_group.mock_calls: + ops = call.args[0] + assert all(op.job_type == "uninstall" for op in ops) + executed_groups.append({op.package.name for op in ops}) + + # Uninstalls executed serially in separate groups. + assert executed_groups == [{"pytest"}, {"attrs"}, {"requests"}, {"clikit"}] + + def test_execute_shows_skipped_operations_if_verbose(config, pool, io): config = Config() config.merge({"cache-dir": "/foo"})