pysaml2: 7.5.0 -> 7.5.2 #381222
pysaml2: 7.5.0 -> 7.5.2 #381222
Conversation
|
|
Seahub will be fixed in #369550, just needs its pyopenssl version to be downgraded to match. It was already broken anyways so it shouldn't be an issue. |
pkgs/top-level/python-packages.nix
Outdated
| pyopenssl_24_2_1 = pyopenssl.overridePythonAttrs (old: rec { | ||
| version = "24.2.1"; | ||
| src = pkgs.fetchFromGitHub { | ||
| owner = "pyca"; | ||
| repo = "pyopenssl"; | ||
| tag = version; | ||
| hash = "sha256-/TQnDWdycN4hQ7ZGvBhMJEZVafmL+0wy9eJ8hC6rfio="; | ||
| }; | ||
| }); | ||
|
|
There was a problem hiding this comment.
I think that in this case, the override should be declared in the pysaml2 derivation, not as a new package.
Any thoughts @mweinelt ?
There was a problem hiding this comment.
Some dependents of pysaml2 (i.e. seahub) also depend directly on pyopenssl, and so they have to also somehow use the same version. pysaml2 could maybe expose it but that seems more complicated.
There was a problem hiding this comment.
If they are downstream dependencies, don't they get pyopenssl as a transitive dependency?
There was a problem hiding this comment.
Yes, but in that case they would have to remove pyopenssl from their list of direct dependencies. It's only a stylistic choice at this point, but IMO it would be better for the derivations to keep their direct dependencies explicit if possible.
There was a problem hiding this comment.
I'm not fond of this duplication.
There is an initiative to switch to cryptography upstream: IdentityPython/pysaml2#977
Maybe, we could wait a bit to see how this turns out.
There was a problem hiding this comment.
Now that a solution is actually in sight, it does seem much better to wait.
|
Seeing that the upstream PR has been stalled, maybe a temporary fix is still worth it? |
Ok why not, but I would move the override in the |
|
Very late but I did get around to fixing this. |
|
Oops hadn't tested on master, something's still broken |
|
The only clean solution with recent openssl/pycryptography versions seems to be to patch in the PR, but I'm not sure how acceptable this is. |
|
|
This update adds a limit on the allowed version of
pyopenssl, so an override for the correct version of the package was added (pyopenssl_24_2_1).There is one new failing test:
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.