Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

With --use-pep517, file:/// and tar.gz sdists support setup.cfg only source trees, but implicit file paths do not #10531

Closed
1 task done
graingert opened this issue Oct 2, 2021 · 42 comments · Fixed by #10577
Labels
C: PEP 517 impact Affected by PEP 517 processing PEP implementation Involves some PEP project: setuptools Related to setuptools type: bug A confirmed bug or unintended behavior
Milestone

Comments

@graingert
Copy link
Contributor

Description

Interestingly, while testing this I noticed that, assuming ./pkgdir contains setup.cfg but no setup.py:

pip install file://$PWD/pkgdir works, but pip install ./pkgdir complains that Neither 'setup.py' nor 'pyproject.toml' found..

The "fix" for that would be easy (in is_installable_dir()), but I'm not sure how far these ripples will go...

Originally posted by @sbidoul in #9945 (comment)

Expected behavior

$ echo '[metadata]\nname=foo' > setup.cfg && pip install --use-pep517 file:///$(pwd)
ERROR: Directory '.' is not installable. Neither 'setup.py' nor 'pyproject.toml' found.

pip version

pip 21.2.4 from /home/graingert/.virtualenvs/testing39/lib/python3.9/site-packages/pip (python 3.9)

Python version

Python 3.9.7

OS

Ubuntu 20.04.3 LTS

How to Reproduce

$ echo '[metadata]\nname=foo' > setup.cfg && pip install --use-pep517 file:///$(pwd)
Processing //home/graingert/projects/foo
  DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
   pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Building wheels for collected packages: foo
  Building wheel for foo (PEP 517) ... done
  Created wheel for foo: filename=foo-0.0.0-py3-none-any.whl size=933 sha256=b0ec4bd32d639456067a52c0297824efbc46d9a1f7b3894c63106ed25c9168fa
  Stored in directory: /tmp/pip-ephem-wheel-cache-qxna4rsa/wheels/4c/98/64/53a57a6326621aeffd6d95082ac196bdb3e2a8459e121fa09a
Successfully built foo
Installing collected packages: foo
  Attempting uninstall: foo
    Found existing installation: foo 0.0.0
    Uninstalling foo-0.0.0:
      Successfully uninstalled foo-0.0.0
Successfully installed foo-0.0.0

Output

No response

Code of Conduct

@graingert graingert added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Oct 2, 2021
@graingert
Copy link
Contributor Author

there's also this on the setuptools end pypa/setuptools#2329

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

Hi @graingert

My comment you mentioned above has now been addressed: both pip install file://$PWD/pkgdir and pip install ./pkgdir fail when there is no pyproject.toml nor setup.py.

When enforcing the use of pep 517 in such situation, the spec says this:

If the pyproject.toml file is absent, or the build-backend key is missing,
the source tree is not using this specification, and tools should revert to the legacy behaviour
of running setup.py (either directly, or by implicitly invoking the setuptools.build_meta:__legacy__ backend).

So when --use-pep517 is set and there is no pyproject.toml, pip uses the default build system table and builds using the setuptools.build_meta:__legacy__ build backend, which does support setup.cfg-only configurations.

So I think the current behaviour is correct.

@graingert
Copy link
Contributor Author

@sbidoul do you know when this was fixed because I can still reproduce this

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

I just tested with the pip main branch. It might have been fixed as part of #8212

@graingert
Copy link
Contributor Author

graingert commented Oct 2, 2021

I tried it against pip installed via git and got this:

 ✘  graingert@onomastic  testing39  ~/projects/foo  pip install -U git+https://github.com/pypa/pip.git
Collecting git+https://github.com/pypa/pip.git
  Cloning https://github.com/pypa/pip.git to /tmp/pip-req-build-m1oeci5v
  Running command git clone -q https://github.com/pypa/pip.git /tmp/pip-req-build-m1oeci5v
  Resolved https://github.com/pypa/pip.git to commit 7616583dbb2dcbda5a19d78873642d6751fbf017
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Building wheels for collected packages: pip
  Building wheel for pip (PEP 517) ... done
  Created wheel for pip: filename=pip-21.3.dev0-py3-none-any.whl size=1719972 sha256=f575d0ae1f0c372b5be154eeaa58063d01aefd086f528a50ebba8a089f6fe54c
  Stored in directory: /tmp/pip-ephem-wheel-cache-dsnjyudn/wheels/fb/cc/15/cf3753138466b998bef3949351cf6218eb857a9f27aa0be84a
Successfully built pip
Installing collected packages: pip
  Attempting uninstall: pip
    Found existing installation: pip 21.2.4
    Uninstalling pip-21.2.4:
      Successfully uninstalled pip-21.2.4
Successfully installed pip-21.3.dev0
 graingert@onomastic  testing39  ~/projects/foo  echo '[metadata]\nname=foo' > setup.cfg && pip install --use-pep517 file:///$(pwd)                                         
