Fix editable-mode logic when pyproject.toml present

[cjerdonek]

This is the second of two PR's to address installing in editable mode when a pyproject.toml file is present. The first part was PR #6331, which has already been merged.

Whereas PR #6331 addressed the case when PEP 517 requires handling the source tree as pyproject.toml-style, this PR addresses the case where it is optional (i.e. PEP 517 doesn't mandate it).

Specifically, this PR does four things:

1. Raises an informative InstallationError if editable mode was requested, a pyproject.toml file is present but PEP 517 processing is optional, and no other PEP 517-related options were passed. Among other things, the error message tells the user they can pass --no-use-pep517 to bypass pyproject.toml-style processing and use editable mode. This behavior was discussed and agreed to on the comment thread for PR Show a nice error if editable mode is attempted with a pyproject.toml source tree #6331. The error currently looks like this--

   Error installing 'my-package': editable mode is not supported for pyproject.toml-style projects. pip is processing this project as pyproject.toml-style because it has a pyproject.toml file. Since the project has a setup.py and the pyproject.toml has no "build-backend" key for the "build_system" value, you may pass --no-use-pep517 to opt out of pyproject.toml-style processing. See PEP 517 for details on pyproject.toml-style projects.

2. Gets pip install -e working again for the PEP 517-is-optional case (provided --no-use-pep517 was passed). Previously, this case was broken because when PEP 517 processing was turned on by default for pip, the build-system.requires section was ignored in the non-PEP 517 case (in the IsSDist.prep_for_dist() method).

3. Adds a lot more unit test coverage of resolve_pyproject_toml() (the pure function responsible for interpreting the various flags in relation to the contents of pyproject.toml). It also fixes test_constraints_local_editable_install_pep518() by passing --no-use-pep517 to the pip install -e invocation in the test, which it should have been doing before.

4. Fixes some exception handling that occurs in IsSDist.prep_for_dist() when conflicts are found while processing build dependencies. The prior code constructed an exception message using a variable (conflicting) that hadn't been defined yet. So I don't think that code path was working before.

[cjerdonek]

