python3Packages.setuptoolsBuildHook: deprecate 👋#405745
python3Packages.setuptoolsBuildHook: deprecate 👋#405745mweinelt wants to merge 2 commits intoNixOS:masterfrom
Conversation
|
@wolfgangwalther Do you think it’s a wise idea to add a check to Lines 118 to 121 in f294786 |
The way this hook directly calls setup.py has been deprecated for years now and we need to get off of it, before it eventually breaks. All packages should migrate to `pyproject = true` and put the relevant PEP517 builder (setuptools in this case) into `build-system`. See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
38041d9 to
37e34f0
Compare
I wouldn't go that route. That would probably become complicated - and not immediately clear to users, why some warnings are OK and others are not.
I think you actually want CI to break here. The approach for deprecation should be:
This means, that once you deprecate something, nixpkgs should already not use it anymore. Or put differently: There is not really something like "deprecation in nixpkgs". In this case, it looks like This then has the benefit that people are blocked from adding this again inside nixpkgs. |
Scratch that, I should have grepped for |
|
In any case, my point still stands: If the deprecation doesn't cause a CI failure, it's a pointless warning added. People will still do it in nixpkgs. |
|
Fair enough. Thank you for the detailed writeup, much appreciated.
Given we don’t have merge queues, there could be a bunch of new instances merged after, though I guess the eval check on master will quickly find those at least. |
|
I’d be willing to write a script to mechanically migrate the simple cases (probably using tree-sitter because I don’t think string replacement is good enough here, though it might just work fine 🤷♀️) if you want, @mweinelt. |
|
I hate everything about this. We have no mechanism to nudge people and instead have to either develop a scripted approach or do the work in a small group that cares.
Yeah, the problem is with the packages that don't set |
|
I believe we need a deprecation notice targeted at nixpkgs developers rather than warnings for nix users. This is essentially about creating a mechanism to more actively address the Currently, over 2k packages use While automation scripts would help, this isn't just a mechanical rewrite - it's a build system replacement that often requires manual intervention. Many packages have implicit issues that surface during migration. Ignoring dependency checks might simplify the work in some cases, but that merely postpones the underlying problems. The ideal solution would be implementing a system that alerts developers about the deprecation and encourages them to run migration scripts for packages. |
Actually, we do. I think that mechanism is Maybe the concept of those checks could be applied slightly wider: Currently they just confirm that (a) already confirming files continue to do so and (b) new files will confirm with the rules. We could also invent a category of checks that would fail when the related file is touched, no matter whether it failed or passed before. This way it would be possible to force users who touch a package, e.g. for update, to also clean it up at the same time. If they don't do it, CI would fail. Edit / posted at the same time:
Yes - that's exactly what CI is in general. So we must make CI fail for those cases. Just in a way that doesn't fail CI for everyone, just because some package is still using setuptools. |
|
The problem is that there are many packages that no one touches. I think these cannot be detected by the current |
|
And the other problem is python-updates, where we touch too many packages to migrate them one by one. |
OK, but that's just a different thing then. That's essentially asking for a better way to ask for help for a big treewide migration like this. But I don't see how we could enforce anything like this via any kind of CI / deprecation / warning or so. But yeah, if not enough packages are touched at all, then the effort to implement this kind of check might not be worth it.
This could be handled by only running this specific check on "regular" PRs, but on |
When I deprecated
This worked very well, ultimately. I was able to deal with the biggest number of packages that way, and then had a few remaining that needed manually work. This will probably be more complicated than that, but maybe we can start with something that tries to find leaf packages, which don't cause other rebuilds, do some automated migration, and then verify via nixpkgs-review? Would it be enough to make the packages build - or would they need to be verified manually as well? |
Co-authored-by: Wolfgang Walther <[email protected]>
The way this hook directly calls setup.py has been deprecated for years now and we need to get off of it, before it eventually breaks.
All packages should migrate to
pyproject = trueand put the relevant PEP517 builder (setuptools in this case) intobuild-system.See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.