Closed thmo closed 3 years ago
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.
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
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
https://github.com/git/git/commit/523fa69c36744ae6779e38614cb9bfb2be552923
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>
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.
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.)
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/
It seems the thread there stalled a bit? Or is there a follow-up one?
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.
Thanks for the patch!
Unfortunately, I see another failure with Git 2.30.1 now in t5050-update-orphans.sh
. See #18 .
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.
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
Friendly ping: Any news here?
The topgit-0.19.13 release contains a resolution for this (and the other reported problems).
Thank you!
While the testsuite passes with Git 2.28.0, it fails in
t6100-tag-reflog
with Git 2.29.2.