Merge pull request #13449 from fzakaria/issue-10431

Add error message when git returns non-0 for fetch

src/libfetchers/git-utils.cc

@@ -545,15 +545,20 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
             append(gitArgs, {"--depth", "1"});
         append(gitArgs, {std::string("--"), url, refspec});
 
-        runProgram(RunOptions {
+        auto [status, output] = runProgram(RunOptions {
             .program = "git",
             .lookupPath = true,
             // FIXME: git stderr messes up our progress indicator, so
             // we're using --quiet for now. Should process its stderr.
             .args = gitArgs,
             .input = {},
+            .mergeStderrToStdout = true,
             .isInteractive = true
         });
+
+        if (status > 0) {
+            throw Error("Failed to fetch git repository %s : %s", url, output);
+        }
     }
 
     void verifyCommit(

src/libfetchers/git.cc

@@ -444,7 +444,11 @@ struct GitInputScheme : InputScheme
         // repo, treat as a remote URI to force a clone.
         static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; // for testing
         auto url = parseURL(getStrAttr(input.attrs, "url"));
-        bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git");
+
+        // Why are we checking for bare repository?
+        // well if it's a bare repository we want to force a git fetch rather than copying the folder
+        bool isBareRepository = url.scheme == "file" && pathExists(url.path) &&
+                                                     !pathExists(url.path + "/.git");
         //
         // FIXME: here we turn a possibly relative path into an absolute path.
         // This allows relative git flake inputs to be resolved against the
@@ -462,6 +466,12 @@ struct GitInputScheme : InputScheme
                     "See https://github.com/NixOS/nix/issues/12281 for details.",
                     url);
             }
+
+            // If we don't check here for the path existence, then we can give libgit2 any directory
+            // and it will initialize them as git directories.
+            if (!pathExists(url.path)) {
+                throw Error("The path '%s' does not exist.", url.path);
+            }
             repoInfo.location = std::filesystem::absolute(url.path);
         } else {
             if (url.scheme == "file")
@@ -599,7 +609,7 @@ struct GitInputScheme : InputScheme
                 ? cacheDir / ref
                 : cacheDir / "refs/heads" / ref;
 
-            bool doFetch;
+            bool doFetch = false;
             time_t now = time(0);
 
             /* If a rev was specified, we need to fetch if it's not in the

tests/functional/fetchGit.sh

@@ -53,6 +53,27 @@ rm -rf $TEST_HOME/.cache/nix
 path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
 [[ $(cat $path/hello) = world ]]
 
+# Fetch again. This should be cached.
+# NOTE: This has to be done before the test case below which tries to pack-refs
+# the reason being that the lookup on the cache uses the ref-file `/refs/heads/master`
+# which does not exist after packing.
+mv $repo ${repo}-tmp
+path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
+[[ $path = $path2 ]]
+
+[[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]]
+[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]]
+[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]]
+
+# Fetching with a explicit hash should succeed.
+path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath")
+[[ $path = $path2 ]]
+
+path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath")
+[[ $(cat $path2/hello) = utrecht ]]
+
+mv ${repo}-tmp $repo
+
 # Fetch when the cache has packed-refs
 # Regression test of #8822
 git -C $TEST_HOME/.cache/nix/gitv3/*/ pack-refs --all
@@ -83,24 +104,6 @@ path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"
 # But without a hash, it fails.
 expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' doesn't fetch unlocked input"
 
-# Fetch again. This should be cached.
-mv $repo ${repo}-tmp
-path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
-[[ $path = $path2 ]]
-
-[[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]]
-[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]]
-[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]]
-
-# Fetching with a explicit hash should succeed.
-path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath")
-[[ $path = $path2 ]]
-
-path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath")
-[[ $(cat $path2/hello) = utrecht ]]
-
-mv ${repo}-tmp $repo
-
 # Using a clean working tree should produce the same result.
 path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath")
 [[ $path = $path2 ]]