fetch{url,zip,git}: restructure with lib.extendMkDerivation#455994
fetch{url,zip,git}: restructure with lib.extendMkDerivation#455994philiptaron merged 8 commits intoNixOS:masterfrom
lib.extendMkDerivation#455994Conversation
fe3c465 to
71d314e
Compare
546c7d4 to
b76d5e8
Compare
|
philiptaron
left a comment
There was a problem hiding this comment.
I'm not at all fussed by including the formatting in the commits that introduce lib.extendMkDerivation and skipping .git-blame-ignore-revs
|
Nice work! |
b76d5e8 to
825b4ef
Compare
|
There are some extra rebuilding packages after the name-attribute changes. i'll take a look. |
philiptaron
left a comment
There was a problem hiding this comment.
I won't be able to do intensive testing today, but the code looks immaculate.
The current implementation of Should we keep the original logic, or add another Update: I choose to observe the original logic with some modification to make
|
f7a395d to
fbf6f38
Compare
|
I like your solution to the conundrum. |
|
The extra rebuilds are now gone. |
…erivation reformatting
fbf6f38 to
1ec0227
Compare
lib.extendMkDerivation
|
|
Despite reporting depent packages build successfully, I'm not entirely sure what is going on. I also rebuild all the changed test sets (shown in the CI summary) manually, and they all build. nix build --no-link --print-out-paths -L -f . \
tests.fetchDebianPatch tests.fetchFirefoxAddon \
tests.fetchFromGitHub tests.fetchNextcloudApp \
tests.fetchPypiLegacy tests.fetchgit tests.fetchpatch \
tests.fetchpatch2 tests.fetchtorrent tests.fetchurl \
tests.fetchzip tests.haskell.cabalSdist tests.testers.runCommand |
|
There's no issue. This is a |
|
Now I can confirm that it's a nixpkgs-review issue. The issue is gone after applying changes similar to Mic92/nixpkgs-review#384 (but specify |
|
Can someone help me updating https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/prefer-remote-fetch/default.nix#L25 to fit this? I removed the overlay from my config and then was greeted with which probably means some other change is required: |
The The safest way to extend these build helper in an overlay and avoid breaking transient compatibility layer such as #271762's is to handle plain attrset and unfixed set differently. You can write it this way: self: super:
let
inherit (self) lib;
safeExtends =
newArgs: args:
if lib.isFunction args then
lib.extend (_: lib.toFunction newArgs) args
else
args // lib.toFunction newArgs args;
{
fetchurl =
fpArgs:
super.fetchurl (safeExtends (args: { preferLocalBuild = args.preferLocalBuild or false; }) fpArgs);
}Things will get much easier without considering the potential difference between self: super:
let
inherit (self) lib;
in
{
fetchurl =
fpArgs:
super.fetchurl (lib.extends
(finalAttrs: args: { preferLocalBuild = args.preferLocalBuild or false; })
(lib.toFunction fpArgs));
}
While |
|
I forgot to mention the error I received and will try out the things you suggested. |
|
That's one is understandable. It can be reproduced simply by By the way, thank you for letting me know that people could wrap build helpers this way in their overlays. This means that we shouldn't rely on the |
|
I'll tried to fix this in #457086 |
|
Sorry for the out-of-tree project breakage caused by this PR. I opened PR #458150 to mitigate them with a compatibility layer. Please take a look. |
This PR restructures
fetchurl,fetchzipandfetchgitwithlib.extendMkDerivation.This PR doesn't contain overriding fixes required to make
finalAttrsand<pkg>.overrideAttrsfunction without surprises, so I'm not claiming that their fixed-point arguments support are available.Changed semantics after overriding:
fetchurl: The following arguments are now passed down tostdenvNoCC.mkDerivation:recursiveHashnetrcPhasenetrcImpureEnvVarsfetchgit: The following arguments are now passed down tostdenvNoCC.mkDerivation:netrcPhase(already included inpostFetch)netrcImpureEnvVarsalready included inimpureEnvVarsThe rebuilded packages are all test cases under
pkgs.tests.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.