pythonPackages.requests: patch in CA bundles

[kini]

This is an attempt to do what #94024 was trying to do, but at a deeper level in the code. In fact the file I edit here has a docstring in it saying the following:

 If you are packaging Requests, e.g., for a Linux distribution or a managed
 environment, you can change the definition of where() to return a separately
 packaged CA bundle.

So I guess this is the "right" way to do this (?).

Motivation for this change

The requests library defaults to using the certificates from the
certifi library when not otherwise specified. If I understand the
discussion at #8247 correctly, we should instead patch it so that it
follows the following priority order:

1. the path pointed to by the environment variable $NIX_SSL_CERT_FILE

2. /etc/ssl/certs/ca-certificates.crt

3. whatever it was doing before (in this case, using certifi)

This commit implements that.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[jonringer]

makes sense to me

cc @FRidh

[r-rmcgibbo]

Result of nixpkgs-review pr 123318 at fe155726 run on aarch64-linux 1

206 packages marked as broken and skipped:

• agdaPackages.iowa-stdlib
• aqemu
• awsebcli
• bareos
• belle-sip
• bonfire
• cassandra_2_1
• cassandra_2_2
• cvc4
• deeptools
• ...

9548 packages skipped due to time constraints:

• DisnixWebService
• EBTKS
• R
• abcl
• acd-cli
• adapta-gtk-theme
• adoptopenjdk-icedtea-web
• aerc
• afew
• agda-pkg
• ...

14 packages built successfully:

• python38Packages.betamax
• python38Packages.flask
• python38Packages.pytest-localserver
• python38Packages.requests
• python38Packages.requests-mock
• python38Packages.requests_download
• python38Packages.responses
• python38Packages.werkzeug
• python39Packages.betamax
• python39Packages.flask
• python39Packages.pytest-localserver
• python39Packages.requests
• python39Packages.responses
• python39Packages.werkzeug

1 suggestion:

• warning: missing-patch-comment

  Consider adding a comment explaining the purpose of this patch on the line preceeding.
  Near pkgs/development/python-modules/requests/default.nix:23:15:

     |
  23 |   patches = [ ./0001-Prefer-NixOS-Nix-default-CA-bundles-over-certifi.patch ];
     |               ^
  

---

Result of nixpkgs-review pr 123318 at fe155726 run on x86_64-linux 1

144 packages marked as broken and skipped:

• agdaPackages.iowa-stdlib
• aqemu
• awsebcli
• bareos
• bonfire
• cassandra_2_1
• cassandra_2_2
• gnuradio3_7
• gnuradio3_7Packages.ais
• gnuradio3_7Packages.gnuradio
• ...

9781 packages skipped due to time constraints:

• DisnixWebService
• EBTKS
• R
• abcl
• acd-cli
• acousticbrainz-client
• adapta-gtk-theme
• adoptopenjdk-icedtea-web
• aerc
• afew
• ...

16 packages built successfully:

• knockpy
• python38Packages.betamax
• python38Packages.flask
• python38Packages.pytest-localserver
• python38Packages.requests
• python38Packages.requests-mock
• python38Packages.requests_download
• python38Packages.responses
• python38Packages.werkzeug
• python39Packages.betamax
• python39Packages.flask
• python39Packages.pytest-localserver
• python39Packages.requests
• python39Packages.requests_download
• python39Packages.responses
• python39Packages.werkzeug

1 suggestion:

• warning: missing-patch-comment

  Consider adding a comment explaining the purpose of this patch on the line preceeding.
  Near pkgs/development/python-modules/requests/default.nix:23:15:

     |
  23 |   patches = [ ./0001-Prefer-NixOS-Nix-default-CA-bundles-over-certifi.patch ];
     |               ^
  

[kini]

Actually, sorry, I think I've gotten something backwards - $NIX_SSL_CERT_FILE, when available, should be preferred over /etc/ssl/certs/ca-certificates.crt, not vice versa. There was some argument about that ordering on #8247, but the current state of nixpkgs, e.g. in the patches to openssl, seem to reflect that $NIX_SSL_CERT_FILE takes precedence. I'll reverse the order of those two and rebase.

EDIT: fixed.

[jonringer]

Actually, sorry, I think I've gotten something backwards - $NIX_SSL_CERT_FILE, when available, should be preferred over /etc/ssl/certs/ca-certificates.crt, not vice vers

Hmm, I don't think the distinction is super necessary. As it likely points to something valid if present. But I could also be wrong about this.

[kini]

@FRidh Could you please take a look? Thanks.

[jonringer]

cc @mweinelt

[mweinelt]

Makes sense, just two recommendations to give it the finishing touch.

[jonringer]

LGTM

[mweinelt]

Merged into my local working tree for python-unstable. Thank you.

[kini]

Should I have opened this pull request against a different target branch so that it could be merged rather than closed?

[mweinelt]

No, the python-unstable branch is something we create when we need it, because it triggers build jobs over on hydra quite regularly. I could also have brought up the branch first and then have you rebase on top of it, but it was easier for me to just pick it.

[kini]

I see. I'd rather have rebased because otherwise I don't think viewing the final commit on github will point back to the PR anymore. I often find it useful, when looking at the history of the nixpkgs repository, to look up a commit in the github interface, find the link to the PR that introduced the commit, and then read the discussion on the PR. Just my two cents. Thanks for picking the commit.

[mweinelt]

Agreed, thanks for the feedback.