From: Siddharth Asthana <siddharthasthana31@gmail•com>
To: git@vger•kernel.org
Cc: christian.couder@gmail•com, phillip.wood123@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,
Siddharth Asthana <siddharthasthana31@gmail•com>
Subject: [PATCH v5 0/3] replay: make atomic ref updates the default
Date: Wed, 29 Oct 2025 03:16:06 +0530 [thread overview]
Message-ID: <20251028214609.10041-1-siddharthasthana31@gmail.com> (raw)
In-Reply-To: <20251022185045.29256-1-siddharthasthana31@gmail.com>
This is v5 of the git-replay atomic updates series.
This version addresses all feedback from v4 reviews. Thanks to Junio,
Christian, and Phillip for the detailed technical reviews that helped
refine the implementation to Git standards.
## Changes in v5
**Added enum trailing comma**
Per Junio's suggestion, added trailing comma to enum definition for
future extensibility. This follows Git's established pattern and
minimizes patch noise when adding new enum values.
**Fixed error message formatting**
Following CodingGuidelines, wrapped ref names in single quotes in
error messages:
- error(_("failed to update ref '%s': %s"), ...)
This provides better visual clarity and matches Git's error reporting
conventions throughout the codebase.
**Extracted helper functions for config parsing**
Per Christian and Junio's feedback, refactored config parsing into
clean helper functions:
- parse_ref_action_mode(): String-to-enum conversion with source context
- get_ref_action_mode(): Handles command-line vs config precedence
This eliminates code duplication and provides a single point for
validation logic, making the code more maintainable.
**Improved test suite with Git best practices**
Following Phillip and Christian's suggestions:
- Switched from grep to test_grep for better error reporting
- Used test_config for automatic config cleanup
- Improved test isolation with proper state management
- Used topic1 tag instead of $(git rev-parse) where appropriate
**Documentation improvements**
Fixed terminology and wording per Christian's feedback:
- "ergonomical" → "ergonomic"
- "configuration option" → "configuration variable"
- "By default (with `--ref-action=update`)" → "By default, or with `--ref-action=update`,"
**Reverted unnecessary style change**
Per Junio's feedback, reverted the `const char * const` → `const char *const`
spacing change. The original spacing follows the prevalent codebase style.
## Technical Implementation
The atomic ref updates leverage Git's ref transaction API:
- ref_store_transaction_begin() with default atomic behavior
- ref_transaction_update() to stage each ref update
- ref_transaction_commit() for atomic application (all succeed or all fail)
The helper functions provide clean separation of concerns:
- parse_ref_action_mode() validates strings and converts to enum
- get_ref_action_mode() implements command-line > config > default precedence
- handle_ref_update() uses type-safe enum with switch statement
The on-demand config reading via repo_config_get_string_tmp() is simpler
than the traditional repo_config() callback pattern for this single-variable
case, while maintaining proper precedence behavior.
## Testing
All tests pass:
- t3650-replay-basics.sh (20 tests pass)
- New atomic behavior tests verify direct ref updates
- Config tests verify proper precedence and error handling
- Existing pipeline tests ensure backward compatibility
CI results: https://gitlab.com/gitlab-org/git/-/pipelines/2123403204
Siddharth Asthana (3):
replay: use die_for_incompatible_opt2() for option validation
replay: make atomic ref updates the default behavior
replay: add replay.refAction config option
Documentation/config/replay.adoc | 11 +++
Documentation/git-replay.adoc | 65 +++++++++++------
builtin/replay.c | 121 +++++++++++++++++++++++++++----
t/t3650-replay-basics.sh | 91 +++++++++++++++++++++--
4 files changed, 245 insertions(+), 43 deletions(-)
create mode 100644 Documentation/config/replay.adoc
Range-diff against v4:
1: baa0cfdd4a = 1: 3e27d07d3b replay: use die_for_incompatible_opt2() for option validation
2: 3b5df166f3 ! 2: 643d9ca86a replay: make atomic ref updates the default behavior
@@ Metadata
Author: Siddharth Asthana <siddharthasthana31@gmail•com>
## Commit message ##
replay: make atomic ref updates the default behavior
[Commit message unchanged - explains problem and solution]
@@ builtin/replay.c: #include <tree.h>
+enum ref_action_mode {
+ REF_ACTION_UPDATE,
-+ REF_ACTION_PRINT
++ REF_ACTION_PRINT,
+};
@@ builtin/replay.c: int cmd_replay
- ret = error(_("failed to update ref %s: %s"),
- decoration->name, transaction_err.buf);
+ ret = error(_("failed to update ref '%s': %s"),
@@ builtin/replay.c: int cmd_replay
- ret = error(_("failed to update ref %s: %s"),
- advance_name, transaction_err.buf);
+ ret = error(_("failed to update ref '%s': %s"),
@@ Documentation/git-replay.adoc
-+ almost certainly find it more ergonomical to simply have the updating
++ almost certainly find it more ergonomic to simply have the updating
@@ Documentation/git-replay.adoc
-+The default mode can be configured via `replay.refAction` configuration option.
++The default mode can be configured via the `replay.refAction` configuration variable.
@@ Documentation/git-replay.adoc: OUTPUT
-+By default (with `--ref-action=update`), this command produces no output on
++By default, or with `--ref-action=update`, this command produces no output on
- const char * const replay_usage[] = {
-+ const char *const replay_usage[] = {
++ const char * const replay_usage[] = {
3: c35049881d ! 3: 334da71911 replay: add replay.refAction config option
@@ Metadata
Author: Siddharth Asthana <siddharthasthana31@gmail•com>
## Commit message ##
replay: add replay.refAction config option
[Commit message unchanged]
@@ builtin/replay.c: static struct commit *pick_regular_commit
return create_commit(repo, result->tree, pickme, replayed_base);
}
++static enum ref_action_mode parse_ref_action_mode(const char *mode_str, const char *source)
++{
++ if (!mode_str || !strcmp(mode_str, "update"))
++ return REF_ACTION_UPDATE;
++ if (!strcmp(mode_str, "print"))
++ return REF_ACTION_PRINT;
++ die(_("invalid %s value: '%s'"), source, mode_str);
++}
++
++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;
++}
++
@@ builtin/replay.c: int cmd_replay
die_for_incompatible_opt2(!!advance_name_opt, "--advance",
contained, "--contained");
-+ /* Set default mode from config if not specified on command line */
-+ if (!ref_action_str) {
-+ const char *config_value = NULL;
-+ if (!repo_config_get_string_tmp(repo, "replay.refAction", &config_value)) {
-+ if (!strcmp(config_value, "update"))
-+ ref_action_str = "update";
-+ else if (!strcmp(config_value, "print"))
-+ ref_action_str = "print";
-+ else
-+ die(_("invalid value for replay.refAction: '%s'"), config_value);
-+ }
-+ }
-+
-+ /* Default to update mode if still not set */
-+ 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);
@@ t/t3650-replay-basics.sh
+test_expect_success 'replay.refAction config option' '
+ START=$(git rev-parse topic2) &&
-+ test_when_finished "git branch -f topic2 $START && git config --unset replay.refAction" &&
++ test_when_finished "git branch -f topic2 $START" &&
++ test_when_finished "git config --unset replay.refAction || true" &&
+
+ git config replay.refAction print &&
+ git replay --onto main topic1..topic2 >output &&
+ test_line_count = 1 output &&
-+ grep "^update refs/heads/topic2 " output &&
++ test_grep "^update refs/heads/topic2 " output &&
+
+ git branch -f topic2 $START &&
+ git config replay.refAction update &&
+test_expect_success 'command-line --ref-action overrides config' '
+ START=$(git rev-parse topic2) &&
-+ test_when_finished "git branch -f topic2 $START && git config --unset replay.refAction" &&
++ test_when_finished "git branch -f topic2 $START" &&
+
-+ git config replay.refAction update &&
++ test_config replay.refAction update &&
+ git replay --ref-action=print --onto main topic1..topic2 >output &&
+ test_line_count = 1 output &&
-+ grep "^update refs/heads/topic2 " output
++ test_grep "^update refs/heads/topic2 " output
+'
+
+test_expect_success 'invalid replay.refAction value' '
-+ test_when_finished "git config --unset replay.refAction" &&
-+ git config replay.refAction invalid &&
++ test_config replay.refAction invalid &&
+ test_must_fail git replay --onto main topic1..topic2 2>error &&
-+ grep "invalid value for replay.refAction" error
++ test_grep "invalid.*replay.refAction.*value" error
+'
--
2.51.0
base-commit: 419c72cb8ada252b260efc38ff91fe201de7c8c3
Thanks
- Siddharth
next prev parent reply other threads:[~2025-10-28 21:46 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 ` Siddharth Asthana [this message]
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
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=20251028214609.10041-1-siddharthasthana31@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