refs: return conflict error when checking packed refs

Commit Message

Ivan Tse April 30, 2024, 2:50 p.m. UTC

From: Ivan Tse <[e-mail]>

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 <[e-mail]>
---
    refs: return conflict error when checking packed refs
    
    I wanted to provide extra context around the error message I'm referring
    to so it's clear what I'm trying to achieve.
    
    First, let's get into a directory/file conflict situation:
    
    git branch dir_file_conflict
    git push origin dir_file_conflict
    git update-ref -d refs/remotes/origin/dir_file_conflict
    git update-ref -d refs/heads/dir_file_conflict
    git update-ref refs/remotes/origin/dir_file_conflict/file master
    
    
    From above, the remote origin has a dir_file_conflict ref name and the
    local repository has a dir_file_conflict/file ref name for the remote.
    This situation can occur if dir_file_conflict/file used to exist and but
    was later deleted in the remote.
    
    Then when we git fetch:
    
    yolo10[master] $ git fetch
    error: cannot lock ref 'refs/remotes/origin/dir_file_conflict': 'refs/remotes/origin/dir_file_conflict/file' exists; cannot create 'refs/remotes/origin/dir_file_conflict'
    From github.com:ivantsepp/yolo10
     ! [new branch]      dir_file_conflict -> origin/dir_file_conflict  (unable to update local ref)
    error: some local refs could not be updated; try running
     'git remote prune origin' to remove any old, conflicting branches
    
    
    We get the helpful error message to run git remote prune origin to
    remove old, conflicting branches.
    
    However, when the ref is stored in packed refs:
    
    yolo10[master] $ git pack-refs --all
    yolo10[master] $ git fetch
    error: cannot lock ref 'refs/remotes/origin/dir_file_conflict': 'refs/remotes/origin/dir_file_conflict/file' exists; cannot create 'refs/remotes/origin/dir_file_conflict'
    From github.com:ivantsepp/yolo10
     ! [new branch]      dir_file_conflict -> origin/dir_file_conflict  (unable to update local ref)
    
    
    The helpful message is not there! I believe this error message should
    show up regardless of how the ref is stored (loose vs packed-refs). I
    attempted to track down the necessary change to make this happen and it
    seems like a straightforward change. I hope I didn't overlook anything!

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

 refs/files-backend.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8

Comments

Patrick Steinhardt May 2, 2024, 12:38 p.m. UTC | #1

On Tue, Apr 30, 2024 at 02:50:47PM +0000, Ivan Tse via GitGitGadget wrote:
> From: Ivan Tse <[e-mail]>
> 
> 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`.

The change itself makes sense to me. But I'd like to see a test that
demonstrates the new behaviour so that we don't regress this in the
future.

Thanks!

Patrick

Patch

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;