Re: [PATCH v6 3/3] replay: add replay.refAction config option
- From
Siddharth Asthana <siddharthasthana31@gmail.com>
- Date
- Nov 5, 2025, 19:10 UTC
- Message-ID
- <3d1dcfe2-3d41-4c96-b44f-0611b11ce853@gmail.com>
- In-Reply-To
- <CABPp-BE_pAQ8f-jjv16Ts-KRTEr3Qc402qRuJKFFW6G3J9shtA@mail.gmail.com>
On 01/11/25 00:19, Elijah Newren wrote:
Show 17 quoted lines
> On Thu, Oct 30, 2025 at 12:20 PM Siddharth Asthana > <siddharthasthana31@gmail.com> wrote: >> Add a configuration option to control the default behavior of git replay >> for updating references. This allows users who prefer the traditional >> pipeline output to set it once in their config instead of passing >> --ref-action=print with every command. >> >> The config option uses string values that mirror the behavior modes: >> * replay.refAction = update (default): atomic ref updates >> * replay.refAction = print: output commands for pipeline >> >> The command-line --ref-action option always overrides the config setting, >> allowing users to temporarily change behavior for a single invocation. > The above paragraph merely states that we follow git practices with > this config options and its corresponding command line; I think we'd > need to call it out if we didn't do that, but calling out that we do > follow git conventions seems unnecessary.
Fair point. Will remove the paragraph about command-line precedence.
Show 38 quoted lines
>
>> Implementation details:
>>
>> In cmd_replay(), after parsing command-line options, we check if
>> --ref-action was provided. If not, we read the configuration using
>> repo_config_get_string_tmp(). If the config variable is set, we validate
>> the value and use it to set the ref_action_str:
>>
>> Config value Internal mode Behavior
>> ──────────────────────────────────────────────────────────────
>> "update" "update" Atomic ref updates (default)
>> "print" "print" Pipeline output
>> (not set) "update" Atomic ref updates (default)
>> (invalid) error Die with helpful message
>>
>> If an invalid value is provided, we die() immediately with an error
>> message explaining the valid options. This catches configuration errors
>> early and provides clear guidance to users.
>>
>> The command-line --ref-action option, when provided, overrides the
>> config value. This precedence allows users to set their preferred default
>> while still having per-invocation control:
>>
>> git config replay.refAction print # Set default
>> git replay --ref-action=update --onto main topic # Override once
>>
>> The config and command-line option use the same value names ('update'
>> and 'print') for consistency and clarity. This makes it immediately
>> obvious how the config maps to the command-line option, addressing
>> feedback about the relationship between configuration and command-line
>> options being clear to users.
> An implementation details section may make sense if it answers a
> "why?" question, or it explains something counter-intuitive, or it
> provides high enough level details that it makes the patch easier to
> read/follow, or it otherwise does something more than just repackage
> the patch in an alternate format. I appreciate the attempt to provide
> these, but I think they simply make the commit message longer without
> adding value.Understood. Will remove the implementation details and configuration precedence table—they're just restating what's in the code.
Show 16 quoted lines
> >> Examples: >> >> $ git config --global replay.refAction print >> $ git replay --onto main topic1..topic2 | git update-ref --stdin >> >> $ git replay --ref-action=update --onto main topic1..topic2 >> >> $ git config replay.refAction update >> $ git replay --onto main topic1..topic2 # Updates refs directly >> >> The implementation follows Git's standard configuration precedence: >> command-line options override config values, which matches user >> expectations across all Git commands. > I don't find the Examples section helpful either; it's yet another > re-iteration that we're following conventions.
Will remove the Examples section too.
Show 106 quoted lines
>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Helped-by: Elijah Newren <newren@gmail.com>
>> 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/config/replay.adoc | 11 ++++++++
>> builtin/replay.c | 39 ++++++++++++++++++--------
>> t/t3650-replay-basics.sh | 48 +++++++++++++++++++++++++++++++-
>> 3 files changed, 86 insertions(+), 12 deletions(-)
>> create mode 100644 Documentation/config/replay.adoc
>>
>> diff --git a/Documentation/config/replay.adoc b/Documentation/config/replay.adoc
>> new file mode 100644
>> index 0000000000..7d549d2f0e
>> --- /dev/null
>> +++ b/Documentation/config/replay.adoc
>> @@ -0,0 +1,11 @@
>> +replay.refAction::
>> + Specifies the default mode for handling reference updates in
>> + `git replay`. The value can be:
>> ++
>> +--
>> + * `update`: Update refs directly using an atomic transaction (default behavior).
>> + * `print`: Output update-ref commands for pipeline use.
>> +--
>> ++
>> +This setting can be overridden with the `--ref-action` command-line option.
>> +When not configured, `git replay` defaults to `update` mode.
>> diff --git a/builtin/replay.c b/builtin/replay.c
>> index 0564d4d2e7..810068f8ef 100644
>> --- a/builtin/replay.c
>> +++ b/builtin/replay.c
>> @@ -8,6 +8,7 @@
>> #include "git-compat-util.h"
>>
>> #include "builtin.h"
>> +#include "config.h"
>> #include "environment.h"
>> #include "hex.h"
>> #include "lockfile.h"
>> @@ -289,6 +290,31 @@ static struct commit *pick_regular_commit(struct repository *repo,
>> return create_commit(repo, result->tree, pickme, replayed_base);
>> }
>>
>> +static enum ref_action_mode parse_ref_action_mode(const char *ref_action, const char *source)
>> +{
>> + if (!ref_action || !strcmp(ref_action, "update"))
>> + return REF_ACTION_UPDATE;
>> + if (!strcmp(ref_action, "print"))
>> + return REF_ACTION_PRINT;
>> + die(_("invalid %s value: '%s'"), source, ref_action);
>> +}
>> +
>> +static enum ref_action_mode get_ref_action_mode(struct repository *repo, const char *ref_action_str)
>> +{
>> + const char *config_value = NULL;
>> +
>> + /* Command line option takes precedence */
>> + if (ref_action_str)
>> + return parse_ref_action_mode(ref_action_str, "--ref-action");
>> +
>> + /* Check config value */
>> + if (!repo_config_get_string_tmp(repo, "replay.refAction", &config_value))
>> + return parse_ref_action_mode(config_value, "replay.refAction");
>> +
>> + /* Default to update mode */
>> + return REF_ACTION_UPDATE;
>> +}
>> +
>> static int handle_ref_update(enum ref_action_mode mode,
>> struct ref_transaction *transaction,
>> const char *refname,
>> @@ -367,17 +393,8 @@ 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);
>> + /* Parse ref action mode from command line or config */
>> + ref_action = get_ref_action_mode(repo, ref_action_str);
>>
>> advance_name = xstrdup_or_null(advance_name_opt);
>>
>> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
>> index 123734b49f..2e90227c2f 100755
>> --- a/t/t3650-replay-basics.sh
>> +++ b/t/t3650-replay-basics.sh
>> @@ -219,7 +219,8 @@ test_expect_success 'merge.directoryRenames=false' '
>>
>> test_expect_success 'default atomic behavior updates refs directly' '
>> # Store original state for cleanup
>> - test_when_finished "git branch -f topic2 topic1" &&
>> + START=$(git rev-parse topic2) &&
>> + test_when_finished "git branch -f topic2 $START" &&
> Yes, these three lines are a good fix, but they belong in the previous patch.Right—the START/test_when_finished cleanup fixes should go in commit 2.
Show 11 quoted lines
> >> # Test default atomic behavior (no output, refs updated) >> git replay --onto main topic1..topic2 >output && >> @@ -232,6 +233,10 @@ test_expect_success 'default atomic behavior updates refs directly' ' >> ' >> >> test_expect_success 'atomic behavior in bare repository' ' >> + # Store original state for cleanup >> + START=$(git rev-parse topic2) && >> + test_when_finished "git branch -f topic2 $START" && > Yes, these three lines are good but they belong in a separate patch.
The bare repo cleanup fix should also go in commit 2.
Show 7 quoted lines
>> + >> # Test atomic updates work in bare repo >> git -C bare replay --onto main topic1..topic2 >output && >> test_must_be_empty output && >> @@ -245,4 +250,45 @@ test_expect_success 'atomic behavior in bare repository' ' >> git -C bare update-ref refs/heads/topic2 $(git -C bare rev-parse topic1) > And this line should be removed in the previous patch.
Will remove the manual reset line from commit 2.
Thanks for pointing out which fixes belong where!
Show 48 quoted lines
> >> ' >> >> +test_expect_success 'replay.refAction config option' ' >> + # Store original state >> + START=$(git rev-parse topic2) && >> + test_when_finished "git branch -f topic2 $START" && >> + >> + # Set config to print >> + test_config replay.refAction print && >> + git replay --onto main topic1..topic2 >output && >> + test_line_count = 1 output && >> + test_grep "^update refs/heads/topic2 " output && >> + >> + # Reset and test update mode >> + git branch -f topic2 $START && >> + test_config replay.refAction update && >> + 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 'command-line --ref-action overrides config' ' >> + # Store original state >> + START=$(git rev-parse topic2) && >> + test_when_finished "git branch -f topic2 $START" && >> + >> + # Set config to update but use --ref-action=print >> + test_config replay.refAction update && >> + git replay --ref-action=print --onto main topic1..topic2 >output && >> + test_line_count = 1 output && >> + test_grep "^update refs/heads/topic2 " output >> +' >> + >> +test_expect_success 'invalid replay.refAction value' ' >> + test_config replay.refAction invalid && >> + test_must_fail git replay --onto main topic1..topic2 2>error && >> + test_grep "invalid.*replay.refAction.*value" error >> +' >> + >> test_done >> -- >> 2.51.0 > Looks good otherwise.
Thanks for careful review!