Skip to content

Comments

stdenv, cacert: $NIX_SSL_CERT_FILE changes#61179

Merged
vcunat merged 3 commits intoNixOS:stagingfrom
vcunat:p/cacert-NIX_SSL
May 19, 2019
Merged

stdenv, cacert: $NIX_SSL_CERT_FILE changes#61179
vcunat merged 3 commits intoNixOS:stagingfrom
vcunat:p/cacert-NIX_SSL

Conversation

@vcunat
Copy link
Member

@vcunat vcunat commented May 9, 2019

Motivation for this change

Some SSL libs don't react to $SSL_CERT_FILE. That actually makes sense to me, as we add this behavior as nixpkgs-specific, so it seems "safer" to use $NIX_*.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date: docs are still missing ;-)
  • Fits CONTRIBUTING.md.

@vcunat
stdenv, cacert: consider $NIX_SSL_CERT_FILE in hooks
79bd4ad 
Some SSL libs don't react to $SSL_CERT_FILE.
That actually makes sense to me, as we add this behavior
as nixpkgs-specific, so it seems "safer" to use $NIX_*.
This was referenced May 9, 2019
Open
Merged
@ofborg ofborg bot added 6.topic: stdenv Standard environment 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels May 9, 2019
layus
layus approved these changes May 9, 2019
View reviewed changes
Copy link
Member

@layus layus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It totally makes sense, and should have been done before :-)

vcunat added 2 commits May 9, 2019 09:49
@vcunat
stdenv: also override cert files in pure nix-shell
b27cc37 
That's very much consistent with the spirit of nix-shell --pure

BTW, nix 1.x shells will be always treated as pure;
in that version detection isn't possible.
NixOS/nix@1bffd83e1a9c
@vcunat
pypi2nix: fix $IN_NIX_SHELL test
99760ed 
In nix 2.0 this changed: NixOS/nix@1bffd83
I only kept the original intention and did no kind of verification.
@vcunat
Copy link
Member Author

vcunat commented May 9, 2019

/cc pypi2nix maintainer @garbas. It couldn't work for years in the intended way, so I don't know.

@ofborg ofborg bot requested a review from garbas May 9, 2019 08:16
@vcunat vcunat changed the title stdenv, cacert: consider $NIX_SSL_CERT_FILE in hooks stdenv, cacert: $NIX_SSL_CERT_FILE changes May 9, 2019
@LnL7
Copy link
Member

LnL7 commented May 9, 2019

My reasoning back when I added it was to go trough the upstream codepath instead of our patches by default. But that doesn't really make sense since we want NIX_SSL_CERT_FILE to work for everything.

@vcunat vcunat merged commit 99760ed into NixOS:staging May 19, 2019
vcunat added a commit that referenced this pull request May 19, 2019
@vcunat
Merge #61179: stdenv, cacert: $NIX_SSL_CERT_FILE changes
96a6043 
... into staging
@vcunat vcunat deleted the p/cacert-NIX_SSL branch May 19, 2019 12:48
vcunat added a commit that referenced this pull request May 19, 2019
@vcunat
gnutls: fix tests after 79bd4ad (PR #61179)
347cd8a 
It's one of the places that would reach out to /etc/ otherwise,
so I expect we have to pay this price to get the effect.
Hopefully there won't be too many places to patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@layus layus layus approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@garbas garbas Awaiting requested review from garbas

Assignees

No one assigned

Labels

6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vcunat @LnL7 @layus