diff --git a/news/13540.bugfix.rst b/news/13540.bugfix.rst new file mode 100644 index 00000000000..a365f3edf17 --- /dev/null +++ b/news/13540.bugfix.rst @@ -0,0 +1,3 @@ +Avoid concurrency issues and improve performance when storing wheels into the +cache of locally built wheels, when the temporary build directory is on a +different filesystem than the cache directory. diff --git a/src/pip/_internal/operations/build/wheel.py b/src/pip/_internal/operations/build/wheel.py index 5e404c6102c..8595e45c548 100644 --- a/src/pip/_internal/operations/build/wheel.py +++ b/src/pip/_internal/operations/build/wheel.py @@ -14,7 +14,7 @@ def build_wheel_pep517( name: str, backend: BuildBackendHookCaller, metadata_directory: str, - tempd: str, + wheel_directory: str, ) -> str | None: """Build one InstallRequirement using the PEP 517 build process. @@ -22,17 +22,17 @@ def build_wheel_pep517( """ assert metadata_directory is not None try: - logger.debug("Destination directory: %s", tempd) + logger.debug("Destination directory: %s", wheel_directory) runner = runner_with_spinner_message( f"Building wheel for {name} (pyproject.toml)" ) with backend.subprocess_runner(runner): wheel_name = backend.build_wheel( - tempd, + wheel_directory=wheel_directory, metadata_directory=metadata_directory, ) except Exception: logger.error("Failed building wheel for %s", name) return None - return os.path.join(tempd, wheel_name) + return os.path.join(wheel_directory, wheel_name) diff --git a/src/pip/_internal/operations/build/wheel_editable.py b/src/pip/_internal/operations/build/wheel_editable.py index 521bd556c4a..f00d9b27ecb 100644 --- a/src/pip/_internal/operations/build/wheel_editable.py +++ b/src/pip/_internal/operations/build/wheel_editable.py @@ -14,7 +14,7 @@ def build_wheel_editable( name: str, backend: BuildBackendHookCaller, metadata_directory: str, - tempd: str, + wheel_directory: str, ) -> str | None: """Build one InstallRequirement using the PEP 660 build process. @@ -22,7 +22,7 @@ def build_wheel_editable( """ assert metadata_directory is not None try: - logger.debug("Destination directory: %s", tempd) + logger.debug("Destination directory: %s", wheel_directory) runner = runner_with_spinner_message( f"Building editable for {name} (pyproject.toml)" @@ -30,7 +30,7 @@ def build_wheel_editable( with backend.subprocess_runner(runner): try: wheel_name = backend.build_editable( - tempd, + wheel_directory=wheel_directory, metadata_directory=metadata_directory, ) except HookMissing as e: @@ -44,4 +44,4 @@ def build_wheel_editable( except Exception: logger.error("Failed building editable for %s", name) return None - return os.path.join(tempd, wheel_name) + return os.path.join(wheel_directory, wheel_name) diff --git a/src/pip/_internal/operations/build/wheel_legacy.py b/src/pip/_internal/operations/build/wheel_legacy.py index 02ef8e57075..cef0bd06687 100644 --- a/src/pip/_internal/operations/build/wheel_legacy.py +++ b/src/pip/_internal/operations/build/wheel_legacy.py @@ -33,7 +33,7 @@ def format_command_result( def get_legacy_build_wheel_path( names: list[str], - temp_dir: str, + wheel_directory: str, name: str, command_args: list[str], command_output: str, @@ -55,7 +55,7 @@ def get_legacy_build_wheel_path( msg += format_command_result(command_args, command_output) logger.warning(msg) - return os.path.join(temp_dir, names[0]) + return os.path.join(wheel_directory, names[0]) def build_wheel_legacy( @@ -64,7 +64,7 @@ def build_wheel_legacy( source_dir: str, global_options: list[str], build_options: list[str], - tempd: str, + wheel_directory: str, ) -> str | None: """Build one unpacked package using the "legacy" build process. @@ -89,12 +89,12 @@ def build_wheel_legacy( setup_py_path, global_options=global_options, build_options=build_options, - destination_dir=tempd, + destination_dir=wheel_directory, ) spin_message = f"Building wheel for {name} (setup.py)" with open_spinner(spin_message) as spinner: - logger.debug("Destination directory: %s", tempd) + logger.debug("Destination directory: %s", wheel_directory) try: output = call_subprocess( @@ -108,10 +108,10 @@ def build_wheel_legacy( logger.error("Failed building wheel for %s", name) return None - names = os.listdir(tempd) + names = os.listdir(wheel_directory) wheel_path = get_legacy_build_wheel_path( names=names, - temp_dir=tempd, + wheel_directory=wheel_directory, name=name, command_args=wheel_args, command_output=output, diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py index beed02f00c5..fa7e3521e75 100644 --- a/src/pip/_internal/wheel_builder.py +++ b/src/pip/_internal/wheel_builder.py @@ -5,8 +5,8 @@ import logging import os.path import re -import shutil from collections.abc import Iterable +from tempfile import TemporaryDirectory from pip._vendor.packaging.utils import canonicalize_name, canonicalize_version from pip._vendor.packaging.version import InvalidVersion, Version @@ -24,7 +24,6 @@ from pip._internal.utils.misc import ensure_dir, hash_file from pip._internal.utils.setuptools_build import make_setuptools_clean_args from pip._internal.utils.subprocess import call_subprocess -from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.urls import path_to_url from pip._internal.vcs import vcs @@ -189,7 +188,7 @@ def _build_one_inside_env( global_options: list[str], editable: bool, ) -> str | None: - with TempDirectory(kind="wheel") as temp_dir: + with TemporaryDirectory(dir=output_dir) as wheel_directory: assert req.name if req.use_pep517: assert req.metadata_directory @@ -207,14 +206,14 @@ def _build_one_inside_env( name=req.name, backend=req.pep517_backend, metadata_directory=req.metadata_directory, - tempd=temp_dir.path, + wheel_directory=wheel_directory, ) else: wheel_path = build_wheel_pep517( name=req.name, backend=req.pep517_backend, metadata_directory=req.metadata_directory, - tempd=temp_dir.path, + wheel_directory=wheel_directory, ) else: wheel_path = build_wheel_legacy( @@ -223,7 +222,7 @@ def _build_one_inside_env( source_dir=req.unpacked_source_directory, global_options=global_options, build_options=build_options, - tempd=temp_dir.path, + wheel_directory=wheel_directory, ) if wheel_path is not None: @@ -231,7 +230,10 @@ def _build_one_inside_env( dest_path = os.path.join(output_dir, wheel_name) try: wheel_hash, length = hash_file(wheel_path) - shutil.move(wheel_path, dest_path) + # We can do a rename here because wheel_path is guaranteed to be + # in the same filesystem as output_dir. An atomic is rename + # to avoid concurrency issues when populating the cache. + os.rename(wheel_path, dest_path) logger.info( "Created wheel for %s: filename=%s size=%d sha256=%s", req.name, diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 028ad3a4603..dcc9f10c33f 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -45,7 +45,7 @@ def call_get_legacy_build_wheel_path( ) -> str | None: wheel_path = get_legacy_build_wheel_path( names=names, - temp_dir="/tmp/abcd", + wheel_directory="/tmp/abcd", name="pendulum", command_args=["arg1", "arg2"], command_output="output line 1\noutput line 2\n",