Subject: Re: [PATCH v6 2/3] replay: make atomic ref updates the default behavior

From: Siddharth Asthana <[email-masked]>
Date: Tue, 4 Nov 2025 01:02:03 +0530

On 03/11/25 21:55, Phillip Wood wrote:
> Hi Siddharth
>
> On 30/10/2025 19:19, Siddharth Asthana wrote:
>
>> +    case REF_ACTION_UPDATE:
>> +        return ref_transaction_update(transaction, refname, new_oid, 
>> old_oid,
>> +                          NULL, NULL, 0, "git replay", err);
>
> I wonder if we should use a more descriptive reflog message here that 
> says what git replay was doing. For example "git replay --onto 
> <new-base> <revs>" could include the new base in the reflog message 
> like "git rebase" does. For "git replay --advance" we could include 
> the commits that have been picked. It would be helpful to test the 
> reflog message in the new tests as well.
>
> Thanks
>
> Phillip

Hi Phillip,

Thanks for the suggestion! I agree that a more descriptive reflog
message would be helpful for users tracking what happened.

I looked at how `git rebase` constructs its reflog messages (it uses
something like "rebase (finish): refs/heads/feature onto abc123def")
and I'm thinking of using a simpler format for replay:

     "replay --onto main"
     "replay --advance main"

This shows the mode and target in a way that mirrors what the user
typed. I chose to use the symbolic name (e.g., "main") rather than
the commit SHA because it seems more user-friendly, though I notice
git rebase uses oid_to_hex().

Regarding "include the commits that have been picked" for --advance
mode - would you prefer:
   - The revision range as specified by the user (e.g., "topic1..topic2")?
   - Just the target branch like I have above?

The revision range would provide more context, but it might make the
reflog message quite long if the user specified something complex. I'm
happy to include it if that's what you think would be most useful.

I'll add tests for the reflog messages in both modes.

Thanks,
Siddharth

>
>
>> +    default:
>> +        BUG("unknown ref_action_mode %d", mode);
>> +    }
>> +}
>> +
>>   int cmd_replay(int argc,
>>              const char **argv,
>>              const char *prefix,
>> @@ -294,6 +321,8 @@ int cmd_replay(int argc,
>>       struct commit *onto = NULL;
>>       const char *onto_name = NULL;
>>       int contained = 0;
>> +    const char *ref_action_str = NULL;
>> +    enum ref_action_mode ref_action = REF_ACTION_UPDATE;
>>         struct rev_info revs;
>>       struct commit *last_commit = NULL;
>> @@ -302,12 +331,14 @@ int cmd_replay(int argc,
>>       struct merge_result result;
>>       struct strset *update_refs = NULL;
>>       kh_oid_map_t *replayed_commits;
>> +    struct ref_transaction *transaction = NULL;
>> +    struct strbuf transaction_err = STRBUF_INIT;
>>       int ret = 0;
>>   -    const char * const replay_usage[] = {
>> +    const char *const replay_usage[] = {
>>           N_("(EXPERIMENTAL!) git replay "
>>              "([--contained] --onto <newbase> | --advance <branch>) "
>> -           "<revision-range>..."),
>> +           "[--ref-action[=<mode>]] <revision-range>..."),
>>           NULL
>>       };
>>       struct option replay_options[] = {
>> @@ -319,6 +350,9 @@ int cmd_replay(int argc,
>>                  N_("replay onto given commit")),
>>           OPT_BOOL(0, "contained", &contained,
>>                N_("advance all branches contained in revision-range")),
>> +        OPT_STRING(0, "ref-action", &ref_action_str,
>> +               N_("mode"),
>> +               N_("control ref update behavior (update|print)")),
>>           OPT_END()
>>       };
>>   @@ -333,6 +367,18 @@ int cmd_replay(int argc,
>>       die_for_incompatible_opt2(!!advance_name_opt, "--advance",
>>                     contained, "--contained");
>>   +    /* 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;
>> +    else if (!strcmp(ref_action_str, "print"))
>> +        ref_action = REF_ACTION_PRINT;
>> +    else
>> +        die(_("unknown --ref-action mode '%s'"), ref_action_str);
>> +
>>       advance_name = xstrdup_or_null(advance_name_opt);
>>         repo_init_revisions(repo, &revs, prefix);
>> @@ -389,6 +435,17 @@ int cmd_replay(int argc,
>>       determine_replay_mode(repo, &revs.cmdline, onto_name, 
>> &advance_name,
>>                     &onto, &update_refs);
>>   +    /* Initialize ref transaction if using update mode */
>> +    if (ref_action == REF_ACTION_UPDATE) {
>> +        transaction = 
>> ref_store_transaction_begin(get_main_ref_store(repo),
>> +                              0, &transaction_err);
>> +        if (!transaction) {
>> +            ret = error(_("failed to begin ref transaction: %s"),
>> +                    transaction_err.buf);
>> +            goto cleanup;
>> +        }
>> +    }
>> +
>>       if (!onto) /* FIXME: Should handle replaying down to root 
>> commit */
>>           die("Replaying down to root commit is not supported yet!");
>>   @@ -434,10 +491,15 @@ int cmd_replay(int argc,
>>               if (decoration->type == DECORATION_REF_LOCAL &&
>>                   (contained || strset_contains(update_refs,
>>                                 decoration->name))) {
>> -                printf("update %s %s %s\n",
>> -                       decoration->name,
>> - oid_to_hex(&last_commit->object.oid),
>> -                       oid_to_hex(&commit->object.oid));
>> +                if (handle_ref_update(ref_action, transaction,
>> +                              decoration->name,
>> +                              &last_commit->object.oid,
>> +                              &commit->object.oid,
>> +                              &transaction_err) < 0) {
>> +                    ret = error(_("failed to update ref '%s': %s"),
>> +                            decoration->name, transaction_err.buf);
>> +                    goto cleanup;
>> +                }
>>               }
>>               decoration = decoration->next;
>>           }
>> @@ -445,10 +507,23 @@ int cmd_replay(int argc,
>>         /* In --advance mode, advance the target ref */
>>       if (result.clean == 1 && advance_name) {
>> -        printf("update %s %s %s\n",
>> -               advance_name,
>> -               oid_to_hex(&last_commit->object.oid),
>> -               oid_to_hex(&onto->object.oid));
>> +        if (handle_ref_update(ref_action, transaction, advance_name,
>> +                      &last_commit->object.oid,
>> +                      &onto->object.oid,
>> +                      &transaction_err) < 0) {
>> +            ret = error(_("failed to update ref '%s': %s"),
>> +                    advance_name, transaction_err.buf);
>> +            goto cleanup;
>> +        }
>> +    }
>> +
>> +    /* Commit the ref transaction if we have one */
>> +    if (transaction && result.clean == 1) {
>> +        if (ref_transaction_commit(transaction, &transaction_err)) {
>> +            ret = error(_("failed to commit ref transaction: %s"),
>> +                    transaction_err.buf);
>> +            goto cleanup;
>> +        }
>>       }
>>         merge_finalize(&merge_opt, &result);
>> @@ -460,6 +535,9 @@ int cmd_replay(int argc,
>>       ret = result.clean;
>>     cleanup:
>> +    if (transaction)
>> +        ref_transaction_free(transaction);
>> +    strbuf_release(&transaction_err);
>>       release_revisions(&revs);
>>       free(advance_name);
>>   diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
>> index 58b3759935..123734b49f 100755
>> --- a/t/t3650-replay-basics.sh
>> +++ b/t/t3650-replay-basics.sh
>> @@ -52,7 +52,7 @@ test_expect_success 'setup bare' '
>>   '
>>     test_expect_success 'using replay to rebase two branches, one on 
>> top of other' '
>> -    git replay --onto main topic1..topic2 >result &&
>> +    git replay --ref-action=print --onto main topic1..topic2 >result &&
>>         test_line_count = 1 result &&
>>   @@ -68,7 +68,7 @@ test_expect_success 'using replay to rebase two 
>> branches, one on top of other' '
>>   '
>>     test_expect_success 'using replay on bare repo to rebase two 
>> branches, one on top of other' '
>> -    git -C bare replay --onto main topic1..topic2 >result-bare &&
>> +    git -C bare replay --ref-action=print --onto main topic1..topic2 
>> >result-bare &&
>>       test_cmp expect result-bare
>>   '
>>   @@ -86,7 +86,7 @@ test_expect_success 'using replay to perform 
>> basic cherry-pick' '
>>       # 2nd field of result is refs/heads/main vs. refs/heads/topic2
>>       # 4th field of result is hash for main instead of hash for topic2
>>   -    git replay --advance main topic1..topic2 >result &&
>> +    git replay --ref-action=print --advance main topic1..topic2 
>> >result &&
>>         test_line_count = 1 result &&
>>   @@ -102,7 +102,7 @@ test_expect_success 'using replay to perform 
>> basic cherry-pick' '
>>   '
>>     test_expect_success 'using replay on bare repo to perform basic 
>> cherry-pick' '
>> -    git -C bare replay --advance main topic1..topic2 >result-bare &&
>> +    git -C bare replay --ref-action=print --advance main 
>> topic1..topic2 >result-bare &&
>>       test_cmp expect result-bare
>>   '
>>   @@ -115,7 +115,7 @@ test_expect_success 'replay fails when both 
>> --advance and --onto are omitted' '
>>   '
>>     test_expect_success 'using replay to also rebase a contained 
>> branch' '
>> -    git replay --contained --onto main main..topic3 >result &&
>> +    git replay --ref-action=print --contained --onto main 
>> main..topic3 >result &&
>>         test_line_count = 2 result &&
>>       cut -f 3 -d " " result >new-branch-tips &&
>> @@ -139,12 +139,12 @@ test_expect_success 'using replay to also 
>> rebase a contained branch' '
>>   '
>>     test_expect_success 'using replay on bare repo to also rebase a 
>> contained branch' '
>> -    git -C bare replay --contained --onto main main..topic3 
>> >result-bare &&
>> +    git -C bare replay --ref-action=print --contained --onto main 
>> main..topic3 >result-bare &&
>>       test_cmp expect result-bare
>>   '
>>     test_expect_success 'using replay to rebase multiple divergent 
>> branches' '
>> -    git replay --onto main ^topic1 topic2 topic4 >result &&
>> +    git replay --ref-action=print --onto main ^topic1 topic2 topic4 
>> >result &&
>>         test_line_count = 2 result &&
>>       cut -f 3 -d " " result >new-branch-tips &&
>> @@ -168,7 +168,7 @@ test_expect_success 'using replay to rebase 
>> multiple divergent branches' '
>>   '
>>     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 &&
>> @@ -217,4 +217,32 @@ test_expect_success 
>> 'merge.directoryRenames=false' '
>>           --onto rename-onto rename-onto..rename-from
>>   '
>>   +test_expect_success 'default atomic behavior updates refs directly' '
>> +    # Store original state for cleanup
>> +    test_when_finished "git branch -f topic2 topic1" &&
>> +
>> +    # Test default atomic behavior (no output, refs updated)
>> +    git replay --onto main topic1..topic2 >output &&
>> +    test_must_be_empty output &&
>> +
>> +    # Verify ref was updated
>> +    git log --format=%s topic2 >actual &&
>> +    test_write_lines E D M L B A >expect &&
>> +    test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'atomic behavior in bare repository' '
>> +    # Test atomic updates work in bare repo
>> +    git -C bare replay --onto main topic1..topic2 >output &&
>> +    test_must_be_empty output &&
>> +
>> +    # Verify ref was updated in bare repo
>> +    git -C bare log --format=%s topic2 >actual &&
>> +    test_write_lines E D M L B A >expect &&
>> +    test_cmp expect actual &&
>> +
>> +    # Reset for other tests
>> +    git -C bare update-ref refs/heads/topic2 $(git -C bare rev-parse 
>> topic1)
>> +'
>> +
>>   test_done
>