Re: [PATCH v3 2/3] Modify pseudo refs through ref backend storage

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Han-Wen,

On Thu, 16 Jul 2020, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> The previous behavior was introduced in commit 74ec19d4be
> ("pseudorefs: create and use pseudoref update and delete functions",
> Jul 31, 2015), with the justification "alternate ref backends still
> need to store pseudorefs in GIT_DIR".
>
> Refs such as REBASE_HEAD are read through the ref backend. This can
> only work consistently if they are written through the ref backend as
> well. Tooling that works directly on files under .git should be
> updated to use git commands to read refs instead.
>
> The following behaviors change:
>
> * Updates to pseudorefs (eg. ORIG_HEAD) with
>   core.logAllRefUpdates=always will create reflogs for the pseudoref.
>
> * non-HEAD pseudoref symrefs are also dereferenced on deletion. Update
>   t1405 accordingly.

Unfortunately, there is still a breakage in t1405, although it only really
shows up on Windows and macOS because of case-insensitive filesystems.
https://github.com/gitgitgadget/git/runs/879772942?check_suite_focus=true
shows the concrete problem: t1405.17 'delete_ref(refs/heads/foo)' will
fail thusly:

	++ test-tool ref-store main delete-ref msg refs/heads/foo c90e4dc5e12224a428dedfbd45ba11e5531706a2 0
	error: cannot lock ref 'refs/heads/foo': is at 1e995a94b5a60488b6ebaf78ec779d85a55ea700 but expected c90e4dc5e12224a428dedfbd45ba11e5531706a2
	error: last command exited with $?=1

The reason is that there exists a `refs/heads/foo` _and_ the symref `FOO`
(which points to `refs/heads/master`, which is at 1e995a).

An earlier symptom of this bug is visible on Linux, too, though: when
you run t1405.4 'delete_refs(FOO, refs/tags/new-tag)', it shows this:

	+ git rev-parse FOO --
	warning: ignoring dangling symref FOO
	fatal: bad revision 'FOO'

(Seeing this warning suggests to me that this should probably be caught
_by the regression test_, i.e. stderr should be redirected, and it should
be verified via `test_i18ngrep` that that warning is not shown.)

The reason for this warning is that the symref `.git/FOO` is no longer
removed as part of t1405.4 'delete_refs(FOO, refs/tags/new-tag)'. I've
used this patch to bisect the breakage down to this here patch:

-- snip --
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index a8d695cd4f1..779906d607f 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -34,6 +34,7 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '
 	m=$(git rev-parse master) &&
 	$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&
 	test_must_fail git rev-parse FOO -- &&
+	test_path_is_missing .git/FOO &&
 	test_must_fail git rev-parse refs/tags/new-tag --&&
 	test_must_fail git rev-parse master -- &&
 	git update-ref refs/heads/master $m
-- snap --

Hopefully you can figure out what is going wrong.

Ciao,
Dscho