pypa / pip

The Python package installer
https://pip.pypa.io/
MIT License
9.5k stars 3.01k forks source link

Raise a warning when pip falls back to the "legacy" direct install code #8102

Closed pfmoore closed 1 year ago

pfmoore commented 4 years ago

What's the problem this feature will solve? The legacy code (which calls setup.py install directly) relies on setuptools to generate entry point wrappers. The wrappers setuptools generates require pkg_resources at runtime, and are reported as being very slow (see https://github.com/pypa/setuptools/issues/510#issuecomment-617031254 and the discussion leading to it).

Having pip warn in this case would be helpful in diagnosing the subtle behaviour change when wheel isn't present.

Describe the solution you'd like When pip uses the setup.py install legacy code path, issue a warning pointing out that because the wheel library isn't present, pip is running setup.py install directly, so old-style script wrappers will be generated.

Alternative Solutions The solution is to install wheel, but this is not obvious from the information pip currently provides.

Additional context The warning has the potential to be "noisy", warning users when they don't care about the issue it's describing. But the legacy code path is just that - legacy - and ultimately we'd like to remove it. Warning people that they are inadvertently using a legacy code path prepares them for that possibility.

Conversely, the requirement for wheel to be installed is not well-managed currently (the stdlib venv doesn't install wheel by default, people don't typically realise that they should install wheel when just doing pip install ., etc. The proposed warning will give the requirement more visibility, which might help trigger a more useful discussion on how to manage the requirement for wheel.

Longer term the solution will be to switch fully to PEP 517 processing, but that is likely to be some time off yet.

marcelm commented 4 years ago

The answer may be obvious, but I’ve often wondered: As far as I understand, quite a few libraries are already vendored with pip, why isn’t wheel simply one of them?

pfmoore commented 4 years ago

Basically, because pip doesn't itself use wheel, it's the build subprocess that we call that needs wheel. Vendoring doesn't help in that situation. (Well, it may be that we could make it work somehow, but this issue is obsoleted by pyproject.toml, so there's a better solution for new projects).

pradyunsg commented 4 years ago

This looks similar to #7709, which got fixed in #7768, which is in pip 20.1b1.

sbidoul commented 4 years ago

+1 for this.

Do we want to start the deprecation process now (and collect feedback in a deprecation issue), or simply raise awareness with louder messages?

We could raise the level from info to warning in _should_build, and/or allude to a future deprecation:

https://github.com/pypa/pip/blob/814c54f5d7e17682f9c476f5978c4d5666abb52c/src/pip/_internal/wheel_builder.py#L72-L78

pip will also fallback to legacy install when wheel building failed for other reasons, so we may want to warn there also, and encourage people to make sure that setup.py bdist_wheel works for their packages:

https://github.com/pypa/pip/blob/814c54f5d7e17682f9c476f5978c4d5666abb52c/src/pip/_internal/commands/install.py#L368-L370

pradyunsg commented 4 years ago

Do we want to start the deprecation process now (and collect feedback in a deprecation issue)

I'm on board for this, though we'd need to decide on how long this should be.

pfmoore commented 4 years ago

+1 from me. Part of me wants to say 21.0, to match the Python 2 desupport, because that makes it a single breakpoint for users (the "desupport everything" release 🙂).

The converse argument is that users won't want to have to deal with 2 different issues in the same timeframe, but my instinct is that the affected groups of users are likely pretty disjoint (and/or the same solution, pin or upgrade your project, fixes both issues at the same time).

pradyunsg commented 4 years ago

Part of me wants to say 21.0, to match the Python 2 desupport

I'm glad I didn't say this first. :)

I'll flag that the sheer amount of traffic on PEP 517 and projects that can't install via wheels (~46k views, most viewed) tells me that there are lots of users who use the direct "setup.py install" code path.

The next topic in terms of number of views is something that, IIRC, made it to the front page of Hackernews, had to be closed by moderators, and THAT has just about half the number of views: https://discuss.python.org/top/all.


Thinking about this more, I'm uncomfortable suggesting "let's warn/deprecate now" without us spending some time checking if we can do something better in overall "PEP 517/PEP 518/build isolation" situation, which is strongly coupled to this IMO.

I really don't have a good feel for what is a reasonable approach/strategy for eventually transitioning off the legacy build/install logic would be, and it feels like we should talk about the broader problem first before discussing the specifics here.

sbidoul commented 4 years ago

OTOH, our deprecation mechanism, combined with a friendly message, is a nice way to ask concerned users to report feedback in a GitHub issue. Based on that feedback we can still decide not to deprecate at the planned date or otherwise adjust the strategy.

