From 5192fb743b659ff523ed51604f60449f84649c87 Mon Sep 17 00:00:00 2001
From: Atemu <git@atemu.net>
Date: Wed, 4 Dec 2024 01:37:01 +0100
Subject: [PATCH] fetchgit{,hub}: assert illegal tag + rev combinations

It's quite a bit more complex due to this but this was asked for during review
---
 pkgs/build-support/fetchgit/default.nix    | 27 ++++++++++++++++++----
 pkgs/build-support/fetchgithub/default.nix | 11 +++++----
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/pkgs/build-support/fetchgit/default.nix b/pkgs/build-support/fetchgit/default.nix
index 6f4cbba982a0e7..14636e43ade89e 100644
--- a/pkgs/build-support/fetchgit/default.nix
+++ b/pkgs/build-support/fetchgit/default.nix
@@ -15,14 +15,14 @@ lib.makeOverridable (lib.fetchers.withNormalizedHash { } (
 #   doc/build-helpers/fetchers.chapter.md
 { url
 , tag ? null
-, rev ? if tag != null then "refs/tags/${tag}" else "HEAD" # FIXME fetching HEAD by default is problematic at best
+, rev ? null
 , leaveDotGit ? deepClone
 , outputHash ? lib.fakeHash, outputHashAlgo ? null
 , fetchSubmodules ? true, deepClone ? false
 , branchName ? null
 , sparseCheckout ? []
 , nonConeMode ? false
-, name ? urlToName url rev
+, name ? null
 , # Shell code executed after the file has been fetched
   # successfully. This can do things like check or transform the file.
   postFetch ? ""
@@ -62,12 +62,30 @@ lib.makeOverridable (lib.fetchers.withNormalizedHash { } (
 assert deepClone -> leaveDotGit;
 assert nonConeMode -> (sparseCheckout != []);
 
+let
+  revWithTag =
+    let
+      warningMsg = "fetchgit requires either `rev` xor `tag`.";
+      otherIsNull = other: lib.assertMsg (other == null) warningMsg;
+    in
+    if tag != null then
+      assert (otherIsNull rev);
+      "refs/tags/${tag}"
+    else if rev != null then
+      assert (otherIsNull tag);
+      rev
+    else
+      # FIXME fetching HEAD if no rev or tag is provided is problematic at best
+      "HEAD";
+in
+
 if builtins.isString sparseCheckout then
   # Changed to throw on 2023-06-04
   throw "Please provide directories/patterns for sparse checkout as a list of strings. Passing a (multi-line) string is not supported any more."
 else
 stdenvNoCC.mkDerivation {
-  inherit name;
+  name = if name != null then name else urlToName url revWithTag;
+
   builder = ./builder.sh;
   fetcher = ./nix-prefetch-git;
 
@@ -82,7 +100,8 @@ stdenvNoCC.mkDerivation {
   # > from standard in as a newline-delimited list instead of from the arguments.
   sparseCheckout = builtins.concatStringsSep "\n" sparseCheckout;
 
-  inherit url rev leaveDotGit fetchLFS fetchSubmodules deepClone branchName nonConeMode postFetch;
+  inherit url leaveDotGit fetchLFS fetchSubmodules deepClone branchName nonConeMode postFetch;
+  rev = revWithTag;
 
   postHook = if netrcPhase == null then null else ''
     ${netrcPhase}
diff --git a/pkgs/build-support/fetchgithub/default.nix b/pkgs/build-support/fetchgithub/default.nix
index de9912465a9374..03c39a1fba9398 100644
--- a/pkgs/build-support/fetchgithub/default.nix
+++ b/pkgs/build-support/fetchgithub/default.nix
@@ -3,7 +3,7 @@
 lib.makeOverridable (
 { owner, repo
 , tag ? null
-, rev ? if tag != null then "refs/tags/${tag}" else null
+, rev ? null
 , name ? "source"
 , fetchSubmodules ? false, leaveDotGit ? null
 , deepClone ? false, private ? false, forceFetchGit ? false
@@ -14,13 +14,16 @@ lib.makeOverridable (
 , ... # For hash agility
 }@args:
 
-assert (lib.assertMsg (rev != null) "You must provide `fetchFromGitHub with a `rev` or `tag`.");
+assert (lib.assertMsg (lib.xor (tag == null) (rev == null)) "fetchFromGitHub requires either `rev` xor `tag`.");
 
 let
 
   position = (if args.meta.description or null != null
     then builtins.unsafeGetAttrPos "description" args.meta
-    else builtins.unsafeGetAttrPos "rev" args
+    else if tag != null then
+      builtins.unsafeGetAttrPos "tag" args
+    else
+      builtins.unsafeGetAttrPos "rev" args
   );
   baseUrl = "https://${githubBase}/${owner}/${repo}";
   newMeta = meta // {
@@ -61,7 +64,7 @@ let
       inherit tag rev deepClone fetchSubmodules sparseCheckout fetchLFS; url = gitRepoUrl;
     } // lib.optionalAttrs (leaveDotGit != null) { inherit leaveDotGit; }
     else {
-      url = "${baseUrl}/archive/${rev}.tar.gz";
+      url = "${baseUrl}/archive/${if tag != null then "refs/tags/${tag}" else rev}.tar.gz";
 
       passthru = {
         inherit gitRepoUrl;