Refactor WheelBuilder.build somewhat

sbidoul · Nov 3, 2019 · 7d57860 · 7d57860
1 parent 30f2740
commit 7d57860
Showing 1 changed file with 32 additions and 49 deletions.
diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
@@ -1102,7 +1102,7 @@ def build(
         :param should_unpack: If True, after building the wheel, unpack it
             and replace the sdist with the unpacked version in preparation
             for installation.
-        :return: True if all the wheels built correctly.
+        :return: The list of InstallRequirement that failed to build.
         """
         # pip install uses should_unpack=True.
         # pip install never provides a _wheel_dir.
@@ -1114,64 +1114,51 @@ def build(
         )
 
         buildset = []
-        cache_available = bool(self.wheel_cache.cache_dir)
 
+        # Determine what to build and whether to cache it or not.
+        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)
+                cache_dir = self.wheel_cache.get_path_for_link(req.link)
             else:
-                output_dir = self.wheel_cache.get_ephem_path_for_link(
+                cache_dir = self.wheel_cache.get_ephem_path_for_link(req.link)
-                    req.link
+            buildset.append((req, cache_dir))
-                )
-
-            buildset.append((req, output_dir))
 
         if not buildset:
             return []
 
-        # TODO by @pradyunsg
+        python_tag = None
-        # Should break up this method into 2 separate methods.
+        if should_unpack:
+            python_tag = pep425tags.implementation_tag
 
         # Build the wheels.
         logger.info(
             'Building wheels for collected packages: %s',
             ', '.join([req.name for (req, _) in buildset]),
         )
-
+        build_success, build_failure = [], []
-        python_tag = None
-        if should_unpack:
-            python_tag = pep425tags.implementation_tag
-
         with indent_log():
-            build_success, build_failure = [], []
+            for req, cache_dir in buildset:
-            for req, output_dir in buildset:
                 try:
-                    ensure_dir(output_dir)
+                    ensure_dir(cache_dir)
-                except OSError as e:
+                    wheel_file = self._build_one(
-                    logger.warning(
+                        req, cache_dir,
-                        "Building wheel for %s failed: %s",
+                        python_tag=python_tag,
-                        req.name, e,
                     )
-                    build_failure.append(req)
+                    if not wheel_file:
-                    continue
+                        build_failure.append(req)
-
+                        continue
-                wheel_file = self._build_one(
-                    req, output_dir,
-                    python_tag=python_tag,
-                )
-                if wheel_file:
                     self.wheel_filenames.append(
-                        os.path.relpath(wheel_file, output_dir)
+                        os.path.relpath(wheel_file, cache_dir)
                     )
                     if should_unpack:
                         # XXX: This is mildly duplicative with prepare_files,
@@ -1198,24 +1185,20 @@ def build(
                         # extract the wheel into the dir
                         unpack_file(req.link.file_path, req.source_dir)
                     else:
-                        # copy from cache to target durectory
+                        # Copy from cache to target directory.
-                        try:
+                        ensure_dir(self._wheel_dir)
-                            ensure_dir(self._wheel_dir)
+                        shutil.copy(
-                            shutil.copy(
+                            os.path.join(cache_dir, wheel_file),
-                                os.path.join(output_dir, wheel_file),
+                            self._wheel_dir,
-                                self._wheel_dir,
+                        )
-                            )
+                except OSError as e:
-                        except OSError as e:
+                    logger.warning(
-                            logger.warning(
+                        "Building wheel for %s failed: %s",
-                                "Building wheel for %s failed: %s",
+                        req.name, e,
-                                req.name, e,
+                    )
-                            )
-                            build_failure.append(req)
-                            continue
-                    build_success.append(req)
-                else:
                     build_failure.append(req)
-
+                else:
+                    build_success.append(req)
         # notify success/failure
         if build_success:
             logger.info(