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

pip 19.1 and develop mode (with pyproject.toml - build-backend present) #6434

Closed
gaborbernat opened this issue Apr 24, 2019 · 66 comments · Fixed by #6442
Closed

pip 19.1 and develop mode (with pyproject.toml - build-backend present) #6434

gaborbernat opened this issue Apr 24, 2019 · 66 comments · Fixed by #6442
Labels
auto-locked Outdated issues that have been locked by automation

Comments

@gaborbernat
Copy link

gaborbernat commented Apr 24, 2019

So I get now pip is PEP compliant, and the reasons why it was done.

However, this now breaks peoples workflow without any sane workaround (the only one to work is to remove pyproject.toml file before install and put it back afterwards), I consider it as a PEP oversight to not specify that it does not apply for editable installs.

PEP-517/8 on purpose did not want to cover editable installs, considering something to be addressed down the line (there's some discussion on the topic upstream, but that's ages away from reality). 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.

We have two options now:

  • In develop mode, pip should fallback to the old system, maintain the status quo until we figure out how to support editable installs in the new world.
  • Alternatively, provide a flag that triggers the old build system independent of the presence/content of the pyproject.toml.

Note this now breaks a lot of dependent systems (pipenv, tox, poetry, etc).

@pfmoore
Copy link
Member

pfmoore commented Apr 24, 2019

While I'm not trying to apportion blame here, it should be noted that a much simpler fix would be to simply delete pyproject.toml. The reason this isn't possible appears to be because projects like black chose to use that file for their general configuration, which is arguably allowed by PEP 518 (it depends on how you define "build tool", basically) but might be considered a bit premature, when the intended use of that file (PEPs 517 and 518 were still in development).

We need to find a way forward, and not look back at how we got here, but it might be worth someone looking at the possibility of black adding support for configuraion in setup.cfg. I don't know if other tools have pyproject.toml-only configuration - if the problem is significantly more widespread than just black, then this may not be a practical solution.

@gaborbernat
Copy link
Author

@pfmoore it's not important how we got here or who is to blame.

The workflow that this breaks is the following: you want to distribute your code as PEP-517/518. And you're testing with pure PEP-517/518 in your CI/tox. However, while developing the codebase it's helpful to install the code in developer mode (to avoid needing to reinstall the package after every code-change to run your tests). In such case, you do want to install the code in editable mode (but also to keep the pyprojec.toml). Deleting the file is not possible in this case. Specifying no pep 517 builds also doesn't work.

@pfmoore
Copy link
Member

pfmoore commented Apr 24, 2019

Ah, OK. There's so many cases being discussed across various issues that I'd missed that this was talking about a different situation1. Sorry.

That's definitely a fair comment then - although it's probably worth being clear when documenting this that you would be doing your development with what is essentially a completely different build system than the one you're going to deploy with. As an example, console script wrappers are handled in a completely different way depending on whether you use the legacy direct install method or the PEP 517 install via wheel method.

1 It might be worthwhile for someone to collect together the various use cases we're seeing reported here and document them, as a starting point for the inevitable "how to extend PEP 517 to support editable mode" discussion. If nothing else, this whole debate makes it clear that the need for a standard covering editable mode isn't something we can put off too long.

@gaborbernat
Copy link
Author

@pfmoore that's okay though, editable mode always was a close enough approximation; the non-editable test suits are the ones that confirm 100% the code is good. For example, if one would load resources they can behave differently. But this is already the case. As such needing to pass some flags is alright, however, for now, there's no way to do this. Can we get a fix? --really-ignore-pep517-for-develop-mode is fine by me.

@mattaustin
Copy link

Specifying no pep 517 worked for me. For anyone who hit this when using tox, adding a custom install_command line to tox.ini is my current workaround (I only have pyproject.toml for setting the line length for black).

[testenv]
install_command =
    python -m pip install --no-use-pep517 {opts} {packages}

@gaborbernat
Copy link
Author

That's a different problem, as you don't have build-backend and requires specified.

@cjerdonek
Copy link
Member

@gaborbernat Can you do something like have a simple wrapper script you use when developing locally that moves the file prior to installing in editable mode? Or copying the pyproject.toml file into place when running in CI, etc? I'm not sure the exact situation, but it seems like you'd have a fair amount of flexibility in coming up with a workflow.

