On Tue, Mar 01 2022, Taylor Blau wrote: > @@ -11,7 +11,7 @@ SYNOPSIS > [verse] > 'git remote' [-v | --verbose] > 'git remote add' [-t <branch>] [-m <master>] [-f] [--[no-]tags] [--mirror=(fetch|push)] <name> <URL> > -'git remote rename' <old> <new> > +'git remote' [-v | --verbose] 'rename' <old> <new> I realize that you're just copying an existing pattern within builtin/reflog.c here, but we should really call this "--no-progress", not "-v" or "--verbose". I did that in the last 3 patches here, which I was waiting on re-submitting: https://lore.kernel.org/git/cover-00.12-00000000000-20211130T213319Z-avarab@xxxxxxxxx/ I.e. the whole reason we use --verbose here is entirely historical, it used to (well, still is without those patches) very verbose line-by-line output. Whereas for other things we turn progress on by default, and --verbose is something very different. So please have this by "int progress = 1", and have a "PARSE_OPT_NONEG" "no-progress" option instead, there's no reason we need to propagate the existing UX mistake in "reflog expire". [I reversed the order you wrote the following, due to the obvious digression...] :) > 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. As an aside I think the reftable code "emulates" the D/F conflicts of the files backend, but I'm not sure (this is from vague memory). And I've run into the same limitations you're describing here with the ref transactions code. I tried to transaction-ize the "branch rename" the other day, there we copy an existing reflog in its entirety. It would be really nice if you could plug things into the transaction. It would have to be things that could support the same do-stuff/abort-stuff semantics, e.g. being able to create a file with arbitrary contents, then unlink() it if we're rolling back. But in any case, all of that is a much larger change than is really required here, so I (also) digress :) As an even further aside: If we had a slight "twist" on the ref backend where we were able to store the reflogs next to the actual ref files, e.g.: .git/refs/heads/master .git/refs/heads/.master.reflog Which would work since that '.master.reflog' name isn't a valid refname (it contains "."), couldn't we safely do this whole operation on a POSIX FS with a simple: mv .git/refs/remotes/origin .git/refs/remotes/newname I.e. even if you had concurrent *.lock files in play from other transactions having those would mean that we had a file descriptor open to file in question, and a file being renamed underneath you doesn't change your ability to write to that open file. Although I guess we rename() them into place, and if that target moves..., so we'd need to pre-open them. Hrm. Anyway, that's an even larger digression & can of worms, but something I wondered about when I saw this take a LOOOONG time in the past, "why doesn't it just rename the folders?".