Processing //home/graingert/projects/foo
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing wheel metadata (pyproject.toml) ... done
Building wheels for collected packages: foo
  Building wheel for foo (pyproject.toml) ... done
  Created wheel for foo: filename=foo-0.0.0-py3-none-any.whl size=933 sha256=ac036b06383d468d783199debe6aade285a5f9f806540901297c422e89b738c9
  Stored in directory: /tmp/pip-ephem-wheel-cache-xxfz2l75/wheels/4c/98/64/53a57a6326621aeffd6d95082ac196bdb3e2a8459e121fa09a
Successfully built foo
Installing collected packages: foo
  Attempting uninstall: foo
    Found existing installation: foo 0.0.0
    Uninstalling foo-0.0.0:
      Successfully uninstalled foo-0.0.0
Successfully installed foo-0.0.0
 ✘  graingert@onomastic  testing39  ~/projects/foo  ls
build  foo-0.0.0-py3-none-any.whl  foo.egg-info  setup.cfg  _setup.py
 graingert@onomastic  testing39  ~/projects/foo  echo '[metadata]\nname=foo' > setup.cfg && pip install --use-pep517 .
ERROR: Directory '.' is not installable. Neither 'setup.py' nor 'pyproject.toml' found.

eg the issue is still present and setup.py-less builds are supported with file:/// uris and rejected with "looks-like-path" args

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

Ok, I see, I had not tried the combination of --use-pep517 and a local directory URL. Surprising. I'll have a look.

@layday
Copy link
Member

layday commented Oct 2, 2021

So when --use-pep517 is set and there is no pyproject.toml, pip uses the default build system table and builds using the setuptools.build_meta:__legacy__ build backend, which does support setup.cfg-only configurations.

I think you're misinterpreting the PEP here. The fallback applies only to legacy projects. A project is considered legacy only if it has a setup.py. If a project is legacy you can either (a) invoke setup.py directly or (b) invoke the __legacy__ backend via PEP 517. The quoted paragraph is poorly worded and the information is scattered between the two PEPs, but I don't think it's the intention that the build-backend should default to setuptool's __legacy__ backend in the absence of both pyproject.toml and setup.py.

Worth nothing that pypa/build rejects pyproject.toml-less projects without a setup.py, so pip is able to build projects that build can't.

@layday
Copy link
Member

layday commented Oct 2, 2021

Edited my message above since I'm not so sure myself anymore after rereading the PEP...

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

Edited my message above since I'm not so sure myself anymore after rereading the PEP...

Yep, same with me, it's not obvious.

When we don't say --use-pep517, it's fine to reject it, I agree.
When we enforce the use of PEP 517, my instinct would be to use the default build-system and delegate to setuptools.build_meta:__legacy__ to build, leaving the responsibility to do the right thing to it.

I don't have a strong opinion on this, though.

@pradyunsg pradyunsg changed the title under --use-pep517 file:/// and tar.gz sdists support setup.cfg only source trees, but implicit file paths do not With --use-pep517, file:/// and tar.gz sdists support setup.cfg only source trees, but implicit file paths do not Oct 2, 2021
@sbidoul sbidoul removed the S: needs triage Issues/PRs that need to be triaged label Oct 2, 2021
@pradyunsg pradyunsg added C: PEP 517 impact Affected by PEP 517 processing PEP implementation Involves some PEP labels Oct 2, 2021
@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

This test is currently done in constructors.py, _get_url_from_path. Purely from a pip internals perspective I'd say this function should not be responsible to do such a check, if only looking at it's name.

@FFY00
Copy link
Member

FFY00 commented Oct 2, 2021

Relevant: #9945 (comment)

@layday
Copy link
Member

layday commented Oct 2, 2021

When we don't say --use-pep517, it's fine to reject it, I agree.
When we enforce the use of PEP 517, my instinct would be to use the default build-system and delegate to setuptools.build_meta:__legacy__ to build, leaving the responsibility to do the right thing to it.

My thinking is that the fallback is there to help with the transition from the setuptools way of doing things to PEP 517. We don't want to elevate one specific backend above the rest for all eternity. If there's no setup.py and build-backend is absent, and pip (and build) does not error when forcing PEP 517, people will inevitably come to depend on PEP 517 defaulting to setuptools, which would be rather counterproductive. The wording's a bit muddled in the PEP but we should be able to agree on the intent.

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

But then if setuptools.build_meta:__legacy__ must be interpreted strictly as a way to invoke setup.py, then why does it work without setup.py ?

