pip: PEP-518 support is broken

  • Pip version: 10.0.0b2-17-gc76452ad
  • Python version: Python 3.6
  • Operating system: Arch Linux

Description:

Steps to reproduce (run from pip’ source directory):

> python setup.py -q egg_info
> mkdir test_project
> >test_project/setup.py <<\EOF
from setuptools import setup
setup(name='project', version='0.1')
EOF
> >test_project/pyproject.toml <<\EOF
[build-system]              
requires = ["setuptools>=34.4.0", "wheel"]
EOF
> python -m venv --without-pip venv
> PYTHONPATH=$PWD/src/ ./venv/bin/python -m pip freeze --all
pip==10.0.0b2
> PYTHONPATH=$PWD/src/ ./venv/bin/python -m pip install ./test_project
Processing ./test_project
  None does not include 'setuptools' as a buildtime requirement in its pyproject.toml.
  This version of pip does not implement PEP 517 so it cannot build a wheel without setuptools.
  Installing build dependencies ... done
Installing collected packages: project
Could not import setuptools which is required to install from a source distribution.
Please install setuptools.

Note the 2 additional minor issues:

  • None instead of the package name because it’s not yet known
  • the check for setuptools presence in the build requirements fails

Additionally, now that PEP 518 is implemented, building a wheel should be possible even if the wheel package is not installed (as long as it’s in the build requirements):

> PYTHONPATH=$PWD/src/ ./venv/bin/python -m pip wheel ./test_project
ERROR: 'pip wheel' requires the 'wheel' package. To fix this, run: pip install wheel

Tentative patch:

 src/pip/_internal/commands/wheel.py     | 21 -----------------
 src/pip/_internal/operations/prepare.py | 24 ++++++++++---------
 src/pip/_internal/req/req_install.py    | 42 ++++++++++-----------------------
 3 files changed, 26 insertions(+), 61 deletions(-)

diff --git i/src/pip/_internal/commands/wheel.py w/src/pip/_internal/commands/wheel.py
index 8a85d873..a6788e37 100644
--- i/src/pip/_internal/commands/wheel.py
+++ w/src/pip/_internal/commands/wheel.py
@@ -102,28 +102,7 @@ class WheelCommand(RequirementCommand):
         self.parser.insert_option_group(0, index_opts)
         self.parser.insert_option_group(0, cmd_opts)
 
-    def check_required_packages(self):
-        import_or_raise(
-            'wheel.bdist_wheel',
-            CommandError,
-            "'pip wheel' requires the 'wheel' package. To fix this, run: "
-            "pip install wheel"
-        )
-
-        need_setuptools_message = (
-            "'pip wheel' requires setuptools >= 0.8 for dist-info support. "
-            "To fix this, run: pip install --upgrade setuptools>=0.8"
-        )
-        pkg_resources = import_or_raise(
-            'pkg_resources',
-            CommandError,
-            need_setuptools_message
-        )
-        if not hasattr(pkg_resources, 'DistInfoDistribution'):
-            raise CommandError(need_setuptools_message)
-
     def run(self, options, args):
-        self.check_required_packages()
         cmdoptions.check_install_build_global(options)
 
         index_urls = [options.index_url] + options.extra_index_urls
diff --git i/src/pip/_internal/operations/prepare.py w/src/pip/_internal/operations/prepare.py
index 2c4ff94c..c61d8873 100644
--- i/src/pip/_internal/operations/prepare.py
+++ w/src/pip/_internal/operations/prepare.py
@@ -126,25 +126,27 @@ class IsSDist(DistAbstraction):
         build_requirements, isolate = self.req.get_pep_518_info()
         should_isolate = build_isolation and isolate
 
