Re: [PATCH 2/4] refs: trim newline from reflog message

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 24, 2021, 18:53 UTC
Message-ID: <xmqqpmqp2xmz.fsf@gitster.g>
In-Reply-To: <CAFQ2z_Mb+NzQfXfMhdQoPs09p2j1AWRAdXcX3JSznn+H9+T0Bw@mail.gmail.com>

Han-Wen Nienhuys <hanwen@google.com> writes:

Show 16 quoted lines

>> ...  Doesn't the ref backend already ensure that
>> the message is not an incomplete line?  If you feed a single
>> complete line when updating, I do not think the backend should add
>> any extra LF relative to the given message:
>
> But it does, currently. As of
>
>   commit 523fa69c36744ae6779e38614cb9bfb2be552923
>   Author: Junio C Hamano <gitster@pobox.com>
>   ..
>   - 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
>
> it is the job of the files ref backend to add a LF, and the input to
> the ref backend is a string without trailing LF. But the files backend
> then produces that message with a LF, when reading back the data, eg.

Ah, perhaps our confusion comes from the fact that I view the ref API as a whole and do not draw a fine line of distinction between the "ref API implementation common across backends" vs "what ref files backend does". But as the implementor of one backend, you obviously have to care.

In any case, when I did that commit, I clearly meant that the normalization implemented by the patch belong to the common part to be used by all backends for uniform handling of refs. If a call to refs API in a repository that is configured to use reftable backend bypasses the normalization function introduced in that commit, that is a bug.

So, yes "ref API ensures that the message is not an incomplete line to turn 0, 1, or multiple lines input into a single line", which is why the experiments you omitted from your quote (reproduced here)

    $ git update-ref -m 'creating a new branch manually
    ' refs/heads/newtest master
    $ git update-ref -m 'updating a new branch manually
    ' refs/heads/newtest master~1
    $ git reflog refs/heads/newtest
    4150a1677b refs/heads/newtest@{0}: updating a new branch manually
    5f439a0ecf refs/heads/newtest@{1}: updating the reference

to give a complete line when recording the reflog entries does not result in an extra LF shown on the output.

Show 9 quoted lines

>> I think the rule for "msg" is that:
>>
>>    a multi-line message, or a message on a single incomplete-line,
>>    are normalized into a single complete line, and callback gets a
>>    single complete line.
>
> That is not how it works today. The files backend verbatimly dumps the
> message supplied to it. (Maybe it should crash if there is a '\n' in
> the message).

As I said, if parts of refs API implementation forgets to call normalization, it is a bug. Is there different codepath other than the one that is exercised with the "git update -m ''" experiment above and you found such a bug there? It needs to be fixed.

Thanks.