Skip to content

fix(cli): require correct setuptools version#4492

Merged
clason merged 1 commit intotree-sitter:masterfrom
rabbiveesh:veesh/fix-setuptools
Jun 27, 2025
Merged

fix(cli): require correct setuptools version#4492
clason merged 1 commit intotree-sitter:masterfrom
rabbiveesh:veesh/fix-setuptools

Conversation

@rabbiveesh
Copy link
Contributor

@rabbiveesh rabbiveesh commented Jun 6, 2025

The build process uses the module setuptools.command.build which was added in 62.4.0 as
per https://github.com/pypa/setuptools/blob/main/NEWS.rst#v6240

also ref tree-sitter-perl/tree-sitter-perl#214

@clason clason added ci:backport release-0.25 Backport label build Related to building (Makefile, CMake, etc.) labels Jun 6, 2025
ObserverOfTime
ObserverOfTime reviewed Jun 6, 2025
View reviewed changes
Copy link
Member

@ObserverOfTime ObserverOfTime left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backwards compatibility we could fall back to distutils.command.build instead.

Several distros have outdated packages according to https://pkgs.org/search/?q=setuptools

ALT Linux: 42.0.0
Amazon Linux 2: 49.1.3
Amazon Linux 2023: 59.6.0
Debian 11: 44.1.1
openSUSE Leap 15.6: 44.1.1
PCLinuxOS: 45.2.0
RHEL 8 & derivatives: 39.2.0
RHEL 9 & derivatives: 53.0.0
Slackware 15.0: 57.5.0
Ubuntu 22.04: 59.6.0

@ObserverOfTime ObserverOfTime mentioned this pull request Jun 6, 2025
Closed
@rabbiveesh
Copy link
Contributor Author

rabbiveesh commented Jun 6, 2025

From what I understand when you declare like I did the pip will install the build deps in a temporary env; so I'm not sure there's benefit to implementing fallback

@ObserverOfTime
Copy link
Member

ObserverOfTime commented Jun 6, 2025

Not if you use the system packages.

@clason
Copy link
Contributor

clason commented Jun 7, 2025

Can you please rebase?

@ObserverOfTime
Copy link
Member

ObserverOfTime commented Jun 7, 2025

Can we merge #4493 instead?

@clason
Copy link
Contributor

clason commented Jun 7, 2025

Why? This is a much simpler fix for the actual issue, and 62.4.0 was released three years ago.

@ObserverOfTime
Copy link
Member

ObserverOfTime commented Jun 7, 2025 •
edited
Loading

The bdist_wheel command also needs a fallback since the setuptools version was introduced in v70.1.0 which is too recent. We might as well have a fallback for the build command too in that case.

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Jun 8, 2025

I agree with @clason. I would like the bindings to stay simple; I don’t think it’s worth adding the fallback scheme to support people who do not want to install recent versions of setup tools.

@ObserverOfTime
Copy link
Member

ObserverOfTime commented Jun 8, 2025

Then should we bump all the way to v70.1.0?

@clason
Copy link
Contributor

clason commented Jun 8, 2025 •
edited
Loading

Then should we bump all the way to v70.1.0?

Why? What will that buy us over 62.4.0?

@ObserverOfTime
Copy link
Member

ObserverOfTime commented Jun 8, 2025

Adopted the bdist_wheel command from the wheel project

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Jun 8, 2025

We could also just keep using the wheel library directly, right? I don't have a strong opinion on that one. Wheel is not deprecated like distutils is. @ObserverOfTime what do you think about this PR's current approach of just upgrading setuptools and keeping wheel?

@ObserverOfTime
Copy link
Member

ObserverOfTime commented Jun 8, 2025

The package isn't deprecated but the bdist_wheel import is.
And switching to the setuptools version will allow us to drop wheel from requires.

@rabbiveesh
Copy link
Contributor Author

rabbiveesh commented Jun 13, 2025

Do we have a conclusion here? I'm happy to pull in latest master and bump further to 70 whatever as needed.
Could I get a clarification for exactly when the install would fail, btw? I didn't quite get the case (I assume it's my ignorance)

@clason
Copy link
Contributor

clason commented Jun 26, 2025 •
edited
Loading

Let's start with a simple rebase(!) on master. If it fixes a concrete problem, we can merge and backport quickly and then iterate further (if and when someone comes up with an actual problem that can be verified to be solved).

@rabbiveesh rabbiveesh force-pushed the veesh/fix-setuptools branch from b3be589 to 378619f Compare June 27, 2025 10:49
@rabbiveesh
Copy link
Contributor Author

rabbiveesh commented Jun 27, 2025

K, i have rebased and force-pushed and its all clean.
The version that i've bumped to does solve a concrete problem, so i'd love to see this merged

@clason clason merged commit b09a15e into tree-sitter:master Jun 27, 2025
12 checks passed
@tree-sitter-ci-bot
Copy link

tree-sitter-ci-bot bot commented Jun 27, 2025

Successfully created backport PR for release-0.25:

@tree-sitter-ci-bot tree-sitter-ci-bot bot mentioned this pull request Jun 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ObserverOfTime ObserverOfTime ObserverOfTime left review comments

@clason clason clason approved these changes

Assignees

No one assigned

Labels

build Related to building (Makefile, CMake, etc.) ci:backport release-0.25 Backport label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rabbiveesh @ObserverOfTime @clason @maxbrunsfeld