Re: [PATCH v4 2/3] replay: make atomic ref updates the default behavior
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Oct 24, 2025, 10:37 UTC
- Message-ID
- <CAP8UFD00rE7gF+baidmoi7nYwVKa3UDQgj+TB4wJLtjJF7u9gA@mail.gmail.com>
- In-Reply-To
- <20251022185045.29256-3-siddharthasthana31@gmail.com>
On Wed, Oct 22, 2025 at 8:51 PM Siddharth Asthana <siddharthasthana31@gmail.com> wrote:
[...]
> However, it should be noted that all three of these are somewhat > special cases; users, whether on the client or server side, would > almost certainly find it more ergonomical to simply have the updating
Nit: maybe: s/ergonomical/ergonomic/
> of refs be the default.
[...]
Show 12 quoted lines
> Change the default behavior to update refs directly, and atomically (at > least to the extent supported by the refs backend in use). This > eliminates the process coordination overhead for the common case. > > For users needing the traditional pipeline workflow, add a new > --ref-action=<mode> option that preserves the original behavior: > > git replay --ref-action=print --onto main topic1..topic2 | git update-ref --stdin > > The mode can be: > * update (default): Update refs directly using an atomic transaction > * print: Output update-ref commands for pipeline use
Nit: maybe it should be mentioned that the command is still experimental, so it's OK to change the default like this.
Show 11 quoted lines
> +--ref-action[=<mode>]:: > + Control how references are updated. The mode can be: > ++ > +-- > + * `update` (default): Update refs directly using an atomic transaction. > + All refs are updated or none are (all-or-nothing behavior). > + * `print`: Output update-ref commands for pipeline use. This is the > + traditional behavior where output can be piped to `git update-ref --stdin`. > +-- > ++ > +The default mode can be configured via `replay.refAction` configuration option.
Nit: s/via `replay.refAction` configuration option/via the `replay.refAction` configuration variable/
(It seems that "configuration variable" is used around 6 times more than "configuration option", so we may want to standardize this wording.)
Show 7 quoted lines
> @@ -54,8 +68,11 @@ include::rev-list-options.adoc[] > OUTPUT > ------ > > -When there are no conflicts, the output of this command is usable as > -input to `git update-ref --stdin`. It is of the form: > +By default (with `--ref-action=update`), this command produces no output on
Nit: s/By default (with `--ref-action=update`)/By default, or with `--ref-action=update`,/
I think it's better to be very explicit here, especially as we mention `--ref-action=print` below.
[...]
> - const char * const replay_usage[] = {
> + const char *const replay_usage[] = {Nit: Not sure this change is worth it, but I understand that it might help pass some automated/CI tests, so not a big issue.
[...]
Show 7 quoted lines
> + /* Default to update mode if not specified */ > + if (!ref_action_str) > + ref_action_str = "update"; > + > + /* Parse ref action mode */ > + if (!strcmp(ref_action_str, "update")) > + ref_action = REF_ACTION_UPDATE;
Nit: maybe:
if (!ref_action_str || !strcmp(ref_action_str, "update"))
ref_action = REF_ACTION_UPDATE;Show 5 quoted lines
> + else if (!strcmp(ref_action_str, "print"))
> + ref_action = REF_ACTION_PRINT;
> + else
> + die(_("unknown --ref-action mode '%s'"), ref_action_str);
> +[...]
Show 6 quoted lines
> test_expect_success 'using replay on bare repo to rebase multiple divergent branches, including contained ones' ' > - git -C bare replay --contained --onto main ^main topic2 topic3 topic4 >result && > + git -C bare replay --ref-action=print --contained --onto main ^main topic2 topic3 topic4 >result && > > test_line_count = 4 result && > cut -f 3 -d " " result >new-branch-tips &&
Are there tests with the new default behavior added? It looks like all the changes in the test script are about adding "--ref-action=print" to an existing test.