Skip to content

Comments

fetchurl: Don't force-override curl's gssSupport to on unnecessarily#66506

Merged
infinisil merged 1 commit intoNixOS:masterfrom
nh2:fetchurl-dont-force-set-gssSupport
Aug 12, 2019
Merged

fetchurl: Don't force-override curl's gssSupport to on unnecessarily#66506
infinisil merged 1 commit intoNixOS:masterfrom
nh2:fetchurl-dont-force-set-gssSupport

Conversation

@nh2
Copy link
Contributor

@nh2 nh2 commented Aug 12, 2019

Motivation for this change

Bug #66499

The original intent in commit a1fec86 treewide: assemble all `fetchurlBoot` uses in overrides to `fetchurl` itself was to turn gssSupport off on Darwin, but the code actually also forced
it on on Linux.

This resulted in previous (e.g. overlays) .override { gssSupport = false; } being ignored (#66499).

This commit fixes it by just respecting the old value when it doesn't need to be forced to off.

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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @oxij @infinisil @matthewbauer @Ericson2314

@nh2 nh2 requested a review from infinisil August 12, 2019 04:45
@nh2 nh2 changed the title Fetchurl dont force set gss support fetchurl: Don't force-override curl's gssSupport to on unnecessarily Aug 12, 2019
@nh2 nh2 force-pushed the fetchurl-dont-force-set-gssSupport branch from e13b9e9 to 65b4b27 Compare August 12, 2019 05:08
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Aug 12, 2019
…6499.

The original intent in commit

    a1fec86 treewide: assemble all `fetchurlBoot` uses in overrides to `fetchurl` itself

was to turn `gssSupport` *off* on Darwin, but the code actually also forced
it *on* on Linux.
This resulted in previous (e.g. overlays) `.override { gssSupport = false; }`
being ignored (NixOS#66499).

This commit fixes it by just respecting the old value when it doesn't need
to be forced to off.
@nh2 nh2 force-pushed the fetchurl-dont-force-set-gssSupport branch from 65b4b27 to b7dfc72 Compare August 12, 2019 22:43
@nh2
Copy link
Contributor Author

nh2 commented Aug 12, 2019

That was merged, I have rebased.

@nh2
Copy link
Contributor Author

nh2 commented Aug 12, 2019

Just for clarification, the previous version of the code was equivalent to this:

       gssSupport =
        if stdenv.isDarwin || stdenv.hostPlatform.isWindows
          then false
          else true

and I've made it this:

       gssSupport =
        if stdenv.isDarwin || stdenv.hostPlatform.isWindows
          then false
          else old.gssSupport or true; # `? true` is the default

@infinisil infinisil merged commit 47fa2f1 into NixOS:master Aug 12, 2019
@oxij
Copy link
Member

oxij commented Aug 12, 2019 via email

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Oct 20, 2019
With NixOS/nixpkgs#66506 being merged,
we no longer need that workaround.
nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Oct 20, 2019
With NixOS/nixpkgs#66506 being merged,
we no longer need that workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants