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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 30, 2021, 18:56 UTC
Message-ID: <xmqqa6sqp827.fsf@gitster.c.googlers.com>
In-Reply-To: <fec7ef37962da584a89012234ae4a1a@72481c9465c8b2c4aaff8b77ab5e23c>

"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.