Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions news/13540.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like it to be just a bit more clear that a temporary directory is created inside the cache directory, e.g.

Avoid concurrency issues and improve performance, caused by the temporary build directory being on a different filesystem than the cache directory, by building wheels in a temporary sub-directory inside the cache directory.

8 changes: 4 additions & 4 deletions src/pip/_internal/operations/build/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,25 @@ 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.

Returns path to wheel if successfully built. Otherwise, returns None.
"""
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)
8 changes: 4 additions & 4 deletions src/pip/_internal/operations/build/wheel_editable.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,23 @@ 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.

Returns path to wheel if successfully built. Otherwise, returns None.
"""
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)"
)
with backend.subprocess_runner(runner):
try:
wheel_name = backend.build_editable(
tempd,
wheel_directory=wheel_directory,
metadata_directory=metadata_directory,
)
except HookMissing as e:
Expand All @@ -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)
14 changes: 7 additions & 7 deletions src/pip/_internal/operations/build/wheel_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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.

Expand All @@ -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(
Expand All @@ -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,
Expand Down
16 changes: 9 additions & 7 deletions src/pip/_internal/wheel_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -223,15 +222,18 @@ 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:
wheel_name = os.path.basename(wheel_path)
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,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading