Merge pull request #7476 from sbidoul/wheel-builder-disentangle-2-sbi

Some more trivial refactorings in WheelBuilder

Author: chrahunt

src/pip/_internal/wheel_builder.py

@@ -32,7 +32,7 @@
 
 if MYPY_CHECK_RUNNING:
     from typing import (
-        Any, Callable, Iterable, List, Optional, Pattern, Text, Union,
+        Any, Callable, Iterable, List, Optional, Pattern, Text, Tuple,
     )
 
     from pip._internal.cache import WheelCache
@@ -263,6 +263,37 @@ def _build_wheel_pep517(
     return os.path.join(tempd, wheel_name)
 
 
+def _collect_buildset(
+    requirements,  # type: Iterable[InstallRequirement]
+    wheel_cache,  # type: WheelCache
+    check_binary_allowed,  # type: BinaryAllowedPredicate
+    need_wheel,  # type: bool
+):
+    # type: (...) -> List[Tuple[InstallRequirement, str]]
+    """Return the list of InstallRequirement that need to be built,
+    with the persistent or temporary cache directory where the built
+    wheel needs to be stored.
+    """
+    buildset = []
+    cache_available = bool(wheel_cache.cache_dir)
+    for req in requirements:
+        if not should_build(
+            req,
+            need_wheel=need_wheel,
+            check_binary_allowed=check_binary_allowed,
+        ):
+            continue
+        if (
+            cache_available and
+            should_cache(req, check_binary_allowed)
+        ):
+            cache_dir = wheel_cache.get_path_for_link(req.link)
+        else:
+            cache_dir = wheel_cache.get_ephem_path_for_link(req.link)
+        buildset.append((req, cache_dir))
+    return buildset
+
+
 def _always_true(_):
     # type: (Any) -> bool
     return True
@@ -292,8 +323,6 @@ def __init__(
         self.build_options = build_options or []
         self.global_options = global_options or []
         self.check_binary_allowed = check_binary_allowed
-        # file names of built wheel names
-        self.wheel_filenames = []  # type: List[Union[bytes, Text]]
 
     def _build_one(
         self,
@@ -389,29 +418,12 @@ def build(
             (not should_unpack and self._wheel_dir)
         )
 
-        buildset = []
-        cache_available = bool(self.wheel_cache.cache_dir)
-
-        for req in requirements:
-            if not should_build(
-                req,
-                need_wheel=not should_unpack,
-                check_binary_allowed=self.check_binary_allowed,
-            ):
-                continue
-
-            if (
-                cache_available and
-                should_cache(req, self.check_binary_allowed)
-            ):
-                output_dir = self.wheel_cache.get_path_for_link(req.link)
-            else:
-                output_dir = self.wheel_cache.get_ephem_path_for_link(
-                    req.link
-                )
-
-            buildset.append((req, output_dir))
-
+        buildset = _collect_buildset(
+            requirements,
+            wheel_cache=self.wheel_cache,
+            check_binary_allowed=self.check_binary_allowed,
+            need_wheel=not should_unpack,
+        )
         if not buildset:
             return []
 
@@ -426,9 +438,9 @@ def build(
 
         with indent_log():
             build_success, build_failure = [], []
-            for req, output_dir in buildset:
+            for req, cache_dir in buildset:
                 try:
-                    ensure_dir(output_dir)
+                    ensure_dir(cache_dir)
                 except OSError as e:
                     logger.warning(
                         "Building wheel for %s failed: %s",
@@ -437,7 +449,7 @@ def build(
                     build_failure.append(req)
                     continue
 
-                wheel_file = self._build_one(req, output_dir)
+                wheel_file = self._build_one(req, cache_dir)
                 if wheel_file:
                     if should_unpack:
                         # XXX: This is mildly duplicative with prepare_files,
@@ -467,10 +479,7 @@ def build(
                         # copy from cache to target directory
                         try:
                             ensure_dir(self._wheel_dir)
-                            shutil.copy(
-                                os.path.join(output_dir, wheel_file),
-                                self._wheel_dir,
-                            )
+                            shutil.copy(wheel_file, self._wheel_dir)
                         except OSError as e:
                             logger.warning(
                                 "Building wheel for %s failed: %s",

tests/functional/test_wheel.py

@@ -98,6 +98,18 @@ def test_basic_pip_wheel_downloads_wheels(script, data):
     assert "Saved" in result.stdout, result.stdout
 
 
+def test_pip_wheel_build_relative_cachedir(script, data):
+    """
+    Test 'pip wheel' builds and caches with a non-absolute cache directory.
+    """
+    result = script.pip(
+        'wheel', '--no-index', '-f', data.find_links,
+        '--cache-dir', './cache',
+        'simple==3.0',
+    )
+    assert result.returncode == 0
+
+
 def test_pip_wheel_builds_when_no_binary_set(script, data):
     data.packages.joinpath('simple-3.0-py2.py3-none-any.whl').touch()
     # Check that the wheel package is ignored