python3Packages.aiomysql: fix dependency on PyMySQL #122126
Conversation
| @@ -18,6 +19,13 @@ buildPythonPackage rec { | |||
| sha256 = "1qvy3phbsxp55161dnppjfx2m1kn82v0irc3xzqw0adfd81vaiad"; | |||
| }; | |||
|
|
|||
| patches = [ | |||
| (fetchpatch { | |||
| url = "https://github.com/aio-libs/aiomysql/commit/919b997a9de7f53d721af76762fba425e306531e.patch"; | |||
There was a problem hiding this comment.
Please add a comment what the patch accomplishes.
| url = "https://github.com/aio-libs/aiomysql/commit/919b997a9de7f53d721af76762fba425e306531e.patch"; | |
| # vendor functions previously provided by pymysql.util | |
| # https://github.com/aio-libs/aiomysql/pull/554 | |
| url = "https://github.com/aio-libs/aiomysql/commit/919b997a9de7f53d721af76762fba425e306531e.patch"; |
There was a problem hiding this comment.
would it be better to link to the patch such that it shows which fork the patch is from?
otherwise
There was a problem hiding this comment.
Which is why I suggested adding the link to the pull request.
There was a problem hiding this comment.
My impression is that there's some concern from a security perspective about patches that appear as if they're from one repository, but are actually from a fork: #20836 (comment). In this case, there's no security issue, but in the future, nixpkgs-hammering might flag this as a possible issue: jtojnar/nixpkgs-hammering#96
There was a problem hiding this comment.
What if fork removed, are patch links immutable? Otherwise I would prefer to keep link to original repo
There was a problem hiding this comment.
Is there a pull request that this commit is part of?
Yes, changing it to https://github.com/aio-libs/aiomysql/pull/554/commits/919b997a9de7f53d721af76762fba425e306531e.patch would also be more clear.
There was a problem hiding this comment.
LGTM
failures are broken on target branch
https://github.com/NixOS/nixpkgs/pull/122126
2 packages failed to build:
python38Packages.fastapi python39Packages.fastapi
10 packages built:
python38Packages.aiomysql python38Packages.databases python38Packages.orm python38Packages.slack-sdk python38Packages.starlette python39Packages.aiomysql python39Packages.databases python39Packages.orm python39Packages.slack-sdk python39Packages.starlette
Motivation for this change
pythonPackages.databases tests break on import error https://hydra.nixos.org/build/142623477
ZHF: #122042
Applied patch from aio-libs/aiomysql@919b997 that just copy paste from:
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)