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

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

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?