Subject: [PATCH v3] refs: return conflict error when checking packed refs

From: "Ivan Tse via GitGitGadget" <[email-masked]>
Date: Sat, 04 May 2024 03:04:08 +0000

From: Ivan Tse <[email-masked]>

The TRANSACTION_NAME_CONFLICT error code refers to a failure to create a
ref due to a name conflict with another ref. An example of this is a
directory/file conflict such as ref names A/B and A.

"git fetch" uses this error code to more accurately describe the error
by recommending to the user that they try running "git remote prune" to
remove any old refs that are deleted by the remote which would clear up
any directory/file conflicts.

This helpful error message is not displayed when the conflicted ref is
stored in packed refs. This change fixes this by ensuring error return
code consistency in `lock_raw_ref`.

Signed-off-by: Ivan Tse <[email-masked]>
---
    refs: return conflict error when checking packed refs
    
    Changes against v2:
    
     * move test_grep to follow after their respective git fetch commands
     * Use "${SQ}" instead of '\''
    
    Thanks for the explanation and for the comments. I've updated the
    changes to incorporate your feedback!

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1716%2Fivantsepp%2Freturn_name_conflict_error_for_packed_refs-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1716/ivantsepp/return_name_conflict_error_for_packed_refs-v3
Pull-Request: https://github.com/git/git/pull/1716

Range-diff vs v2:

 1:  58b2cda5c18 ! 1:  943a8629b5f refs: return conflict error when checking packed refs
     @@ t/t5510-fetch.sh: test_expect_success 'branchname D/F conflict resolved by --pru
      +	(
      +		cd df-conflict-error &&
      +		git update-ref refs/remotes/origin/dir_conflict/file HEAD &&
     -+		test_must_fail git fetch 2>../err &&
     ++		test_must_fail git fetch 2>err &&
     ++		test_grep "error: some local refs could not be updated; try running" err &&
     ++		test_grep " ${SQ}git remote prune origin${SQ} to remove any old, conflicting branches" err &&
      +		git pack-refs --all &&
     -+		test_must_fail git fetch 2>../err-packed
     -+	) &&
     -+	test_grep "error: some local refs could not be updated; try running" err &&
     -+	test_grep " '\''git remote prune origin'\'' to remove any old, conflicting branches" err &&
     -+	test_grep "error: some local refs could not be updated; try running" err-packed &&
     -+	test_grep " '\''git remote prune origin'\'' to remove any old, conflicting branches" err-packed
     ++		test_must_fail git fetch 2>err-packed &&
     ++		test_grep "error: some local refs could not be updated; try running" err-packed &&
     ++		test_grep " ${SQ}git remote prune origin${SQ} to remove any old, conflicting branches" err-packed
     ++	)
      +'
      +
       test_expect_success 'fetching a one-level ref works' '

 refs/files-backend.c |  4 +++-
 t/t5510-fetch.sh     | 16 ++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index a098d14ea00..97473f377d1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -794,8 +794,10 @@ static int lock_raw_ref(struct files_ref_store *refs,
 		 */
 		if (refs_verify_refname_available(
 				    refs->packed_ref_store, refname,
-				    extras, NULL, err))
+				    extras, NULL, err)) {
+			ret = TRANSACTION_NAME_CONFLICT;
 			goto error_return;
+		}
 	}
 
 	ret = 0;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 33d34d5ae9e..530369266fd 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1091,6 +1091,22 @@ test_expect_success 'branchname D/F conflict resolved by --prune' '
 	test_cmp expect actual
 '
 
+test_expect_success 'branchname D/F conflict rejected with targeted error message' '
+	git clone . df-conflict-error &&
+	git branch dir_conflict &&
+	(
+		cd df-conflict-error &&
+		git update-ref refs/remotes/origin/dir_conflict/file HEAD &&
+		test_must_fail git fetch 2>err &&
+		test_grep "error: some local refs could not be updated; try running" err &&
+		test_grep " ${SQ}git remote prune origin${SQ} to remove any old, conflicting branches" err &&
+		git pack-refs --all &&
+		test_must_fail git fetch 2>err-packed &&
+		test_grep "error: some local refs could not be updated; try running" err-packed &&
+		test_grep " ${SQ}git remote prune origin${SQ} to remove any old, conflicting branches" err-packed
+	)
+'
+
 test_expect_success 'fetching a one-level ref works' '
 	test_commit extra &&
 	git reset --hard HEAD^ &&

base-commit: d4cc1ec35f3bcce816b69986ca41943f6ce21377
-- 
gitgitgadget