-        if 'setuptools' not in build_requirements:
+        if not {'setuptools', 'wheel'}.issubset(
+            pkg_resources.Requirement(r).key
+            for r in build_requirements
+        ):
             logger.warning(
-                "%s does not include 'setuptools' as a buildtime requirement "
-                "in its pyproject.toml.", self.req.name,
+                "%s does not include 'setuptools' and/or 'wheel' as buildtime "
+                "requirements in its pyproject.toml.", self.req,
             )
             logger.warning(
                 "This version of pip does not implement PEP 517 so it cannot "
-                "build a wheel without setuptools."
+                "build a wheel without those."
             )
 
-        if not should_isolate:
-            self.req.build_env = NoOpBuildEnvironment(no_clean=False)
-
-        with self.req.build_env as prefix:
-            if should_isolate:
+        if should_isolate:
+            with self.req.build_env as prefix:
                 _install_build_reqs(finder, prefix, build_requirements)
+        else:
+            self.req.build_env = NoOpBuildEnvironment(no_clean=False)
 
-            self.req.run_egg_info()
-            self.req.assert_source_matches_version()
+        self.req.run_egg_info()
+        self.req.assert_source_matches_version()
 
 
 class Installed(DistAbstraction):
diff --git i/src/pip/_internal/req/req_install.py w/src/pip/_internal/req/req_install.py
index 04bb3fd6..ddd167c6 100644
--- i/src/pip/_internal/req/req_install.py
+++ w/src/pip/_internal/req/req_install.py
@@ -413,24 +413,6 @@ class InstallRequirement(object):
     @property
     def setup_py(self):
         assert self.source_dir, "No source dir for %s" % self
-        cmd = [sys.executable, '-c', 'import setuptools']
-        output = call_subprocess(
-            cmd,
-            show_stdout=False,
-            command_desc='python -c "import setuptools"',
-            on_returncode='ignore',
-        )
-
-        if output:
-            if get_installed_version('setuptools') is None:
-                add_msg = "Please install setuptools."
-            else:
-                add_msg = output
-            # Setuptools is not available
-            raise InstallationError(
-                "Could not import setuptools which is required to "
-                "install from a source distribution.\n%s" % add_msg
-            )
 
         setup_py = os.path.join(self.setup_py_dir, 'setup.py')
 
@@ -496,11 +478,12 @@ class InstallRequirement(object):
                 egg_info_dir = os.path.join(self.setup_py_dir, 'pip-egg-info')
                 ensure_dir(egg_info_dir)
                 egg_base_option = ['--egg-base', 'pip-egg-info']
-            call_subprocess(
-                egg_info_cmd + egg_base_option,
-                cwd=self.setup_py_dir,
-                show_stdout=False,
-                command_desc='python setup.py egg_info')
+            with self.build_env:
+                call_subprocess(
+                    egg_info_cmd + egg_base_option,
+                    cwd=self.setup_py_dir,
+                    show_stdout=False,
+                    command_desc='python setup.py egg_info')
 
         if not self.req:
             if isinstance(parse_version(self.pkg_info()["Version"]), Version):
@@ -788,12 +771,13 @@ class InstallRequirement(object):
             msg = 'Running setup.py install for %s' % (self.name,)
             with open_spinner(msg) as spinner:
                 with indent_log():
-                    call_subprocess(
-                        install_args + install_options,
-                        cwd=self.setup_py_dir,
-                        show_stdout=False,
-                        spinner=spinner,
-                    )
+                    with self.build_env:
+                        call_subprocess(
+                            install_args + install_options,
+                            cwd=self.setup_py_dir,
+                            show_stdout=False,
+                            spinner=spinner,
+                        )
 
             if not os.path.exists(record_filename):
                 logger.debug('Record file %s not found', record_filename)

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 20 (19 by maintainers)

Most upvoted comments

A package without PEP 518 support, should not have build isolation since that’s a (big) change in behavior – that’ll break things in an unexpected way for people. It might happen that someone installs a PEP 518 package and a legacy package in the same pip run and one wants isolation while the other doesn’t. This is an important case to have working for an easy transition – hence we can’t have a all-in-or-nothing approach here.

I do agree there’s a lot of code paths and honestly, the best way to fix that would be refactors to make life easier like #5051.

Or the message could be changed to show the missing requirements.