Patrick Steinhardt <ps@xxxxxx> writes:
> I've quickly hacked something together now, see the work-in-progress
> patch below. The patch does not yet handle reflogs, but that isn't too
> hard to implement.
>
> And these changes indeed speed up things by quite a lot: instead of
> hours it now takes 7 seconds :) I'll polish this patch series and will
> likely send it in tomorrow.
Nice. Not just the "oops, we shouldn't assume symrefs always point
at an existing ref" fix, performance fix comes at the same time ;-)
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 5dd6cbbaeed..072a70e6b45 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -612,36 +612,53 @@ static int add_branch_for_removal(const char *refname,
> struct rename_info {
> const char *old_name;
> const char *new_name;
> - struct string_list *remote_branches;
> uint32_t symrefs_nr;
> + struct ref_transaction *transaction;
> + struct strbuf *err;
> };
OK, as a place to hook the transaction into, rename_info is a
convenient place, as it already is passed around throughout the
relevant code paths.
> -static int read_remote_branches(const char *refname, const char *referent UNUSED,
> - const struct object_id *oid UNUSED,
> - int flags UNUSED, void *cb_data)
> +static int queue_one_rename(const char *refname, const char *referent,
> + const struct object_id *oid,
> + int flags, void *cb_data)
> {
> + struct strbuf new_refname = STRBUF_INIT, new_referent = STRBUF_INIT;
> struct rename_info *rename = cb_data;
> - struct strbuf buf = STRBUF_INIT;
> - struct string_list_item *item;
> - int flag;
> - const char *symref;
> -
> - strbuf_addf(&buf, "refs/remotes/%s/", rename->old_name);
> - if (starts_with(refname, buf.buf)) {
> - item = string_list_append(rename->remote_branches, refname);
> - symref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> - refname, RESOLVE_REF_READING,
> - NULL, &flag);
> - if (symref && (flag & REF_ISSYMREF)) {
> - item->util = xstrdup(symref);
> - rename->symrefs_nr++;
> - } else {
> - item->util = NULL;
> - }
> + int error;
> +
> + strbuf_addf(&new_refname, "refs/remotes/%s/", rename->old_name);
> + if (!starts_with(refname, new_refname.buf)) {
> + error = 0;
> + goto out;
> }
> - strbuf_release(&buf);
>
> - return 0;
> + if (flags & REF_ISSYMREF) {
> + strbuf_addstr(&new_referent, referent);
> + strbuf_splice(&new_referent, strlen("refs/remotes/"), strlen(rename->old_name),
> + rename->new_name, strlen(rename->new_name));
> + oid = NULL;
> + }
> +
> + error = ref_transaction_delete(rename->transaction, refname,
> + oid, referent, REF_NO_DEREF, NULL, rename->err);
Remove old ...
> + if (error < 0)
> + goto out;
> +
> + strbuf_reset(&new_refname);
> + strbuf_addstr(&new_refname, refname);
> + strbuf_splice(&new_refname, strlen("refs/remotes/"), strlen(rename->old_name),
> + rename->new_name, strlen(rename->new_name));
> +
> + error = ref_transaction_update(rename->transaction, new_refname.buf, oid,
> + null_oid(the_hash_algo), (flags & REF_ISSYMREF) ? new_referent.buf : NULL, NULL,
> + REF_SKIP_CREATE_REFLOG | REF_NO_DEREF | REF_SKIP_OID_VERIFICATION,
> + NULL, rename->err);
... and create new. Would we be hit with the same "while renaming A
to A/B, there is a D/F conflict which the ref transaction does not
handle by itself" issue we saw recently here?
> + rename.transaction = ref_store_transaction_begin(get_main_ref_store(the_repository),
> + 0, &err);
> + if (!rename.transaction)
> + goto out;
>
> + result = refs_for_each_rawref(get_main_ref_store(the_repository),
> + queue_one_rename, &rename);
> + if (result < 0)
> + die(_("renaming references failed: %s"), rename.err->buf);
>
> + result = ref_transaction_commit(rename.transaction, &err);
> + if (result < 0)
> + die(_("committing renamed references failed: %s"), rename.err->buf);
>
> handle_push_default(rename.old_name, rename.new_name);
>
> out:
> - string_list_clear(&remote_branches, 1);
> + ref_transaction_free(rename.transaction);
Very nice.
> strbuf_release(&old_remote_context);
> strbuf_release(&buf);
> strbuf_release(&buf2);
> - strbuf_release(&buf3);
> + strbuf_release(&err);
> return result;
> }
>
