Only write lock file when installation is success (#7498)

(affects `poetry add` and `poetry update`)

src/poetry/installation/installer.py

@@ -291,11 +291,9 @@ class Installer:
         lockfile_repo = LockfileRepository()
         lockfile_repo = LockfileRepository()
         self._populate_lockfile_repo(lockfile_repo, ops)
         self._populate_lockfile_repo(lockfile_repo, ops)
 
 
-        if self._update:
-            self._write_lock_file(lockfile_repo)
-
-            if self._lock:
+        if self._lock and self._update:
             # If we are only in lock mode, no need to go any further
             # If we are only in lock mode, no need to go any further
+            self._write_lock_file(lockfile_repo)
             return 0
             return 0
 
 
         if self._groups is not None:
         if self._groups is not None:
@@ -362,7 +360,13 @@ class Installer:
         self._filter_operations(ops, lockfile_repo)
         self._filter_operations(ops, lockfile_repo)
 
 
         # Execute operations
         # Execute operations
-        return self._execute(ops)
+        status = self._execute(ops)
+
+        if status == 0 and self._update:
+            # Only write lock file when installation is success
+            self._write_lock_file(lockfile_repo)
+
+        return status
 
 
     def _write_lock_file(self, repo: LockfileRepository, force: bool = False) -> None:
     def _write_lock_file(self, repo: LockfileRepository, force: bool = False) -> None:
         if self._write_lock and (force or self._update):
         if self._write_lock and (force or self._update):

src/poetry/packages/locker.py

@@ -224,6 +224,18 @@ class Locker:
         return repository
         return repository
 
 
     def set_lock_data(self, root: Package, packages: list[Package]) -> bool:
     def set_lock_data(self, root: Package, packages: list[Package]) -> bool:
+        """Store lock data and eventually persist to the lock file"""
+        lock = self._compute_lock_data(root, packages)
+
+        if self._should_write(lock):
+            self._write_lock_data(lock)
+            return True
+
+        return False
+
+    def _compute_lock_data(
+        self, root: Package, packages: list[Package]
+    ) -> TOMLDocument:
         package_specs = self._lock_packages(packages)
         package_specs = self._lock_packages(packages)
         # Retrieving hashes
         # Retrieving hashes
         for package in package_specs:
         for package in package_specs:
@@ -254,6 +266,10 @@ class Locker:
             "content-hash": self._content_hash,
             "content-hash": self._content_hash,
         }
         }
 
 
+        return lock
+
+    def _should_write(self, lock: TOMLDocument) -> bool:
+        # if lock file exists: compare with existing lock data
         do_write = True
         do_write = True
         if self.is_locked():
         if self.is_locked():
             try:
             try:
@@ -263,8 +279,6 @@ class Locker:
                 pass
                 pass
             else:
             else:
                 do_write = lock != lock_data
                 do_write = lock != lock_data
-        if do_write:
-            self._write_lock_data(lock)
         return do_write
         return do_write
 
 
     def _write_lock_data(self, data: TOMLDocument) -> None:
     def _write_lock_data(self, data: TOMLDocument) -> None:

tests/console/commands/self/test_add_plugins.py

@@ -49,11 +49,11 @@ Using version ^0.1.0 for poetry-plugin
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 1 install, 0 updates, 0 removals
 Package operations: 1 install, 0 updates, 0 removals
 
 
   • Installing poetry-plugin (0.1.0)
   • Installing poetry-plugin (0.1.0)
+
+Writing lock file
 """
 """
     assert_plugin_add_result(tester, expected, "^0.1.0")
     assert_plugin_add_result(tester, expected, "^0.1.0")
 
 
