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

2 messages, 2 authors, 2021-11-24 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-11-23 20:34:34

Han-Wen Nienhuys [off-list ref] writes:
I'm talking about refs/refs-internal.h. It seems you want to add something like

/* The ref backend should add a '\n' relative to the message supplied
to the delete/symref/update functions. */
typedef int for_each_reflog_ent_fn(struct ref_store *ref_store,
                                   const char *refname,
                                   each_reflog_ent_fn fn,
                                   void *cb_data);

?
Sorry, I do not follow.  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:

    $ 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

I think what deserves such a comment more is the prototype for
each_reflog_ent_fn describing what the parameters to that callback
function, to help the callers of the iterator what to expect.  That
is the end-user facing part.

/*
 * Callback to process a reflog entry found by the iteration functions (see
 * below)
 */
typedef int each_reflog_ent_fn(
		struct object_id *old_oid, struct object_id *new_oid,
		const char *committer, timestamp_t timestamp,
		int tz, const char *msg, void *cb_data);

Currently it only says "Callback (see below)" but "below" has only
comments about the difference between refs_for_each_reflog_ent() and
refs_for_each_reflog_entreverse() functions, and does not talk about
what "committer" looks like (i.e. does it give user.name equivalent,
user.name plus <user.email>, or something else?), and what "msg"
looks like (i.e. if a multi-line message was given when a ref was
updated or created, do we get these lines intact? if it gets
mangled, how? if the original was a single-liner incomplete line, do
we lack the final LF?), and how tz is encoded.

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.

Once these rules get specified tightly and fully, it is up to the
ref(log) backends how to implement the interface.  If files-backend
wants to keep LF at the end of the message when storing (so that it
can easily use it as record delimiter), it can do so and reuse the
LF at the end of the message as part of the msg parameter to the
callback.  If another backend wants to drop LF at the end of the
message when storing (to save space), it can do so as long as the
callback function gets the LF just like the other backend.

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

From: Han-Wen Nienhuys <hidden>
Date: 2021-11-24 11:17:18

On Tue, Nov 23, 2021 at 9:34 PM Junio C Hamano [off-list ref] wrote:
Han-Wen Nienhuys [off-list ref] writes:
quoted
I'm talking about refs/refs-internal.h. It seems you want to add something like

/* The ref backend should add a '\n' relative to the message supplied
to the delete/symref/update functions. */
typedef int for_each_reflog_ent_fn(struct ref_store *ref_store,
                                   const char *refname,
                                   each_reflog_ent_fn fn,
                                   void *cb_data);

?
Sorry, I do not follow.  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.

$ git --version
git version 2.34.0.rc2.393.gf8c9666880-goog

$ GIT_TRACE_REFS="1" git branch -m bla blub
..
12:03:59.408705 refs/debug.c:162        rename_ref: refs/heads/bla ->
refs/heads/blub "Branch: renamed refs/heads/bla to refs/heads/blub": 0

$ GIT_TRACE_REFS=1 git reflog show refs/heads/blub
12:04:23.277805 refs/debug.c:294        reflog_ent refs/heads/blub
(ret 0): cd3e606211bb1cf8bc57f7d76bab98cc17a150bc ->
cd3e606211bb1cf8bc57f7d76bab98cc17a150bc, Han-Wen Nienhuys
[off-list ref] 1637751839 "Branch: renamed refs/heads/bla to
refs/heads/blub
"
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).

-- 
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help