Merge pull request #6236 from pradyunsg/fix/pip-wheel-locations

Copy the correct file during PEP 517's build process

Author: pradyunsg

news/6196.bugfix

@@ -0,0 +1 @@
+Ensure the correct wheel file is copied when building PEP 517 distribution is built.

src/pip/_internal/wheel.py

@@ -818,15 +818,14 @@ def _build_one_inside_env(self, req, output_dir, python_tag=None):
                 builder = self._build_one_pep517
             else:
                 builder = self._build_one_legacy
-            if builder(req, temp_dir.path, python_tag=python_tag):
+            wheel_path = builder(req, temp_dir.path, python_tag=python_tag)
+            if wheel_path is not None:
+                wheel_name = os.path.basename(wheel_path)
+                dest_path = os.path.join(output_dir, wheel_name)
                 try:
-                    wheel_name = os.listdir(temp_dir.path)[0]
-                    wheel_path = os.path.join(output_dir, wheel_name)
-                    shutil.move(
-                        os.path.join(temp_dir.path, wheel_name), wheel_path
-                    )
+                    shutil.move(wheel_path, dest_path)
                     logger.info('Stored in directory: %s', output_dir)
-                    return wheel_path
+                    return dest_path
                 except Exception:
                     pass
             # Ignore return, we can't do anything else useful.
@@ -844,29 +843,39 @@ def _base_setup_args(self, req):
         ] + list(self.global_options)
 
     def _build_one_pep517(self, req, tempd, python_tag=None):
+        """Build one InstallRequirement using the PEP 517 build process.
+
+        Returns path to wheel if successfully built. Otherwise, returns None.
+        """
         assert req.metadata_directory is not None
         try:
             req.spin_message = 'Building wheel for %s (PEP 517)' % (req.name,)
             logger.debug('Destination directory: %s', tempd)
-            wheelname = req.pep517_backend.build_wheel(
+            wheel_name = req.pep517_backend.build_wheel(
                 tempd,
                 metadata_directory=req.metadata_directory
             )
             if python_tag:
                 # General PEP 517 backends don't necessarily support
                 # a "--python-tag" option, so we rename the wheel
                 # file directly.
-                newname = replace_python_tag(wheelname, python_tag)
+                new_name = replace_python_tag(wheel_name, python_tag)
                 os.rename(
-                    os.path.join(tempd, wheelname),
-                    os.path.join(tempd, newname)
+                    os.path.join(tempd, wheel_name),
+                    os.path.join(tempd, new_name)
                 )
-            return True
+                # Reassign to simplify the return at the end of function
+                wheel_name = new_name
         except Exception:
             logger.error('Failed building wheel for %s', req.name)
-            return False
+            return None
+        return os.path.join(tempd, wheel_name)
 
     def _build_one_legacy(self, req, tempd, python_tag=None):
+        """Build one InstallRequirement using the "legacy" build process.
+
+        Returns path to wheel if successfully built. Otherwise, returns None.
+        """
         base_args = self._base_setup_args(req)
 
         spin_message = 'Building wheel for %s (setup.py)' % (req.name,)
@@ -881,11 +890,12 @@ def _build_one_legacy(self, req, tempd, python_tag=None):
             try:
                 call_subprocess(wheel_args, cwd=req.setup_py_dir,
                                 show_stdout=False, spinner=spinner)
-                return True
             except Exception:
                 spinner.finish("error")
                 logger.error('Failed building wheel for %s', req.name)
-                return False
+                return None
+            # listdir's return value is sorted to be deterministic
+            return os.path.join(tempd, sorted(os.listdir(tempd))[0])
 
     def _clean_one(self, req):
         base_args = self._base_setup_args(req)

tests/functional/test_wheel.py

@@ -14,6 +14,13 @@ def auto_with_wheel(with_wheel):
     pass
 
 
+def add_files_to_dist_directory(folder):
+    (folder / 'dist').makedirs()
+    (folder / 'dist' / 'a_name-0.0.1.tar.gz').write("hello")
+    # Not adding a wheel file since that confuses setuptools' backend.
+    # (folder / 'dist' / 'a_name-0.0.1-py2.py3-none-any.whl').write("hello")
+
+
 def test_wheel_exit_status_code_when_no_requirements(script):
     """
     Test wheel exit status code when no requirements specified
@@ -210,3 +217,31 @@ def test_pip_wheel_with_user_set_in_config(script, data, common_wheels):
         '--no-index', '-f', common_wheels
     )
     assert "Successfully built withpyproject" in result.stdout, result.stdout
+
+
+def test_pep517_wheels_are_not_confused_with_other_files(script, tmpdir, data):
+    """Check correct wheels are copied. (#6196)
+    """
+    pkg_to_wheel = data.src / 'withpyproject'
+    add_files_to_dist_directory(pkg_to_wheel)
+
+    result = script.pip('wheel', pkg_to_wheel, '-w', script.scratch_path)
+    assert "Installing build dependencies" in result.stdout, result.stdout
+
+    wheel_file_name = 'withpyproject-0.0.1-py%s-none-any.whl' % pyversion[0]
+    wheel_file_path = script.scratch / wheel_file_name
+    assert wheel_file_path in result.files_created, result.stdout
+
+
+def test_legacy_wheels_are_not_confused_with_other_files(script, tmpdir, data):
+    """Check correct wheels are copied. (#6196)
+    """
+    pkg_to_wheel = data.src / 'simplewheel-1.0'
+    add_files_to_dist_directory(pkg_to_wheel)
+
+    result = script.pip('wheel', pkg_to_wheel, '-w', script.scratch_path)
+    assert "Installing build dependencies" not in result.stdout, result.stdout
+
+    wheel_file_name = 'simplewheel-1.0-py%s-none-any.whl' % pyversion[0]
+    wheel_file_path = script.scratch / wheel_file_name
+    assert wheel_file_path in result.files_created, result.stdout