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

From: Junio C Hamano <[email-masked]>
Date: Wed, 24 Nov 2021 10:53:40 -0800

Han-Wen Nienhuys <[email-masked]> writes:

>> ...  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 <[email-masked]>
>   ..
>   - 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.

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