Also, in my understanding, deprecating the setup.py install code path is "softer" that enforcing build isolation.

pfmoore commented 4 years ago

Thinking about this more, I'm uncomfortable suggesting "let's warn/deprecate now" without us spending some time checking if we can do something better in overall "PEP 517/PEP 518/build isolation" situation, which is strongly coupled to this IMO.

Wait - what I thought we were discussing the deprecation of was falling through to setup.py install. All that would involve would saying something like "Cannot build wheel for legacy install, please install the wheel project or switch to pyproject.toml". And converting that to a hard error and ripping out the legacy setup.py install code 🙂 once we get to 20.1.

Forcing everything to use PEP 517/518/build isolation is a much bigger issue, and I agree that for that, we need to disentangle some of the complexities around isolation, --no-binary, etc.

sbidoul commented 4 years ago

Another reason to deprecate the setup.py install code path is that some features (namely direct_url.json and REQUESTED) will only work when installing via wheel.

dstufft commented 4 years ago

I've long wished we had a way to get some metrics on pip usage, to try and suss out these kinds of questions. As it is right now, we really have no good way to determine if 100% of pip install invocations are relying on the legacy fallback, or if 0%, or somewhere in between.

pfmoore commented 4 years ago

Agreed. So (possibly over-eager) deprecation is a blunt tool, but it's really the only way we have of judging impact.

There's also in this instance the case of catching people who simply forgot to install wheel, and have absolutely no problem installing it if reminded. I do that all the time when I use venv.

pradyunsg commented 4 years ago

Wait - what I thought we were discussing the deprecation of was falling through to setup.py install.

And you are correct. I just derailed and started mixing this with the other bigger problem in my head (we use "legacy" for way too many things now). :)

OTOH, our deprecation mechanism, combined with a friendly message, is a nice way to ask concerned users to report feedback in a GitHub issue.

Indeed! It is a nice way, and I don't think we have any other option here TBH. :)

xavfernandez commented 4 years ago

All that would involve would saying something like "Cannot build wheel for legacy install, please install the wheel project or switch to pyproject.toml". And converting that to a hard error and ripping out the legacy setup.py install code :slightly_smiling_face: once we get to 20.1.

We could also try to install those using build isolation and a default pyproject.toml build system (setuptools & wheel).

uranusjr commented 4 years ago

We could also try to install those using build isolation and a default pyproject.toml build system (setuptools & wheel).

The down side to this would be more confusion. We already have people asking why pip fails to install setuptools even though pip list clearly shows setuptools is installed, and this would be much more of a problem for packages that perform install-time feature-detection (e.g. install this extra thing if Numpy is already installed) if we switch them to use an isolated build. And I persume those packages should be the main holdouts to the isolation feature.

I don’t think this precludes the possibility, to be clear, but we’ll need to consider the support cost trade-off 🙂

pfmoore commented 4 years ago

