Re: [PATCH] builtin/remote.c: show progress when renaming remote references

[Derrick Stolee <derrickstolee@github.com>]

On 3/1/2022 5:20 PM, Taylor Blau wrote:
> When renaming a remote, Git needs to rename all remote tracking
> references to the remote's new name (e.g., renaming
> "refs/remotes/old/foo" to "refs/remotes/new/foo" when renaming a remote
> from "old" to "new").
> 
> This can be somewhat slow when there are many references to rename,
> since each rename is done in a separate call to rename_ref() as opposed
> to grouping all renames together into the same transaction. It would be
> nice to execute all renames as a single transaction, but there is a
> snag: the reference transaction backend doesn't support renames during a
> transaction (only individually, via rename_ref()).
> 
> The reasons there are described in more detail in [1], but the main
> problem is that in order to preserve the existing reflog, it must be
> moved while holding both locks (i.e., on "oldname" and "newname"), and
> the ref transaction code doesn't support inserting arbitrary actions
> into the middle of a transaction like that.
> 
> As an aside, adding support for this to the ref transaction code is
> less straightforward than inserting both a ref_update() and ref_delete()
> call into the same transaction. rename_ref()'s special handling to
> detect D/F conflicts would need to be rewritten for the transaction code
> if we wanted to proactively catch D/F conflicts when renaming a
> reference during a transaction. The reftable backend could support this
> much more readily because of its lack of D/F conflicts.
> 
> Instead of a more complex modification to the ref transaction code,
> display a progress meter when running verbosely in order to convince the
> user that Git is doing work while renaming a remote.

Thanks for this patch. It improves the user experience through
useful feedback.

> @@ -682,7 +686,8 @@ static int mv(int argc, const char **argv)
>  		old_remote_context = STRBUF_INIT;
>  	struct string_list remote_branches = STRING_LIST_INIT_DUP;
>  	struct rename_info rename;
> -	int i, refspec_updated = 0;
> +	int i, j = 0, refspec_updated = 0;

My only complaint is that 'j' is not informative enough here.

'j' as a loop iterator is good, but we aren't looping "on" j,
but instead tracking a progress_count across multiple loops.

Thanks,
-Stolee