Improve message when setuptools is missing (PEP 518)

[pradyunsg]

No description provided.

[benoit-pierre]

Does it make sense to tell the user to upgrade to a non-existent version of pip? PEP 517 is still not supported, even in master, no?

[pradyunsg]

How does the following read? (current msg quoted)

This version of pip does not implement PEP 517, so it cannot build a wheel without setuptools. You need to upgrade to a newer version of pip that implements PEP 517.

This version of pip does not implement PEP 517, so it cannot build a wheel without setuptools. The next major version release of pip would implement PEP 517.

[pfmoore]

I agree woth @benoit-pierre . I'll need to check the context of this (no time now) but I'm confused as to what's going on here. I thought that pre-PEP 517, projects wouldn't explicitly include setuptools in their build-requires, and we'd simply assume that as now, setuptools and wheel would be required. Post PEP 517, we'd allow build tools to be specified, but still fall back to setuptools/wheel. So still entirely possible for setuptools nort to be present.

I'll check the wider code later, but as things stand I hope this is all irrelevant, as this message should never appear in practice (makes me wonder why it's appearing in the forkbomb example, though...)

[pradyunsg]

no time now

No hurries. I'm gonna sleep now anyways. =)

[pfmoore]

Ah, I now see what's going on here. PEP 518 states that if pyproject.toml isn't present, build-system.requires should default to ['setuptools', 'wheel'] (for backward compatibility, as that's what legacy setup.py needs. If it is present, build-system.requires "will have a value of a list of strings representing the PEP 508 dependencies required to execute the build system (currently that means what dependencies are required to execute a setup.py file)."

The case we're catching here is when there is a pyproject.toml, but it doesn't specify the build system. That's cool - you've said what your build system is, and it's not setuptools, so we don't support it yet (because PEP 517 isn't supported yet).

But, #4647 has a pyproject.toml that looks like the following:

[build-system]
requires = ["python-xlib"]

That's not specifying the build system, but rather additional requirements needed to execute setup.py in addition to the assumed setuptools and wheel.

It looks like #4647 is based on a mistaken assumption about how PEP 518 works. Ideally, we'd need to catch cases like this and give a more meaningful error, which the current proposed error doesn't really do. How about this:

Currently, pip does not support PEP 517, and can only build wheels using setuptools. Your pyproject.toml specifies ["python-xlib"] as its build system, which needs PEP 517 support.

[pradyunsg]

How about reversing the order of those statements?

[pradyunsg]

Your pyproject.toml specifies ["python-xlib"] as its build system, which needs PEP 517 support. Currently, pip does not support PEP 517, and can only build wheels using setuptools.

[pradyunsg]

(huh, cmd+enter posts a comment, interesting)

[pradyunsg]

Except, maybe instead of "Your" we need to mention the project name.

[pfmoore]

Sounds OK to me.

(Offtopic thought - if @benoit-pierre assumed in #4647, and no-one who reviewed that issue corrected him, that you only need to specify additional requirements needed for setup.py, maybe that means there's a need to be clearer in PEP 518 how the build-system.requires field should be used...?)

[pradyunsg]

(Offtopic thought - if @benoit-pierre assumed in #4647, and no-one who reviewed that issue corrected him, that you only need to specify additional requirements needed for setup.py, maybe that means there's a need to be clearer in PEP 518 how the build-system.requires field should be used...?)

Yes, I think a line making this clear would be useful.

---

I've committed the following:

pip-forkbomb-test 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.

It just came out of my head as I typed in what we'd discussed above.

[pfmoore]

lgtm

[ghost]

Please do not merge this until after gh-4799 as it will create merge conflicts and is a lower priority.

[BrownTruck]

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

[pfmoore]

Something's wrong here as when I look at the PR, it says "0 files changed"...

[sersorrel]

Indeed, the warning message got moved into prep_for_dist in src/pip/_internal/operations/prepare.py - the relevant commit on master is 163149f. The merge restored the existing message.

[pradyunsg]

Whoops! I'd messed up... I had local changes that didn't get added before the merge commit.

Anyway, rebased and squashed everything. :)

Changes have been made.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.