I agree with @uranusjr here. PEP 517 (and 518, and build isolation) has up to this point been an opt-in choice (at least in the sense that unless you change your project or your build command, pip won't change its behaviour. Erroring out sticks to that policy, telling the users how to opt in (or remain opted out for a little bit longer), but not opting them in silently.

xavfernandez commented 4 years ago

Fair points but I suspect we'll then also get users asking "why does pip complain that wheel is not installed and does not install it already ?" ^^ It seems more user friendly to try to build those legacy projects via the default PEP-517 configuration than completely error.

pfmoore commented 4 years ago

@xavfernandez I'm somewhat confused. The original proposal here was essentially "if we get through the build code paths, and end up at the point where we currently do setup.py install (which is when building a wheel for the project has failed), we switch to writing a deprecation warning, and then later hard error".

Basically add a deprecation/error at this line in the code, ultimately removing the following lines and install_legacy.

What you seem to be suggesting is that much earlier in the process, we check whether the user has wheel installed, and if not then we effectively set the --use-pep517 flag. In addition, we'd still need to add a warning/error at the same point, as we still have the possibility that building the wheel failed and we didn't trap it earlier (as @sbidoul pointed out here).

I'm not against your proposal, but it's more than what I was proposing, and it seems like it would be a more controversial change (because my proposal is a strict subset of yours, if for no other reason 🙂).

pradyunsg commented 4 years ago

Fair points but I suspect we'll then also get users asking "why does pip complain that wheel is not installed and does not install it already ?" ^^

I think you're referring to #7709?

sbidoul commented 4 years ago

Digging into this a little bit I remember that --no-binary implies falling back to setup.py install too.

So this warning would also mean deprecating that behavior of --no-binary. Which is fine with me, as installing from source should not preclude building a wheel locally.

bluetech commented 4 years ago

For me this happened due to this:

virtualenvs created with virtualenv have wheel installed:

$ virtualenv venv
created virtual environment CPython3.8.2.final.0-64 in 222ms
  creator CPython3Posix(dest=/home/ran/venv, clear=False, global=False)
  seeder FromAppData(download=False, pip=latest, setuptools=latest, wheel=latest, via=copy, app_data_dir=/home/ran/.local/share/virtualenv/seed-app-data/v1.0.1)
  activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
$ . venv/bin/activate
(venv) $ pip list
Package    Version
---------- -------
pip        20.0.2 
setuptools 46.1.3 
wheel      0.34.2 

but virtualenvs created by python -m venv don't:

$ python3.8 -m venv venv
$ . venv/bin/activate
(venv) $ pip list
Package    Version
---------- -------
pip        19.2.3 
setuptools 41.2.0 

I don't know the details, but I think many people create virtualenvs using python -m venv and don't know they should install wheel. At least I didn't know to do this until I debugged some issue a while ago. I don't know if it makes sense, but is it feasible to have python -m venv install wheel?

pfmoore commented 4 years ago

I don't know if it makes sense, but is it feasible to have python -m venv install wheel?

This is how I hit the issue too. To get venv changed would need an issue raising for the core Python project. My feeling is that they would probably reject it on the grounds that the stdlib doesn't want to sanction anything other than the bare minimum of 3rd party projects - IIRC even setuptools is only present as an "implementation detail" of having pip.

IMO, the next step after adding the warning wouldn't be to worry about getting wheel more commonly available, but moving to PEP 517/8 as default, as that removes the need for the user to have setuptools and wheel installed at all.

uranusjr commented 4 years ago

Ref: https://bugs.python.org/issue31634

Following the line of thought, maybe the warning should prompt the user to nudge package maintainers to migrate to PEP 517/518, instead of installing wheel. One way to do this would be to open a locked issue on GitHub explaining the situation, and reference the link in the message.

xavfernandez commented 4 years ago

@xavfernandez I'm somewhat confused. The original proposal here was essentially "if we get through the build code paths, and end up at the point where we currently do setup.py install (which is when building a wheel for the project has failed), we switch to writing a deprecation warning, and then later hard error".

Maybe I wasn't clear with my proposal. It applies to pip installing a sdist/vcs checkout/local directory (i.e. not a wheel).

Currently:

With what I understand from the proposal:

And my suggestion:

pfmoore commented 4 years ago

@xavfernandez That seems sensible, although you have "otherwise behave as if pyproject.toml was present" followed by "otherwise error out". The second "otherwise" doesn't seem like it would ever happen? Or was that meant to be "otherwise (if there's no pyproject.toml and no setup.py)"?

This basically means that we opt people into PEP 517, build isolation, etc, if they don't have wheel installed. That's quite a big change in behavior triggered by something as subtle as wheel not being installed - but on the presumption that our ultimate goal is to switch over to always using PEP 517 etc, I'm OK with this. We will almost certainly get pushback when this is implemented (heck, we get pushback on everything 🙂), but we can simply explain our reasoning and the long term goal - there's an easy workaround for users in terms of just installing wheel.

xavfernandez commented 4 years ago

The second "otherwise" doesn't seem like it would ever happen? Or was that meant to be "otherwise (if there's no pyproject.toml and no setup.py)"?

You're right. Someone must have copy-pasted it from the other cases :roll_eyes:

That's quite a big change in behavior triggered by something as subtle as wheel not being installed

I agree but I think it is still better than erroring out while for a vast majority of simple pure-python setuptools based projects, the default pyproject.toml is likely to just work.

uranusjr commented 4 years ago

One minor nitpick, if we’re going to go with the “pretend there’s a pyproject.toml” route, the default backend should be setuptools.build_meta.__legacy__ instead, which mimics the non-isolated build slightly better.

dennylab commented 4 years ago

python 363: I have one (maybe dumb) question:

Why are we gatting this warning when installing a library using pip? Example: We install ls-stacktrace (sdist on own pypi server), which has a requirement to flask in the setup.py. We're getting:

Could not build wheels for flask, since package 'wheel' is not installed.

I mean, it's not up to the user to have wheel installed, but it's up to the publisher to publish using a bdist instead of sdist, right?

When installing wheel before installing the other lib, the warning is gone. But it doesn't change anything!?

Or does wheel on the user's side change something regarding "pkg_resources" or whatever?

I tried it with pillow on Windows: With "wheel" installed and without: Using pip verbose I saw that the execution and the result is exactly the same. The *.whl is used, which is fine.