Someone just reported as a new issue (#6375) the issue that this PR is meant to address.

[cjerdonek]

@pfmoore I'm hoping you can take a look at this PR at some point. I think it would be good to try to get it into the next release in some form so that we can head off people unwittingly trying to combine editable mode with PEP 517 behavior before it gets too ingrained and harder to change. I'm worried some people may already be doing this -- see e.g. the just-filed #6375, as well as the Poetry issue #6314 from before. This PR will also serve another important function by helping to educate people through the error message.

Finally, the PR also unit tests pretty much all of the logical interactions between the various flags and error messages so we can encode that behavior and prevent regressions.

[con-f-use]

If I understand correctly, then accepting the proposed solution here means:

1. Calling pip install --no-use-pep517 --editable . with a pyproject.toml present that does not specify a back-end, causes
   • ...build dependencies specified by requires=[...] in the pyproject.toml will be installed by pip (which btw. is not case yet, see pyproject.toml prevents pip install --user --editable (v19.0.3) #6375)
   • ...pip will then proceed installation not treating the project as specified by PEP517 but rather in the old way.
2. Calling pip install --editable . with a pyproject.toml present that does not specify a back-end, on the other hand, will result in
   • ...display of editable mode is not supported for pyproject.toml-style projects error
   • ...pip exiting with an error status code

I might be wrong, but to me that means, the only canonical way to specify build dependencies in such a project is the --no-use-pep517 option (again, which currently does not install the build dependencies due to what might be a bug or a misunderstanding on my part).

Regardless, even if it did install the specified build dependencies, some other tools like Pipenv, have no way to specify this option on a by-requirement bases. What I mean is that in a Pipenv Pipfile, there is no way to do something like:

# Pipfile
[packages]
i_require_gitpython_to_build = { path = ".", editable = true, requires=["gitpython"] }
# or alternatively:
i_require_gitpython_to_build = { path = ".", editable = true, extra_pip_options=["--no-use-pep517"] }

Which effectively means, one cannot use those tools to develop projects with old-style setup.py scripts that have build dependencies.

I'm not at all sure if everything I wrote is correct and if the following is a good idea, but maybe, case 1.) should just display a warning instead of failing and the proceed with installing the build dependencies and not processing according to PEP517?

---

Btw. education on PEP517/518 and the new canonical way of building, developing and installing python package is, from my user perspective, a bit scarce at the moment. So every little bit as soon as possible is appreciated. Just as an example, it took me a long time to find out, that setup-tools does not have its own section in a pyproject.toml akin to [tools.flit.metadata], i.e. it does not parse:

[tool.setuptools.metadata]
module = "mymodule"
author = "confus"
author-email = "[email protected]"

And other little things like PEP517/518, pyproject.toml, flit, hatch and poetry just being a small footnote in the official python packaging guide or a user-friendly comprehensive official documentation on setuptools sorely missing. But that's offtopic, sorry.

[cjerdonek]

@con-f-use I believe your summary is correct. A few comments:

• It's a bug that the dependencies in pyproject.toml aren't being installed when editable mode is used. (That's one of the issues this PR is meant to fix.)
• It looks like pipenv may need an update then. (Poetry also needed its own update, for a similar reason. See "conflict with the backend dependencies" error running pip install in editable mode with pyproject.toml / PEP-517 enabled project #6314.) Would you be able to let them know?

I'm not at all sure if everything I wrote is correct and if the following is a good idea, but maybe, case 1.) should just display a warning instead of failing and the proceed with installing the build dependencies and not processing according to PEP517?

The issue you're raising here was discussed in "part 1" of this PR. (I was the one that brought it up there.) See e.g. this comment: #6331 (comment)

Basically, the issue is that the presence of pyproject.toml was meant to be the way for users to opt in to the new PEP 517 behavior. So allowing pyproject.toml to result in the old behavior would defeat / undermine that purpose. For example, if and when we add support for editable mode to PEP 517, we would then have a backwards compatibility issue if we were to let invocations happen today. This is the point that @pfmoore was making in his comment I linked to above (and that I agree with, for the same reasons). Yes, it will be a harder pill for users to swallow today (but it's still very early in PEP 517's adoption). But that pill will become a much bigger problem later on if we permit the old behavior to happen with pyproject.toml (without an explicit flag from the user like --no-use-pep517).

One reason I want this PR to land sooner rather than later is to head off this problem earlier, because the longer it's postponed, the more users will be negatively affected by waiting.

[cjerdonek]

PS - yes, I agree there needs to be better documentation. Having said that, I do think the error messages in this PR and the previous will help to educate people and point them to where to find more information on this topic. That was a deliberate goal on my part and one of the reasons the error messages are a bit longer compared with other error messages by pip.

[con-f-use]

Again, thanks for you patience.

* It looks like pipenv may need an update then. (Poetry also needed its own update, for a similar reason. See #6314.) Would you be able to let them know?

Already kinda did yesterday. Maybe a gentle nudge from someone more known in the packaging work wold be additional motivation, tough ;-)

[cjerdonek]

Thanks, @pfmoore.

[con-f-use]

@con-f-use original post:

I'm not at all sure if everything I wrote is correct and if the following is a good idea, but maybe, case 1.) should just display a warning instead of failing and the proceed with installing the build dependencies and not processing according to PEP517?

@cjerdonek response:

Basically, the issue is that the presence of pyproject.toml was meant to be the way for users to opt in to the new PEP 517 behavior. So allowing pyproject.toml to result in the old behavior would defeat / undermine that purpose. For example, if and when we add support for editable mode to PEP 517, we would then have a backwards compatibility issue if we were to let invocations happen today. This is the point that @pfmoore was making in his comment I linked to above (and that I agree with, for the same reasons). Yes, it will be a harder pill for users to swallow today (but it's still very early in PEP 517's adoption). But that pill will become a much bigger problem later on if we permit the old behavior to happen with pyproject.toml (without an explicit flag from the user like --no-use-pep517).

Another thing that crossed my mind is to have something like this:

# pyproject.toml
[build-system]
requires = ['some_requirement']

[tool.pip]
extra_options = ['--no-use-pep517']
# or:
no_pep517 = true

Not sure if it's a good idea either, but it would not break a Pipenv installation in the way I outlined in #6370 (comment) and would also not break backwards compatibility. Maybe make it more concrete with pip_no_pep517 = true so not open the whole can of worms that is exposing pip options.

[Evpok]

This breaks editable installs for people who use pyproject.toml for unrelated configuration (eg black). I think I understand the rationale, but going through a gentler deprecation path would have been nice.

[tom-dalton-fanduel]

Echoing the above, a bunch of our builds have started failing for the same reason - we use the new toml file purely for 2 lines of black configuration. We have a project structure like:

setup.py  # Contains the actual project/package requirements
pyproject.toml  # Contains only 2 lines of black configuration
requirements/base.txt  # Contains only `-e .`
requirements/testing.txt  # Contains `-r base.txt` and then other stuff like pytest etc.

This setup allows us to

• Specify the project requirements exactly once (in the setup.py)
• Use a command like pip install -r requirements/testing.txt to install both the project requirements along with other stuff needed to run the tests.
• Build the package with python setup.py sdist

It's not immediately clear from the docs I've seen so far what the minimal change we need to make is - I'd guessing in the short term are options are to

• Downgrade pip, or
• Add the --no-use-pep517 to our build tool config

I'd echo the author above that a deprecation warning would have been nice here, but mostly I'd like to understand if there's a small change/addition we can make to the pyproject.toml file to resume previous behaviour. The examples I've seen so far use poetry or flit as example build tools, I'm surprised there's been no example using 'historical' tooling (e.g. pip/setuptools) so I'm wondering if I'm missing something here.

[con-f-use]

Just making unqualified comments here, but doesn't black have a specific config file other than pyproject.toml?

Btw. what was the reason again to not automatically fall back to the old setup.py build method, when there is a pyproject.toml with no build-backend and backend-path specified? I.e. implying --no-use-pep517 in thoses cases or at least try again with the old way (no virtualenv) before failing?
I feel this was discussed somewhere but can't find it. Not specifying a backend, not even build-backend='setuptools', basically screams "old way" to me.

[tom-dalton-fanduel]

Unfortunately (for me at least) black doesn't offer anything other than pyproject.toml. Opinions seem a bit split on it, personally I'm disappointed it doesn't offer setup.cfg as an option but I can understand the maintainer wanting to keep it simple. Ref psf/black#683

[cjerdonek]

I think I understand the rationale, but going through a gentler deprecation path would have been nice.

A little background on this: It was a bug / oversight that this was ever working. (It shouldn't have ever been working in the first place.) Yes, we know it's going to be an inconvenience for some that started using this. But PEP 517 is still a very new feature, and it would create much bigger problems further down the road if people started relying on this behavior more, which is why we wanted to stop it earlier while adoption is still small.

Btw. what was the reason again to not automatically fall back to setup.py when there is a pyproject.toml with no build-backend and backend-path specified, even without `--no-use-pep517? I feel this was discussed somewhere but can't find it.

Here are three comments with more detailed background / rationale:

• "conflict with the backend dependencies" error running pip install in editable mode with pyproject.toml / PEP-517 enabled project #6314 (comment)

• Show a nice error if editable mode is attempted with a pyproject.toml source tree #6331 (comment)

• Fix editable-mode logic when pyproject.toml present #6370 (comment)

The simplest work-around for people should be to add --no-use-pep517 to any invocation that requires editable mode (which I believe any error message you're seeing should say). Other options would be not to use editable mode, or not use a pyproject.toml file.

[gaborbernat]

In some cases editable install is desirable (Cython code coverage, development of a library), so the later two options are often no-go from the get-go. Will --no-use-pep517 work even when both build-backend and requires section is satisfied? Does this mean that the flag is now supported for the foreseeable future?

[tom-dalton-fanduel]

Hmm. I might be doing something wrong but the new flag doesn't seem to work for me:

pip install --no-use-pep517 -q --upgrade --requirement /var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/src/requirements/development.txt
ERROR: Error installing 'file:///var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion (from -r /var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/src/requirements/_base.txt (line 1))':
  editable mode is not supported for pyproject.toml-style projects. pip is processing this project as pyproject.toml-style because it has a pyproject.toml file.
  Since the project has a setup.py and the pyproject.toml has no "build-backend" key for the "build_system" value, you may pass --no-use-pep517 to opt out of pyproject.toml-style processing.
  See PEP 517 for details on pyproject.toml-style projects.

make: *** [/var/lib/jenkins/workspace/EC2_SQS_Queue_Deletion/sqs-deletion/_build/requirements.txt] Error 1```

[tom-dalton-fanduel]

I'm going to open a new issue if only to prevent cluttering up this merged PR.

[tom-dalton-fanduel]

:) I'll add my info to the bug above

[con-f-use]

@gaborbernat Yes, mixed caeses were my concern, too, like:

echo "
-e pep517_project/
non-pep517_project/
" | pip install --no-use-pep517 -r /dev/stdin

would this work:

echo "
-e --no-use-pep517 pep517_project/
non-pep517_project/
" | pip install  -r /dev/stdin

I mean Pipenv and similar tools still have now way to pass options to pip on a per-package basis, but it be nice.

[pablogsal]

The simplest work-around for people should be to add --no-use-pep517 to any invocation that requires editable mode (which I believe any error message you're seeing should say)

That fails with:

ERROR: Disabling PEP 517 processing is invalid: project specifies a build backend of setuptools.build_meta in pyproject.toml

This seems to imply that PIP refuses to opt-out of PEP 517 if there is a build-backend in the pyproject.toml even if the environment allows to just run setup.py directly.

[cjerdonek]

Will --no-use-pep517 work even when both build-backend and requires section is satisfied?

It will only work when PEP 517 doesn't mandate that pyproject.toml-style be used. If the "build-backend" key is present, the PEP says pyproject.toml-style must be used, so the flag won't work in that case.

The flag for the legacy behavior will eventually go away (that's the plan). Here's the tracking issue: #6334

[gaborbernat]

So I get now pip is PEP compliant, but this now breaks peoples workflow without any sane workaround, I consider it as a PEP oversight. PEP-517/8 on purpose did not want to cover editable installs, considering something to be addressed down the line. As such I would argue PEP-517/PEP-518 no longer applies when editable install mode is enabled. It's the build frontend dependent on how this case is handled. In such a case, pip should fallback to the old system, until we define editable installs inside a PEP. @pfmoore want to weigh in?

[tom-dalton-fanduel]

Will --no-use-pep517 work even when both build-backend and requires section is satisfied?

It will only work when PEP 517 doesn't mandate that pyproject.toml-style be used. If the "build-backend" key is present, the PEP says pyproject.toml-style must be used, so the flag won't work in that case.

I have no build-backend section in my pyproject.toml file but the --no-use-pep517 flag still doesnt appear to work as expected. See #6433 for details.

[tom-dalton-fanduel]

Raised #6433 for further discussion

[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.