🐛 fix(pypi): address review on clear_caches concurrency fix

webknjaz's review left several requests on the parallel-rebuild fix.

The renamed-aside directory variable moves from the generic `stale` to
`old_downloads_dir` so the name says what it holds. The suppressed
`OSError` now logs at `-v` rather than vanishing: a racing peer or an
OS-level rename refusal is worth a breadcrumb, and the error codes that
trigger it shift across platforms, so silently swallowing it hides a
real signal.

The changelog fragment moves out of imperative mood into the "X now
does Y" form the towncrier convention expects, routes the second
`pip-compile --rebuild` mention through the `{command}` role, and gains
the `by` keyword in the byline. `changelog.d/2384.bugfix.md` is a
symlink to `2393.bugfix.md` so the note renders under both the issue
and the PR.

The `pytest-mock` lower bound drops to match the unbounded style of the
sibling test dependencies (`pytest-rerunfailures`, `pytest-xdist`,
`tomli-w`); pinning one test helper to a single tested version while
the rest float was inconsistent. `test_clear_caches_swallows_replace_
failure` now asserts the rename was attempted against the live download
directory before the OSError short-circuits the rmtree.

Author: gaborbernat

changelog.d/2384.bugfix.md

@@ -0,0 +1 @@
+2393.bugfix.md

changelog.d/2393.bugfix.md

@@ -1,6 +1,6 @@
-Atomically rename the download directory before deleting it in
-{meth}`piptools.repositories.PyPIRepository.clear_caches` so a parallel
+{meth}`piptools.repositories.PyPIRepository.clear_caches` now atomically
+renames the download directory before deleting it, so a parallel
 {command}`pip-compile --rebuild` no longer trips over a half-deleted
-tree. The patch narrows one observable symptom and does not fully
-serialise concurrent `--rebuild` runs; broader cache locking remains
-future work -- {user}`gaborbernat`.
+tree. The fix narrows one observable symptom; it does not fully
+serialise concurrent {command}`pip-compile --rebuild` runs, and broader
+cache locking remains future work -- by {user}`gaborbernat`.

piptools/repositories/pypi.py

@@ -112,12 +112,17 @@ def clear_caches(self) -> None:
         # around the resolve, which is out of scope here.
         if not os.path.exists(self._download_dir):
             return
-        stale = f"{self._download_dir}.stale-{os.getpid()}"
+        old_downloads_dir = f"{self._download_dir}.stale-{os.getpid()}"
         try:
-            os.replace(self._download_dir, stale)
-        except OSError:
+            os.replace(self._download_dir, old_downloads_dir)
+        except OSError as exc:
+            # A racing peer renamed the tree first, or the OS refused the
+            # move (cross-device, permissions). The directory is no longer
+            # ours to clear; log at -v since the suppressed OSError is
+            # otherwise invisible and its error codes shift across platforms.
+            log.debug(f"clear_caches skipped {self._download_dir!r}: {exc}")
             return
-        rmtree(stale, ignore_errors=True)
+        rmtree(old_downloads_dir, ignore_errors=True)
 
     @property
     def options(self) -> optparse.Values:

pyproject.toml

@@ -58,7 +58,7 @@ changelog = "https://github.com/jazzband/pip-tools/releases"
 [project.optional-dependencies]
 testing = [
   "pytest >= 7.2.0",
-  "pytest-mock >= 3.15.1",
+  "pytest-mock >= 3.3.0",
   "pytest-rerunfailures",
   "pytest-xdist",
   "tomli-w",

tests/test_repository_pypi.py

@@ -201,15 +201,19 @@ def test_clear_caches_when_download_dir_missing_is_a_no_op(
 def test_clear_caches_swallows_replace_failure(
     repo_with_cache_dir: PyPIRepository, mocker: MockerFixture
 ) -> None:
-    Path(repo_with_cache_dir._download_dir).mkdir(parents=True, exist_ok=True)
-    mocker.patch(
+    download_dir = Path(repo_with_cache_dir._download_dir)
+    download_dir.mkdir(parents=True, exist_ok=True)
+    replace = mocker.patch(
         "piptools.repositories.pypi.os.replace",
         side_effect=OSError("racing peer renamed first"),
     )
     rmtree = mocker.patch("piptools.repositories.pypi.rmtree")
 
     repo_with_cache_dir.clear_caches()
 
+    # The rename targets the live download dir; the OSError short-circuits
+    # before the rmtree so a racing peer keeps ownership of the tree.
+    assert replace.call_args.args[0] == str(download_dir)
     rmtree.assert_not_called()