Re: Bug report: Strange behavior with `git gc` and `reference-transaction` hook

[+cc pks]

On Wed, Nov 17, 2021 at 04:52:46PM -0800, Bryan Turner wrote:

quoted

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