From: Phillip Wood <phillip.wood123@gmail•com>
To: Siddharth Asthana <siddharthasthana31@gmail•com>, git@vger•kernel.org
Cc: christian.couder@gmail•com, phillip.wood@dunelm•org.uk,
newren@gmail•com, gitster@pobox•com, ps@pks•im,
karthik.188@gmail•com, code@khaugsbakk•name, rybak.a.v@gmail•com,
jltobler@gmail•com, toon@iotcl•com, johncai86@gmail•com,
johannes.schindelin@gmx•de
Subject: Re: [PATCH v6 2/3] replay: make atomic ref updates the default behavior
Date: Mon, 3 Nov 2025 16:25:23 +0000 [thread overview]
Message-ID: <a7f9c31b-2342-405e-848d-08b9de837dc6@gmail.com> (raw)
In-Reply-To: <20251030191931.30837-3-siddharthasthana31@gmail.com>
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
> + 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
next prev parent reply other threads:[~2025-11-03 16:25 UTC|newest]
Thread overview: 125+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-08 4:36 [PATCH 0/2] replay: add --update-refs option Siddharth Asthana
2025-09-08 4:36 ` [PATCH 1/2] " Siddharth Asthana
2025-09-08 9:54 ` Patrick Steinhardt
2025-09-09 6:58 ` Siddharth Asthana
2025-09-09 9:00 ` Patrick Steinhardt
2025-09-09 7:32 ` Elijah Newren
2025-09-10 17:58 ` Siddharth Asthana
2025-09-08 4:36 ` [PATCH 2/2] replay: document --update-refs and --batch options Siddharth Asthana
2025-09-08 6:00 ` Christian Couder
2025-09-09 6:36 ` Siddharth Asthana
2025-09-09 7:26 ` Christian Couder
2025-09-10 20:26 ` Siddharth Asthana
2025-09-08 14:40 ` Kristoffer Haugsbakk
2025-09-09 7:06 ` Siddharth Asthana
2025-09-09 19:20 ` Andrei Rybak
2025-09-10 20:28 ` Siddharth Asthana
2025-09-08 6:07 ` [PATCH 0/2] replay: add --update-refs option Christian Couder
2025-09-09 6:36 ` Siddharth Asthana
2025-09-08 14:33 ` Kristoffer Haugsbakk
2025-09-09 7:04 ` Siddharth Asthana
2025-09-09 7:13 ` Elijah Newren
2025-09-09 7:47 ` Christian Couder
2025-09-09 9:19 ` Elijah Newren
2025-09-09 16:44 ` Junio C Hamano
2025-09-09 19:52 ` Elijah Newren
2025-09-26 23:08 ` [PATCH v2 0/1] replay: make atomic ref updates the default behavior Siddharth Asthana
2025-09-26 23:08 ` [PATCH v2 1/1] " Siddharth Asthana
2025-09-30 8:23 ` Christian Couder
2025-10-02 22:16 ` Siddharth Asthana
2025-10-03 7:30 ` Christian Couder
2025-10-02 22:55 ` Elijah Newren
2025-10-03 7:05 ` Christian Couder
2025-09-30 10:05 ` Phillip Wood
2025-10-02 10:00 ` Karthik Nayak
2025-10-02 22:20 ` Siddharth Asthana
2025-10-02 22:20 ` Siddharth Asthana
2025-10-08 14:01 ` Phillip Wood
2025-10-08 20:09 ` Siddharth Asthana
2025-10-08 20:59 ` Elijah Newren
2025-10-08 21:16 ` Siddharth Asthana
2025-10-09 9:40 ` Phillip Wood
2025-10-02 16:32 ` Elijah Newren
2025-10-02 18:27 ` Junio C Hamano
2025-10-02 23:42 ` Siddharth Asthana
2025-10-02 23:27 ` Siddharth Asthana
2025-10-03 7:59 ` Christian Couder
2025-10-08 19:59 ` Siddharth Asthana
2025-10-03 19:48 ` Elijah Newren
2025-10-03 20:32 ` Junio C Hamano
2025-10-08 20:06 ` Siddharth Asthana
2025-10-08 20:59 ` Junio C Hamano
2025-10-08 21:10 ` Siddharth Asthana
2025-10-08 21:30 ` Elijah Newren
2025-10-08 20:05 ` Siddharth Asthana
2025-10-02 17:14 ` [PATCH v2 0/1] " Kristoffer Haugsbakk
2025-10-02 23:36 ` Siddharth Asthana
2025-10-03 19:05 ` Kristoffer Haugsbakk
2025-10-08 20:02 ` Siddharth Asthana
2025-10-08 20:56 ` Elijah Newren
2025-10-08 21:16 ` Kristoffer Haugsbakk
2025-10-08 21:18 ` Siddharth Asthana
2025-10-13 18:33 ` [PATCH v3 0/3] replay: make atomic ref updates the default Siddharth Asthana
2025-10-13 18:33 ` [PATCH v3 1/3] replay: use die_for_incompatible_opt2() for option validation Siddharth Asthana
2025-10-13 18:33 ` [PATCH v3 2/3] replay: make atomic ref updates the default behavior Siddharth Asthana
2025-10-13 22:05 ` Junio C Hamano
2025-10-15 5:01 ` Siddharth Asthana
2025-10-13 18:33 ` [PATCH v3 3/3] replay: add replay.defaultAction config option Siddharth Asthana
2025-10-13 19:39 ` [PATCH v3 0/3] replay: make atomic ref updates the default Junio C Hamano
2025-10-15 4:57 ` Siddharth Asthana
2025-10-15 10:33 ` Christian Couder
2025-10-15 14:45 ` Junio C Hamano
2025-10-22 18:50 ` [PATCH v4 " Siddharth Asthana
2025-10-22 18:50 ` [PATCH v4 1/3] replay: use die_for_incompatible_opt2() for option validation Siddharth Asthana
2025-10-22 18:50 ` [PATCH v4 2/3] replay: make atomic ref updates the default behavior Siddharth Asthana
2025-10-22 21:19 ` Junio C Hamano
2025-10-28 19:03 ` Siddharth Asthana
2025-10-24 10:37 ` Christian Couder
2025-10-24 15:23 ` Junio C Hamano
2025-10-28 20:18 ` Siddharth Asthana
2025-10-28 19:39 ` Siddharth Asthana
2025-10-22 18:50 ` [PATCH v4 3/3] replay: add replay.refAction config option Siddharth Asthana
2025-10-24 11:01 ` Christian Couder
2025-10-24 15:30 ` Junio C Hamano
2025-10-28 20:08 ` Siddharth Asthana
2025-10-28 19:26 ` Siddharth Asthana
2025-10-24 13:28 ` Phillip Wood
2025-10-24 13:36 ` Phillip Wood
2025-10-28 19:47 ` Siddharth Asthana
2025-10-28 19:46 ` Siddharth Asthana
2025-10-23 18:47 ` [PATCH v4 0/3] replay: make atomic ref updates the default Junio C Hamano
2025-10-25 16:57 ` Junio C Hamano
2025-10-28 20:19 ` Siddharth Asthana
2025-10-24 9:39 ` Christian Couder
2025-10-28 21:46 ` [PATCH v5 " Siddharth Asthana
2025-10-28 21:46 ` [PATCH v5 1/3] replay: use die_for_incompatible_opt2() for option validation Siddharth Asthana
2025-10-28 21:46 ` [PATCH v5 2/3] replay: make atomic ref updates the default behavior Siddharth Asthana
2025-10-28 21:46 ` [PATCH v5 3/3] replay: add replay.refAction config option Siddharth Asthana
2025-10-29 16:19 ` Christian Couder
2025-10-29 17:00 ` Siddharth Asthana
2025-10-30 19:19 ` [PATCH v6 0/3] replay: make atomic ref updates the default Siddharth Asthana
2025-10-30 19:19 ` [PATCH v6 1/3] replay: use die_for_incompatible_opt2() for option validation Siddharth Asthana
2025-10-31 18:47 ` Elijah Newren
2025-11-05 18:39 ` Siddharth Asthana
2025-10-30 19:19 ` [PATCH v6 2/3] replay: make atomic ref updates the default behavior Siddharth Asthana
2025-10-31 18:49 ` Elijah Newren
2025-10-31 19:59 ` Junio C Hamano
2025-11-05 19:07 ` Siddharth Asthana
2025-11-03 16:25 ` Phillip Wood [this message]
2025-11-03 19:32 ` Siddharth Asthana
2025-11-04 16:15 ` Phillip Wood
2025-10-30 19:19 ` [PATCH v6 3/3] replay: add replay.refAction config option Siddharth Asthana
2025-10-31 7:08 ` Christian Couder
2025-11-05 19:03 ` Siddharth Asthana
2025-10-31 18:49 ` Elijah Newren
2025-11-05 19:10 ` Siddharth Asthana
2025-10-31 18:51 ` [PATCH v6 0/3] replay: make atomic ref updates the default Elijah Newren
2025-11-05 19:15 ` [PATCH v7 " Siddharth Asthana
2025-11-05 19:15 ` [PATCH v7 1/3] replay: use die_for_incompatible_opt2() for option validation Siddharth Asthana
2025-11-05 19:16 ` [PATCH v7 2/3] replay: make atomic ref updates the default behavior Siddharth Asthana
2025-11-05 19:16 ` [PATCH v7 3/3] replay: add replay.refAction config option Siddharth Asthana
2025-11-06 19:32 ` [PATCH v7 0/3] replay: make atomic ref updates the default Elijah Newren
2025-11-08 13:22 ` Siddharth Asthana
2025-11-08 17:11 ` Elijah Newren
2025-11-07 15:48 ` Phillip Wood
2025-11-08 13:23 ` Siddharth Asthana
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a7f9c31b-2342-405e-848d-08b9de837dc6@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=code@khaugsbakk$(echo .)name \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=jltobler@gmail$(echo .)com \
--cc=johannes.schindelin@gmx$(echo .)de \
--cc=johncai86@gmail$(echo .)com \
--cc=karthik.188@gmail$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=ps@pks$(echo .)im \
--cc=rybak.a.v@gmail$(echo .)com \
--cc=siddharthasthana31@gmail$(echo .)com \
--cc=toon@iotcl$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox