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 24.1.2 does not include wheel as a dependency. #124

Closed
1 task done
tribeiro opened this issue Jul 31, 2024 · 40 comments · Fixed by #125
Closed
1 task done

Pip 24.1.2 does not include wheel as a dependency. #124

tribeiro opened this issue Jul 31, 2024 · 40 comments · Fixed by #125
Labels

Comments

@tribeiro
Copy link

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

pip 24.1.2 was released on 2024-07-31 07:20:55. Ever since it was released and our build system started to pick it up we have a build error that says:

error: invalid command 'bdist_wheel'

After doing a diff on the dependencies installed I noticed the following:

26c26
<     pip:               24.0-pyhd8ed1ab_0          conda-forge
---
>     pip:               24.1.2-pyhd8ed1ab_0        conda-forge
38d37
<     wheel:             0.43.0-pyhd8ed1ab_1        conda-forge

it seems like going from version 24.0 to 24.1.2 no longer installs the wheel package as a dependency, which seems to be the source of the problem.

Installed packages

_libgcc_mutex:     0.1-conda_forge            conda-forge
    _openmp_mutex:     4.5-2_gnu                  conda-forge
    bzip2:             1.0.8-h4bc722e_7           conda-forge
    ca-certificates:   2024.7.4-hbcca054_0        conda-forge
    ld_impl_linux-64:  2.40-hf3520f5_7            conda-forge
    libblas:           3.9.0-23_linux64_openblas  conda-forge
    libcblas:          3.9.0-23_linux64_openblas  conda-forge
    libexpat:          2.6.2-h59595ed_0           conda-forge
    libffi:            3.4.2-h7f98852_5           conda-forge
    libgcc-ng:         14.1.0-h77fa898_0          conda-forge
    libgfortran-ng:    14.1.0-h69a702a_0          conda-forge
    libgfortran5:      14.1.0-hc5f4f2c_0          conda-forge
    libgomp:           14.1.0-h77fa898_0          conda-forge
    liblapack:         3.9.0-23_linux64_openblas  conda-forge
    libnsl:            2.0.1-hd590300_0           conda-forge
    libopenblas:       0.3.27-pthreads_hac2b453_1 conda-forge
    libsqlite:         3.46.0-hde9e2c9_0          conda-forge
    libstdcxx-ng:      14.1.0-hc0a3c3a_0          conda-forge
    libuuid:           2.38.1-h0b41bf4_0          conda-forge
    libxcrypt:         4.4.36-hd590300_1          conda-forge
    libzlib:           1.3.1-h4ab18f5_1           conda-forge
    ncurses:           6.5-h59595ed_0             conda-forge
    numpy:             1.25.2-py311h64a7726_0     conda-forge
    openssl:           3.3.1-h4bc722e_2           conda-forge
    packaging:         24.1-pyhd8ed1ab_0          conda-forge
    pip:               24.1.2-pyhd8ed1ab_0        conda-forge
    python:            3.11.9-hb806964_0_cpython  conda-forge
    python_abi:        3.11-4_cp311               conda-forge
    readline:          8.2-h8228510_1             conda-forge
    setuptools:        68.2.2-pyhd8ed1ab_0        conda-forge
    setuptools-scm:    7.0.5-pyhd8ed1ab_1         conda-forge
    setuptools_scm:    7.0.5-hd8ed1ab_1           conda-forge
    tk:                8.6.13-noxft_h4845f30_101  conda-forge
    tomli:             2.0.1-pyhd8ed1ab_0         conda-forge
    typing-extensions: 4.12.2-hd8ed1ab_0          conda-forge
    typing_extensions: 4.12.2-pyha770c72_0        conda-forge
    tzdata:            2024a-h0c530f3_0           conda-forge
    xz:                5.2.6-h166bdaf_0           conda-forge

Environment info