So I'd be inclined to remove the check from pip's _get_url_from_path. And if it is the consensus that the __legacy__ backend must only supports setup.py, then enforce that in setuptools.build_meta:__legacy__, or alternatively in pep517 (that we vendor and is the part that creates the default build-system table) [updated: we don't rely on pep517 to set the default build-system table].

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

Also, testing for the presence of pyproject.toml is not sufficient ? Because a pyproject.toml without a build-system table also means using the default backend ?

@FFY00
Copy link
Member

FFY00 commented Oct 2, 2021

No, the project should have a build-system table. pyproject.toml is not only used by packages, it is also used for tool configurations. It is not tied to packages.

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

I'm aware that pyproject.toml can be used for tool configuration. But its mere presence does trigger the pep517 build code path in pip, whether there is a build-system table in it or not.

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

For that matter, pip install --use-pep517 of an empty directory does also trigger the __legacy__ build backend, which happily builds UNKNOWN-0.0.0 :) So yeah I'm now quite convinced that a fix has to be done in setuptools.

@FFY00
Copy link
Member

FFY00 commented Oct 2, 2021

But then if setuptools.build_meta:__legacy__ must be interpreted strictly as a way to invoke setup.py, then why does it work without setup.py ?

That is merely coincidental.

For that matter, pip install --use-pep517 of an empty directory does also trigger the __legacy__ build backend, which happily builds UNKNOWN-0.0.0 :)

Which sounds like a bug to me 😅

So yeah I'm now quite convinced that a fix has to be done in setuptools.

I disagree, setuptools could change the behavior in order to mitigate this, but nobody should be relying on this in the first place.

@pfmoore
Copy link
Member

pfmoore commented Oct 2, 2021

For that matter, pip install --use-pep517 of an empty directory does also trigger the legacy build backend, which happily builds UNKNOWN-0.0.0 :)

Which sounds like a bug to me 😅

A bug in setuptools, yes. The user asked for PEP 517 behaviour, which explicitly states that if pyproject.toml is missing, the setuptools legacy backend should be used. It's up to that backend to check that it's being invoked in a directory which has whatever it needs to build a wheel.

I disagree, setuptools could change the behavior in order to mitigate this, but nobody should be relying on this in the first place.

It's not a matter of whether people rely on it, it's a matter of backends having decent error handling. I consider "I did something wrong and the tool produced garbage rather than spotting it and telling me" a legitimate bug report (against setuptools in this case).

I'm not sure where this discussion is going any more. And I don't think that abstract arguments about what is whose issue are that helpful. The title of the issue says "with --use-pep517", but the report is talking about cases where --use-pep517 is not specified (and hence pip works out whether to use PEP 517 itself). Can someone please confirm what the actual behaviour is that is being claimed to be incorrect here? Then we can focus on that.

@graingert
Copy link
Contributor Author

For that matter, pip install --use-pep517 of an empty directory does also trigger the legacy build backend, which happily builds UNKNOWN-0.0.0 :)

Which sounds like a bug to me 😅

A bug in setuptools, yes. The user asked for PEP 517 behaviour, which explicitly states that if pyproject.toml is missing, the setuptools legacy backend should be used. It's up to that backend to check that it's being invoked in a directory which has whatever it needs to build a wheel.

I disagree, setuptools could change the behavior in order to mitigate this, but nobody should be relying on this in the first place.

It's not a matter of whether people rely on it, it's a matter of backends having decent error handling. I consider "I did something wrong and the tool produced garbage rather than spotting it and telling me" a legitimate bug report (against setuptools in this case).

I'm not sure where this discussion is going any more. And I don't think that abstract arguments about what is whose issue are that helpful. The title of the issue says "with --use-pep517", but the report is talking about cases where --use-pep517 is not specified (and hence pip works out whether to use PEP 517 itself). Can someone please confirm what the actual behaviour is that is being claimed to be incorrect here? Then we can focus on that.

I ran this echo '[metadata]\nname=foo' > setup.cfg && pip install --use-pep517 file:///$(pwd) and it should behave same as echo '[metadata]\nname=foo' > setup.cfg && pip install --use-pep517 .`

@sbidoul
Copy link
Member

sbidoul commented Oct 2, 2021