@gaborbernat
Copy link
Author

gaborbernat commented Apr 24, 2019

Moving around the files is a really bad workaround and would generate a lot more trouble. This now breaks tox for example. tox it's no longer able to create a develop environment.

While moving the pyproject.toml out and back would workaround the issue (in a scary horrible way), it would break parallel mode. Imagine if you want to run 5 tox environments in parallel, 4 of them use PEP 517/518 and one wants an editable install (for example is the only way cython code coverage can be done). Again feels to me like a bad place to be, now one would need some lock to hold why altering the work tree, just to get an editable install through. On this point, I either downgrade to pip < 19.1, or use python setup.py develop... I would really prefer a true PEP-517/PEP-518 disable behind an obscure flag. Wrapper scripts at that point can pass through that, rather than altering the source directory in-place.

@cjerdonek
Copy link
Member

cjerdonek commented Apr 24, 2019

Imagine if you want to run 5 tox environments in parallel, 4 of them use PEP 517/518 and one wants an editable install

Okay, but that's a different scenario from what you mentioned above. I was responding to what you wrote. Above you just said you wanted to install in develop mode when developing locally, and run CI/tox in pure PEP-517/518.

Like I said above, I don't know your exact situation. It just feels to me like you would have a lot of options given that you can structure things however you want in your repo and local environment.

@gaborbernat
Copy link
Author

Well, I never said that I do install in develop mode when developing locally manually (I use tox for that too) or not in parallel with some other operation on the same repo. pip installs at the moment are parallel safe, your suggestion would break that constant, so it's not a true workaround.

@pfmoore
Copy link
Member

pfmoore commented Apr 24, 2019

Can we get a fix?

Even assuming someone pulls together a proposal that covers all the various issues that are getting raised, and gets agreement from everyone, it'll still need implementing. We're still only at the stage of everyone contributing their use case, and we haven't got much clarity on what is the best way forward yet. Stage one is probably for someone to step up and manage the various threads and discussions going on here (we really need one proposal to handle all the use cases, not multiple "temporary" solutions).

Also, the next pip release isn't for 3 months, so there's no huge rush. Let's think things through and make sure we have a good solution before rushing into someone writing a PR.

But yes, we can get a fix, in due course.

PS It might actually be possible to get a PEP 517 extension supporting editable mode sorted in the 3 months before pip's next release - let's not waste all of the energy here on temporary solutions if we can harness it to address the underlying problem!

@pfmoore
Copy link
Member

pfmoore commented Apr 24, 2019

One other question - is this something new in pip 19.1? The PEP 517 support was introduced in 19.0, a few months ago - is there something in 19.1 that makes it worse? If not, then how come this issue has suddenly flared up now? Just coincidence?

@gaborbernat
Copy link
Author

Also, the next pip release isn't for 3 months, so there's no huge rush. Let's think things through and make sure we have a good solution before rushing into someone writing a PR.

This breaks a lot of CIs and workflows. Notably mypy, tox, virtualenv (but a lot-lot more within my workplace). I consider this to be a bug-fix request, so waiting for three months feels to me not the right way to go.

@gaborbernat
Copy link
Author

gaborbernat commented Apr 24, 2019

pip 19.0.0 allowed develop mode (without satisfying the requires section) when pyproject.toml was present. pip 19.1 via #6370 decided to hard error instead.

@pfmoore
Copy link
Member

pfmoore commented Apr 24, 2019

So #6370 says to use --no-use-pep517. Are you saying that the PR is broken, and it doesn't (as stated) "Get pip install -e working again for the PEP 517-is-optional case (provided --no-use-pep517 was passed)"?

If the PR is not delivering what it claims to deliver, then let's fix that (and yes, such a fix might warrant a 19.1.1 release), not add yet more options and workarounds. But we'd need instructions on how to reproduce the breakage in that case.

@gaborbernat
Copy link
Author

see #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)

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
Copy link
Member

cjerdonek commented Apr 24, 2019

@pfmoore This issue is about the first of the two PR's: #6331 rather than the second.

The first is the case where PEP 517 mandates that pyproject.toml-style be used and so --no-use-pep517 isn't permitted.

@gaborbernat
Copy link
Author

I think PEP-517/518 does not cover/handle editable install (on purpose, TBD) so there's nothing mandating things actually once someone asks editable install. But at the very least we should be able to disable entirely PEP-517 in case of editable installs.

@pfmoore
Copy link
Member

pfmoore commented Apr 24, 2019

@cjerdonek Thanks. This still seems correct - PEP 517 doesn't support editable mode, so projects shouldn't use it if they want to support editable mode.

This does emphasise the need for someone to reopen the standardisation process for editable mode under PEP 517. And when it's been agreed, pip will (of course) follow it.

The decision for pip to switch to PEP 517 mode when a PEP 517-style source layout was detected was made right back in the beginning of the implementation. I pushed for it (I believed then, and still do, that it's important to exercise the new code path where possible, and writing your source code to use the new features seems like a pretty strong opt-in), but it was generally agreed to be the correct choice. And I still believe it is - if we'd held back on using the new code path, how would we have found out that the lack of editable mode was such a significant issue? We'd just have been delaying the point when this issue arose.

Moving forward, though, we do need to work out what to do next.

@gaborbernat
Copy link
Author

gaborbernat commented Apr 24, 2019

@pfmoore in the meantime we agree on editable installs do we still plan to offer the legacy editable install mode (behind some weird flag) or pip 19.0.0 has dropped editable install support for now, and worked only by chance pre 19.1?

I don't agree this is a good approach. Basically, you're saying whoever wants to use editable install for development needs to drop pyproject.toml PEP-517/518 for deployment too.

@cjerdonek
Copy link
Member

@gaborbernat When asking these questions, for clarity you should distinguish between the two cases of whether "build-backend" is present in pyproject.toml or not because the answer is different in the two cases. If you don't distinguish, it can create confusion because people won't know which case is being referred to.

@gaborbernat gaborbernat changed the title pip 19.1 and develop mode (with pyproject.toml) pip 19.1 and develop mode (with pyproject.toml - build-backend present) Apr 24, 2019
@gaborbernat
Copy link
Author

Changed name of the issue to reflect scope.

@pganssle
Copy link
Member

pganssle commented Apr 24, 2019

I have proposed discussing this at the Packaging Mini-summit, to the extent that "likes" are considered votes, liking it will help get it prioritized.

Either way, maybe we can get the new standard jump-started at PyCon next week? I'm thinking an open space to flesh out ideas among people interested would be good, if there's time.

@blueyed
Copy link
Contributor

blueyed commented Apr 30, 2019

Just noticed that pip install -e failed with pip itself when using 19.1, but works now after installing it from master non-editable first.
I also typically use editable installs, because it easily allows to edit the version-controlled files for patches.

cas-- added a commit to cas--/Deluge that referenced this issue May 3, 2019
 * Using pyproject.toml for black config pip version 19.1 errors out
   about using editable install with pyproject.toml.
   Workaround is to not use pip 19.1 in tox.
 * Pin to 18.1 to avoid pip-wheel-metadata-folder creation

Ref:
 - pypa/pip#6434
 - pypa/pip#6213
jaraco added a commit to jaraco/skeleton that referenced this issue May 9, 2019
clrpackages pushed a commit to clearlinux-pkgs/keyring that referenced this issue May 20, 2019
Jason R. Coombs (14):
      Add black config, pre-commit including black, check code with black.
      Rely on alabaster theme to support sidebar rendering.
      Use nicer, simpler phrasing
      Enable tox-pip-extensions ext_venv_update if available. Fixes jaraco/skeleton#6
      Rely on tox 3.2 and pip 10 or later for all builds
      It adds no value to add a pip requirement for the tox install
      Pin to pip 19.0 for now for pypa/pip#6434.
      Revert "Pin to pip 19.0 for now for pypa/pip#6434."
      Only install and invoke pytest-black on Python 3
      Use pytest-black-multipy to enable simple support for pytest-black where available. Ref pytest-dev/pytest#5272.
      Update skeleton documentation to reflect black adoption.
      Add support for automatic publishing of release notes
      Fade to black
      Update changelog
clipos-anssi pushed a commit to clipos/toolkit that referenced this issue May 21, 2019
This Python packages bump is required in order to upgrade Pip to
version 19.1.1 which fixes a bug causing "pyproject.toml" file to be
considered eventhough "--no-use-pep517" is specified on the command
line (see pypa/pip#6434 ).
This bug was causing our CI systems to fail on the install of package
sphinx-autodoc-typehints which makes use of "pyproject.toml" file.

Change-Id: Ie102facaf16baac7699e92237db61c3ccb817783
Signed-off-by: Nicolas Godinho <[email protected]>
clrpackages pushed a commit to clearlinux-pkgs/pytest-runner that referenced this issue May 24, 2019
…support setuptools older than 30.4. Fixes #49.

Jason R. Coombs (57):
      Bump minimum pytest version
      Add pyproject.toml declaring build dependencies.
      When ignoring linter warnings, document the reason.
      Disable the (broken) IPv6 in Travis. Ref travis-ci/travis-ci#8361.
      Don't match issues if preceeded by some other indicator.
      skip_upload_docs is default
      Drop the dot; http://blog.pytest.org/2016/whats-new-in-pytest-30/
      Rely on declarative config to create long_description.
      Remove workaround for pyyaml 126.
      Revert "Remove workaround for pyyaml 126."
      We're getting close, but Python 3.7 still requires a workaround
      Use xenial to include support for Python 3.7.
      Remove release, no longer needed. Use twine instead.
      Also ignore W504 in flake8, following the indication in OCA/maintainer-quality-tools that neither W503 nor W504 are worthwhile in general.
      Release of pyyaml 3.13 seems to have fixed install issues on Python 3.7.
      Block pytest 3.7.3 due to pytest-dev/pytest#3888.
      Move most package config to declarative config
      Ignore pycodestyle warning. Seems it's not going to be fixed anytime soon.
      Also ignore flake8 error
      Require setuptools 34.4 to support python_requires in declarative config.
      Add workaround for Teemu/pytest-sugar#159.
      Add black config, pre-commit including black, check code with black.
      Remove workaround for pytest-sugar 159, now fixed.
      Remove pytest-sugar plugin from standard pipelines as recommended in Teemu/pytest-sugar#159.
      Prefer pytest-checkdocs to collective.checkdocs
      Suppress deprecation warning in docutils
      Remove use of setup_requires. Builders now require pip 10 or later to build/install from sdist. Older installers will still install the packages from wheels. Ref tox-dev/tox#809.
      Revert "Remove use of setup_requires. Builders now require pip 10 or later to build/install from sdist. Older installers will still install the packages from wheels. Ref tox-dev/tox#809."
      Indicate build backend of setuptools
      Add support for cutting releases without DPL and using pep517.
      Rely on pep517 0.5
      Add documentation on the skeleton. Fixes #5.
      Add workaround for DeprecationWarning in flake8
      Use consistent encoding quoting in pyproject.toml
      Clarify purpose of local/upstream extras
      Suppress E117 as workaround for PyCQA/pycodestyle#836
      Amend skeleton documentation to expand on the value of the approach.
      Pin to old versions of pytest as workaround for man-group/pytest-plugins#110.
      Add workaround for man-group/pytest-plugins/issues/122
      Update changelog. Ref #42.
      pytest-runner has a single module, not a package. Fixes #44.
      Remove sudo declaration in Travis config.
      Enable tox-pip-extensions ext_venv_update if available. Fixes jaraco/skeleton#6
      Rely on tox 3.2 and pip 10 or later for all builds
      It adds no value to add a pip requirement for the tox install
      Pin to pip 19.0 for now for pypa/pip#6434.
      Revert "Pin to pip 19.0 for now for pypa/pip#6434."
      Only install and invoke pytest-black on Python 3
      Use pytest-black-multipy to enable simple support for pytest-black where available. Ref pytest-dev/pytest#5272.
      Update skeleton documentation to reflect black adoption.
      Fade to black
      Update changelog.
      Update changelog from master.
      Update changelog.
      Update changelog for 5.0
      Clarify changelog. Ref #49.
      Duplicately supply a minimal metadata in setup.py to support setuptools older than 30.4. Fixes #49.

Sebastian Kriems (1):
      spaces, style and formatters (#4)
@lock
Copy link

lock bot commented May 30, 2019

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.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging a pull request may close this issue.