active environment : base
    active env location : /home/saluser/miniconda3
            shell level : 1
       user config file : /home/saluser/.condarc
 populated config files : /home/saluser/miniconda3/.condarc
                          /home/saluser/.condarc
          conda version : 24.5.0
    conda-build version : 24.5.1
         python version : 3.11.9.final.0
                 solver : libmamba (default)
       virtual packages : __archspec=1=skylake_avx512
                          __conda=24.5.0=0
                          __glibc=2.28=0
                          __linux=4.14.343=0
                          __unix=0=0
       base environment : /home/saluser/miniconda3  (writable)
      conda av data dir : /home/saluser/miniconda3/etc/conda
  conda av metadata url : None
           channel URLs : https://conda.anaconda.org/lsstts/linux-64
                          https://conda.anaconda.org/lsstts/noarch
                          https://conda.anaconda.org/conda-forge/linux-64
                          https://conda.anaconda.org/conda-forge/noarch
          package cache : /home/saluser/miniconda3/pkgs
                          /home/saluser/.conda/pkgs
       envs directories : /home/saluser/miniconda3/envs
                          /home/saluser/.conda/envs
               platform : linux-64
             user-agent : conda/24.5.0 requests/2.31.0 CPython/3.11.9 Linux/4.14.343-261.564.amzn2.x86_64 almalinux/8.10 glibc/2.28 solver/libmamba conda-libmamba-solver/24.1.0 libmambapy/1.5.8
                UID:GID : 73006:73006
             netrc file : None
           offline mode : False
@tribeiro tribeiro added the bug label Jul 31, 2024
@tribeiro
Copy link
Author

I wonder if this is a result of the changed made in #121

@jakirkham
Copy link
Member

It is. Quoting my reply from there ( #121 (comment) ):

The change was intentional in the bullet points above:

  • Remove the run requirements on setuptools and wheel. They are only needed to build the package not at runtime.

That said, no strong feelings if folks would like to add them back

@jakirkham
Copy link
Member

cc @conda-forge/pip @conda-forge/core (for feedback)

@beckermr
Copy link
Member

Sorry for the crossed issues/PRs. Let's keep the discussion here. My latest comment copied over

Yep. I see that the change was intentional.

I am looking for further discussion. Do we want to take the pure path here or go more "batteries included"?

AFAIK, we have shipped setuptools+wheel as run deps with pip for ~8 years now (5f1aff6) since this feedstock was created.

Given how low this package sits in the typical dev stack, the removal of these deps is definitely non-trivial.

@tribeiro
Copy link
Author

Do you mind if I open a PR to mark the package as broken while the discussion is still ongoing? I am happy to wait until a final decision is made before merging it. Just want to have the ducks lined up so we can get our project build up and running as soon as possible.

@jakirkham
Copy link
Member

Think that an open PR is a good thing to have. Maybe mark it as WIP & draft?

Though yeah let's see what others think in this discussion

@bollwyvl
Copy link

bollwyvl commented Jul 31, 2024

One the one hand, it's making a lot of work to update feedstocks.

On the other: setuptools is a bit of a beast, and runtime use of it or its various other site-packages spawn is something of a smell. It's probably healthy for it to not be a dependency of pip, so that pip check can help us find where setuptools is a run (or host) dependency, and plan for the inevitable API breakages that are ominously threatened every once and a while.

Perhaps moving to a pip-split, which shipped

  • pip-base, with 0 dependencies, appropriate for e.g. test: {requires: [pip-base]}
  • pip, with the historical dependencies and pip-base

... would provide a way to start transitioning to more accurate host deps.

@beckermr
Copy link
Member

One the one hand, it's making a lot of work to update feedstocks.

We face similar issues with matplotlib+qt, moved to matplotlib-base + matplotlib as suggested above, ended up using the bot to rename deps in its PRs, and having the linter warn folks.

I'd be ok with that solution here.

Normally, I'd be in the camp above "you should not have been using transitive deps in the first place." However, in this case the pain we're causing might just be too high to take the pure road without a transition plan as suggested by @bollwyvl and this comment.

@jjhelmus
Copy link
Contributor

One the one hand, it's making a lot of work to update feedstocks.

These feedstocks do require updating but they also are incorrect in the current state in my opinion. Feedstocks that do not list setuptools and wheel are not properly declaring their build backend. Projects that use alternative build backends like flit-core or hatchling already need to declare these dependencies, setuptools based packages should have the same requirement. With the old setup non-setuptools based projects were building in environments with setuptools and wheel which served no purpose with no good option for excluding them.

@beckermr
Copy link
Member

These feedstocks do require updating but they also are incorrect in the current state in my opinion. Feedstocks that do not list setuptools and wheel are not properly declaring their build backend. Projects that use alternative build backends like flit-core or hatchling already need to declare these dependencies, setuptools based packages should have the same requirement. With the old setup non-setuptools based projects were building in environments with setuptools and wheel which served no purpose with no good option for excluding them.

I agree 100%. I simply want to ease the transition to the new way of doing things so that folks are not caught by surprise. After a sufficent amount of time, I'd consider merging the pip-base and pip packages as well.

@jjhelmus
Copy link
Contributor

I simply want to ease the transition

Splitting pip into pip-core and pip creates two transitions.
The first would is packages to using pip-core to run pip check in tests or when they use non-setuptools backends. This would not be required but it will crop up.

The second is when pip-core is removed. At that point all the feedstocks that made the change above need to switch back.

IMHO adding pip-core creates future work and does not offer people who want to follow best practices (declaring their full dependencies) a straight forward path. I'd rather see:

  • feedstocks update their dependencies
  • re-add setuptools and wheel as pip run requirements for the time being and set up a linter to flag packages that only depend on pip. After some amount of time (6 months?) drop the dependencies on setuptools and wheel.

@jjhelmus
Copy link
Contributor

@tribeiro To address the issue you reported, you should add wheel and potentially setuptools as dependencies to your build process. bdist_wheel is a command provided by the wheel package not pip

@jjhelmus
Copy link
Contributor

I'll add that starting with Python 3.12, setuptools is not installed into environments created using venv or when using ensurepip. python/cpython#95299 has details and discussion on this change.

@maresb
Copy link

maresb commented Jul 31, 2024

Maybe too simple-minded, but why not just:

  1. readd wheel as a dependency of pip as a temporary lie
  2. update the bulk of the most important feedstocks to explicitly declare wheel explicitly
  3. release a build 1 of pip that drops the wheel dependency and clean up the remaining breakage

Introducing multiple pip packages seems really confusing, like more bloat for the knowledge base.

@dholth
Copy link

dholth commented Jul 31, 2024

Which version of setuptools includes bdist_wheel as a vendored package, instead of a separate project?

@jjhelmus
Copy link
Contributor

Maybe too simple-minded, but why not just:

  1. readd wheel as a dependency of pip as a temporary lie
  2. update the bulk of the most important feedstocks to explicitly declare wheel explicitly
  3. release a build 1 of pip that drops the wheel dependency and clean up the remaining breakage

Introducing multiple pip packages seems really confusing, like more bloat for the knowledge base.

I like this approach, but you need to add both wheel and setuptools or just setuptools >= 70.1.0 (wheel is vendored in setuptools starting in 70.1.0). setuptools is the build backend which many feedstocks will call when running python -m pip install .....

@jjhelmus
Copy link
Contributor

Which version of setuptools includes bdist_wheel as a vendored package, instead of a separate project?

bdist_wheel was adopted from the wheel project in setuptools 70.1.0, pypa/setuptools#1386.

@beckermr
Copy link
Member

I simply want to ease the transition

Splitting pip into pip-core and pip creates two transitions.
The first would is packages to using pip-core to run pip check in tests or when they use non-setuptools backends. This would not be required but it will crop up.

The second is when pip-core is removed. At that point all the feedstocks that made the change above need to switch back.

IMHO adding pip-core creates future work and does not offer people who want to follow best practices (declaring their full dependencies) a straight forward path. I'd rather see:

feedstocks update their dependencies
re-add setuptools and wheel as pip run requirements for the time being and set up a linter to flag packages that only depend on pip. After some amount of time (6 months?) drop the dependencies on setuptools and wheel.

This plan is fine by me, but the breakages go beyond the considerations of feedstocks. The change in deps is breaking people who use pip in conda environments outside of conda-forge feedstocks. Those folks will experience a breaking change if we remove the deps from the pip package permanently. We can make that change, but in this case for this package, it should be announced in our news feed to give people warning.

@beckermr
Copy link
Member

The second is when pip-core is removed. At that point all the feedstocks that made the change above need to switch back.

To be clear, and I was not above, my original proposal would involve never removing pip-core/pip-base. It would be a shell package that would live on forever and simply depend on pip.

@jjhelmus
Copy link
Contributor

jjhelmus commented Jul 31, 2024

The change in deps is breaking people who use pip in conda environments outside of conda-forge feedstocks.

Can you expand on how this will break? Pip will install the declared build backend or setuptools from PyPI automatically unless --no-build-isolation is specified on the command line. Users who though pip provided the bdist_wheel command were misinformed and were depending on a transitive dependency.

@beckermr
Copy link
Member

unless --no-build-isolation is specified on the command line

Exactly. Many people install everything they need from conda-forge and then use say an editable install of their package with pip install --no-build-isolation -e . for their work, in their CI jobs etc. instead of letting the pip provided by conda make a venv with build deps it needs.

@beckermr
Copy link
Member

Users who though pip provided the bdist_wheel command were misinformed and were depending on a transitive dependency.

100% agree on that.

@beckermr
Copy link
Member

Thanks for the bump on that issue @isuruf. IMHO us breaking CI pipelines for pytorch is not a great end result.

@jakirkham
Copy link
Member

jakirkham commented Jul 31, 2024

Is it fair to say that given the overwhelming response here, we should...?

  • Add back the dependencies for now
  • Devise a plan to drop the dependencies and communicate this out

@jjhelmus
Copy link
Contributor

Many people install everything they need from conda-forge and then use say an editable install of their package with pip install --no-build-isolation -e .

This command never worked if the project used flit or hatching, only setuptools. That said setuptools was the backend for so long many people never were aware they were using it. Re-adding the dependency will allow this misunderstanding to continue but does smooth it out for many users. My concern is that there will never be a good time to drop the dependency.

@isuruf
Copy link
Member

isuruf commented Jul 31, 2024

My concern is that there will never be a good time to drop the dependency.

We can do this for the upcoming python 3.13 release.

@beckermr
Copy link
Member

I think we drop them in three to six months after we have time to cleanup feedstocks and give people notice.

My concern here is easing the transition, not dropping them in and of itself. I'm ok with the break but folks need warning. I'm also ok with the split recipe approach. As long as we give people notice, I'm perfectly happy.

@jjhelmus
Copy link
Contributor

#125 Adds setuptools and wheel to the run requirements and update the recipe to the latest version, 24.2.

@jakirkham
Copy link
Member

Thanks Jonathan! 🙏

Both for helping raise awareness about this issue we need to address and helping us think through planning of this change

Agree with Isuru that aligning with a migration like 3.13 could be a good approach

@dholth
Copy link

dholth commented Jul 31, 2024

Wouldn't pip (more dependencies) normally depend on pip-core (fewer dependencies)?

@jjhelmus
Copy link
Contributor

On the topic of 3.13. I've been building out packages using the 3.13.0b4 release (available on ad-testing/label/py313) for Anaconda and found ~20% of our recipes were missing setuptools and wheel. So the problem is quite common.

@jakirkham
Copy link
Member

While Jonathan's PR has been merged, think the points he raised here are important and need further discussion

As that discussion is already happening here, will reopen so that can continue

@beckermr
Copy link
Member

I can help with announcements, bot prs, and the linter in the next few weeks.

@jakirkham
Copy link
Member

Thanks Matt! 🙏

Would be interested in your thoughts on issue ( regro/cf-scripts#2888 ), when you have a moment 🙂

@tribeiro
Copy link
Author

It seems like the decision is to keep wheel for the time being.

I opened a PR to mark the 24.2.1 package as broken on Conda-forge.

Do you agree with that (mark it as broken)?

This is the PR by the way: conda-forge/admin-requests#1041

@jakirkham
Copy link
Member

Thanks Tiago! 🙏

Have responded in that PR. Let's see if others have thoughts

@tribeiro
Copy link
Author

Thank you everyone! I really appreciate the time and attention! 🙏

@h-vetinari
Copy link
Member

We can do this for the upcoming python 3.13 release.

+1 for doing this with the 3.13 migration

@beckermr
Copy link
Member

FYI, I've moved discussion and tracking of the transition plan to a central issue as usual: conda-forge/conda-forge.github.io#2252. Please redirect any additional comments on the transition there.

@jakirkham
Copy link
Member

Thanks Matt! 🙏

Will close this out then as there are no more items to do here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants