refs: don't clear oid before read_raw_ref in the debug ref backend

From: Xinyu Ruan <[email protected]>

The debug_read_raw_ref function clears the oid to null_oid before calling read_raw_ref, which causes the oid to be lost even when read_raw_ref successfully reads the reference. This leads to failures when executing commands like "git branch new_branch <commit_id>" with GIT_TRACE_REFS=1, as the command cannot find a valid branch point because the oid is null.

---
refs: don't clear oid before read_raw_ref in the debug ref backend

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2089%2FRuanXinyu%2Fref-debug-disable-clear-oid-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2089/RuanXinyu/ref-debug-disable-clear-oid-v1
Pull-Request: https://github.com/git/git/pull/2089

 refs/debug.c | 1 -
 1 file changed, 1 deletion(-)

index 697adbd0dc..c8dde3558d 100644
@@ -227,7 +227,6 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,
 	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
 	int res = 0;
 
-	oidcpy(oid, null_oid(ref_store->repo->hash_algo));
 	res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,
 					    type, failure_errno);
 

base-commit: a99f379adf116d53eb11957af5bab5214915f91d

gitgitgadget

This sentence doesn't quite make sense to me. If read_raw_ref() succeeds, wouldn't we expect that the OID was set to the target's object ID anyway? So why does it hurt to set the object ID to the null ID if it's going to get rewritten anyway?

Another question is why we were setting it to the null OID in the first place. Ideally, this should be discussed in the commit message.

This smells like an issue that can be be demonstrated via a unit test. Right now though we got zero testing for GIT_TRACE_REFS in our test suite. Maybe this could be used as a starting point for a new test suite "t0620-ref-debug.sh" that exercises the different callbacks?

Thanks!

Patrick