Re: [PATCH v3 2/3] Modify pseudo refs through ref backend storage
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 17, 2020, 08:52 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2007171042461.54@tvgsbejvaqbjf.bet>
- In-Reply-To
- <79cd5dd480a4218d4563120c03046375594cc23e.1594925141.git.gitgitgadget@gmail.com>
Hi Han-Wen,
On Thu, 16 Jul 2020, Han-Wen Nienhuys via GitGitGadget wrote:
Show 19 quoted lines
> 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