Re: [PATCH 1/2] t/t1417: test symbolic-ref effects on ref logs

[Junio C Hamano <gitster@pobox.com>]

"Kyle J. McKay" <mackyle@gmail.com> writes:

> The git command `git symbolic-ref <refname1> <refname2>` updates
> <refname1> to be a "symbolic" pointer to <refname2>.  No matter
> what future value <refname2> takes on, <refname1> will continue
> to refer to that future value since it's "symbolic".

Correct.  While you are on the my-topic branch, HEAD (that's the
<refname1> in your description) points at refs/heads/my-topic
(that's the <refname2> in your description).

And when you create a new commit from this state, the logs of these
two refs will gain one entry each.  "git log HEAD@{now} would show
the recent progress of HEAD, "git log my-topic@{now}" would show the
recent progress of my-topic ("my-topic@"now}" can also be spelled as
"@{now}).

The <refname1> (HEAD) will keep referring to <refname2> (my-topic)
until you switch branches, and does not change even if <refname2>
points at a different commit, as it is "symbolic".

> Since commit 523fa69c36744ae6 ("reflog: cleanse messages in the refs.c
> layer", 2020-07-10, v2.29.0), the effect of using the aforementioned
> "symbolic-ref" command on ref logs has changed in an unexpected way.

Please explain "in an unexpected way" here in the log message.  Not
every reader can read your mind and would expect the same as you do.

The said commit came as part of this topic, ...

https://lore.kernel.org/git/pull.669.v2.git.1594401593.gitgitgadget@gmail.com/

... so I've added the true author of it on the Cc: list.

> Add a new set of tests to exercise and demonstrate this change
> in preparation for correcting it (at which point the failing tests
> will be flipped from `test_expect_failure` to `test_expect_success`).

We usually prefer not to do the two-step "expect_failure first and
then in a separate patch flip that to _success", as it makes it hard
to see the "fix" step (because the change in the test would be shown
only 3 lines before and after _failure->_success line, and what the
test is doing cannot be seen in the patch by itself).

Thanks.