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

Han-Wen Nienhuys [off-list ref] writes:

quoted

...  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 [off-list ref]
  ..
  - 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.

quoted

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.

On Wed, Nov 24, 2021 at 7:53 PM Junio C Hamano [off-list ref] wrote:

Han-Wen Nienhuys [off-list ref] writes:

quoted

quoted

...  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 [off-list ref]
  ..
  - 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.

Having the read function return something different than what the
write gets still seems odd to me.
How about having copy_reflog_msg() trim trailing space and then always add LF?

The files backend can assert that the string ends in LF, and doesn't
need to add LF itself.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado