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

From: Han-Wen Nienhuys <hanwen@google.com>
Date: Nov 23, 2021, 16:35 UTC
Message-ID: <CAFQ2z_PE7TMj=qfQVroK_gRfZk-xF9PKhk2yxqF-bB+2aA7eoQ@mail.gmail.com>
In-Reply-To: <xmqq35nnddw7.fsf@gitster.g>

On Mon, Nov 22, 2021 at 11:27 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

>
> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Han-Wen Nienhuys <hanwen@google.com>
> >
> > Commit 523fa69c ("reflog: cleanse messages in the refs.c layer") standardizes
> > how write entries into the reflog. This commit standardizes how we get messages
> > out of the reflog. Before, the files backend implicitly added '\n' to the end of
> > reflog message on reading, which creates a subtle incompatibility with alternate
> > ref storage backends, such as reftable.
> >
> > We address this by stripping LF from the message before we pass it to the
> > user-provided callback.
>
> If this were truly "user-provided", then I'd argue that all backends
> should follow whatever the files backend has been doing forever---if
> the files added LF implicitly, others should, too, because that is
> pretty much what these "user-provided" callbacks have been expecting
> to see.

I think it's just wrong. If you pass `msg` to a storage API, you should get `msg` when you read it back, not (msg + "\n").

Show 9 quoted lines

> Ah, the $RUN is hiding what is really going on; it is running the
> "test-tool ref-store" helper, and we did not adjust that helper.  So
> if we make a compensating change to the test-tool then we do not
> have to have these changes at all?  But that point may be moot.
>
> In any case, in order to lose 5 lines from show-branch.c, and 2
> lines from reflog-walk.c, I see that we had to touch 30+ lines in
> refs/files-backend.c.  I find it a bit hard to sell this as an
> improvement to the API, to be honest.

The test-tool ref-store adds its own '\n', so you always get a blank line in the output. That serves no purpose, and leads to the

  tail-n2  | head -n1

in order to read the last log line. I think it's silly, and should be dropped.

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