From 59a5a61ec211d6425e40fcdbf929ffcf1f7510ac Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 10 Jan 2026 21:58:58 -0800 Subject: [PATCH 1/2] SQUASH ME: Fixups This includes several fixes I highlighted in my review and needs to be split up and squashed into the relevant previous patches. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- Documentation/git-history.adoc | 2 +- builtin/history.c | 16 ++++------------ builtin/replay.c | 10 ++-------- replay.c | 10 ++++------ replay.h | 21 +-------------------- 5 files changed, 12 insertions(+), 47 deletions(-) diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc index 4eea317e5c..457a1314a5 100644 --- a/Documentation/git-history.adoc +++ b/Documentation/git-history.adoc @@ -63,7 +63,7 @@ OPTIONS `--ref-action=(branches|head|print)`:: Control which references will be updated by the command, if any. With `branches`, all local branches that point to commits which are - decendants of the original commit will be rewritten. With `head`, only + descendants of the original commit will be rewritten. With `head`, only the current `HEAD` reference will be rewritten. With `print`, all updates as they would be performed with `branches` are printed in a format that can be consumed by linkgit:git-update-ref[1]. diff --git a/builtin/history.c b/builtin/history.c index 28db6fc5b9..60c5b5c5c7 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -178,9 +178,7 @@ static int handle_reference_updates(enum ref_action action, { const struct name_decoration *decoration; struct replay_revisions_options opts = { 0 }; - struct replay_result result = { - .final_oid = rewritten->object.oid, - }; + struct replay_result result = { 0 }; struct ref_transaction *transaction = NULL; struct strvec args = STRVEC_INIT; struct strbuf err = STRBUF_INIT; @@ -233,7 +231,7 @@ static int handle_reference_updates(enum ref_action action, goto out; } - strvec_push(&args, oid_to_hex(&head->object.oid)); + strvec_push(&args, "HEAD"); } else { strvec_push(&args, "--branches"); } @@ -244,13 +242,14 @@ static int handle_reference_updates(enum ref_action action, opts.onto = oid_to_hex_r(hex, &rewritten->object.oid); - ret = replay_revisions(repo, &revs, &opts, &result); + ret = replay_revisions(&revs, &opts, &result); if (ret) goto out; switch (action) { case REF_ACTION_DEFAULT: case REF_ACTION_BRANCHES: + case REF_ACTION_HEAD: transaction = ref_store_transaction_begin(get_main_ref_store(repo), 0, &err); if (!transaction) { ret = error(_("failed to begin ref transaction: %s"), err.buf); @@ -300,13 +299,6 @@ static int handle_reference_updates(enum ref_action action, } break; - case REF_ACTION_HEAD: - ret = refs_update_ref(get_main_ref_store(repo), reflog_msg, "HEAD", - &result.final_oid, &head->object.oid, 0, - UPDATE_REFS_MSG_ON_ERR); - if (ret) - goto out; - break; case REF_ACTION_PRINT: for (size_t i = 0; i < result.updates_nr; i++) printf("update %s %s %s\n", diff --git a/builtin/replay.c b/builtin/replay.c index da8b7202f6..f66e1a1fcd 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -167,7 +167,7 @@ int cmd_replay(int argc, revs.simplify_history = 0; } - ret = replay_revisions(repo, &revs, &opts, &result); + ret = replay_revisions(&revs, &opts, &result); if (ret) goto cleanup; @@ -220,11 +220,5 @@ cleanup: strbuf_release(&reflog_msg); release_revisions(&revs); - if (ret) { - if (result.merge_conflict) - return 1; - return 128; - } - - return 0; + return ret; } diff --git a/replay.c b/replay.c index 74e45ed27a..ea300e3c36 100644 --- a/replay.c +++ b/replay.c @@ -5,11 +5,11 @@ #include "hex.h" #include "merge-ort.h" #include "object-name.h" -#include "oidset.h" #include "parse-options.h" #include "refs.h" #include "replay.h" #include "revision.h" +#include "strmap.h" #include "tree.h" static const char *short_commit_name(struct repository *repo, @@ -256,7 +256,7 @@ static void replay_result_queue_update(struct replay_result *result, result->updates_nr++; } -int replay_revisions(struct repository *repo, struct rev_info *revs, +int replay_revisions(struct rev_info *revs, struct replay_revisions_options *opts, struct replay_result *out) { @@ -265,6 +265,7 @@ int replay_revisions(struct repository *repo, struct rev_info *revs, struct commit *last_commit = NULL; struct commit *commit; struct commit *onto = NULL; + struct repository *repo = revs->repo; struct merge_options merge_opt; struct merge_result result = { .clean = 1, @@ -328,8 +329,7 @@ int replay_revisions(struct repository *repo, struct rev_info *revs, } if (!result.clean) { - out->merge_conflict = true; - ret = -1; + ret = 1; goto out; } @@ -339,8 +339,6 @@ int replay_revisions(struct repository *repo, struct rev_info *revs, &onto->object.oid, &last_commit->object.oid); - out->final_oid = last_commit->object.oid; - ret = 0; out: diff --git a/replay.h b/replay.h index f8f9889112..9b31c85dd9 100644 --- a/replay.h +++ b/replay.h @@ -43,25 +43,6 @@ struct replay_result { struct object_id new_oid; } *updates; size_t updates_nr, updates_alloc; - - /* Set to true in case the replay failed with a merge conflict. */ - bool merge_conflict; - - /* - * The final object ID that was rewritten. Note that this field has - * somewhat special semantics and may or may not be what you want: - * - * - If no commits were rewritten it will remain uninitialized. - * - * - If a thicket of branches is rewritten it is undefined in which - * order those branches will be rewritten, and thus the final object - * ID may point to a different commit than you'd expect. - * - * That being said, this field can still be useful when you know that - * you only replay a single strand of commits. In that case, the final - * commit will point to the tip of the rewritten strand of commits. - */ - struct object_id final_oid; }; void replay_result_release(struct replay_result *result); @@ -73,7 +54,7 @@ void replay_result_release(struct replay_result *result); * * Returns 0 on success, a negative error code otherwise. */ -int replay_revisions(struct repository *repo, struct rev_info *revs, +int replay_revisions(struct rev_info *revs, struct replay_revisions_options *opts, struct replay_result *out); From 555dea73fda807778bbe14ad015339a9d7445906 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 10 Jan 2026 21:58:59 -0800 Subject: [PATCH 2/2] history: fix detached HEAD handling The default behavior for history is to work on all local branches. When HEAD is detached, it should be treated like a local branch as well. The primary fix for this is just to make sure that in addition to passing --branches to the revision machinery that we pass HEAD as well. However, that doesn't quite do the trick, because we also process the "decorations" that point at commits that we have processed, and we do this in two places -- in replay_revisions() as we replay commits, and in handle_reference_updates() when there are no commits to replay because the commit at the tip of the revision range was the one edited. In both cases, we previously keyed off of DECORATION_REF_LOCAL to make sure we only looked at local branches. Now, we need to also pay attention to DECORATION_REF_HEAD. However, in order to avoid doing two updates to the same branch (which will the ref transaction framework would throw an error on), we need to only pay attention to DECORATION_REF_HEAD when we have a detached HEAD. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/history.c | 15 +++++++++++-- replay.c | 18 ++++++++++++--- t/t3451-history-reword.sh | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/builtin/history.c b/builtin/history.c index 60c5b5c5c7..dd0df89c94 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -183,10 +183,18 @@ static int handle_reference_updates(enum ref_action action, struct strvec args = STRVEC_INIT; struct strbuf err = STRBUF_INIT; struct commit *head = NULL; + char *head_ref = NULL; + bool detached_head = false; struct rev_info revs; char hex[GIT_MAX_HEXSZ + 1]; int ret; + head_ref = refs_resolve_refdup(get_main_ref_store(repo), "HEAD", + RESOLVE_REF_READING, NULL, NULL); + if (!strcmp(head_ref, "HEAD")) + detached_head = true; + free(head_ref); + repo_init_revisions(repo, &revs, NULL); strvec_push(&args, "ignored"); strvec_push(&args, "--reverse"); @@ -234,6 +242,7 @@ static int handle_reference_updates(enum ref_action action, strvec_push(&args, "HEAD"); } else { strvec_push(&args, "--branches"); + strvec_push(&args, "HEAD"); } setup_revisions_from_strvec(&args, &revs, NULL); @@ -278,9 +287,11 @@ static int handle_reference_updates(enum ref_action action, decoration; decoration = decoration->next) { - if (decoration->type != DECORATION_REF_LOCAL) + if ((decoration->type != DECORATION_REF_HEAD || + (action != REF_ACTION_HEAD && !detached_head)) && + (decoration->type != DECORATION_REF_LOCAL || + action == REF_ACTION_HEAD)) continue; - ret = ref_transaction_update(transaction, decoration->name, &rewritten->object.oid, diff --git a/replay.c b/replay.c index ea300e3c36..7c98b1d104 100644 --- a/replay.c +++ b/replay.c @@ -151,11 +151,20 @@ static void get_ref_information(struct repository *repo, static void set_up_replay_mode(struct repository *repo, struct rev_cmdline_info *cmd_info, const char *onto_name, + bool *detached_head, char **advance_name, struct commit **onto, struct strset **update_refs) { struct ref_info rinfo; + char *head_ref; + + *detached_head = false; + head_ref = refs_resolve_refdup(get_main_ref_store(repo), "HEAD", + RESOLVE_REF_READING, NULL, NULL); + if (!strcmp(head_ref, "HEAD")) + *detached_head = true; + free(head_ref); get_ref_information(repo, cmd_info, &rinfo); if (!rinfo.positive_refexprs) @@ -271,11 +280,12 @@ int replay_revisions(struct rev_info *revs, .clean = 1, }; char *advance; + bool detached_head; int ret; advance = xstrdup_or_null(opts->advance); - set_up_replay_mode(repo, &revs->cmdline, opts->onto, &advance, - &onto, &update_refs); + set_up_replay_mode(repo, &revs->cmdline, opts->onto, + &detached_head, &advance, &onto, &update_refs); /* FIXME: Should allow replaying commits with the first as a root commit */ @@ -317,7 +327,9 @@ int replay_revisions(struct rev_info *revs, if (!decoration) continue; while (decoration) { - if (decoration->type == DECORATION_REF_LOCAL && + if ((decoration->type == DECORATION_REF_LOCAL || + (decoration->type == DECORATION_REF_HEAD && + detached_head)) && (opts->contained || strset_contains(update_refs, decoration->name))) { replay_result_queue_update(out, decoration->name, diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh index cd5883051d..7ddbab8e67 100755 --- a/t/t3451-history-reword.sh +++ b/t/t3451-history-reword.sh @@ -77,6 +77,53 @@ test_expect_success 'can reword commit in the middle' ' ) ' +test_expect_success 'can reword commit in the middle even on detached head' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + test_commit second && + test_commit third_on_main && + git checkout --detach HEAD^ && + test_commit third_on_head && + + reword_with_message HEAD~ <<-EOF && + second reworded + EOF + + expect_log HEAD --branches --graph <<-\EOF + * third_on_head + | * third_on_main + |/ + * second reworded + * first + EOF + ) +' + +test_expect_success 'can reword the detached head' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + test_commit second && + git checkout --detach HEAD && + test_commit third && + + reword_with_message HEAD <<-EOF && + third reworded + EOF + + expect_log <<-\EOF + third reworded + second + first + EOF + ) +' + test_expect_success 'can reword root commit' ' test_when_finished "rm -rf repo" && git init repo &&