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

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

Closed
pfmoore opened this issue Apr 21, 2020 · 76 comments · Fixed by #11874
Closed

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

pfmoore opened this issue Apr 21, 2020 · 76 comments · Fixed by #11874
Labels
type: deprecation Related to deprecation / removal.

Comments

@pfmoore
Copy link
Member

pfmoore commented Apr 21, 2020

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 pypa/setuptools#510 (comment) 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
Copy link

marcelm commented Apr 21, 2020

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

pfmoore commented Apr 21, 2020

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 pradyunsg added state: needs discussion This needs some more discussion type: deprecation Related to deprecation / removal. labels Apr 21, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Apr 21, 2020
@pradyunsg
Copy link
Member

pradyunsg commented Apr 21, 2020

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

@sbidoul
Copy link
Member

sbidoul commented Apr 21, 2020

+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:

if not req.use_pep517 and not is_wheel_installed():
# we don't build legacy requirements if wheel is not installed
logger.info(
"Could not build wheels for %s, "
"since package 'wheel' is not installed.", req.name,
)
return False

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:

# We don't care about failures building legacy
# requirements, as we'll fall through to a direct
# install for those.

@pradyunsg pradyunsg removed the resolution: duplicate Duplicate of an existing issue/PR label Apr 21, 2020
@pradyunsg
Copy link
Member

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

pfmoore commented Apr 21, 2020

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

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

sbidoul commented Apr 21, 2020

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

pfmoore commented Apr 21, 2020

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

sbidoul commented Apr 21, 2020

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

dstufft commented Apr 21, 2020

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

pfmoore commented Apr 21, 2020

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

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

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.

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

@uranusjr
Copy link
Member

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

pfmoore commented Apr 22, 2020

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

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

pfmoore commented Apr 24, 2020

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

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

sbidoul commented Apr 26, 2020

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

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?

@sbidoul
Copy link
Member

sbidoul commented Sep 17, 2022

So, after more head scratching than I care to admit, #11359, #11452 and #11454 finally conclude the series of deprecations towards removing setup.py install. These complement #8369, #11331 that were merged before.

@pypa/pip-committers do you think we should already announce a deprecation deadline (gone_in) in (some of) these, or should we wait for (more) user feedback? I tend to think the packaging landscape has matured quite a lot in the last year, but I may be underestimating the number of legacy projects that can't build a wheel for any reason.

@sbidoul sbidoul removed the state: needs discussion This needs some more discussion label Sep 17, 2022
@pfmoore
Copy link
Member Author

pfmoore commented Sep 17, 2022

I'm inclined to say we should make the timetable explicit. We've been clear on our intention for some time now that we plan on removing setup.py installs, and setuptools has publicly deprecated it, so I actually think we'll be doing any remaining projects a favour if we give them an explicit deadline. If there's a lot of pushback that our choice of date is too soon, we can easily change it, so being too aggressive isn't a fatal issue.

@pradyunsg
Copy link
Member

We should, yes.

I’m personally OK with picking a deadline anything from 6-12 months after 22.3. I’d lean toward 6 months… 6 months is the shortest we’re supposed to be doing, based on what we’ve agreed as a baseline for deprecations.

@sbidoul
Copy link
Member

sbidoul commented Sep 17, 2022

Ok, I'll change the PRs to set gone_in 23.1.

@sbidoul
Copy link
Member

sbidoul commented Sep 17, 2022

Done.

The next question will be if we'll need a period where setup.py install is available behind a --use-deprecated flag (I'd rather not have not implement that, TBH).

@pradyunsg
Copy link
Member

I don't have a good sense of how much churn this would cause for users.

The intuition I have for adding no-deprecated flags is basically whether we expect to get users who will show up with a pitchfork and demand that we give them that functionality back.

@pfmoore
Copy link
Member Author

pfmoore commented Sep 17, 2022

