setuptools: build_meta doesn't use a fresh dist directory, which causes ValueError unpacking tuple

If you use pip install . on a source directory that already has a dist, the existing directory structure is used when preparing the wheel. This causes an exception if an existing wheel with a different name exists, because build_meta assumes there’s only one wheel in dist.

Here’s a script to create a MWE repo:

#!/usr/bin/bash

mkdir /tmp/demo_dist_517
cd /tmp/demo_dist_517
echo "from setuptools import setup; setup()" > setup.py
echo "0.0.1" > VERSION
cat > setup.cfg << EOF
[metadata]
name = foo
version = file: VERSION
EOF
cat > pyproject.toml << EOF
[build-system]
requires = ["setuptools", "wheel"]
build-backend = "setuptools.build_meta"
EOF

At this point your repo looks like this:

$ tree
├── pyproject.toml
├── setup.cfg
├── setup.py
└── VERSION

Create a wheel in dist, then change the version:

pip wheel . --no-deps -w dist
echo "0.0.2" > VERSION

Now try to create a wheel from the repo:

pip wheel . -w dist

This will trigger an error in build_meta: File “/tmp/pip-build-env-plomixa1/overlay/…/setuptools/build_meta.py”, line 157, in _file_with_extension file, = matching ValueError: too many values to unpack (expected 1) Building wheel for foo (PEP 517) … error Failed building wheel for foo

This is pretty easy to work around, obviously, since the user can just remove dist before invoking any pip commands, but it might be better to do the wheel build in a clean directory if possible.

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 2
Comments: 33 (31 by maintainers)

Commits related to this issue

Ignore dists/ for now Because of a bug in setuptools (https://github.com/pypa/setuptools/issues/1671), building our files in `dist/` can cause build failures if dist/ is not cleaned out between build... — committed to pganssle/grid-strategy by pganssle 5 years ago
Ignore dists/ for now Because of a bug in setuptools (https://github.com/pypa/setuptools/issues/1671), building our files in `dist/` can cause build failures if dist/ is not cleaned out between build... — committed to pganssle/grid-strategy by pganssle 5 years ago
refactored the circleci wheel building workflow, specifically build_manylinux_wheels.sh, to leave all wheels in the wheelhouse after a run until a final run step in the circle config moves them all in... — committed to rndrr/pymssql by jsmntv 5 years ago
Improve stability of manylinux1 build script This introduces workaround for setuptools' bug in PEP517 build backend: https://github.com/pypa/setuptools/issues/1671 — committed to aio-libs/aiohttp by webknjaz 5 years ago
Merge pull request #1726 from florisla/issue-1671-test Add failing test for issue #1671 (ValueError when .whl is present) — committed to pypa/setuptools by mergify[bot] 5 years ago
Add fix for #1671 — committed to shashanksingh28/setuptools by shashanksingh28 5 years ago
Fix error when wheels already exist in dist/ `build_meta.build_wheel` assumes that the only wheel in its output directory is the one it builds, but prior to this, it also used the `dist/` folder as i... — committed to shashanksingh28/setuptools by shashanksingh28 5 years ago
Fix error when wheels already exist in dist/ `build_meta.build_wheel` assumes that the only wheel in its output directory is the one it builds, but prior to this, it also used the `dist/` folder as i... — committed to shashanksingh28/setuptools by shashanksingh28 5 years ago

Most upvoted comments

I’ll do it.

benoit-pierre on Apr 22, 2019

@webknjaz I have no objections, but I won’t have much time until this weekend at the earliest. @benoit-pierre or @jaraco feel free to cut a release.

pganssle on Apr 22, 2019

Er, hold up, I just remembered that #1726 already implements a test for this and was just blocked on a bug that has since been fixed. Let me merge that.

pganssle on Apr 20, 2019

I like @benoit-pierre’s proposal with a tmpdir. It’s clean and easy to understand. One minor suggestion: move the return instruction outside of with-block.

webknjaz on Apr 20, 2019

OK, I don’t think there’s any harm in creating our own temporary directory for this. In that case we’ll probably also want to add a test (we can just parametrize the first test) for the situation where wheel_directory already has a wheel in it.

pganssle on Apr 20, 2019

It’s not explicitly mentioned in the PEP that it’s a temporary directory, at least not in the build_wheel section.

benoit-pierre on Apr 20, 2019

That’s what I was thinking:

 setuptools/build_meta.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git i/setuptools/build_meta.py w/setuptools/build_meta.py
index 47cbcbf6..51113856 100644
--- i/setuptools/build_meta.py
+++ w/setuptools/build_meta.py
@@ -32,6 +32,7 @@
 import tokenize
 import shutil
 import contextlib
+import tempfile
 
 import setuptools
 import distutils
@@ -182,14 +183,15 @@ def build_wheel(self, wheel_directory, config_settings=None,
                     metadata_directory=None):
         config_settings = self._fix_config(config_settings)
         wheel_directory = os.path.abspath(wheel_directory)
-        sys.argv = sys.argv[:1] + ['bdist_wheel'] + \
-            config_settings["--global-option"]
-        self.run_setup()
-        if wheel_directory != 'dist':
-            shutil.rmtree(wheel_directory)
-            shutil.copytree('dist', wheel_directory)
-
-        return _file_with_extension(wheel_directory, '.whl')
+        with tempfile.TemporaryDirectory(dir=wheel_directory) as tmp_dist_dir:
+            sys.argv = sys.argv[:1] + [
+                'bdist_wheel', '--dist-dir', tmp_dist_dir
+            ] + config_settings["--global-option"]
+            self.run_setup()
+            wheel_basename = _file_with_extension(tmp_dist_dir, '.whl')
+            os.rename(os.path.join(tmp_dist_dir, wheel_basename),
+                      os.path.join(wheel_directory, wheel_basename))
+            return wheel_basename
 
     def build_sdist(self, sdist_directory, config_settings=None):
         config_settings = self._fix_config(config_settings)

benoit-pierre on Apr 20, 2019