t6100-tag-reflog failures with Git 2.29.2

[thmo]

While the testsuite passes with Git 2.28.0, it fails in t6100-tag-reflog with Git 2.29.2.

*** t6100-tag-reflog.sh ***
1..44
ok 1 - setup main
ok 2 - setup linked
not ok 3 - verify setup
#      failed: t6100-tag-reflog.sh:498: 3 - verify setup
#
#      	test -f "main/.git/objects/pack/$objpack.idx" &&
#      	test -f "main/.git/objects/pack/$objpack.pack" &&
#      	cd main &&
#      	mv -f ".git/objects/pack/$objpack.idx" ".git/objects/pack/$objpack.no" &&
#      	test_must_fail git fsck --full &&
#      	mv -f ".git/objects/pack/$objpack.no" ".git/objects/pack/$objpack.idx" &&
#      	git fsck --full &&
#      	git log -g --pretty=fuller HEAD >actual &&
#      	test_cmp actual ../HEAD_main.log &&
#      	git log -g --pretty=fuller master >actual &&
#      	test_cmp actual ../master.log &&
#      	if test_have_prereq GIT_2_5; then
#      		cd ../linked &&
#      		git log -g --pretty=fuller HEAD >actual &&
#      		test_cmp actual ../HEAD_linked.log &&
#      		git log -g --pretty=fuller linked >actual &&
#      		test_cmp actual ../linked.log
#      	fi &&
#      	test_when_finished test_set_prereq SETUP
#
ok 4 # skip tag -g HEAD (missing SETUP)
[... remaining tests all skipped ...]

# t6100 failed 1 among 44 test(s)

[mackyle]

Thanks for the report.
A quick perusal of the Git release notes from 2.28.0 to 2.29.2 does not make anything immediately pop out at me as the cause.
I will investigate in more detail as I have the time.

[mackyle]

I have been able to reproduce this locally. Apparently the contents of the ref log have changed in a subtle way -- not sure if it's by design or an accidental Git change.

To see the difference yourself use the -x -i options and run that specific test like so:

cd topgit # topgit is the top-level checkout directory of the topgit repository
make
cd t
./t6100-tag-reflog.sh -x -i

[mackyle]

I love git bisect.

The following commit has been identified as introducing the change that causes t6100 to fail:

https://git.kernel.org/pub/scm/git/git.git/commit/?id=523fa69c36744ae6779e38614cb9bfb2be552923
git/git@523fa69

commit 523fa69c36744ae6779e38614cb9bfb2be552923
Author:     Junio C Hamano <gitster@pobox.com>
AuthorDate: Fri Jul 10 17:19:53 2020 +0000
Commit:     Junio C Hamano <gitster@pobox.com>
CommitDate: Fri Jul 10 13:53:37 2020 -0700

reflog: cleanse messages in the refs.c layer

Regarding reflog messages:

 - We expect that a reflog message consists of a single line.  The
   file format used by the files backend may add a LF after the
   message as a delimiter, and output by commands like "git log -g"
   may complete such an incomplete line by adding a LF at the end,
   but philosophically, the terminating LF is not a part of the
   message.

 - We however allow callers of refs API to supply a random sequence
   of NUL terminated bytes.  We cleanse caller-supplied message by
   squashing a run of whitespaces into a SP, and by trimming trailing
   whitespace, before storing the message.  This is how we tolerate,
   instead of erring out, a message with LF in it (be it at the end,
   in the middle, or both).

Currently, the cleansing of the reflog message is done by the files
backend, before the log is written out.  This is sufficient with the
current code, as that is the only backend that writes reflogs.  But
new backends can be added that write reflogs, and we'd want the
resulting log message we would read out of "log -g" the same no
matter what backend is used, and moving the code to do so to the
generic layer is a way to do so.

An added benefit is that the "cleansing" function could be updated
later, independent from individual backends, to e.g. allow
multi-line log messages if we wanted to, and when that happens, it
would help a lot to ensure we covered all bases if the cleansing
function (which would be updated) is called from the generic layer.

Side note: I am not interested in supporting multi-line reflog
messages right at the moment (nobody is asking for it), but I
envision that instead of the "squash a run of whitespaces into a SP
and rtrim" cleansing, we can %urlencode problematic bytes in the
message *AND* append a SP at the end, when a new version of Git that
supports multi-line and/or verbatim reflog messages writes a reflog
record.  The reading side can detect the presense of SP at the end
(which should have been rtrimmed out if it were written by existing
versions of Git) as a signal that decoding %urlencode recovers the
original reflog message.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

[mackyle]

After more analysis and testing, it turns out that commit has a completely undocumented and unintended? side-effect.

Whenever the git symbolic-ref HEAD refs/heads/<whatever> command is used, as of Git v2.29.0 (the first released version with the above commit), the HEAD ref log (or the ref log for whatever symbolic ref is being updated provided it actually has a ref log) receives a new entry with an empty message.

It's rather obvious what happened from looking at the changes in that commit, the question is whether or not it was intentional -- I suspect it was an accident.

I will follow up on the git list to find out.

[thmo]

Many thanks for doing that analysis!

(And will try to remember -x -i, that's really helpful, was instead looking at the files in question myself, but without much insight.)

[mackyle]

FYI, you can always find the full list of options that can be passed to individual tests in the topgit t/README-TESTLIB file in the "Test Options" section:

https://github.com/mackyle/topgit/blob/master/t/README-TESTLIB#L382

A patch set has been submitted to the git@ list regarding this issue. How exactly the topgit test gets fixed will depend on the outcome of that discussion.

You can follow the email thread at this link:

https://lore.kernel.org/git/7c7e8679f2da7e1475606d698b2da8c@72481c9465c8b2c4aaff8b77ab5e23c/

[thmo]

It seems the thread there stalled a bit? Or is there a follow-up one?

[mackyle]

I apologize, I've had some "real life" issues to deal with that popped up and have unexpectedly dragged on and on…

However, in the meanwhile, this patch can be applied:

https://github.com/mackyle/topgit/blob/f04fac9d9a0c7e2ad43d19736f3af71e6bd462af/patch.txt

It allows the test to run properly on any version of Git.

[thmo]

Thanks for the patch!

Unfortunately, I see another failure with Git 2.30.1 now in t5050-update-orphans.sh. See #18 .

[mackyle]

I've finally been able to get back to this project.

Unfortunately Git v2.31.0 has more breakage in store for t6100 as it changes how Git interprets <refname>@{0} which, although it corrects a long-standing issue, breaks more checks in t6100.

A forthcoming TopGit release will include a proper fix for this issue and others as soon as I figure out what to do about the @{0} problem.

[thmo]

Indeed, with Git 2.31.1, I see these tests failing:

t6100-tag-reflog.sh             (Wstat: 0 Tests: 44 Failed: 11)
  Failed tests:  26-29, 37-40, 42-44

[thmo]

Friendly ping: Any news here?

[mackyle]

The topgit-0.19.13 release contains a resolution for this (and the other reported problems).

[thmo]

Thank you!