[+cc pks]
On Wed, Nov 17, 2021 at 04:52:46PM -0800, Bryan Turner wrote:
> > The expected behavior would be that the latest reference transaction
> > hook refers to the state of the references on disk. That is, either
> > `master` should point to 0 (be deleted), or it should have said that
> > `master` pointed to `e197d1`.
> >
> > But if we actually examine `master`, it's set to `e197d1`, just as you
> > would expect. The GC should have been a no-op overall.
>
> One of the subtasks of "git gc" is "git pack-refs". If you inspect in
> more detail, I suspect you'll find that "refs/heads/master" was loose
> before "git gc" ran (as in, there was a file
> "$GIT_DIR/refs/heads/master") and "packed-refs" either didn't have a
> "refs/heads/master" entry or had a different hash. (Loose refs always
> "win" over packed, since ref updates only write loose refs.)
It seems totally broken to me that we'd trigger the
reference-transaction hook for ref packing. The point of the hook is to
track logical updates to the refs. But during ref packing that does not
change at all; the value remains the same. So I don't think we should be
triggering the hook at all, let alone with confusing values.
This snippet shows a simple case that I think is wrong:
-- >8 --
git init -q repo
cd repo
cat >.git/hooks/reference-transaction <<\EOF
#!/bin/sh
echo >&2 "==> reference-transaction $*"
sed 's/^/ /'
EOF
chmod +x .git/hooks/reference-transaction
echo >&2 "running commit..."
git commit --allow-empty -qm foo
echo >&2 "running pack-refs..."
git pack-refs --all --prune
-- >8 --
It produces:
running commit...
==> reference-transaction prepared
0000000000000000000000000000000000000000 77bcab0d950aee3021e8aa13a15d40e7a9a5f71b HEAD
0000000000000000000000000000000000000000 77bcab0d950aee3021e8aa13a15d40e7a9a5f71b refs/heads/main
==> reference-transaction committed
0000000000000000000000000000000000000000 77bcab0d950aee3021e8aa13a15d40e7a9a5f71b HEAD
0000000000000000000000000000000000000000 77bcab0d950aee3021e8aa13a15d40e7a9a5f71b refs/heads/main
running pack-refs...
==> reference-transaction prepared
0000000000000000000000000000000000000000 77bcab0d950aee3021e8aa13a15d40e7a9a5f71b refs/heads/main
==> reference-transaction committed
0000000000000000000000000000000000000000 77bcab0d950aee3021e8aa13a15d40e7a9a5f71b refs/heads/main
==> reference-transaction prepared
77bcab0d950aee3021e8aa13a15d40e7a9a5f71b 0000000000000000000000000000000000000000 refs/heads/main
==> reference-transaction committed
77bcab0d950aee3021e8aa13a15d40e7a9a5f71b 0000000000000000000000000000000000000000 refs/heads/main
I think the final four invocations should be skipped entirely. They're
pointless at best (nothing actually changed), and extremely misleading
at worst (they look like the ref ended up deleted!).
-Peff
