Patrick Steinhardt <ps@xxxxxx> writes:
> When migrating reflog entries between different storage formats we must
> reconstruct the identity of reflog entries. This is done by passing the
> committer passed to the `migrate_one_reflog_entry()` callback function
> to `fmt_ident()`.
>
> This results in an invalid identity though: `fmt_ident()` expects the
> caller to provide both name and mail of the author, but we pass the full
> identity as mail. This leads to an identity like:
>
> pks <Patrick Steinhardt ps@xxxxxx>
>
> Fix the bug by splitting the identity line first. This allows us to
> extract both the name and mail so that we can pass them to `fmt_ident()`
> separately.
>
> This commit does not yet add any tests as there is another bug in the
> reflog migration that will be fixed in a subsequent commit. Once that
> bug is fixed we'll make the reflog verification in t1450 stricter, and
> that will catch both this bug here and the other bug.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> refs.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 188989e4113..64544300dc3 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2945,7 +2945,7 @@ struct migration_data {
> struct ref_store *old_refs;
> struct ref_transaction *transaction;
> struct strbuf *errbuf;
> - struct strbuf sb;
> + struct strbuf sb, name, mail;
> };
>
> static int migrate_one_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid,
> @@ -2984,7 +2984,7 @@ struct reflog_migration_data {
> struct ref_store *old_refs;
> struct ref_transaction *transaction;
> struct strbuf *errbuf;
> - struct strbuf *sb;
> + struct strbuf *sb, *name, *mail;
> };
>
> static int migrate_one_reflog_entry(struct object_id *old_oid,
> @@ -2994,13 +2994,22 @@ static int migrate_one_reflog_entry(struct object_id *old_oid,
> const char *msg, void *cb_data)
> {
> struct reflog_migration_data *data = cb_data;
> + struct ident_split ident;
> const char *date;
> int ret;
>
> + if (split_ident_line(&ident, committer, strlen(committer)) < 0)
> + return -1;
> +
> + strbuf_reset(data->name);
> + strbuf_add(data->name, ident.name_begin, ident.name_end - ident.name_begin);
> + strbuf_reset(data->mail);
> + strbuf_add(data->mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
> +
> date = show_date(timestamp, tz, DATE_MODE(NORMAL));
> strbuf_reset(data->sb);
> /* committer contains name and email */
This comment doesn't make sense here no more. Better leave it out.
> - strbuf_addstr(data->sb, fmt_ident("", committer, WANT_BLANK_IDENT, date, 0));
> + strbuf_addstr(data->sb, fmt_ident(data->name->buf, data->mail->buf, WANT_BLANK_IDENT, date, 0));
I was checking whether we could use IDENT_NO_NAME instead of parsing the
ident. But fmt_ident() calls strbuf_addstr_without_crud() which strips
out the angle brackets from the email address, which builds an invalid
identity like: "C O Mitter committer@xxxxxxxxxxx 1112911993 -0700"
We could opt to add a flag IDENT_LEAVE_CRUD which calls strbuf_addstr()
instead, but I'm not sure we want to go there.
--
Cheers,
Toon