My intuition says that we'll get people complaining, but they will be the sort of people who aren't looking for a transition mechanism, but rather for a "make it work again" flag. So they'll simply complain again when the --use-deprecated flag is removed.

So I'm fine with not adding a --use-deprecated option. Adding pyproject.toml is straightforward, and if users can't do that, and can't fix the underlying issue that makes it impossible for them to do that within the deprecation period, and have ignored everything that's been said in various channels about looking at moving to pyproject.toml for ages now, then I'm happy saying that they should build and publish wheels manually as a workaround, or advise their users to pin pip until they work out a better fix.

If during the deprecation period we get anyone raising a serious and genuinely intractable issue, we can review and if necessary extend the deprecation period. If they don't engage with us until it's too late, well, I guess it's too late 🤷

@sbidoul
Copy link
Member

sbidoul commented Sep 18, 2022

Agreed.

I think users who may be most impacted are application authors who are (perhaps unknowingly) on the setup.py install code path and rely on (unmaintained) libraries which install successfully but don't support bdist_wheel for some reason.

They might be using setup.py install simply because they created a virtual environment with python -m venv and don't have wheel installed.

@pradyunsg
Copy link
Member

Hmm... I wonder if we should extend the isolation enabling logic, from #10717 to include wheel -- i.e. use isolated pyproject.toml builds if the environment does not have both setuptools and wheel installed. That's one way to avoid falling back to setup.py install in plain python -m venv environments -- before we actually make the changes in Python 3.12 for python/cpython#95299.

@pradyunsg
Copy link
Member

pradyunsg commented Oct 7, 2022

Actually, here's a shower thought...

It might make sense to entirely disable both setup.py install (this issue) and setup.py develop (#11457) on Python 3.12+. There's currently no users who are on that version -- such a change wouldn't be disruptive and, instead, it would be a minor hassle for performing the 3.12 upgrade. To be clear, I don't think we should do this for setup.py bdist_wheel just yet, since there's a few more pieces to change around that (#9175).

This could render some packages unusable on Python 3.12+, but I reckon that would be acceptable -- if they are actively maintained, the maintainers should be able to update the build system to behave correctly on newer versions of Python.

How do people feel about this, and do you reckon this is a good idea?

@sbidoul
Copy link
Member

sbidoul commented Oct 7, 2022

@pradyunsg does that imply keeping support for setup.py install and setup.py develop until we drop support for Python 3.11?

@pradyunsg
Copy link
Member

pradyunsg commented Oct 7, 2022

No.

I'm thinking of it as putting a maximum limit of when it will definitely be dead -- when 3.11 support is dropped in ~5-6 years.

We could still drop support for these mechanisms on older versions of Python before then, but it certainly won't be a problem after 6 years.

@sbidoul
Copy link
Member

sbidoul commented Oct 7, 2022

In any case, we have currently merged gone_in=23.1 for setup.py install :)

@pradyunsg
Copy link
Member

pradyunsg commented Oct 7, 2022

Assuming that works out, this only applies to the setup.py develop case then. :)

Let's move the discussion there.

coalsont pushed a commit to Washington-University/gradunwarp that referenced this issue Jan 6, 2023
pip has deprecated (pypa/pip#8102) version names that do not comply with PEP440 (https://peps.python.org/pep-0440/).
Making this change also avoids what seems to be a bug in newer versions of setuptools which can mangle the shebang (#!) when installing gradient_unwarp.py using the fallback option used by pip for non-compliant packages.
@pradyunsg
Copy link
Member

@sbidoul Would you have time/energy to file follow ups removing the relevant logic in this case? I’m down to do so, if you don’t have the bandwidth.

@sbidoul
Copy link
Member

sbidoul commented Mar 1, 2023

Hi @pradyunsg, yes I'm happy to do it. I'm kind of planning to work on this around mid/end of March.

@pradyunsg
Copy link
Member

Awesome! I'm looking forward to seeing this happen! ❤️

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: deprecation Related to deprecation / removal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.