[PATCH v5 0/3] replay: make atomic ref updates the default

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
     +'