Merge pull request #247527 from oxij/metrics/fetchzip

fetchzip: cleanup and improve metrics

2 files changed, 29 insertions, 40 deletions

diff --git a/pkgs/build-support/fetchurl/default.nix b/pkgs/build-support/fetchurl/default.nix
index bcab54e273dcf..d1a886a34a701 100644
--- a/pkgs/build-support/fetchurl/default.nix
+++ b/pkgs/build-support/fetchurl/default.nix
@@ -132,6 +132,13 @@ let
     else throw "fetchurl requires a hash for fixed-output derivation: ${lib.concatStringsSep ", " urls_}";
 in
 
+assert (lib.isList curlOpts) -> lib.warn ''
+    fetchurl for ${toString (builtins.head urls_)}: curlOpts is a list (${lib.generators.toPretty { multiline = false; } curlOpts}), which is not supported anymore.
+    - If you wish to get the same effect as before, for elements with spaces (even if escaped) to expand to multiple curl arguments, use a string argument instead:
+      curlOpts = ${lib.strings.escapeNixString (toString curlOpts)};
+    - If you wish for each list element to be passed as a separate curl argument, allowing arguments to contain spaces, use curlOptsList instead:
+      curlOptsList = [ ${lib.concatMapStringsSep " " lib.strings.escapeNixString curlOpts} ];'' true;
+
 stdenvNoCC.mkDerivation ((
   if (pname != "" && version != "") then
     { inherit pname version; }
@@ -161,12 +168,7 @@ stdenvNoCC.mkDerivation ((
 
   outputHashMode = if (recursiveHash || executable) then "recursive" else "flat";
 
-  curlOpts = lib.warnIf (lib.isList curlOpts) ''
-    fetchurl for ${toString (builtins.head urls_)}: curlOpts is a list (${lib.generators.toPretty { multiline = false; } curlOpts}), which is not supported anymore.
-    - If you wish to get the same effect as before, for elements with spaces (even if escaped) to expand to multiple curl arguments, use a string argument instead:
-      curlOpts = ${lib.strings.escapeNixString (toString curlOpts)};
-    - If you wish for each list element to be passed as a separate curl argument, allowing arguments to contain spaces, use curlOptsList instead:
-      curlOptsList = [ ${lib.concatMapStringsSep " " lib.strings.escapeNixString curlOpts} ];'' curlOpts;
+  inherit curlOpts;
   curlOptsList = lib.escapeShellArgs curlOptsList;
   inherit showURLs mirrorsFile postFetch downloadToTemp executable;
 
diff --git a/pkgs/build-support/fetchzip/default.nix b/pkgs/build-support/fetchzip/default.nix
index e980f9d65214d..0446851d64098 100644
--- a/pkgs/build-support/fetchzip/default.nix
+++ b/pkgs/build-support/fetchzip/default.nix
@@ -7,41 +7,34 @@
 
 { lib, fetchurl, unzip, glibcLocalesUtf8 }:
 
-{ # Optionally move the contents of the unpacked tree up one level.
-  stripRoot ? true
+{ name ? "source"
 , url ? ""
 , urls ? []
-, extraPostFetch ? ""
+, nativeBuildInputs ? []
 , postFetch ? ""
-, name ? "source"
-, pname ? ""
-, version ? ""
-, nativeBuildInputs ? [ ]
-, # Allows to set the extension for the intermediate downloaded
-  # file. This can be used as a hint for the unpackCmdHooks to select
-  # an appropriate unpacking tool.
-  extension ? null
-, ... } @ args:
+, extraPostFetch ? ""
 
+# Optionally move the contents of the unpacked tree up one level.
+, stripRoot ? true
+# Allows to set the extension for the intermediate downloaded
+# file. This can be used as a hint for the unpackCmdHooks to select
+# an appropriate unpacking tool.
+, extension ? null
+
+# the rest are given to fetchurl as is
+, ... } @ args:
 
-lib.warnIf (extraPostFetch != "") "use 'postFetch' instead of 'extraPostFetch' with 'fetchzip' and 'fetchFromGitHub'."
+assert (extraPostFetch != "") -> lib.warn "use 'postFetch' instead of 'extraPostFetch' with 'fetchzip' and 'fetchFromGitHub'." true;
 
-(let
+let
   tmpFilename =
     if extension != null
     then "download.${extension}"
     else baseNameOf (if url != "" then url else builtins.head urls);
 in
 
-fetchurl ((
-  if (pname != "" && version != "") then
-    {
-      name = "${pname}-${version}";
-      inherit pname version;
-    }
-  else
-    { inherit name; }
-) // {
+fetchurl ({
+  inherit name;
   recursiveHash = true;
 
   downloadToTemp = true;
@@ -61,8 +54,7 @@ fetchurl ((
       mv "$downloadedFile" "$renamed"
       unpackFile "$renamed"
       chmod -R +w "$unpackDir"
-    ''
-    + (if stripRoot then ''
+    '' + (if stripRoot then ''
       if [ $(ls -A "$unpackDir" | wc -l) != 1 ]; then
         echo "error: zip file must contain a single file or directory."
         echo "hint: Pass stripRoot=false; to fetchzip to assume flat list of files."
@@ -75,16 +67,11 @@ fetchurl ((
       mv "$unpackDir/$fn" "$out"
     '' else ''
       mv "$unpackDir" "$out"
-    '')
-    + ''
+    '') + ''
       ${postFetch}
-    '' + ''
       ${extraPostFetch}
-    ''
-
-    # Remove non-owner write permissions
-    # Fixes https://github.com/NixOS/nixpkgs/issues/38649
-    + ''
       chmod 755 "$out"
     '';
-} // removeAttrs args [ "stripRoot" "extraPostFetch" "postFetch" "extension" "nativeBuildInputs" ]))
+    # ^ Remove non-owner write permissions
+    # Fixes https://github.com/NixOS/nixpkgs/issues/38649
+} // removeAttrs args [ "stripRoot" "extraPostFetch" "postFetch" "extension" "nativeBuildInputs" ])