From: Siddharth Asthana <siddharthasthana31@gmail•com>
To: Elijah Newren <newren@gmail•com>
Cc: git@vger•kernel.org, christian.couder@gmail•com,
phillip.wood123@gmail•com, phillip.wood@dunelm•org.uk,
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: Thu, 6 Nov 2025 00:37:46 +0530 [thread overview]
Message-ID: <f24dd910-855d-4594-9e56-b0c8586eb1f9@gmail.com> (raw)
In-Reply-To: <CABPp-BGmHegyqvN48vJO1Y9gWVDk5u2SO5_i9KMw2aoAtmNuyw@mail.gmail.com>
On 01/11/25 00:19, Elijah Newren wrote:
> On Thu, Oct 30, 2025 at 12:20 PM Siddharth Asthana
> <siddharthasthana31@gmail•com> wrote:
>> The git replay command currently outputs update commands that can be
>> piped to update-ref to achieve a rebase, e.g.
>>
>> git replay --onto main topic1..topic2 | git update-ref --stdin
>>
>> This separation had advantages for three special cases:
>> * it made testing easy (when state isn't modified from one step to
>> the next, you don't need to make temporary branches or have undo
>> commands, or try to track the changes)
>> * it provided a natural can-it-rebase-cleanly (and what would it
>> rebase to) capability without automatically updating refs, similar
>> to a --dry-run
>> * it provided a natural low-level tool for the suite of hash-object,
>> mktree, commit-tree, mktag, merge-tree, and update-ref, allowing
>> users to have another building block for experimentation and making
>> new tools
>>
>> 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 ergonomic to simply have the updating
>> of refs be the default.
>>
>> For server-side operations in particular, the pipeline architecture
>> creates process coordination overhead. Server implementations that need
>> to perform rebases atomically must maintain additional code to:
>>
>> 1. Spawn and manage a pipeline between git-replay and git-update-ref
>> 2. Coordinate stdout/stderr streams across the pipe boundary
>> 3. Handle partial failure states if the pipeline breaks mid-execution
>> 4. Parse and validate the update-ref command output
>>
>> 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
> Looks good up to here.
>
>> Implementation details:
>>
>> The atomic ref updates are implemented using Git's ref transaction API.
>> In cmd_replay(), when not in `print` mode, we initialize a transaction
>> using ref_store_transaction_begin() with the default atomic behavior.
>> As commits are replayed, ref updates are staged into the transaction
>> using ref_transaction_update(). Finally, ref_transaction_commit()
>> applies all updates atomically—either all updates succeed or none do.
>>
>> To avoid code duplication between the 'print' and 'update' modes, this
>> commit extracts a handle_ref_update() helper function. This function
>> takes the mode (as an enum) and either prints the update command or
>> stages it into the transaction. Using an enum rather than passing the
>> string around provides type safety and allows the compiler to catch
>> typos. The switch statement makes it easy to add future modes.
>>
>> The helper function signature:
>>
>> static int handle_ref_update(enum ref_action_mode mode,
>> struct ref_transaction *transaction,
>> const char *refname,
>> const struct object_id *new_oid,
>> const struct object_id *old_oid,
>> struct strbuf *err)
>>
>> The enum is defined as:
>>
>> enum ref_action_mode {
>> REF_ACTION_UPDATE,
>> REF_ACTION_PRINT
>> };
>>
>> The mode string is converted to enum immediately after parse_options()
>> to avoid string comparisons throughout the codebase and provide compiler
>> protection against typos.
> I'm not sure the implementation details section above makes sense to
> include in the commit message; it feels like it's not providing much
> high level information nor much "why" information, but just presenting
> an alternative view of the information people will find in the patch.
> Perhaps leave it out?
Make sense. Will remove the "Implementation details" section.
>
>> Test suite changes:
>>
>> All existing tests that expected command output now use
>> --ref-action=print to preserve their original behavior. This keeps
>> the tests valid while allowing them to verify that the pipeline workflow
>> still works correctly.
>>
>> New tests were added to verify:
>> - Default atomic behavior (no output, refs updated directly)
>> - Bare repository support (server-side use case)
>> - Equivalence between traditional pipeline and atomic updates
>> - Real atomicity using a lock file to verify all-or-nothing guarantee
>> - Test isolation using test_when_finished to clean up state
>>
>> The bare repository tests were fixed to rebuild their expectations
>> independently rather than comparing to previous test output, improving
>> test reliability and isolation.
> The above paragraph sounds like you are comparing to an earlier
> series, which will confuse future readers who only compare to code
> that existed before your patches.
Right—"The bare repository tests were fixed..." belongs in the cover
letter, not the commit message. Will remove it.
>
>> A following commit will add a replay.refAction configuration
>> option for users who prefer the traditional pipeline output as their
>> default behavior.
>>
>> Helped-by: Elijah Newren <newren@gmail•com>
>> Helped-by: Patrick Steinhardt <ps@pks•im>
>> Helped-by: Christian Couder <christian.couder@gmail•com>
>> Helped-by: Phillip Wood <phillip.wood123@gmail•com>
>> Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail•com>
>> ---
>> Documentation/git-replay.adoc | 65 +++++++++++++++--------
>> builtin/replay.c | 98 +++++++++++++++++++++++++++++++----
>> t/t3650-replay-basics.sh | 44 +++++++++++++---
>> 3 files changed, 167 insertions(+), 40 deletions(-)
>>
>> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
>> index 0b12bf8aa4..037b093196 100644
>> --- a/Documentation/git-replay.adoc
>> +++ b/Documentation/git-replay.adoc
>> @@ -9,15 +9,16 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
>> SYNOPSIS
>> --------
>> [verse]
>> -(EXPERIMENTAL!) 'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>...
>> +(EXPERIMENTAL!) 'git replay' ([--contained] --onto <newbase> | --advance <branch>) [--ref-action[=<mode>]] <revision-range>...
>>
>> DESCRIPTION
>> -----------
>>
>> Takes ranges of commits and replays them onto a new location. Leaves
>> -the working tree and the index untouched, and updates no references.
>> -The output of this command is meant to be used as input to
>> -`git update-ref --stdin`, which would update the relevant branches
>> +the working tree and the index untouched. By default, updates the
>> +relevant references using an atomic transaction (all refs update or
>> +none). Use `--ref-action=print` to avoid automatic ref updates and
>> +instead get update commands that can be piped to `git update-ref --stdin`
>> (see the OUTPUT section below).
>>
>> THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
>> @@ -29,18 +30,31 @@ OPTIONS
>> Starting point at which to create the new commits. May be any
>> valid commit, and not just an existing branch name.
>> +
>> -When `--onto` is specified, the update-ref command(s) in the output will
>> -update the branch(es) in the revision range to point at the new
>> -commits, similar to the way how `git rebase --update-refs` updates
>> -multiple branches in the affected range.
>> +When `--onto` is specified, the branch(es) in the revision range will be
>> +updated to point at the new commits (or update commands will be printed
>> +if `--ref-action=print` is used), similar to the way `git rebase --update-refs`
>> +updates multiple branches in the affected range.
> I'm not sure if the parenthetical comment is necessary; we tend not to
> try to document every combinatorial combination with every sentence.
Good point. Will remove the `--ref-action=print` parentheticals from
the `--onto` and `--advance` descriptions in git-replay.adoc.
> For example, in the `git rebase` manpage under the description of the
> `--strategy` flag, it says "Because git rebase replays each commit
> from the working branch on top of the <upstream> branch using the
> given strategy", which is technically incorrect if either the --onto
> or --keep-base flags are specified, but belaboring all the details at
> that location would just burden the reader and the explanations of
> --onto and --keep-base are sufficient for users to understand. I
> think we tend to just describe the option in combination with the
> default, and only mention other options if the combination is
> ambiguous or confusing. I don't think users would find anything
> ambiguous or confusing about how --ref-action=print would combine with
> these options, so I don't think it's necessary to make the description
> longer.
>
>> --advance <branch>::
>> Starting point at which to create the new commits; must be a
>> branch name.
>> +
>> -When `--advance` is specified, the update-ref command(s) in the output
>> -will update the branch passed as an argument to `--advance` to point at
>> -the new commits (in other words, this mimics a cherry-pick operation).
>> +The history is replayed on top of the <branch> and <branch> is updated to
>> +point at the tip of the resulting history (or an update command will be
>> +printed if `--ref-action=print` is used). This is different from `--onto`,
>> +which uses the target only as a starting point without updating it.
> Same comment as above about this parenthetical comment as well.
>
>> +
>> +--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 the `replay.refAction` configuration variable.
> This last sentence conflicts with the commit message; if the
> configuration option isn't added until a later commit, then this last
> sentence shouldn't be added until then either.
Absolutely right. Will move "The default mode can be configured via the
`replay.refAction` configuration variable." to commit 3.
>
>> <revision-range>::
>> Range of commits to replay. More than one <revision-range> can
>> @@ -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, or with `--ref-action=update`, this command produces no output on
>> +success, as refs are updated directly using an atomic transaction.
>> +
>> +When using `--ref-action=print`, the output is usable as input to
>> +`git update-ref --stdin`. It is of the form:
>>
>> update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
>> update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
>> @@ -81,6 +98,14 @@ To simply rebase `mybranch` onto `target`:
>>
>> ------------
>> $ git replay --onto target origin/main..mybranch
>> +------------
>> +
>> +The refs are updated atomically and no output is produced on success.
>> +
>> +To see what would be updated without actually updating:
>> +
>> +------------
>> +$ git replay --ref-action=print --onto target origin/main..mybranch
>> update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
>> ------------
>>
>> @@ -88,33 +113,29 @@ To cherry-pick the commits from mybranch onto target:
>>
>> ------------
>> $ git replay --advance target origin/main..mybranch
>> -update refs/heads/target ${NEW_target_HASH} ${OLD_target_HASH}
>> ------------
>>
>> Note that the first two examples replay the exact same commits and on
>> top of the exact same new base, they only differ in that the first
>> -provides instructions to make mybranch point at the new commits and
>> -the second provides instructions to make target point at them.
>> +updates mybranch to point at the new commits and the second updates
>> +target to point at them.
>>
>> What if you have a stack of branches, one depending upon another, and
>> you'd really like to rebase the whole set?
>>
>> ------------
>> $ git replay --contained --onto origin/main origin/main..tipbranch
>> -update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
>> -update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
>> -update refs/heads/tipbranch ${NEW_tipbranch_HASH} ${OLD_tipbranch_HASH}
>> ------------
>>
>> +All three branches (`branch1`, `branch2`, and `tipbranch`) are updated
>> +atomically.
>> +
>> When calling `git replay`, one does not need to specify a range of
>> commits to replay using the syntax `A..B`; any range expression will
>> do:
>>
>> ------------
>> $ git replay --onto origin/main ^base branch1 branch2 branch3
>> -update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
>> -update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
>> -update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
>> ------------
>>
>> This will simultaneously rebase `branch1`, `branch2`, and `branch3`,
>> diff --git a/builtin/replay.c b/builtin/replay.c
>> index b64fc72063..0564d4d2e7 100644
>> --- a/builtin/replay.c
>> +++ b/builtin/replay.c
>> @@ -20,6 +20,11 @@
>> #include <oidset.h>
>> #include <tree.h>
>>
>> +enum ref_action_mode {
>> + REF_ACTION_UPDATE,
>> + REF_ACTION_PRINT,
>> +};
>> +
>> static const char *short_commit_name(struct repository *repo,
>> struct commit *commit)
>> {
>> @@ -284,6 +289,28 @@ static struct commit *pick_regular_commit(struct repository *repo,
>> return create_commit(repo, result->tree, pickme, replayed_base);
>> }
>>
>> +static int handle_ref_update(enum ref_action_mode mode,
>> + struct ref_transaction *transaction,
>> + const char *refname,
>> + const struct object_id *new_oid,
>> + const struct object_id *old_oid,
>> + struct strbuf *err)
>> +{
>> + switch (mode) {
>> + case REF_ACTION_PRINT:
>> + printf("update %s %s %s\n",
>> + refname,
>> + oid_to_hex(new_oid),
>> + oid_to_hex(old_oid));
>> + return 0;
>> + case REF_ACTION_UPDATE:
>> + return ref_transaction_update(transaction, refname, new_oid, old_oid,
>> + NULL, NULL, 0, "git replay", err);
>> + 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" &&
> Why are you resetting topic2 back to topic1? Shouldn't it be set back
> to what it was before the test ran instead, e.g.
> START=$(git rev-parse topic2) &&
> test_when_finished "git branch -f topic2 $START" &&
> ?
Yes, you're right. Should be:
START=$(git rev-parse topic2) &&
test_when_finished "git branch -f topic2 $START" &&
>> +
>> + # 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)
> This reset happens too late to help if the earlier commands fail, and
> also resets to the wrong ref. You should instead use a
> test_when_finished block, and make sure to reset to what topic2 used
> to point to, not reset it to what topic1 points to.
Will fix both test cleanup issues using test_when_finished with the
proper START variable.
Thanks for the thorough review!
>
>
> Otherwise, the patch looks good. This is really close to being ready
> to merge; just a few minor fixups needed that I highlighted above.
next prev parent reply other threads:[~2025-11-05 19:07 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 [this message]
2025-11-03 16:25 ` Phillip Wood
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=f24dd910-855d-4594-9e56-b0c8586eb1f9@gmail.com \
--to=siddharthasthana31@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.wood123@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=ps@pks$(echo .)im \
--cc=rybak.a.v@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