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
pythonPackages.bcrypt: use 3.1.x for python older than 3.6 #97996
Conversation
@mvnetbiz the standard way of doing this is overriding the package set to create a custom python installation with the version that you want. See for example https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/tools/prospector/default.nix. That is, for
|
Since it is deprecated it seems like it would make sense to keep the definition in the package that requires it, so it can be removed once it isn't needed any more, but this is used by at least 2 used packages, also the expression might deviate significantly from the new version, would that end up being too verbose keeping in a per-use override? Would it be better to add a separate pythonPackages.bcrypt_3_1, and use that in these packages? |
I took about 10 mins and looked at usages of bcrypt throughout nixpkgs. In the applications that need it, pinging maintainer of |
IMHO, in that example, it is bcrypt's reverse dependency (home-assistant) breaking compatibility with it, so it makes sense to override it there. The reason for this patch is that bcrypt itself, not it's user, is dropping compatibility with python < 3.6, so anything requiring bcrypt on python2 will have to use this version, so I thought it made sense to have a more general patch. I can make the PRs for mysql-workbench, syncthing-gtk, (poretools?) if this gets closed. |
There are many modules defined this way in pkgs/top-level/python-packages.nix, at least the first 4 I looked at result from the same upstream dropping of old python version, this doesn't seem too atypical. |
@drewrisinger that's only if it's a python application, if he still needs python2 to work, this is the correct method. We do it for packages such as numpy. |
also, please disable python2Packages.trytond:
|
NVM I guess that makes sense if it's bcrypt related |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- A few unnecessary things in diff
- Commits LGTM
- Partial build tested:
nix-review -p python27Packages.bcrypt -p mysql-workbench
- Maybe add
disabled
topython-modules/bcrypt/default.nix
to be explicit in the file about where it's supported??
https://github.com/NixOS/nixpkgs/pull/97996
2 packages built:
mysql-workbench python27Packages.bcrypt
de75804
to
c7f8249
Compare
@drewrisinger, thanks for the comments. |
Looks like enabling this breaks
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Diff LGTM
- Commits LGTM
- Builds via
nix-review
(mostly, see above comments). Failing packages should be addressed in follow-up ZHF PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff LGTM
Motivation for this change
pythonPackages.bcrypt was disabled for python older than 3.6: 87371b8
We still have some applications that cannot be upgraded from python 2 that require python-bcrypt
mysql-workbench, syncthing-gtk: #97642
ZHF: #97479
Things done
Added back pythonPackages.bcrypt 3.1.7 only for python older than 3.6
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)