Thanks

uranusjr commented 4 years ago

I mean, it's not up to the user to have wheel installed, but it's up to the publisher to publish using a bdist instead of sdist, right?

This is actually one of the root problems. There are a number of reasons a project cannot produce pre-built wheels, e.g. lack of development resources, or maybe it simply needs to depend on information only available on installation. pip cannot outright refuse those usages, and emitting a warning on the installation site is the best effort for pip to make this information known, so at least the user knows what’s going on behind the scenes.

dennylab commented 4 years ago

But in most cases the end user don't care about this, since most libraries don't even have compiled c code, so the install is in reality just copy paste of py files... no?

Also, the end user has zero advantages by installing wheel, since it doesn't change anything in the install process or elsewhere. These are just warnings he doesn't want to see. If you don't agree, it should at least be displayed only ONCE when using -r, not x times.

I don't see why users are getting spammed with messages they can't really fix, they can just hide them by installing another thing that isn't even needed! It sounds strange to me...

(end user = person installing a lib)

One small related question: Is the lib "wheel" pre-delivered from a specific interpreter version or will just everybody get that warning right now until they install it?

pfmoore commented 4 years ago

so the install is in reality just copy paste of py files... no?

No. The install also involves generating metadata, checking that metadata for dependencies and installing those, and a number of other actions.

Agreed that users having to install wheel is not ideal. But projects have to fix that by moving to pyproject.toml and/or distributing wheels. From the Python packaging tools end, we've publicised that, but projects move slowly. While the projects don't change, users hit issues and the best we (pip) can do is (1) tell them how to work around the issue (install wheel) and (2) hope they put extra pressure on the projects to move forward.

It's a problem that users don't really know (or want/need to know) about the details of what happens with an install, and why Python code needs "building", but I don't know how we can solve that (without forcing end users to learn about this stuff regardless 🙁)

dennylab commented 4 years ago

Interesting, I've never heard about pyproject.toml, and I'm a full time py dev. So you might see that it doesn't get as much attention as you want... We're distributing source dists just because it's the easiest way, but I understand that wheels are better.

I understand your point. You want to push the new method because of x advantages. That's good, because we shouldn't stuck with old stuff, but you can also see that it's confusing, since we should try to blame the devs, not the "installers". But yeah, you're pip, not setuptools...

Thanks for your answers!

davidbullado commented 4 years ago

Hi everybody,

I'm just a regular user. I started to see this message appearing everywhere and wondered what it could mean: "Could not build wheels for *, since package 'wheel' is not installed"

Now, I understand that I should install the wheels package, because it will be soon mandatory, and because it could make the build faster. But that was not the case the first time I saw this message. (I thought it was just a random message because I mess with my computer or something like that).

Why not make the message more explicit like that: " could be build faster with package 'wheels'. We encourage you to install it"

What do you guys think?

pradyunsg commented 4 years ago

@davidbullado That message has been improved and those changes will be released in a bugfix by Monday. See https://github.com/pypa/pip/pull/8180/.

adampl commented 4 years ago

Still, why not include wheel package as pip package dependency, or at least add a log message encouraging users to install it?

uranusjr commented 4 years ago

You’re definitely not the first one to propose this. Suffice to say things don’t really work like this :)

pradyunsg commented 4 years ago

I think we need to start saying "generate metadata and distributions" instead of "build" to be honest.

johnthagen commented 4 years ago

Another regular user showing after seeing messages like Using legacy setup.py install for Kivy-Garden, since package 'wheel' is not installed. from venvs created with python3 -m venv venv.

I want to make sure I understand the takeaways:

  1. After creating a venv using the venv module, rather than running python -m pip install --upgrade pip, my common practice should now be to run python -m pip install --upgrade pip wheel instead.
  2. I should be encouraging / helping third party libraries I use to publish .whl files in addition to sdist files.

Is that an accurate summary?

uranusjr commented 4 years ago

I think the summary is correct. Some additional advice:

sbidoul commented 4 years ago

I tend to agree with @xavfernandez's proposal in https://github.com/pypa/pip/issues/8102#issuecomment-622401675. I'd say it can be further discussed when we reach the end of the deprecation period and we have received more feedback.

I have identified 3 cases where pip falls back to setup.py install for source requirements without pyproject.toml:

  1. when --no-binary is used (and --use-pep517 is not)
  2. when wheel is not installed
  3. when setup.py bdist_wheel fails

I propose

@pypa/pip-committers does that sound like a plan? If yes we can discuss the exact message and solution offered to users for each in separate PRs.

pfmoore commented 4 years ago

Sounds like an excellent plan 🙂

sbidoul commented 4 years ago

Noting that deprecating setup.py install when no-binary is active implies also deprecating --install-options, --build-options, --global-options (#2677). I would be inclined to add a 4th deprecation so we collect feedback for that too and try to better understand how important it is to implement PEP 517 config settings (#5711) as a replacement.

sbidoul commented 4 years ago

I'm starting with 3. (the easiest / most obvious) in #8369. If the approach looks good I'll continue with 2. and then see how to address 1., which is much more hairy due to the interaction with --install-options and friends.

sbidoul commented 4 years ago

Noting that deprecating setup.py install when no-binary is active implies also deprecating --install-options, --build-options, --global-options (#2677). I would be inclined to add a 4th deprecation so we collect feedback for that too and try to better understand how important it is to implement PEP 517 config settings (#5711) as a replacement.

Following https://github.com/pypa/pip/issues/2677#issuecomment-640027700, an intermediary step is to deprecate --install-option while adding --build-option to pip install. It is the path that I am exploring now.

sbidoul commented 4 years ago

The fact that --no-binary implies setup.py install is annoying and hard to transition. If we deprecate setup.py install we want to provide the user with a mechanism to set future-proof options and therefore silence the deprecation messages. When --no-binary (1) is used there is no way to do that, contrarily to (2) and (3) where there are solutions (respectively install wheel and fix the build error).

So I think we need to create a new, transitory option: --always-install-via-wheel or -no-use-setuppy-install. It would default to False with the goal of making it the default in the future.

Then the deprecation message can encourage users of --no-binary to activate that option and report problems they discover with that approach.

sbidoul commented 4 years ago

Following the decision about feature flags, I propose --use-feature=always-install-via-wheel.

So when using --no-binary we will print a deprecation warning saying that --no-binary currently implies a deprecated call to setup.py install, and tell the user to silence the warning by using --use-feature=always-install-via-wheel.

When the transition is done, --use-feature=always-install-via-wheel becomes a no-op.

If we decide to keep the setup.py install code path for some time after the transition, then we'll have --use-deprecated=setup-py-install.

cc/ @nlhkabu

pfmoore commented 2 years ago

I have identified 3 cases where pip falls back to setup.py install for source requirements without pyproject.toml:

  1. when --no-binary is used (and --use-pep517 is not)
  2. when wheel is not installed
  3. when setup.py bdist_wheel fails

[...]

I'm starting with 3. (the easiest / most obvious) in https://github.com/pypa/pip/pull/8369. If the approach looks good I'll continue with 2. and then see how to address 1., which is much more hairy due to the interaction with --install-options and friends.

I'm trying to pull together the position on this.

So we're still some way from getting setup.py install fully deprecated, let alone removed. No pressure, I just wanted to pull together where we are.

My interest in this is with regard to #11307, which is unlikely to support the setup.py install code path. As a pre-emptive measure, I'd expect to add an explicit check in #11307 to fail if we end up at setup.py install (assuming that feature gets completed before the deprecation here does 🙂).

sbidoul commented 2 years ago

Part of this deprecation of the setup.py install code path, I was also considering to remove the installation of setuptools from ensurepip.

Indeed the current situation with a default venv from python -m venv is the worst possible:

What do you think? Should this be raised on discuss?

sbidoul commented 2 years ago

To complement, the above: since 22.1 we automatically enable PEP 517 when setuptools is not installed (#10717).

pfmoore commented 2 years ago

I was also considering to remove the installation of setuptools from ensurepip.

+1 on that. PEP 453 says "The private copy of setuptools will be removed from ensurepip once it is no longer needed. This is likely to be at the point when get-pip.py stops installing setuptools by default." I'd be OK with considering the fact that we enable PEP 517 (and isolated builds) when setuptools is not present as sufficient here, and removing setuptools from both ensurepip and get-pip.

What do you think? Should this be raised on discuss?

Edit: Aargh, I got in a complete muddle here. The following is what I meant to say first time...

I don't think it needs to go on Discourse. It's easy enough for anyone who gets an error to do pip install setuptools, and we don't normally pre-announce changes.

I think we can (and should) change get-pip.py to no longer include setuptools and wheel pretty much as soon as we want. And we can encourage virtualenv to do the same. The ensurepip change might need to wait for Python 3.12, though, to fit with the Python release schedule. I think it's too late for the initial release of 3.11, and while we may be able to get approval for doing it in a 3.11 patch release (it's documented as an internal detail and subject to change without warning) I don't think it's worth doing so. But if we want to, checking with the RM and/or python-dev should be enough.