@@ -71,11 +71,11 @@ def test_add_with_constraint(
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 1 install, 0 updates, 0 removals
 Package operations: 1 install, 0 updates, 0 removals
 
 
   • Installing poetry-plugin (0.2.0)
   • Installing poetry-plugin (0.2.0)
+
+Writing lock file
 """
 """
 
 
     assert_plugin_add_result(tester, expected, "^0.2.0")
     assert_plugin_add_result(tester, expected, "^0.2.0")
@@ -93,12 +93,12 @@ def test_add_with_git_constraint(
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 2 installs, 0 updates, 0 removals
 Package operations: 2 installs, 0 updates, 0 removals
 
 
   • Installing pendulum (2.0.5)
   • Installing pendulum (2.0.5)
   • Installing poetry-plugin (0.1.2 9cf87a2)
   • Installing poetry-plugin (0.1.2 9cf87a2)
+
+Writing lock file
 """
 """
 
 
     assert_plugin_add_result(
     assert_plugin_add_result(
@@ -119,13 +119,13 @@ def test_add_with_git_constraint_with_extras(
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 3 installs, 0 updates, 0 removals
 Package operations: 3 installs, 0 updates, 0 removals
 
 
   • Installing pendulum (2.0.5)
   • Installing pendulum (2.0.5)
   • Installing tomlkit (0.7.0)
   • Installing tomlkit (0.7.0)
   • Installing poetry-plugin (0.1.2 9cf87a2)
   • Installing poetry-plugin (0.1.2 9cf87a2)
+
+Writing lock file
 """
 """
 
 
     assert_plugin_add_result(
     assert_plugin_add_result(
@@ -162,12 +162,12 @@ def test_add_with_git_constraint_with_subdirectory(
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 2 installs, 0 updates, 0 removals
 Package operations: 2 installs, 0 updates, 0 removals
 
 
   • Installing pendulum (2.0.5)
   • Installing pendulum (2.0.5)
   • Installing poetry-plugin (0.1.2 9cf87a2)
   • Installing poetry-plugin (0.1.2 9cf87a2)
+
+Writing lock file
 """
 """
 
 
     constraint = {
     constraint = {
@@ -262,11 +262,11 @@ Using version ^2.3.4 for poetry-plugin
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 0 installs, 1 update, 0 removals
 Package operations: 0 installs, 1 update, 0 removals
 
 
   • Updating poetry-plugin (1.2.3 -> 2.3.4)
   • Updating poetry-plugin (1.2.3 -> 2.3.4)
+
+Writing lock file
 """
 """
 
 
     assert_plugin_add_result(tester, expected, "^2.3.4")
     assert_plugin_add_result(tester, expected, "^2.3.4")
@@ -298,12 +298,12 @@ Using version ^1.2.3 for poetry-plugin
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 1 install, 1 update, 0 removals
 Package operations: 1 install, 1 update, 0 removals
 
 
   • Updating tomlkit (0.7.1 -> 0.7.2)
   • Updating tomlkit (0.7.1 -> 0.7.2)
   • Installing poetry-plugin (1.2.3)
   • Installing poetry-plugin (1.2.3)
+
+Writing lock file
 """
 """
 
 
     assert_plugin_add_result(tester, expected, "^1.2.3")
     assert_plugin_add_result(tester, expected, "^1.2.3")

tests/console/commands/self/test_remove_plugins.py

@@ -73,11 +73,11 @@ def test_remove_installed_package(tester: CommandTester):
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 0 installs, 0 updates, 1 removal
 Package operations: 0 installs, 0 updates, 1 removal
 
 
   • Removing poetry-plugin (1.2.3)
   • Removing poetry-plugin (1.2.3)
+
+Writing lock file
 """
 """
     assert tester.io.fetch_output() == expected
     assert tester.io.fetch_output() == expected
 
 

tests/console/commands/self/test_update.py

@@ -71,12 +71,12 @@ Using version ^{new_version} for poetry
 Updating dependencies
 Updating dependencies
 Resolving dependencies...
 Resolving dependencies...
 
 
-Writing lock file
-
 Package operations: 0 installs, 2 updates, 0 removals
 Package operations: 0 installs, 2 updates, 0 removals
 
 
   • Updating cleo (0.8.2 -> 1.0.0)
   • Updating cleo (0.8.2 -> 1.0.0)
   • Updating poetry ({__version__} -> {new_version})
   • Updating poetry ({__version__} -> {new_version})
+
+Writing lock file
 """
 """
 
 
     assert tester.io.fetch_output() == expected_output
     assert tester.io.fetch_output() == expected_output

tests/installation/test_installer.py

@@ -105,6 +105,7 @@ class Locker(BaseLocker):
         self._lock = lock_path / "poetry.lock"
         self._lock = lock_path / "poetry.lock"
         self._written_data = None
         self._written_data = None
         self._locked = False
         self._locked = False
+        self._lock_data = None
         self._content_hash = self._get_content_hash()
         self._content_hash = self._get_content_hash()
 
 
     @property
     @property
@@ -2415,6 +2416,29 @@ def test_installer_can_handle_old_lock_files(
     assert installer.executor.installations_count == 8
     assert installer.executor.installations_count == 8
 
 
 
 
+def test_installer_does_not_write_lock_file_when_installation_fails(
+    installer: Installer,
+    locker: Locker,
+    repo: Repository,
+    package: ProjectPackage,
+    mocker: MockerFixture,
+):
+    repo.add_package(get_package("A", "1.0"))
+    package.add_dependency(Factory.create_dependency("A", "~1.0"))
+
+    locker.locked(False)
+
+    mocker.patch("poetry.installation.installer.Installer._execute", return_value=1)
+    result = installer.run()
+    assert result == 1  # error
+
+    assert locker._lock_data is None
+
+    assert installer.executor.installations_count == 0
+    assert installer.executor.updates_count == 0
+    assert installer.executor.removals_count == 0
+
+
 @pytest.mark.parametrize("quiet", [True, False])
 @pytest.mark.parametrize("quiet", [True, False])
 def test_run_with_dependencies_quiet(
 def test_run_with_dependencies_quiet(
     installer: Installer,
     installer: Installer,

tests/installation/test_installer_old.py

@@ -62,6 +62,7 @@ class Locker(BaseLocker):
         self._lock = lock_path / "poetry.lock"
         self._lock = lock_path / "poetry.lock"
         self._written_data = None
         self._written_data = None
         self._locked = False
         self._locked = False
+        self._lock_data = None
         self._content_hash = self._get_content_hash()
         self._content_hash = self._get_content_hash()
 
 
     @property
     @property