I think I found the next best place to fix this (#10534) - the best being in setuptools, IMO.

@layday
Copy link
Member

layday commented Oct 2, 2021

It's not a matter of whether people rely on it, it's a matter of backends having decent error handling. I consider "I did something wrong and the tool produced garbage rather than spotting it and telling me" a legitimate bug report (against setuptools in this case).

This is not generic to all backends having "decent error handling" - it's specific to setuptools, because only setuptools is invoked implicitly. I think we're struggling to stay on top of when or not to invoke the legacy setuptools backend. There's at least six different variables to consider and all of their combinations: has setup.py, has pyproject.toml, has build-table, has requires, has build-backend, and --[no-]use-pep517. Something has to give - either we invoke setuptools implicitly in fewer situations, or if we can remove the first variable, by delegating the setup.py existence check to the setuptools legacy backend, then we should probably do that.

@pfmoore
Copy link
Member

pfmoore commented Oct 2, 2021

This is not generic to all backends having "decent error handling" - it's specific to setuptools, because only setuptools is invoked implicitly.

OK, it's related to setuptools having decent error handling. Is there any non-contrived reason why setuptools shouldn't be telling the user that they made a mistake if they invoke setuptools on an empty directory?

We seem to be assuming that setuptools don't want to fix this - and I don't know why. Has anyone tried reporting this as a bug to them?

I think we're struggling to stay on top of when or not to invoke the legacy setuptools backend.

Agreed.

Something has to give - either we invoke setuptools implicitly in fewer situations, or if we can remove the first variable, by delegating the setup.py existence check to the setuptools legacy backend, then we should probably do that.

Or we focus on getting the things that matter right, and don't worry about weird corner cases unless someone hits them in a real-world situation 🙂

To be clear, though, the whole --use-pep517 business was a bit of a hack when I first implemented it. There are a lot of different cases, and they are messy. I'd welcome something cleaner. But I think if we want to do that, we should write down the rules we're following, and not just fix things as we find them - otherwise, as you say, we just end up getting bogged down in confusion, and we'll keep going round this whole loop.

@layday
Copy link
Member

layday commented Oct 2, 2021

Or we focus on getting the things that matter right, and don't worry about weird corner cases unless someone hits them in a real-world situation 🙂

Well, it did start out as a weird corner case, but it wasn't clear what to do when passing --use-pep517 (which pypa/build always does!), hence the ensuing discussion. If we relax the check to let setup.py-less source trees through in less cornery cases, that will have some decently far-reaching consequences. But I agree, we should formalise these rules.

@pfmoore
Copy link
Member

pfmoore commented Oct 2, 2021

but it wasn't clear what to do when passing --use-pep517 (which pypa/build always does!)

I'd be curious to know why you do that. I suspect it's intended to make sure you always get PEP 517 behaviour, but that was never the original intention of that flag. The original point was that pip was applying the rule from PEP 517 itself, "If the pyproject.toml file is absent, or the build-backend key is missing, the source tree is not using this specification" and so in those cases we were doing "something else"¹. The --use-pep517 flag was simply a way to override that and try to use PEP 517 (and the other behaviours that it triggers in pip, like build isolation) in any case (by doing what PEP 517 says² and using the setuptools legacy backend)

¹ If the PEP says "this spec doesn't apply in this case" then you can do whatever you like, by definition.
² It's actually quite weird that PEP 517 says what you should do when PEP 517 doesn't apply. But ho hum, it's useful to have that clarification, so I'm not complaining 🙂

@graingert
Copy link
Contributor Author

² It's actually quite weird that PEP 517 says what you should do when PEP 517 doesn't apply. But ho hum, it's useful to have that clarification, so I'm not complaining

The purpose of the system is what it does

@layday
Copy link
Member

layday commented Oct 2, 2021

I'd be curious to know why you do that.

We do it because we don't pre-install setuptools in the isolated build environment so build dependencies without wheels which haven't "opted into" PEP 517 can't be built. This was brought up in pypa/build#231, where flit tried to install every single runtime dependency in the build environment, because it could not parse the AST of the module to extract the version, as I recall.

@pradyunsg
Copy link
Member

pradyunsg commented Oct 2, 2021

Oh, wow. This has blown up. I'm surprised everyone is suddenly so concerned about this. Of all the things I was expecting would get attention this wekeend on this issue tracker, this was not on that list. XD

I don't think this is a particularly useful discussion, so I'm gonna keep my participation limited -- don't expect me to reply to any questions, sorry. What follows, is my perspective on this.


I am in favour of adding a check for setup.cfg in the is_installable_dir and changing nothing else, as well as the PR that's currently open.

For setuptools, even an empty directory is a valid project. It will package up literally any directory on the filesystem and ship that to the user. If you want to complain about that behaviour, go here: pypa/setuptools#2329

I definitely prefer that that pip continues to protect users from setuptools' shitty behaviour, whenever we can, without going overboard. The directory case is a simple one to do this in, given that we already do so.

As far as I'm concerned, is_installable_dir is a hueristic, to protect our users from trying to run pip in an inappropriate directory. This is especially relevant in the PEP 517 case, thanks to setuptools' shitty behavior -- see link above. If it were to model the reality of what setuptools accepts, it should basically "return True" in the PEP 517 case -- setuptools will happily try to package up any directory into a wheel.

If pip install . in a random directory packages up the whole directory, and dumps that into a Python's site-packages, that's... horrible. I don't think we should have that behaviour.

I care much less about pip install file:///blah/foo and pip install sdist.tar.gz -- there's only so much protection that we should provide from setuptools' shitty behaviour here and they're much less common. I could be convinced that we should have these checks there too -- and I'd be happy to merge such a PR -- I probably won't be authoring such a PR though.

@pradyunsg pradyunsg added the project: setuptools Related to setuptools label Oct 2, 2021
@sbidoul sbidoul added this to the 21.3.1 milestone Oct 15, 2021
inmantaci pushed a commit to inmanta/inmanta-core that referenced this issue Oct 22, 2021
Bumps [pip](https://github.com/pypa/pip) from 21.3 to 21.3.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's changelog</a>.</em></p>
<blockquote>
<h1>21.3.1 (2021-10-22)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Always refuse installing or building projects that have no <code>pyproject.toml</code> nor
<code>setup.py</code>. (<code>[#10531](pypa/pip#10531) &lt;https://github.com/pypa/pip/issues/10531&gt;</code>_)</li>
<li>Tweak running-as-root detection, to check <code>os.getuid</code> if it exists, on Unix-y and non-Linux/non-MacOS machines. (<code>[#10565](pypa/pip#10565) &lt;https://github.com/pypa/pip/issues/10565&gt;</code>_)</li>
<li>When installing projects with a <code>pyproject.toml</code> in editable mode, and the build
backend does not support :pep:<code>660</code>, prepare metadata using
<code>prepare_metadata_for_build_wheel</code> instead of <code>setup.py egg_info</code>. Also, refuse
installing projects that only have a <code>setup.cfg</code> and no <code>setup.py</code> nor
<code>pyproject.toml</code>. These restore the pre-21.3 behaviour. (<code>[#10573](pypa/pip#10573) &lt;https://github.com/pypa/pip/issues/10573&gt;</code>_)</li>
<li>Restore compatibility of where configuration files are loaded from on MacOS (back to <code>Library/Application Support/pip</code>, instead of <code>Preferences/pip</code>). (<code>[#10585](pypa/pip#10585) &lt;https://github.com/pypa/pip/issues/10585&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade pep517 to 0.12.0</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/f9914f3ebe223004b1d439a2b1980bd132d14f27"><code>f9914f3</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/f9f2db248fc04b46bd0149eba92882e4932c627e"><code>f9f2db2</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/f2d776be2adffee700ef4563893dc01dc231125c"><code>f2d776b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10607">#10607</a> from pradyunsg/fix-docs-builds</li>
<li><a href="https://github.com/pypa/pip/commit/4a4b613a7ccdf4c4aab8f223f9b97f413b8b3056"><code>4a4b613</code></a> Merge PR <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10577">#10577</a> from sbidoul/fix-pep660-metadata-preparation-fallback</li>
<li><a href="https://github.com/pypa/pip/commit/f4d67ba0c091c5b02096ddb211c7602bf0d95580"><code>f4d67ba</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10592">#10592</a> from pradyunsg/update-ewdurbin-name</li>
<li><a href="https://github.com/pypa/pip/commit/37aef106a325ed7b1115f04028360e03dbfe7ee8"><code>37aef10</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10536">#10536</a> from pradyunsg/docs/fix-wordin</li>
<li><a href="https://github.com/pypa/pip/commit/457564cf38ad5da75672d4de119e4c5ae19fbd56"><code>457564c</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10585">#10585</a> from pradyunsg/fix-config-paths</li>
<li><a href="https://github.com/pypa/pip/commit/8c1f333ba5cb0a8c2cc4c775355bdde4ae06e50f"><code>8c1f333</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10580">#10580</a> from pradyunsg/better-towncrier-template</li>
<li><a href="https://github.com/pypa/pip/commit/cc559ed6237f7461c919d403c93fbc2ac82abe09"><code>cc559ed</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10583">#10583</a> from pradyunsg/fix-vendoring</li>
<li><a href="https://github.com/pypa/pip/commit/0c2574b7ef78791dd98822bca038b6d839b5378c"><code>0c2574b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10566">#10566</a> from n1000/dont_warn_on_bsd</li>
<li>See full diff in <a href="https://github.com/pypa/pip/compare/21.3...21.3.1">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pip&package-manager=pip&previous-version=21.3&new-version=21.3.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>
@henryiii
Copy link
Contributor

Nice to see consistency here. If it helps future readers (I just found out about this discussion from the changelog, I was part of some of the earlier ones), this is the argument that lead me to agree with @FFY00 that PEP 517 requires setup.py to be present if pyproject.toml is missing:

If the pyproject.toml file is absent, or the build-backend key is missing, the source tree is not using this specification, and tools should revert to the legacy behaviour of running setup.py (either directly, or by implicitly invoking the setuptools.build_meta:__legacy__ backend).

This says a valid build backend could either choose to run setup.py directly or invoke setuptools.build_meta:__legacy__. Therefore, a valid PEP 517 builder could exist that runs setup.py if pyproject.toml is missing. Most prefer setuptools.build_meta:__legacy__, but it's not required. So therefore, a pure setup.cfg project is invalid under PEP 517.

And yes, this is making statements about the PEP 517 validity of a project that does not follow PEP 517.1

Footnotes

  1. @pfmoore, FYI, since you are adding your own footnotes above, GitHub now allows real markdown footnotes. :)

@FFY00
Copy link
Member

FFY00 commented Oct 22, 2021

The pyproject.toml presence does not really mean anything, the build-system table presence in pyproject.toml does 😄

@pradyunsg
Copy link
Member

pradyunsg commented Oct 22, 2021

FYI, since you are adding your own footnotes above, GitHub now allows real markdown footnotes. :)

Woah.

The pyproject.toml presence does not really mean anything, the build-system table presence in pyproject.toml does 😄

Are you sure? ;)

mergify bot pushed a commit to andrewbolster/bolster that referenced this issue Oct 22, 2021
Bumps [pip](https://github.com/pypa/pip) from 21.3 to 21.3.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's changelog</a>.</em></p>
<blockquote>
<h1>21.3.1 (2021-10-22)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Always refuse installing or building projects that have no <code>pyproject.toml</code> nor
<code>setup.py</code>. (<code>[#10531](pypa/pip#10531) &lt;https://github.com/pypa/pip/issues/10531&gt;</code>_)</li>
<li>Tweak running-as-root detection, to check <code>os.getuid</code> if it exists, on Unix-y and non-Linux/non-MacOS machines. (<code>[#10565](pypa/pip#10565) &lt;https://github.com/pypa/pip/issues/10565&gt;</code>_)</li>
<li>When installing projects with a <code>pyproject.toml</code> in editable mode, and the build
backend does not support :pep:<code>660</code>, prepare metadata using
<code>prepare_metadata_for_build_wheel</code> instead of <code>setup.py egg_info</code>. Also, refuse
installing projects that only have a <code>setup.cfg</code> and no <code>setup.py</code> nor
<code>pyproject.toml</code>. These restore the pre-21.3 behaviour. (<code>[#10573](pypa/pip#10573) &lt;https://github.com/pypa/pip/issues/10573&gt;</code>_)</li>
<li>Restore compatibility of where configuration files are loaded from on MacOS (back to <code>Library/Application Support/pip</code>, instead of <code>Preferences/pip</code>). (<code>[#10585](pypa/pip#10585) &lt;https://github.com/pypa/pip/issues/10585&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade pep517 to 0.12.0</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/f9914f3ebe223004b1d439a2b1980bd132d14f27"><code>f9914f3</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/f9f2db248fc04b46bd0149eba92882e4932c627e"><code>f9f2db2</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/f2d776be2adffee700ef4563893dc01dc231125c"><code>f2d776b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10607">#10607</a> from pradyunsg/fix-docs-builds</li>
<li><a href="https://github.com/pypa/pip/commit/4a4b613a7ccdf4c4aab8f223f9b97f413b8b3056"><code>4a4b613</code></a> Merge PR <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10577">#10577</a> from sbidoul/fix-pep660-metadata-preparation-fallback</li>
<li><a href="https://github.com/pypa/pip/commit/f4d67ba0c091c5b02096ddb211c7602bf0d95580"><code>f4d67ba</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10592">#10592</a> from pradyunsg/update-ewdurbin-name</li>
<li><a href="https://github.com/pypa/pip/commit/37aef106a325ed7b1115f04028360e03dbfe7ee8"><code>37aef10</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10536">#10536</a> from pradyunsg/docs/fix-wordin</li>
<li><a href="https://github.com/pypa/pip/commit/457564cf38ad5da75672d4de119e4c5ae19fbd56"><code>457564c</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10585">#10585</a> from pradyunsg/fix-config-paths</li>
<li><a href="https://github.com/pypa/pip/commit/8c1f333ba5cb0a8c2cc4c775355bdde4ae06e50f"><code>8c1f333</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10580">#10580</a> from pradyunsg/better-towncrier-template</li>
<li><a href="https://github.com/pypa/pip/commit/cc559ed6237f7461c919d403c93fbc2ac82abe09"><code>cc559ed</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10583">#10583</a> from pradyunsg/fix-vendoring</li>
<li><a href="https://github.com/pypa/pip/commit/0c2574b7ef78791dd98822bca038b6d839b5378c"><code>0c2574b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10566">#10566</a> from n1000/dont_warn_on_bsd</li>
<li>See full diff in <a href="https://github.com/pypa/pip/compare/21.3...21.3.1">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pip&package-manager=pip&previous-version=21.3&new-version=21.3.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>
@pfmoore
Copy link
Member

pfmoore commented Oct 22, 2021

GitHub now allows real markdown footnotes.

Does it? 1

OK, enough silliness for today 🙂

Footnotes

  1. Ooh, yes it does2!

  2. I'm quite attached to my autohotkey macro for "superscript 1", but having proper support is good3

  3. Unfortunately, it looks like Discourse doesn't support footnotes, so I may stick with doing them manually for consistency.

@pradyunsg
Copy link
Member

@FFY00
Copy link
Member

FFY00 commented Oct 22, 2021

Are you sure? ;)

I am. pip's implementation and what the PEP says are different things. pip does not follow the PEP in this detail, it says

If the pyproject.toml file is absent, or the build-backend key is missing, the source tree is not using this specification, and tools should revert to the legacy behaviour of running setup.py (either directly, or by implicitly invoking the setuptools.build_meta:legacy backend).

Which is clear about a pyproject.toml that does not specify the backend being a legacy project, but I am not a pip maintainer so 🤷

@FFY00
Copy link
Member

FFY00 commented Oct 22, 2021

Users might want to use configure a tool in pyproject.toml but still use a legacy project, in which case pip will misbehave. I don't think it's a big deal since this use-case is not common, but pip is not following the PEP.

@pradyunsg
Copy link
Member

Feel free to read distutils-sig about this.

@pradyunsg
Copy link
Member

FWIW, I tend to agree with you, but I couldn’t convince folks that this was a bad idea, and you seem to have a better success rate than I do about these things. :)

@henryiii
Copy link
Contributor

I know some people (cough, @asottile, cough) who hate pyproject.toml for this exact reason. I didn't realize the wording was correct in the PEP, and it's only been the implementations that have been broken, though! Hopefully in the near future, once PEP 517 is on by default, it won't really matter, but interesting that it was specified correctly1 in the PEP.

in which case pip will misbehave

This is one of the three reasons flake8 does not have a pyproject.toml config option; if it moved to only having a single pyproject.toml file for config, it would break users who need the old behavior. The other two reasons are actually no longer true, AFAICK, there is a 1.0 release of the TOML standard, and tomli has become the "accepted" implementation (although not for very long at this point).

Footnotes

  1. Correctly meaning adding a file to configure Black shouldn't change your build system and cause it to do something different.

@uranusjr
Copy link
Member

See https://discuss.python.org/t/2725. If pip's behaviour has changed between then and now, it should be treated as a bug. But if it didn't, then either the proposed amendment or your interpretation is wrong, because the current text was written to exactly describe pip's behaviour (at the time).

@layday
Copy link
Member

layday commented Oct 22, 2021

Yes, the quoted passage from 517 should be read in conjunction with the passage from PEP 518, and even then, it's not easy to follow without knowing the background.

@henryiii
Copy link
Contributor

The code there looks about right, maybe adding a pyproject.toml without a build-backend now has no effect on the build process in pip 20.0+? I never have pyproject.toml's without build-backends, so it's not something I test.

@FFY00
Copy link
Member

FFY00 commented Oct 22, 2021

I think the damage is done at this point, even if we fix pip's behavior, nobody can rely on it as there will still be tons of users stuck with older versions of pip. We would be creating divergence and introducing breakage. It could be worth, but I am not certain that it would, and I don't have any personal stake in this to motivate me to drive it forward.

If someone else wants to try starting a discussion on this with the goal of fixing the behavior, I'd be supportive.

inmantaci pushed a commit to inmanta/inmanta-core that referenced this issue Nov 12, 2021
Bumps [pip](https://github.com/pypa/pip) from 21.0.1 to 21.3.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's changelog</a>.</em></p>
<blockquote>
<h1>21.3.1 (2021-10-22)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Always refuse installing or building projects that have no <code>pyproject.toml</code> nor
<code>setup.py</code>. (<code>[#10531](pypa/pip#10531) &lt;https://github.com/pypa/pip/issues/10531&gt;</code>_)</li>
<li>Tweak running-as-root detection, to check <code>os.getuid</code> if it exists, on Unix-y and non-Linux/non-MacOS machines. (<code>[#10565](pypa/pip#10565) &lt;https://github.com/pypa/pip/issues/10565&gt;</code>_)</li>
<li>When installing projects with a <code>pyproject.toml</code> in editable mode, and the build
backend does not support :pep:<code>660</code>, prepare metadata using
<code>prepare_metadata_for_build_wheel</code> instead of <code>setup.py egg_info</code>. Also, refuse
installing projects that only have a <code>setup.cfg</code> and no <code>setup.py</code> nor
<code>pyproject.toml</code>. These restore the pre-21.3 behaviour. (<code>[#10573](pypa/pip#10573) &lt;https://github.com/pypa/pip/issues/10573&gt;</code>_)</li>
<li>Restore compatibility of where configuration files are loaded from on MacOS (back to <code>Library/Application Support/pip</code>, instead of <code>Preferences/pip</code>). (<code>[#10585](pypa/pip#10585) &lt;https://github.com/pypa/pip/issues/10585&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade pep517 to 0.12.0</li>
</ul>
<h1>21.3 (2021-10-11)</h1>
<h2>Deprecations and Removals</h2>
<ul>
<li>Improve deprecation warning regarding the copying of source trees when installing from a local directory. (<code>[#10128](pypa/pip#10128) &lt;https://github.com/pypa/pip/issues/10128&gt;</code>_)</li>
<li>Suppress location mismatch warnings when pip is invoked from a Python source
tree, so <code>ensurepip</code> does not emit warnings on CPython <code>make install</code>. (<code>[#10270](pypa/pip#10270) &lt;https://github.com/pypa/pip/issues/10270&gt;</code>_)</li>
<li>On Python 3.10 or later, the installation scheme backend has been changed to use
<code>sysconfig</code>. This is to anticipate the deprecation of <code>distutils</code> in Python
3.10, and its scheduled removal in 3.12. For compatibility considerations, pip
installations running on Python 3.9 or lower will continue to use <code>distutils</code>. (<code>[#10358](pypa/pip#10358) &lt;https://github.com/pypa/pip/issues/10358&gt;</code>_)</li>
<li>Remove the <code>--build-dir</code> option and aliases, one last time. (<code>[#10485](pypa/pip#10485) &lt;https://github.com/pypa/pip/issues/10485&gt;</code>_)</li>
<li>In-tree builds are now the default. <code>--use-feature=in-tree-build</code> is now
ignored. <code>--use-deprecated=out-of-tree-build</code> may be used temporarily to ease
the transition. (<code>[#10495](pypa/pip#10495) &lt;https://github.com/pypa/pip/issues/10495&gt;</code>_)</li>
<li>Un-deprecate source distribution re-installation behaviour. (<code>[#8711](pypa/pip#8711) &lt;https://github.com/pypa/pip/issues/8711&gt;</code>_)</li>
</ul>
<h2>Features</h2>
<ul>
<li>Replace vendored appdirs with platformdirs. (<code>[#10202](pypa/pip#10202) &lt;https://github.com/pypa/pip/issues/10202&gt;</code>_)</li>
<li>Support <code>PEP 610 &lt;https://www.python.org/dev/peps/pep-0610/&gt;</code>_ to detect
editable installs in <code>pip freeze</code> and  <code>pip list</code>. The <code>pip list</code> column output</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/f9914f3ebe223004b1d439a2b1980bd132d14f27"><code>f9914f3</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/f9f2db248fc04b46bd0149eba92882e4932c627e"><code>f9f2db2</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/f2d776be2adffee700ef4563893dc01dc231125c"><code>f2d776b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10607">#10607</a> from pradyunsg/fix-docs-builds</li>
<li><a href="https://github.com/pypa/pip/commit/4a4b613a7ccdf4c4aab8f223f9b97f413b8b3056"><code>4a4b613</code></a> Merge PR <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10577">#10577</a> from sbidoul/fix-pep660-metadata-preparation-fallback</li>
<li><a href="https://github.com/pypa/pip/commit/f4d67ba0c091c5b02096ddb211c7602bf0d95580"><code>f4d67ba</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10592">#10592</a> from pradyunsg/update-ewdurbin-name</li>
<li><a href="https://github.com/pypa/pip/commit/37aef106a325ed7b1115f04028360e03dbfe7ee8"><code>37aef10</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10536">#10536</a> from pradyunsg/docs/fix-wordin</li>
<li><a href="https://github.com/pypa/pip/commit/457564cf38ad5da75672d4de119e4c5ae19fbd56"><code>457564c</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10585">#10585</a> from pradyunsg/fix-config-paths</li>
<li><a href="https://github.com/pypa/pip/commit/8c1f333ba5cb0a8c2cc4c775355bdde4ae06e50f"><code>8c1f333</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10580">#10580</a> from pradyunsg/better-towncrier-template</li>
<li><a href="https://github.com/pypa/pip/commit/cc559ed6237f7461c919d403c93fbc2ac82abe09"><code>cc559ed</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10583">#10583</a> from pradyunsg/fix-vendoring</li>
<li><a href="https://github.com/pypa/pip/commit/0c2574b7ef78791dd98822bca038b6d839b5378c"><code>0c2574b</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/10566">#10566</a> from n1000/dont_warn_on_bsd</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/21.0.1...21.3.1">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pip&package-manager=pip&previous-version=21.0.1&new-version=21.3.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: PEP 517 impact Affected by PEP 517 processing PEP implementation Involves some PEP project: setuptools Related to setuptools type: bug A confirmed bug or unintended behavior
Projects
None yet
8 participants