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

[Junio C Hamano <gitster@pobox.com>]

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

> $ 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
> <hanwen@google.com> 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).

I still am puzzled what you wanted to illustrate with the "git
branch -m bla" trace.  The call graph into the refs API looks like
this:

builtin/branch.c::cmd_branch()
 -> branch.c::create_branch()
    -> refs.c::ref_transaction_update()
       -> refs.c::ref_transaction_add_update()
    -> refs.c::ref_transaction_commit()

and the message the user gave is passed through in "msg" variable
without modification, when calling ref_transaction_update(), which
in turn makes a call to ref_transaction_add_update().  It does this:

    struct ref_update *ref_transaction_add_update(
                    struct ref_transaction *transaction,
                    const char *refname, unsigned int flags,
                    const struct object_id *new_oid,
                    const struct object_id *old_oid,
                    const char *msg)
    {
            struct ref_update *update;

            if (transaction->state != REF_TRANSACTION_OPEN)
                    BUG("update called for transaction that is not open");

            FLEX_ALLOC_STR(update, refname, refname);
            ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
            transaction->updates[transaction->nr++] = update;

            update->flags = flags;

            if (flags & REF_HAVE_NEW)
                    oidcpy(&update->new_oid, new_oid);
            if (flags & REF_HAVE_OLD)
                    oidcpy(&update->old_oid, old_oid);
            update->msg = normalize_reflog_message(msg);
            return update;
    }

And normalize_reflog_message() calls copy_reflog_msg() to squash
runs of isspace() bytes (note: that class includes LF) into a single
space, and runs rtrim(), so update->msg will get a single incomplete
line.  As I suspected in a separate message, I think my notion of
what happens in the ref API implementation common to all backends
and what happens in each backend, and hence my statements about the
distinction, were much fuzzier than yours, so I should say that I
consider that all of the above is happening in the common part
across all backends.  If normalize happens in ref-files, it should
happen in reftable backend, too, automatically.

The files-backend has no chance to even see an embedded LF in the
message when the transaction gets committed and the backend is
called, does it?  So I am not sure why we should crash upon seeing a
LF in the message.

In any case, it seems that as a comment to clarify the end-user
facing each_reflog_ent_fn() parameters, what you quoted above from
my message seems correct to me, after following the callgraph like
the above.  A 0-line (i.e. incomplete, like your 'bla' given to "git
branch"), 1-line (i.e. a single complete line, like the message I
gave to "update-ref -m" in my earlier illustration), or multi-line
message given when a reflog entry is created, is normalized and when
the each_reflog_ent_fn() callback is called, it is shown to the
callback function as a single complete line, with a LF at the end.

Phrased without the explanation specific to this discussion, but
with a bit more detail:

  The message given when a reflog entry was created, is normalized
  into a single line by turning each LF into a space, squeezing a
  run of multiple whitespaces into one space, and removing trailing
  whitespaces. The callback gets a single complete line in the 'msg'
  parameter.

perhaps?