* [PATCH v4 0/2] revision.c: implement --reverse=before for walks
From: Mirko Faina @ 2026-04-27 0:24 UTC (permalink / raw)
To: git
Cc: Mirko Faina, Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble
In-Reply-To: <cover.1776984666.git.mroik@delayed.space>
v4 aligns the behaviour for --reverse=before and --reverse=after on
negative numbers that are less than -1 to treat them the same as -1,
which is current behaviour (before this series).
[1/2] revision.c: implement --reverse=before for walks (Mirko Faina)
[2/2] revision.c: reduce memory usage on reverse before (Mirko Faina)
Documentation/rev-list-options.adoc | 16 +++++--
revision.c | 73 +++++++++++++++++++++++++++--
revision.h | 8 +++-
t/t4202-log.sh | 66 ++++++++++++++++++++++++++
4 files changed, 153 insertions(+), 10 deletions(-)
Range-diff against v3:
1: 4864ac46dd = 1: 4864ac46dd revision.c: implement --reverse=before for walks
2: 00489b0e52 ! 2: 7c0bab5d14 revision.c: reduce memory usage on reverse before
@@ Commit message
revision with --reverse=before and --max-count=<k>. We do this through a
simple queue. With N nodes and K as the --max-count argument, assuming K
< N, we go from a space complexity of O(N) to O(K). When it comes down
- to time complexity, the queue has an ammortized time of O(1) for pops,
- so the complexity remains O(N).
+ to time complexity, the queue has an amortized time of O(1) for pops, so
+ the complexity remains O(N).
Signed-off-by: Mirko Faina <mroik@delayed.space>
@@ revision.c: struct commit *get_revision(struct rev_info *revs)
reversed = NULL;
- while ((c = get_revision_internal(revs)))
- commit_list_insert(c, &reversed);
-+ if (revs->reverse == REVERSE_BEFORE && max_count != -1) {
++ if (revs->reverse == REVERSE_BEFORE && max_count >= 0) {
+ retrieve_with_window(revs, max_count, &reversed);
+ } else {
+ while ((c = get_revision_internal(revs)))
base-commit: e8955061076952cc5eab0300424fc48b601fe12d
--
2.54.0
^ permalink raw reply
* [PATCH v4 2/2] revision.c: reduce memory usage on reverse before
From: Mirko Faina @ 2026-04-27 0:24 UTC (permalink / raw)
To: git
Cc: Mirko Faina, Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble
In-Reply-To: <cover.1777249165.git.mroik@delayed.space>
Due to the nature of --reverse=before we have to walk all of the history
and store each non-filtered processed commit, this can be expensive on
memory for projects with a long history. When --max-count is being used
we don't really have to keep every processed commit, we can discard
older commits (as in have been processed before than the ones we're now
considering, from a chronological commit order they are the newer
commits) as we surpass the --max-count limit.
Teach get_revision() to keep only the newer commits as we walk a
revision with --reverse=before and --max-count=<k>. We do this through a
simple queue. With N nodes and K as the --max-count argument, assuming K
< N, we go from a space complexity of O(N) to O(K). When it comes down
to time complexity, the queue has an amortized time of O(1) for pops, so
the complexity remains O(N).
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
revision.c | 42 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index d581f5e38e..41c3d185c5 100644
--- a/revision.c
+++ b/revision.c
@@ -4530,6 +4530,40 @@ static struct commit *get_revision_internal(struct rev_info *revs)
return c;
}
+static void retrieve_with_window(struct rev_info *revs, int max_count,
+ struct commit_list **reversed)
+{
+ struct commit *c;
+ struct commit_list *into_queue = NULL;
+ struct commit_list *outo_queue = NULL;
+ int into_count = 0;
+ int outo_count = 0;
+
+ while ((c = get_revision_internal(revs))) {
+ commit_list_insert(c, &into_queue);
+ into_count++;
+ if (into_count + outo_count > max_count) {
+ if (!outo_count) {
+ while (into_count) {
+ c = pop_commit(&into_queue);
+ into_count--;
+ commit_list_insert(c, &outo_queue);
+ outo_count++;
+ }
+ }
+ pop_commit(&outo_queue);
+ outo_count--;
+ }
+ }
+
+ while ((c = pop_commit(&outo_queue)))
+ commit_list_insert(c, reversed);
+ while ((c = pop_commit(&into_queue)))
+ commit_list_insert(c, &outo_queue);
+ while ((c = pop_commit(&outo_queue)))
+ commit_list_insert(c, reversed);
+}
+
struct commit *get_revision(struct rev_info *revs)
{
struct commit *c;
@@ -4546,8 +4580,12 @@ struct commit *get_revision(struct rev_info *revs)
revs->max_count = -1;
reversed = NULL;
- while ((c = get_revision_internal(revs)))
- commit_list_insert(c, &reversed);
+ if (revs->reverse == REVERSE_BEFORE && max_count >= 0) {
+ retrieve_with_window(revs, max_count, &reversed);
+ } else {
+ while ((c = get_revision_internal(revs)))
+ commit_list_insert(c, &reversed);
+ }
commit_list_free(revs->commits);
revs->commits = reversed;
revs->reverse_output_stage = 1;
--
2.54.0
^ permalink raw reply related
* [PATCH v4 1/2] revision.c: implement --reverse=before for walks
From: Mirko Faina @ 2026-04-27 0:24 UTC (permalink / raw)
To: git
Cc: Mirko Faina, Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble
In-Reply-To: <cover.1777249165.git.mroik@delayed.space>
In a revision walk `--reverse` can only be applied after any commit
limiting option. This makes getting a limited amount of commits from the
tail impossible. E.g.
git log --reverse --max-count=3
Some would expect this to give back the first 3 commits of the project.
Instead it returns the last 3 but in reversed order.
Teach `get_revision()` to accpet an argument `(after|before)` from the
CLI, and apply the reversal before or after the commit limiting options
based on this argument. If no argument is provided default to the
current behaviour, applying `--reverse` after the commit limiting
options.
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Documentation/rev-list-options.adoc | 16 +++++--
revision.c | 31 ++++++++++++--
revision.h | 8 +++-
t/t4202-log.sh | 66 +++++++++++++++++++++++++++++
4 files changed, 113 insertions(+), 8 deletions(-)
diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..e97f6f2aff 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -914,10 +914,18 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5
avoid showing the commits from two parallel development track mixed
together.
-`--reverse`::
- Output the commits chosen to be shown (see 'Commit Limiting'
- section above) in reverse order. Cannot be combined with
- `--walk-reflogs`.
+`--[no-]reverse[=(after|before)]`::
+ Accepts `after` or `before`. Cannot be combined with
+ `--walk-reflogs`. If `after`, output the commits chosen to be
+ shown (see 'Commit Limiting' section above) in reverse order. If
+ `before`, reverse the commits before filtering with `Commit
+ Limiting` options. When multiple `--reverse=` options are given,
+ the final option overrides any previous options. The `--reverse`
+ option (with no specifier) behaves as `--reverse=after`, except
+ that, for historical reasons, it negates any previous reversed
+ state (so `--reverse --reverse` does nothing, nor does
+ `--reverse=before --reverse`. Note that `--reverse=before
+ --reverse --reverse` is the same as `--reverse=after`).
endif::git-shortlog[]
ifndef::git-shortlog[]
diff --git a/revision.c b/revision.c
index 599b3a66c3..d581f5e38e 100644
--- a/revision.c
+++ b/revision.c
@@ -2686,7 +2686,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
git_log_output_encoding = xstrdup("");
return argcount;
} else if (!strcmp(arg, "--reverse")) {
- revs->reverse ^= 1;
+ revs->reverse = !revs->reverse;
+ } else if (skip_prefix(arg, "--reverse=", &optarg)) {
+ if (!strcmp(optarg, "after"))
+ revs->reverse = REVERSE_AFTER;
+ else if(!strcmp(optarg, "before"))
+ revs->reverse = REVERSE_BEFORE;
+ else
+ die(_("unknown value for --reverse: %s"), optarg);
+ } else if (!strcmp(arg, "--no-reverse")) {
+ revs->reverse = NO_REVERSE;
} else if (!strcmp(arg, "--children")) {
revs->children.name = "children";
revs->limited = 1;
@@ -4525,19 +4534,35 @@ struct commit *get_revision(struct rev_info *revs)
{
struct commit *c;
struct commit_list *reversed;
+ int max_count = revs->max_count;
+
+ if (revs->reverse && !revs->reverse_output_stage) {
+ if (revs->reverse == 3) {
+ BUG("allowed values for reverse are 0, 1 and 2");
+ revs->reverse = 1;
+ }
+
+ if (revs->reverse == REVERSE_BEFORE)
+ revs->max_count = -1;
- if (revs->reverse) {
reversed = NULL;
while ((c = get_revision_internal(revs)))
commit_list_insert(c, &reversed);
commit_list_free(revs->commits);
revs->commits = reversed;
- revs->reverse = 0;
revs->reverse_output_stage = 1;
+
+ if (revs->reverse == REVERSE_BEFORE)
+ revs->max_count = max_count;
}
if (revs->reverse_output_stage) {
+ if (revs->reverse == REVERSE_BEFORE && revs->max_count == 0)
+ return NULL;
+
c = pop_commit(&revs->commits);
+ if (revs->reverse == REVERSE_BEFORE)
+ revs->max_count--;
if (revs->track_linear)
revs->linear = !!(c && c->object.flags & TRACK_LINEAR);
return c;
diff --git a/revision.h b/revision.h
index 584f1338b5..02881577dc 100644
--- a/revision.h
+++ b/revision.h
@@ -121,6 +121,12 @@ struct ref_exclusions {
struct oidset;
struct topo_walk_info;
+enum rev_reverse {
+ NO_REVERSE = 0,
+ REVERSE_AFTER = 1,
+ REVERSE_BEFORE = 2,
+};
+
struct rev_info {
/* Starting list */
struct commit_list *commits;
@@ -167,6 +173,7 @@ struct rev_info {
ignore_missing_links:1;
/* Traversal flags */
+ enum rev_reverse reverse:2;
unsigned int dense:1,
prune:1,
no_walk:1,
@@ -196,7 +203,6 @@ struct rev_info {
rewrite_parents:1,
print_parents:1,
show_decorations:1,
- reverse:1,
reverse_output_stage:1,
cherry_pick:1,
cherry_mark:1,
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 05cee9e41b..3bfe2c99b8 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '
test_cmp_graph --name-status tangle..reach
'
+cat >expect <<-\EOF
+c3f451c Merge tag 'reach'
+046b221 to remove
+EOF
+
+test_expect_success 'log --reverse --oneline --max-count=2' '
+ test_when_finished git reset --hard HEAD~1 &&
+ touch to_remove &&
+ git add to_remove &&
+ git commit -m "to remove" &&
+ git log --reverse --oneline --max-count=2 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '
+ test_when_finished git reset --hard HEAD~1 &&
+ touch to_remove &&
+ git add to_remove &&
+ git commit -m "to remove" &&
+ git log --reverse --reverse --reverse --oneline --max-count=2 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --reverse=after --oneline --max-count=2' '
+ test_when_finished git reset --hard HEAD~1 &&
+ touch to_remove &&
+ git add to_remove &&
+ git commit -m "to remove" &&
+ git log --reverse=after --oneline --max-count=2 >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<-\EOF
+3a2fdcb initial
+f7dab8e second
+EOF
+
+test_expect_success 'log --reverse=before --oneline --max-count=2' '
+ test_when_finished rm actual &&
+ git log --reverse=before --oneline --max-count=2 >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<-\EOF
+046b221 to remove
+c3f451c Merge tag 'reach'
+EOF
+
+test_expect_success 'log --reverse --reverse --oneline --max-count=2' '
+ test_when_finished git reset --hard HEAD~1 &&
+ touch to_remove &&
+ git add to_remove &&
+ git commit -m "to remove" &&
+ git log --reverse --reverse --oneline --max-count=2 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '
+ test_when_finished git reset --hard HEAD~1 &&
+ touch to_remove &&
+ git add to_remove &&
+ git commit -m "to remove" &&
+ git log --reverse --no-reverse --oneline --max-count=2 >actual &&
+ test_cmp expect actual
+'
+
cat >expect <<-\EOF
* reach
|
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] l10n: it.po: fix italian usage messages alignment
From: Jiang Xin @ 2026-04-27 2:22 UTC (permalink / raw)
To: Matteo Beniamino; +Cc: Matteo Beniamino, git, Alessandro Menti
In-Reply-To: <d7ec16d9-2707-4c1c-ac64-ac3dde3e0946@beniamino.eu>
On Sun, Apr 26, 2026 at 2:47 PM Matteo Beniamino
<m.beniamino@beniamino.eu> wrote:
>
> Thanks for your answer. The italian repository found in the TEAMS files
> hasn't received an update for more than 5 years. I've opened a PR on the
> git-l10n repo, but it doesn't pass all the checks due to many obsolete
> entries.
Your fix is unrelated to the CI issues reported in PR #918 [1]; feel
free to ignore the errors reported. I will merge your PR in the next
localization window.
[1]: https://github.com/git-l10n/git-po/pull/918
> Maybe Alessandro can shed some light on the current status of
> the italian translation.
>
> Also, notice that when creating a PR the default PR
> message tells the user that the correct way to post a patch is via the
> mailing list: I assume this behaviour is inherited by the main git repo,
> but still can be a bit misleading.
The Git l10n coordinator repository ("git-po/git-l10n") inherits the
pull request template file [2], and there is no way to disable it
without changing the default branch name. I have updated the
repository description as below to clarify:
Git l10n coordinator repository (We use the GitHub pull request
workflow. Please ignore the pull request template inherited from
the upstream git/git repository.)
[2]: https://github.com/git-l10n/git-po/blob/master/.github/PULL_REQUEST_TEMPLATE.md
--
Jiang Xin
^ permalink raw reply
* Re: Doubt on post-merge script
From: balji balaji @ 2026-04-27 3:40 UTC (permalink / raw)
To: git
In-Reply-To: <CAOZVFV9pKfDEG7ikAzb1yV8JcNzJ4sp_YXq9p1dPE2F-vKBO=Q@mail.gmail.com>
Hello,
why is post-merge not hooked when I use git merge --continue after a
merge conflict?
I was expecting it to work, Is there a proper reason why we don't have
this feature, can please elaborate the cons of having this feature?
Thank You
Regards,
R Balaji
^ permalink raw reply
* [PATCH v3 0/3] builtin/history: introduce "fixup" subcommand
From: Patrick Steinhardt @ 2026-04-27 5:53 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, D. Ben Knoble, Tian Yuchen
In-Reply-To: <20260422-b4-pks-history-fixup-v1-0-48d4484243de@pks.im>
Hi,
this short patch series introduces a new "fixup" subcommand. This
command is the first one that I felt is missing in my day to day work,
as I end up doing fixup commits quite often.
The flow is rather simple: the user stages some changes, and then they
execute `git history fixup <commit>` to amend those changes to the given
commit. As with the other subcommands, dependent branches will then be
rebased automatically.
This is the first command that may result in merge conflicts. For now we
simply abort in such cases, but there are plans to introduce first-class
conflicts into Git. So once we have them, we'll also be able to handle
such cases more gracefully. I still think that the command is useful
even without that conflict handling.
Changes in v3:
- Some more polishing of the command's description.
- Link to v2: https://patch.msgid.link/20260423-b4-pks-history-fixup-v2-0-d7571c6d36eb@pks.im
Changes in v2:
- Introduce "--empty=(keep|drop|abort)" to specify what happens with
empty commits.
- Adapt documentation a bit to hopefully clarify how changes are
backported.
- Link to v1: https://patch.msgid.link/20260422-b4-pks-history-fixup-v1-0-48d4484243de@pks.im
Thanks!
Patrick
---
Patrick Steinhardt (3):
replay: allow callers to control what happens with empty commits
builtin/history: generalize function to commit trees
builtin/history: introduce "fixup" subcommand
Documentation/git-history.adoc | 78 ++++-
builtin/history.c | 291 ++++++++++++++++--
replay.c | 29 +-
replay.h | 19 ++
t/meson.build | 1 +
t/t3453-history-fixup.sh | 680 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 1068 insertions(+), 30 deletions(-)
Range-diff versus v2:
1: 8840b18095 = 1: 81240d1959 replay: allow callers to control what happens with empty commits
2: b078354b5a = 2: 4f35bba868 builtin/history: generalize function to commit trees
3: 3d1fec55c7 ! 3: ecaded9415 builtin/history: introduce "fixup" subcommand
@@ Documentation/git-history.adoc: conflicts. This limitation is by design as histo
The following commands are available to rewrite history in different ways:
+`fixup <commit>`::
-+ Apply the currently staged changes to the specified commit. This
-+ is done by performing a three-way merge between the HEAD commit,
-+ the target commit and the tree generated from staged changes.
-+ This is using the same logic as linkgit:git-cherry-pick[1].
++ Apply the currently staged changes to the specified commit. This is
++ similar in nature to `git commit --fixup=<commit>` followed by `git
++ rebase --autosquash <commit>~`. Changes are applied to the target
++ commit by performing a three-way merge between the HEAD commit, the
++ target commit and the tree generated from staged changes.
++
+The commit message and authorship of the target commit are preserved by
+default, unless you specify `--reedit-message`.
---
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
change-id: 20260422-b4-pks-history-fixup-be27e0c4a03e
^ permalink raw reply
* [PATCH v3 1/3] replay: allow callers to control what happens with empty commits
From: Patrick Steinhardt @ 2026-04-27 5:53 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, D. Ben Knoble, Tian Yuchen
In-Reply-To: <20260427-b4-pks-history-fixup-v3-0-cb908f06264b@pks.im>
When replaying commits it may happen that some of the commits become
empty relative to their parent. Such commits are for now automatically
dropped by the replay subsystem without much control from the user.
Introduce a new enum that allows the caller to drop, keep or abort in
this case.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
replay.c | 29 ++++++++++++++++++++++++-----
replay.h | 19 +++++++++++++++++++
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/replay.c b/replay.c
index f96f1f6551..4ef8abb607 100644
--- a/replay.c
+++ b/replay.c
@@ -269,7 +269,8 @@ static struct commit *pick_regular_commit(struct repository *repo,
struct commit *onto,
struct merge_options *merge_opt,
struct merge_result *result,
- enum replay_mode mode)
+ enum replay_mode mode,
+ enum replay_empty_commit_action empty)
{
struct commit *base, *replayed_base;
struct tree *pickme_tree, *base_tree, *replayed_base_tree;
@@ -321,12 +322,25 @@ static struct commit *pick_regular_commit(struct repository *repo,
}
merge_opt->ancestor = NULL;
merge_opt->branch2 = NULL;
+
if (!result->clean)
return NULL;
- /* Drop commits that become empty */
+
+ /* Handle commits that become empty */
if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
- !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
- return replayed_base;
+ !oideq(&pickme_tree->object.oid, &base_tree->object.oid)) {
+ switch (empty) {
+ case REPLAY_EMPTY_COMMIT_DROP:
+ return replayed_base;
+ case REPLAY_EMPTY_COMMIT_KEEP:
+ break;
+ case REPLAY_EMPTY_COMMIT_ABORT:
+ result->clean = error(_("commit %s became empty after replay"),
+ oid_to_hex(&pickme->object.oid));
+ return NULL;
+ }
+ }
+
return create_commit(repo, result->tree, pickme, replayed_base, mode);
}
@@ -417,7 +431,7 @@ int replay_revisions(struct rev_info *revs,
last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
mode == REPLAY_MODE_REVERT ? last_commit : onto,
- &merge_opt, &result, mode);
+ &merge_opt, &result, mode, opts->empty);
if (!last_commit)
break;
@@ -458,6 +472,11 @@ int replay_revisions(struct rev_info *revs,
}
}
+ if (result.clean < 0) {
+ ret = -1;
+ goto out;
+ }
+
if (!result.clean) {
ret = 1;
goto out;
diff --git a/replay.h b/replay.h
index 0ab74b9805..1851a07705 100644
--- a/replay.h
+++ b/replay.h
@@ -6,6 +6,19 @@
struct repository;
struct rev_info;
+/*
+ * Controls what happens when a replayed commit becomes empty (i.e. its tree
+ * is identical to its parent's tree after the replay).
+ */
+enum replay_empty_commit_action {
+ /* Silently discard the empty commit. */
+ REPLAY_EMPTY_COMMIT_DROP,
+ /* Keep the empty commit as-is. */
+ REPLAY_EMPTY_COMMIT_KEEP,
+ /* Abort with an error. */
+ REPLAY_EMPTY_COMMIT_ABORT,
+};
+
/*
* A set of options that can be passed to `replay_revisions()`.
*/
@@ -43,6 +56,12 @@ struct replay_revisions_options {
* Requires `onto` to be set.
*/
int contained;
+
+ /*
+ * Controls what to do when a replayed commit becomes empty.
+ * Defaults to REPLAY_EMPTY_COMMIT_DROP.
+ */
+ enum replay_empty_commit_action empty;
};
/* This struct is used as an out-parameter by `replay_revisions()`. */
--
2.54.0.545.g6539524ca2.dirty
^ permalink raw reply related
* [PATCH v3 2/3] builtin/history: generalize function to commit trees
From: Patrick Steinhardt @ 2026-04-27 5:53 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, D. Ben Knoble, Tian Yuchen
In-Reply-To: <20260427-b4-pks-history-fixup-v3-0-cb908f06264b@pks.im>
The function `commit_tree_with_edited_message_ext()` can be used to
commit a tree with a specific list of parents with an edited commit
message. This function is useful outside of editing the commit message
though, as it also performs the plumbing to extract the original commit
message and strip some headers from it.
Refactor the function to receive a flags field that allows the caller to
control whether or not the commit message should be edited, or whether
it should be retained as-is. This will be used in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/history.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 9526938085..549e352c74 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -91,13 +91,18 @@ static int fill_commit_message(struct repository *repo,
return 0;
}
-static int commit_tree_with_edited_message_ext(struct repository *repo,
- const char *action,
- struct commit *commit_with_message,
- const struct commit_list *parents,
- const struct object_id *old_tree,
- const struct object_id *new_tree,
- struct commit **out)
+enum commit_tree_flags {
+ COMMIT_TREE_EDIT_MESSAGE = (1 << 0),
+};
+
+static int commit_tree_ext(struct repository *repo,
+ const char *action,
+ struct commit *commit_with_message,
+ const struct commit_list *parents,
+ const struct object_id *old_tree,
+ const struct object_id *new_tree,
+ struct commit **out,
+ enum commit_tree_flags flags)
{
const char *exclude_gpgsig[] = {
/* We reencode the message, so the encoding needs to be stripped. */
@@ -122,10 +127,14 @@ static int commit_tree_with_edited_message_ext(struct repository *repo,
original_author = xmemdupz(ptr, len);
find_commit_subject(original_message, &original_body);
- ret = fill_commit_message(repo, old_tree, new_tree,
- original_body, action, &commit_message);
- if (ret < 0)
- goto out;
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ ret = fill_commit_message(repo, old_tree, new_tree,
+ original_body, action, &commit_message);
+ if (ret < 0)
+ goto out;
+ } else {
+ strbuf_addstr(&commit_message, original_body);
+ }
original_extra_headers = read_commit_extra_headers(commit_with_message,
exclude_gpgsig);
@@ -168,8 +177,8 @@ static int commit_tree_with_edited_message(struct repository *repo,
oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
}
- return commit_tree_with_edited_message_ext(repo, action, original, original->parents,
- &parent_tree_oid, tree_oid, out);
+ return commit_tree_ext(repo, action, original, original->parents,
+ &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
}
enum ref_action {
@@ -616,9 +625,8 @@ static int split_commit(struct repository *repo,
* The first commit is constructed from the split-out tree. The base
* that shall be diffed against is the parent of the original commit.
*/
- ret = commit_tree_with_edited_message_ext(repo, "split-out", original,
- original->parents, &parent_tree_oid,
- &split_tree->object.oid, &first_commit);
+ ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
+ &split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing first commit"));
goto out;
@@ -634,9 +642,8 @@ static int split_commit(struct repository *repo,
old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid;
new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- ret = commit_tree_with_edited_message_ext(repo, "split-out", original,
- parents, old_tree_oid,
- new_tree_oid, &second_commit);
+ ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
+ new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing second commit"));
goto out;
--
2.54.0.545.g6539524ca2.dirty
^ permalink raw reply related
* [PATCH v3 3/3] builtin/history: introduce "fixup" subcommand
From: Patrick Steinhardt @ 2026-04-27 5:53 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, D. Ben Knoble, Tian Yuchen
In-Reply-To: <20260427-b4-pks-history-fixup-v3-0-cb908f06264b@pks.im>
The newly introduced git-history(1) command provides functionality to
easily edit commit history while also rebasing dependent branches. The
functionality exposed by this command is still somewhat limited though.
One common use case when editing commit history that is not yet covered
is fixing up a specific commit. Introduce a new subcommand that allows
the user to do exactly that by performing a three-way merge into the
target's commit tree, using HEAD's tree as the merge base. The flow is
thus essentially:
$ echo changes >file
$ git add file
$ git history fixup HEAD~
Like with the other commands, this will automatically rebase dependent
branches, as well. Unlike the other commands though:
- The command does not work in a bare repository as it interacts with
the index.
- The command may run into merge conflicts. If so, the command will
simply abort.
Especially the second item limits the usefulness of this command a bit.
But there are plans to introduce first-class conflicts into Git, which
will help use cases like this one.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-history.adoc | 78 ++++-
builtin/history.c | 246 ++++++++++++++-
t/meson.build | 1 +
t/t3453-history-fixup.sh | 680 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 999 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 24dc907033..2ba8121795 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -8,6 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history
SYNOPSIS
--------
[synopsis]
+git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
@@ -22,8 +23,9 @@ THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
This command is related to linkgit:git-rebase[1] in that both commands can be
used to rewrite history. There are a couple of major differences though:
-* linkgit:git-history[1] can work in a bare repository as it does not need to
- touch either the index or the worktree.
+* Most subcommands of linkgit:git-history[1] can work in a bare repository as
+ they do not need to touch either the index or the worktree. The `fixup`
+ subcommand is an exception to this, as it reads staged changes from the index.
* linkgit:git-history[1] does not execute any linkgit:githooks[5] at the
current point in time. This may change in the future.
* linkgit:git-history[1] by default updates all branches that are descendants
@@ -48,11 +50,28 @@ conflicts. This limitation is by design as history rewrites are not intended to
be stateful operations. The limitation can be lifted once (if) Git learns about
first-class conflicts.
+When using `fixup` with `--empty=drop`, dropping the root commit is not yet
+supported.
+
COMMANDS
--------
The following commands are available to rewrite history in different ways:
+`fixup <commit>`::
+ Apply the currently staged changes to the specified commit. This is
+ similar in nature to `git commit --fixup=<commit>` followed by `git
+ rebase --autosquash <commit>~`. Changes are applied to the target
+ commit by performing a three-way merge between the HEAD commit, the
+ target commit and the tree generated from staged changes.
++
+The commit message and authorship of the target commit are preserved by
+default, unless you specify `--reedit-message`.
++
+If applying the staged changes would result in a conflict, the command
+aborts with an error. All branches that are descendants of the original
+commit are updated to point to the rewritten history.
+
`reword <commit>`::
Rewrite the commit message of the specified commit. All the other
details of this commit remain unchanged. This command will spawn an
@@ -87,6 +106,31 @@ OPTIONS
objects will be written into the repository, so applying these printed
ref updates is generally safe.
+`--reedit-message`::
+ Open an editor to modify the target commit's message.
+
+`--empty=(drop|keep|abort)`::
+ Control what happens when a commit becomes empty as a result of the
+ fixup. This can happen in two situations:
++
+--
+* The fixup target itself becomes empty because the staged changes exactly
+ cancel out all changes introduced by that commit.
+
+* A descendant commit becomes empty during replay because it introduced the
+ same change that was just fixed up into an ancestor.
+--
++
+With `drop` (the default), empty commits are removed from the rewritten
+history. Descendants of a dropped target commit are replayed directly onto
+the target's parent. Note that dropping the root commit is not supported;
+see LIMITATIONS.
++
+With `keep`, empty commits are retained in the rewritten history as-is.
++
+With `abort`, the command stops with an error if any commit would become
+empty.
+
`--update-refs=(branches|head)`::
Control which references will be updated by the command, if any. With
`branches`, all local branches that point to commits which are
@@ -96,6 +140,36 @@ OPTIONS
EXAMPLES
--------
+Fixup a commit
+~~~~~~~~~~~~~~
+
+----------
+$ git log --oneline --stat
+abc1234 (HEAD -> main) third
+ third.txt | 1 +
+def5678 second
+ second.txt | 1 +
+ghi9012 first
+ first.txt | 1 +
+
+$ echo "change" >>unrelated.txt
+$ git add unrelated.txt
+$ git history fixup ghi9012
+
+$ git log --oneline --stat
+jkl3456 (HEAD -> main) third
+ third.txt | 1 +
+mno7890 second
+ second.txt | 1 +
+pqr1234 first
+ first.txt | 1 +
+ unrelated.txt | 1 +
+----------
+
+The staged addition of `unrelated.txt` has been incorporated into the `first`
+commit. All descendant commits have been replayed on top of the rewritten
+history.
+
Split a commit
~~~~~~~~~~~~~~
diff --git a/builtin/history.c b/builtin/history.c
index 549e352c74..0fc06fb204 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -10,6 +10,7 @@
#include "gettext.h"
#include "hex.h"
#include "lockfile.h"
+#include "merge-ort.h"
#include "oidmap.h"
#include "parse-options.h"
#include "path.h"
@@ -23,6 +24,8 @@
#include "unpack-trees.h"
#include "wt-status.h"
+#define GIT_HISTORY_FIXUP_USAGE \
+ N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]")
#define GIT_HISTORY_REWORD_USAGE \
N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
#define GIT_HISTORY_SPLIT_USAGE \
@@ -335,10 +338,13 @@ static int handle_reference_updates(struct rev_info *revs,
struct commit *original,
struct commit *rewritten,
const char *reflog_msg,
- int dry_run)
+ int dry_run,
+ enum replay_empty_commit_action empty)
{
const struct name_decoration *decoration;
- struct replay_revisions_options opts = { 0 };
+ struct replay_revisions_options opts = {
+ .empty = empty,
+ };
struct replay_result result = { 0 };
struct ref_transaction *transaction = NULL;
struct strbuf err = STRBUF_INIT;
@@ -434,6 +440,236 @@ static int handle_reference_updates(struct rev_info *revs,
return ret;
}
+static int commit_became_empty(struct repository *repo,
+ struct commit *original,
+ struct tree *result)
+{
+ struct commit *parent = original->parents ? original->parents->item : NULL;
+ struct object_id parent_tree_oid;
+
+ if (parent) {
+ if (repo_parse_commit(repo, parent))
+ return error(_("unable to parse parent of %s"),
+ oid_to_hex(&original->object.oid));
+
+ parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
+ } else {
+ oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
+ }
+
+ return oideq(&result->object.oid, &parent_tree_oid);
+}
+
+static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
+{
+ enum replay_empty_commit_action *value = opt->value;
+
+ BUG_ON_OPT_NEG(unset);
+
+ if (!strcmp(arg, "drop"))
+ *value = REPLAY_EMPTY_COMMIT_DROP;
+ else if (!strcmp(arg, "keep"))
+ *value = REPLAY_EMPTY_COMMIT_KEEP;
+ else if (!strcmp(arg, "abort"))
+ *value = REPLAY_EMPTY_COMMIT_ABORT;
+ else
+ die(_("unrecognized '--empty=' action '%s'; "
+ "valid values are \"drop\", \"keep\", and \"abort\"."), arg);
+
+ return 0;
+}
+
+static int cmd_history_fixup(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo)
+{
+ const char * const usage[] = {
+ GIT_HISTORY_FIXUP_USAGE,
+ NULL,
+ };
+ enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
+ enum ref_action action = REF_ACTION_DEFAULT;
+ enum commit_tree_flags flags = 0;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+ N_("control which refs should be updated"),
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
+ OPT_BIT(0, "reedit-message", &flags,
+ N_("open an editor to modify the commit message"),
+ COMMIT_TREE_EDIT_MESSAGE),
+ OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
+ N_("how to handle commits that become empty"),
+ PARSE_OPT_NONEG, parse_opt_empty),
+ OPT_END(),
+ };
+ struct merge_result merge_result = { 0 };
+ struct merge_options merge_opts = { 0 };
+ struct strbuf reflog_msg = STRBUF_INIT;
+ struct commit *head_commit, *original, *rewritten;
+ struct tree *head_tree, *original_tree, *index_tree;
+ struct rev_info revs = { 0 };
+ bool skip_commit = false;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision"));
+ goto out;
+ }
+ repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+
+ if (is_bare_repository()) {
+ ret = error(_("cannot run fixup in a bare repository"));
+ goto out;
+ }
+
+ /* Resolve the original commit, which is the one we want to fix up. */
+ original = lookup_commit_reference_by_name(argv[0]);
+ if (!original) {
+ ret = error(_("commit cannot be found: %s"), argv[0]);
+ goto out;
+ }
+
+ /*
+ * Resolve HEAD so we can use its tree as the merge base: the staged
+ * changes are expressed as a diff from HEAD's tree to the index tree.
+ */
+ head_commit = lookup_commit_reference_by_name("HEAD");
+ if (!head_commit) {
+ ret = error(_("cannot look up HEAD"));
+ goto out;
+ }
+
+ head_tree = repo_get_commit_tree(repo, head_commit);
+ if (!head_tree) {
+ ret = error(_("cannot get tree for HEAD"));
+ goto out;
+ }
+
+ if (repo_read_index(repo) < 0) {
+ ret = error(_("unable to read index"));
+ goto out;
+ }
+
+ if (!repo_index_has_changes(repo, head_tree, NULL)) {
+ ret = error(_("nothing to fixup: no staged changes"));
+ goto out;
+ }
+
+ /*
+ * Write the index as a tree object. This is the "theirs" side of the
+ * three-way merge: it is HEAD's tree with the staged changes applied.
+ */
+ index_tree = write_in_core_index_as_tree(repo, repo->index);
+ if (!index_tree) {
+ ret = error(_("unable to write index as a tree"));
+ goto out;
+ }
+
+ original_tree = repo_get_commit_tree(repo, original);
+ if (!original_tree) {
+ ret = error(_("cannot get tree for commit %s"), argv[0]);
+ goto out;
+ }
+
+ /*
+ * Perform the three-way merge to reapply changes in the index onto the
+ * target commit. This is using basically the same logic as a
+ * cherry-pick, where the base commit is our HEAD, ours is the original
+ * tree and theirs is the index tree.
+ */
+ init_basic_merge_options(&merge_opts, repo);
+ merge_opts.ancestor = "HEAD";
+ merge_opts.branch1 = argv[0];
+ merge_opts.branch2 = "staged";
+ merge_incore_nonrecursive(&merge_opts, head_tree,
+ original_tree, index_tree, &merge_result);
+
+ if (merge_result.clean < 0) {
+ ret = error(_("merge failed while applying fixup"));
+ goto out;
+ }
+
+ if (!merge_result.clean) {
+ ret = error(_("fixup would produce conflicts; aborting"));
+ goto out;
+ }
+
+ ret = commit_became_empty(repo, original, merge_result.tree);
+ if (ret < 0)
+ goto out;
+ if (ret > 0) {
+ switch (empty) {
+ case REPLAY_EMPTY_COMMIT_DROP:
+ /*
+ * Drop the target commit by replaying its descendants
+ * directly onto its parent.
+ */
+ rewritten = original->parents ? original->parents->item : NULL;
+
+ /*
+ * TODO: we don't yet have the ability to drop root
+ * commits, but there's ultimately no good reason for
+ * this restriction to exist other than a technical
+ * limitation.
+ */
+ if (!rewritten) {
+ ret = error(_("cannot drop root commit %s: "
+ "it has no parent to replay onto"),
+ argv[0]);
+ goto out;
+ }
+
+ skip_commit = true;
+ break;
+ case REPLAY_EMPTY_COMMIT_KEEP:
+ /* Proceed and record the empty commit. */
+ break;
+ case REPLAY_EMPTY_COMMIT_ABORT:
+ ret = error(_("fixup makes commit %s empty"), argv[0]);
+ goto out;
+ }
+ }
+
+ ret = setup_revwalk(repo, action, original, &revs);
+ if (ret)
+ goto out;
+
+ if (!skip_commit) {
+ ret = commit_tree_ext(repo, "fixup", original, original->parents,
+ &original_tree->object.oid, &merge_result.tree->object.oid,
+ &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing fixed-up commit"));
+ goto out;
+ }
+ }
+
+ strbuf_addf(&reflog_msg, "fixup: updating %s", argv[0]);
+
+ ret = handle_reference_updates(&revs, action, original, rewritten,
+ reflog_msg.buf, dry_run, empty);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ merge_finalize(&merge_opts, &merge_result);
+ strbuf_release(&reflog_msg);
+ release_revisions(&revs);
+ return ret;
+}
+
static int cmd_history_reword(int argc,
const char **argv,
const char *prefix,
@@ -487,7 +723,7 @@ static int cmd_history_reword(int argc,
strbuf_addf(&reflog_msg, "reword: updating %s", argv[0]);
ret = handle_reference_updates(&revs, action, original, rewritten,
- reflog_msg.buf, dry_run);
+ reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
@@ -724,7 +960,7 @@ static int cmd_history_split(int argc,
strbuf_addf(&reflog_msg, "split: updating %s", argv[0]);
ret = handle_reference_updates(&revs, action, original, rewritten,
- reflog_msg.buf, dry_run);
+ reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
@@ -745,12 +981,14 @@ int cmd_history(int argc,
struct repository *repo)
{
const char * const usage[] = {
+ GIT_HISTORY_FIXUP_USAGE,
GIT_HISTORY_REWORD_USAGE,
GIT_HISTORY_SPLIT_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
struct option options[] = {
+ OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
OPT_SUBCOMMAND("split", &fn, cmd_history_split),
OPT_END(),
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5..f502ad8ec9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -397,6 +397,7 @@ integration_tests = [
't3450-history.sh',
't3451-history-reword.sh',
't3452-history-split.sh',
+ 't3453-history-fixup.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3453-history-fixup.sh b/t/t3453-history-fixup.sh
new file mode 100755
index 0000000000..868298e248
--- /dev/null
+++ b/t/t3453-history-fixup.sh
@@ -0,0 +1,680 @@
+#!/bin/sh
+
+test_description='tests for git-history fixup subcommand'
+
+. ./test-lib.sh
+
+fixup_with_message () {
+ cat >message &&
+ write_script fake-editor.sh <<-\EOF &&
+ cp message "$1"
+ EOF
+ test_set_editor "$(pwd)"/fake-editor.sh &&
+ git history fixup --reedit-message "$@" &&
+ rm fake-editor.sh message
+}
+
+expect_changes () {
+ git log --format="%s" --numstat "$@" >actual.raw &&
+ sed '/^$/d' <actual.raw >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+test_expect_success 'errors on missing commit argument' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history fixup 2>err &&
+ test_grep "command expects a single revision" err
+ )
+'
+
+test_expect_success 'errors on too many arguments' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history fixup HEAD HEAD 2>err &&
+ test_grep "command expects a single revision" err
+ )
+'
+
+test_expect_success 'errors on unknown revision' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history fixup does-not-exist 2>err &&
+ test_grep "commit cannot be found: does-not-exist" err
+ )
+'
+
+test_expect_success 'errors when nothing is staged' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history fixup HEAD 2>err &&
+ test_grep "nothing to fixup: no staged changes" err
+ )
+'
+
+test_expect_success 'errors in a bare repository' '
+ test_when_finished "rm -rf repo repo.git" &&
+ git init repo &&
+ test_commit -C repo initial &&
+ git clone --bare repo repo.git &&
+ test_must_fail git -C repo.git history fixup HEAD 2>err &&
+ test_grep "cannot run fixup in a bare repository" err
+'
+
+test_expect_success 'errors with invalid --empty= value' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_must_fail git -C repo history fixup --empty=bogus HEAD 2>err &&
+ test_grep "unrecognized.*--empty.*bogus" err
+'
+
+test_expect_success 'can fixup the tip commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ echo content >file.txt &&
+ git add file.txt &&
+ git commit -m "add file" &&
+
+ echo fix >>file.txt &&
+ git add file.txt &&
+
+ expect_changes <<-\EOF &&
+ add file
+ 1 0 file.txt
+ initial
+ 1 0 initial.t
+ EOF
+
+ git symbolic-ref HEAD >branch-expect &&
+ git history fixup HEAD &&
+ git symbolic-ref HEAD >branch-actual &&
+ test_cmp branch-expect branch-actual &&
+
+ expect_changes <<-\EOF &&
+ add file
+ 2 0 file.txt
+ initial
+ 1 0 initial.t
+ EOF
+
+ # Verify the fix is in the tip commit tree
+ git show HEAD:file.txt >actual &&
+ printf "content\nfix\n" >expect &&
+ test_cmp expect actual &&
+
+ git reflog >reflog &&
+ test_grep "fixup: updating HEAD" reflog
+ )
+'
+
+test_expect_success 'can fixup a commit in the middle of history' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ echo content >file.txt &&
+ git add file.txt &&
+ git commit -m "add file" &&
+ test_commit third &&
+
+ echo fix >>file.txt &&
+ git add file.txt &&
+
+ expect_changes <<-\EOF &&
+ third
+ 1 0 third.t
+ add file
+ 1 0 file.txt
+ first
+ 1 0 first.t
+ EOF
+
+ git history fixup HEAD~ &&
+
+ expect_changes <<-\EOF &&
+ third
+ 1 0 third.t
+ add file
+ 2 0 file.txt
+ first
+ 1 0 first.t
+ EOF
+
+ # Verify the fix landed in the "add file" commit.
+ git show HEAD~:file.txt >actual &&
+ printf "content\nfix\n" >expect &&
+ test_cmp expect actual &&
+
+ # And verify that the replayed commit also has the change.
+ git show HEAD:file.txt >actual &&
+ printf "content\nfix\n" >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'can fixup root commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ echo initial >root.txt &&
+ git add root.txt &&
+ git commit -m "root" &&
+ test_commit second &&
+
+ expect_changes <<-\EOF &&
+ second
+ 1 0 second.t
+ root
+ 1 0 root.txt
+ EOF
+
+ echo fix >>root.txt &&
+ git add root.txt &&
+ git history fixup HEAD~ &&
+
+ expect_changes <<-\EOF &&
+ second
+ 1 0 second.t
+ root
+ 2 0 root.txt
+ EOF
+
+ git show HEAD~:root.txt >actual &&
+ printf "initial\nfix\n" >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'preserves commit message and authorship' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ echo content >file.txt &&
+ git add file.txt &&
+ git commit --author="Original <original@example.com>" -m "original message" &&
+
+ echo fix >>file.txt &&
+ git add file.txt &&
+ git history fixup HEAD &&
+
+ # Message preserved
+ git log -1 --format="%s" >actual &&
+ echo "original message" >expect &&
+ test_cmp expect actual &&
+
+ # Authorship preserved
+ git log -1 --format="%an <%ae>" >actual &&
+ echo "Original <original@example.com>" >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'updates all descendant branches by default' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+ git switch main &&
+
+ echo fix >fix.txt &&
+ git add fix.txt &&
+ git history fixup base &&
+
+ expect_changes --branches <<-\EOF &&
+ theirs
+ 1 0 theirs.t
+ ours
+ 1 0 ours.t
+ base
+ 1 0 base.t
+ 1 0 fix.txt
+ EOF
+
+ # Both branches should have the fix in the base
+ git show main~:fix.txt >actual &&
+ echo fix >expect &&
+ test_cmp expect actual &&
+ git show branch~:fix.txt >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'can fixup commit on a different branch' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch theirs &&
+ test_commit ours &&
+ git switch theirs &&
+ test_commit theirs &&
+
+ # Stage a change while on "theirs"
+ echo fix >fix.txt &&
+ git add fix.txt &&
+
+ # Ensure that "ours" does not change, as it does not contain
+ # the commit in question.
+ git rev-parse ours >ours-before &&
+ git history fixup theirs &&
+ git rev-parse ours >ours-after &&
+ test_cmp ours-before ours-after &&
+
+ git show HEAD:fix.txt >actual &&
+ echo fix >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--dry-run prints ref updates without modifying repo' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit main-tip &&
+ git switch branch &&
+ test_commit branch-tip &&
+ git switch main &&
+
+ echo fix >fix.txt &&
+ git add fix.txt &&
+
+ git refs list >refs-before &&
+ git history fixup --dry-run base >updates &&
+ git refs list >refs-after &&
+ test_cmp refs-before refs-after &&
+
+ test_grep "update refs/heads/main" updates &&
+ test_grep "update refs/heads/branch" updates &&
+
+ expect_changes --branches <<-\EOF &&
+ branch-tip
+ 1 0 branch-tip.t
+ main-tip
+ 1 0 main-tip.t
+ base
+ 1 0 base.t
+ EOF
+
+ git update-ref --stdin <updates &&
+ expect_changes --branches <<-\EOF
+ branch-tip
+ 1 0 branch-tip.t
+ main-tip
+ 1 0 main-tip.t
+ base
+ 1 0 base.t
+ 1 0 fix.txt
+ EOF
+ )
+'
+
+test_expect_success '--update-refs=head updates only HEAD' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit main-tip &&
+ git switch branch &&
+ test_commit branch-tip &&
+
+ echo fix >fix.txt &&
+ git add fix.txt &&
+
+ # Only HEAD (branch) should be updated
+ git history fixup --update-refs=head base &&
+
+ # The main branch should be unaffected.
+ expect_changes main <<-\EOF &&
+ main-tip
+ 1 0 main-tip.t
+ base
+ 1 0 base.t
+ EOF
+
+ # But the currently checked out branch should be modified.
+ expect_changes branch <<-\EOF
+ branch-tip
+ 1 0 branch-tip.t
+ base
+ 1 0 base.t
+ 1 0 fix.txt
+ EOF
+ )
+'
+
+test_expect_success '--update-refs=head refuses to rewrite commits not in HEAD ancestry' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch other &&
+ test_commit main-tip &&
+ git switch other &&
+ test_commit other-tip &&
+
+ echo fix >fix.txt &&
+ git add fix.txt &&
+
+ test_must_fail git history fixup --update-refs=head main-tip 2>err &&
+ test_grep "rewritten commit must be an ancestor of HEAD" err
+ )
+'
+
+test_expect_success 'aborts when fixup would produce conflicts' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ echo "line one" >file.txt &&
+ git add file.txt &&
+ git commit -m "first" &&
+
+ echo "line two" >file.txt &&
+ git add file.txt &&
+ git commit -m "second" &&
+
+ echo "conflicting change" >file.txt &&
+ git add file.txt &&
+
+ git refs list >refs-before &&
+ test_must_fail git history fixup HEAD~ 2>err &&
+ test_grep "fixup would produce conflicts" err &&
+ git refs list >refs-after &&
+ test_cmp refs-before refs-after
+ )
+'
+
+test_expect_success '--reedit-message opens editor for the commit message' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ echo content >file.txt &&
+ git add file.txt &&
+ git commit -m "add file" &&
+
+ echo fix >>file.txt &&
+ git add file.txt &&
+
+ fixup_with_message HEAD <<-\EOF &&
+ add file with fix
+ EOF
+
+ expect_changes --branches <<-\EOF
+ add file with fix
+ 2 0 file.txt
+ initial
+ 1 0 initial.t
+ EOF
+ )
+'
+
+test_expect_success 'retains unstaged working tree changes after fixup' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ touch a b &&
+ git add . &&
+ git commit -m "initial commit" &&
+ echo staged >a &&
+ echo unstaged >b &&
+ git add a &&
+ git history fixup HEAD &&
+
+ # b is still modified in the worktree but not staged
+ cat >expect <<-\EOF &&
+ M b
+ EOF
+ git status --porcelain --untracked-files=no >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'index is clean after fixup when target is HEAD' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit initial &&
+ echo fix >fix.txt &&
+ git add fix.txt &&
+ git history fixup HEAD &&
+
+ git status --porcelain --untracked-files=no >actual &&
+ test_must_be_empty actual
+ )
+'
+
+test_expect_success 'index is unchanged on conflict' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ echo base >file.txt &&
+ git add file.txt &&
+ git commit -m base &&
+ echo change >file.txt &&
+ git add file.txt &&
+ git commit -m change &&
+
+ echo conflict >file.txt &&
+ git add file.txt &&
+
+ git diff --cached >index-before &&
+ test_must_fail git history fixup HEAD~ &&
+ git diff --cached >index-after &&
+ test_cmp index-before index-after
+ )
+'
+
+test_expect_success '--empty=drop removes target commit and replays descendants onto its parent' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git rm second.t &&
+ git history fixup --empty=drop HEAD~ &&
+
+ expect_changes <<-\EOF &&
+ third
+ 1 0 third.t
+ first
+ 1 0 first.t
+ EOF
+ test_must_fail git show HEAD:second.t
+ )
+'
+
+test_expect_success '--empty=drop errors out when dropping the root commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit first &&
+ test_commit second &&
+
+ git rm first.t &&
+ test_must_fail git history fixup --empty=drop HEAD~ 2>err &&
+ test_grep "cannot drop root commit" err
+ )
+'
+
+test_expect_success '--empty=drop can drop the HEAD commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit first &&
+ test_commit second &&
+
+ git rm second.t &&
+ git history fixup --empty=drop HEAD &&
+
+ expect_changes <<-\EOF
+ first
+ 1 0 first.t
+ EOF
+ )
+'
+
+test_expect_success '--empty=drop drops empty replayed commits' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ touch base remove-me &&
+ git add . &&
+ git commit -m "base" &&
+ git rm remove-me &&
+ git commit -m "remove" &&
+ touch reintroduce remove-me &&
+ git add . &&
+ git commit -m "reintroduce" &&
+
+ git rm remove-me &&
+ git history fixup --empty=drop HEAD~2 &&
+
+ expect_changes <<-\EOF
+ reintroduce
+ 0 0 reintroduce
+ 0 0 remove-me
+ base
+ 0 0 base
+ EOF
+ )
+'
+
+test_expect_success '--empty=keep keeps commit when fixup target becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git rm second.t &&
+ git history fixup --empty=keep HEAD~ &&
+
+ expect_changes <<-\EOF
+ third
+ 1 0 third.t
+ second
+ first
+ 1 0 first.t
+ EOF
+ )
+'
+
+test_expect_success '--empty=keep keeps commit when replayed commit becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ touch base remove-me &&
+ git add . &&
+ git commit -m "base" &&
+ git rm remove-me &&
+ git commit -m "remove" &&
+ touch reintroduce remove-me &&
+ git add . &&
+ git commit -m "reintroduce" &&
+
+ git rm remove-me &&
+ git history fixup --empty=keep HEAD~2 &&
+
+ expect_changes <<-\EOF
+ reintroduce
+ 0 0 reintroduce
+ 0 0 remove-me
+ remove
+ base
+ 0 0 base
+ EOF
+ )
+'
+
+test_expect_success '--empty=abort errors out when fixup target becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit first &&
+ test_commit second &&
+
+ git rm first.t &&
+ test_must_fail git history fixup --empty=abort HEAD~ 2>err &&
+ test_grep "fixup makes commit.*empty" err
+ )
+'
+
+test_expect_success '--empty=abort errors out when a descendant becomes empty during replay' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+
+ touch base remove-me &&
+ git add . &&
+ git commit -m "base" &&
+ git rm remove-me &&
+ git commit -m "remove" &&
+ touch reintroduce remove-me &&
+ git add . &&
+ git commit -m "reintroduce" &&
+
+ git rm remove-me &&
+ test_must_fail git history fixup --empty=abort HEAD~2 2>err &&
+ test_grep "became empty after replay" err
+ )
+'
+
+test_done
--
2.54.0.545.g6539524ca2.dirty
^ permalink raw reply related
* for we can offer beer towers and beer faucets
From: JENNY @ 2026-04-27 6:19 UTC (permalink / raw)
To: git@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 483 bytes --]
Dear Manager；
This is Jenny from China, and our factory is Hubei proveince.
we design and produce beer towers, faucets, shanks, keg couplers and cleaning components for bars, breweries,
restaurants and beverage distributors.
If you have anything, please feel free to contact us.
Hope get your reply earlier.
Jenny
HUBEI ZHONGKE MACHINARY CO.,LTD.
电话TEL:+86- 717-4912718
传真FAX:+86-717-4403355
手机：M.P.+86-18571017709
| |
JENNY
|
|
jennyhu666@vip.163.com
|
[-- Attachment #1.2: Type: text/html, Size: 4543 bytes --]
[-- Attachment #2: 1.png --]
[-- Type: image/png, Size: 1914824 bytes --]
[-- Attachment #3: 2.png --]
[-- Type: image/png, Size: 491336 bytes --]
[-- Attachment #4: 3.png --]
[-- Type: image/png, Size: 453880 bytes --]
[-- Attachment #5: 4.png --]
[-- Type: image/png, Size: 484017 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks
From: Junio C Hamano @ 2026-04-27 6:45 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble
In-Reply-To: <4864ac46dd8ef4b704c29efc96c45f4e1412373b.1777249165.git.mroik@delayed.space>
Mirko Faina <mroik@delayed.space> writes:
> In a revision walk `--reverse` can only be applied after any commit
> limiting option. This makes getting a limited amount of commits from the
> tail impossible. E.g.
>
> git log --reverse --max-count=3
Can we rephrase "from the tail" somehow to reduce ambiguity?
Normally we generate a list of commits from newer to older, and you
are saying that it is not possible to take the oldest three commits
and show them from older to newer (i.e., in reverse). But that, to
some readers, is showing commits from the beginning end, not from
the tail end.
Perhaps "... limited number of oldest commits impossible"?
> Teach `get_revision()` to accpet an argument `(after|before)` from the
> CLI, and apply the reversal before or after the commit limiting options
> based on this argument.
I think "after" and "before" comes from "Do other things (including
count limiting) and then apply reverse after all that" and would be
very much understandable to those who know how the machinery works,
but should mere mortals need to know the machinery only to use "git
log"?
To put it another way, do you tnink experienced Git users who
haven't seen the actual implementation of revision traversal can
immediately answer this question:
Now we have --reverse=after and --reverse=before to let you take
a limited history from both ends when used with --max-count.
Which between after and before do you think corresponds to the
traditional --reverse that allowed you to only see the newest
part of the history?
I doubt that the population to answer correctly would not exceed a
half by large margin (if it is 50% then it means nobody understood
the difference correctly and they just flipped a coin).
I wonder --reverse=oldest and --reverse=newest is easier to teach
and explain? I dunno.
^ permalink raw reply
* Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks
From: Johannes Sixt @ 2026-04-27 7:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Mirko Faina
In-Reply-To: <xmqq8qa852b5.fsf@gitster.g>
Am 27.04.26 um 08:45 schrieb Junio C Hamano:
> I think "after" and "before" comes from "Do other things (including
> count limiting) and then apply reverse after all that" and would be
> very much understandable to those who know how the machinery works,
> but should mere mortals need to know the machinery only to use "git
> log"?
I fully share your sentiments regarding "after" and "before" being too
much tied to the machinery, but...
> I wonder --reverse=oldest and --reverse=newest is easier to teach
> and explain? I dunno.
What does it mean to "revert the oldest"? Or "the newest"? If at all,
then this "newest" and "oldest" must be a restriction that applies to
--max-count in some way. Perhaps we need a --max-count-oldest option,
then --reverse does not have to be touched at all, because it is still
applied only after the set of commits to show has been determined.
-- Hannes
^ permalink raw reply
* Re: [PATCH] alias: restore support for simple dotted aliases
From: Jonatan Holmgren @ 2026-04-27 8:36 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, rsch, michael.grossfeld
In-Reply-To: <20260426230125.GA218434@coredump.intra.peff.net>
Sorry, that wasn't a "hey we should deprecate this" code-wise, I was
asking from a documentation point of view, i.e. was curious how you felt
about what is "advisable". Shouldn't've included that in my email
^ permalink raw reply
* Re: [PATCH v2 6/9] update-ref: handle rejections while adding updates
From: Karthik Nayak @ 2026-04-27 8:47 UTC (permalink / raw)
To: Toon Claes, git; +Cc: gitster, ps
In-Reply-To: <87340keh6h.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
[-- Attachment #1: Type: text/plain, Size: 6488 bytes --]
Toon Claes <toon@iotcl.com> writes:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> When using git-update-ref(1) with the '--batch-updates' flag, updates
>> rejected by the reference backend are displayed to the user while other
>> updates are applied. This only applies during the commit phase of the
>> transaction.
>>
>> In the following commits, we'll also extend `ref_transaction_update()`
>> to reject updates before a transaction is prepared/committed. In
>> preparation, modify the code in update-ref to also handle non-generic
>> rejections from `ref_transaction_update()`. This involves propagating
>> information to each of the commands on whether updates are allowed to be
>> rejected, and also checking for rejections and only dying for generic
>> failures.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> builtin/update-ref.c | 100 ++++++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 71 insertions(+), 29 deletions(-)
>>
>> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
>> index 5259cc7226..99deaac6db 100644
>> --- a/builtin/update-ref.c
>> +++ b/builtin/update-ref.c
>> @@ -25,6 +25,15 @@ static unsigned int default_flags;
>> static unsigned create_reflog_flag;
>> static const char *msg;
>>
>> +struct command_options {
>> + /*
>> + * Individual updates are allowed to fail without causing
>> + * update-ref to exit. This is set when using the
>> + * '--batch-updates' flag.
>> + */
>> + bool allow_update_failures;
>> +};
>> +
>> /*
>> * Parse one whitespace- or NUL-terminated, possibly C-quoted argument
>> * and append the result to arg. Return a pointer to the terminator.
>> @@ -268,11 +277,13 @@ static void print_rejected_refs(const char *refname,
>> */
>>
>> static void parse_cmd_update(struct ref_transaction *transaction,
>> - const char *next, const char *end)
>> + const char *next, const char *end,
>> + struct command_options *opts)
>> {
>> struct strbuf err = STRBUF_INIT;
>> char *refname;
>> struct object_id new_oid, old_oid;
>> + enum ref_transaction_error tx_err;
>> int have_old;
>>
>> refname = parse_refname(&next);
>> @@ -289,12 +300,20 @@ static void parse_cmd_update(struct ref_transaction *transaction,
>> if (*next != line_termination)
>> die("update %s: extra input: %s", refname, next);
>>
>> - if (ref_transaction_update(transaction, refname,
>> - &new_oid, have_old ? &old_oid : NULL,
>> - NULL, NULL,
>> - update_flags | create_reflog_flag,
>> - msg, &err))
>> + tx_err = ref_transaction_update(transaction, refname,
>> + &new_oid, have_old ? &old_oid : NULL,
>> + NULL, NULL,
>> + update_flags | create_reflog_flag,
>> + msg, &err);
>> +
>> + if (tx_err && tx_err != REF_TRANSACTION_ERROR_GENERIC &&
>> + opts->allow_update_failures) {
>> + print_rejected_refs(refname, have_old ? &old_oid : NULL,
>> + &new_oid, NULL, NULL, tx_err, err.buf,
>> + NULL);
>> + } else if (tx_err) {
>> die("%s", err.buf);
>
> Why die() only on an ERROR_GENERIC? Is GENERIC the only error that stops
> processing of other refs? Why? Would there be more errors in the future
> that could be added to the pile of "fatal" errors like GENERIC?
>
> I would rather see something that gives a more clear indication this
> current ref is rejected. Maybe have a range in the enum:
>
> enum ref_transaction_error {
> /* Default error code */
> REF_TRANSACTION_ERROR_GENERIC = -1,
>
> /* Ref rejected error range start */
> REF_TRANSACTION_REF_REJECTED = -100,
>
> /* Ref name conflict like A vs A/B */
> REF_TRANSACTION_ERROR_NAME_CONFLICT = -101,
> /* Ref to be created already exists */
> REF_TRANSACTION_ERROR_CREATE_EXISTS = -102,
> /* ref expected but doesn't exist */
> REF_TRANSACTION_ERROR_NONEXISTENT_REF = -103,
> /* Provided old_oid or old_target of reference doesn't match actual */
> REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE = -104,
> /* Provided new_oid or new_target is invalid */
> REF_TRANSACTION_ERROR_INVALID_NEW_VALUE = -105,
> /* Expected ref to be symref, but is a regular ref */
> REF_TRANSACTION_ERROR_EXPECTED_SYMREF = -106,
> /* Cannot create ref due to case-insensitive filesystem */
> REF_TRANSACTION_ERROR_CASE_CONFLICT = -107,
> };
>
> statis inline bool ref_rejected(enum ref_transaction_error err)
> {
> return err < REF_TRANSACTION_REF_REJECTED;
> }
>
> The thing is, now you have this checking on GENERIC in two places, I'm
> worried one or the other might be forgotten in the future.
But it's the same no? We moved from using the error to using a function
which checks the same error. This function will now be used in two
places and one of them could be forgotten.
Anyways, the GENERIC error is the default error code, this is to state
that this isn't a specific recoverable error type.
>
> Now maybe this is a bit of an overkill, so feel free to reject that
> suggestion. But if you want to keep looking at GENERIC, how do you feel
> about this version:
>
> if (tx_err == REF_TRANSACTION_ERROR_GENERIC)
> die("%s", err.buf);
>
> if (tx_err && opts->allow_update_failures)
> print_rejected_refs(refname, have_old ? &old_oid : NULL,
> &new_oid, NULL, NULL, tx_err, err.buf,
> NULL);
>
> And a little line of comment saying why to die() on GENERIC wouldn't
> hurt I think.
>
This is nicer, I can modify to this :)
>> @@ -341,13 +362,21 @@ static void parse_cmd_symref_update(struct ref_transaction *transaction,
>> if (*next != line_termination)
>> die("symref-update %s: extra input: %s", refname, next);
>>
>> - if (ref_transaction_update(transaction, refname, NULL,
>> - have_old_oid ? &old_oid : NULL,
>> - new_target,
>> - have_old_oid ? NULL : old_target,
>> - update_flags | create_reflog_flag,
>> - msg, &err))
>> + tx_err = ref_transaction_update(transaction, refname, NULL,
>> + have_old_oid ? &old_oid : NULL,
>> + new_target,
>> + have_old_oid ? NULL : old_target,
>> + update_flags | create_reflog_flag,
>> + msg, &err);
>> +
>> + if (tx_err && tx_err != REF_TRANSACTION_ERROR_GENERIC &&
>> + opts->allow_update_failures) {
>> + print_rejected_refs(refname, have_old_oid ? &old_oid : NULL,
>> + NULL, have_old_oid ? NULL : old_target,
>> + new_target, tx_err, err.buf, NULL);
>> + } else if (tx_err) {
>> die("%s", err.buf);
>> + }
>
> Obviously the same suggestion could be applied here.
>
> --
> Cheers,
> Toon
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2 7/9] refs: move object parsing to the generic layer
From: Karthik Nayak @ 2026-04-27 9:32 UTC (permalink / raw)
To: Toon Claes, git; +Cc: gitster, ps
In-Reply-To: <87zf2sd0lb.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]
Toon Claes <toon@iotcl.com> writes:
[snip]
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> index 878aa7f0ed..0fb3d57de8 100644
>> --- a/builtin/receive-pack.c
>> +++ b/builtin/receive-pack.c
>> @@ -1641,8 +1641,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
>> ret = NULL; /* good */
>> }
>> strbuf_release(&err);
>> - }
>> - else {
>> + } else {
>> + enum ref_transaction_error err_type;
>
> Shall we also use `tx_err` like in builtin/update-ref.c?
>
Makes sense, let's do that.
>> diff --git a/refs.c b/refs.c
>> index efa16b739d..662a9e6f9e 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -1416,6 +1416,24 @@ enum ref_transaction_error ref_transaction_update(struct ref_transaction *transa
>> flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
>> flags |= (new_target ? REF_HAVE_NEW : 0) | (old_target ? REF_HAVE_OLD : 0);
>>
>> + if ((flags & REF_HAVE_NEW) && !new_target && !is_null_oid(new_oid) &&
>> + !(flags & REF_SKIP_OID_VERIFICATION) && !(flags & REF_LOG_ONLY)) {
>
> Compared to the version we used to have in refs/reftable-backend.c,
> you've added `!new_target`. Why is that? If I understand correctly, that
> only happens when `new_oid` is set. Wouldn't `!is_null_oid(new_oid)`
> guard for that?
>
Since `new_oid` is a pointer here, we don't know if it's NULL or not.
`is_null_oid()` doesn't validate NULL values, so the check must come
before it.
If `REF_HAVE_NEW` is set, it either means that either `new_target` is
set or `new_oid` is set. So we check that `new_target` is NULL.
The reftable check worked as there we operate on top of `struct
ref_update`, which contains `struct object_id new_oid;`, since it is not
a pointer, the value is 0 when `new_target` is set. This works with the
`is_null_oid()` check.
> --
> Cheers,
> Toon
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2 8/9] refs: add peeled object ID to the `ref_update` struct
From: Karthik Nayak @ 2026-04-27 9:33 UTC (permalink / raw)
To: Toon Claes, git; +Cc: gitster, ps
In-Reply-To: <87v7dgcnp6.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
[-- Attachment #1: Type: text/plain, Size: 5714 bytes --]
Toon Claes <toon@iotcl.com> writes:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Certain reference backend {packed, reftable}, have the ability to also
>
> Shouldn't it be:
>
> Certain reference backends {packed, reftable} have the ability to also
>
Oh yeah, thanks!
>> store the peeled object ID for a reference pointing to a tag object.
>> This has the added benefit that during retrieval of such references, we
>> also obtain the peeled object ID without having to use the ODB.
>>
>> To provide this functionality, each backend independently calls the ODB
>> to obtain the peeled OID. To move this functionality to the generic
>> layer, there must be support infrastructure to pass in a peeled OID for
>> reference updates.
>>
>> Add a `peeled` field to the `ref_update` structure and modify
>> `ref_transaction_add_update()` to receive and copy this object ID to the
>> `ref_update` structure. Finally, modify `ref_transaction_update()` to
>> peel tag objects and pass the peeled OID to
>> `ref_transaction_add_update()`.
>>
>> Update all callers of these functions with the new function parameters.
>> Callers which only add reflog updates, need to only pass in NULL, since
>> for reflogs, we don't store peeled OIDs. Reference deletions also only
>> need to pass in NULL. For others, pass along the peeled OID if
>> available.
>>
>> In a following commit, we'll modify the backends to use this peeled OID
>> instead of parsing it themselves.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> refs.c | 15 +++++++++++++--
>> refs/files-backend.c | 20 ++++++++++++--------
>> refs/refs-internal.h | 14 ++++++++++++++
>> refs/reftable-backend.c | 6 +++---
>> 4 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
>> index d103387ebf..307dcb277b 100644
>> --- a/refs/refs-internal.h
>> +++ b/refs/refs-internal.h
>> @@ -39,6 +39,13 @@ struct ref_transaction;
>> */
>> #define REF_LOG_ONLY (1 << 7)
>>
>> +/*
>> + * The reference contains a peeled object ID. This is used when the
>> + * new_oid is pointing to a tag object and the reference backend
>> + * wants to also store the peeled value for optimized retrieval.
>> + */
>> +#define REF_HAVE_PEELED (1 << 15)
>
> How did you end up picking this value?
>
> I did some grepping to figure out if it would conflict with anything:
>
> git grep -h '#define REF_' -- '*.h' '*.c' |
> awk '/0x/{n=strtonum($3);b=0;while(n>1){n/=2;b++};$3="(1 << "b")"} 1' |
> sort -t'<' -k3 -n |
> column -t
>
> (Yeah I got some help from AI to write the `awk` command)
>
> Resulting in:
>
> #define REF_EXCLUSIONS_INIT { \
> #define REF_FILTER_H
> #define REF_FILTER_INIT { \
> #define REF_FORMAT_INIT { \
> #define REF_FORMATTING_STATE_INIT { 0 }
> #define REF_NO_DEREF (1 << 0)
> #define REF_NORMAL (1u << 0)
> #define REF_STATES_INIT { \
> #define REF_STORE_ALL_CAPS (REF_STORE_READ | \
> #define REF_STORE_CREATE_ON_DISK_IS_WORKTREE (1 << 0)
> #define REF_STORE_READ (1 << 0)
> #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \
> #define REF_BRANCHES (1u << 1)
> #define REF_FORCE_CREATE_REFLOG (1 << 1)
> #define REF_STORE_WRITE (1 << 1) /* can perform update operations */
> #define REF_HAVE_NEW (1 << 2)
> #define REF_STORE_ODB (1 << 2) /* has access to object database */
> #define REF_TAGS (1u << 2)
> #define REF_HAVE_OLD (1 << 3)
> #define REF_STORE_MAIN (1 << 3)
> #define REF_DIR (1 << 4)
> #define REF_IS_PRUNING (1 << 4)
> #define REF_DELETING (1 << 5)
> #define REF_INCOMPLETE (1 << 5)
> #define REF_KNOWS_PEELED (1 << 6)
> #define REF_NEEDS_COMMIT (1 << 6)
> #define REF_LOG_ONLY (1 << 7)
> #define REF_UPDATE_VIA_HEAD (1 << 8)
> #define REF_UPDATE_VIA_HEAD (1 << 8)
> #define REF_DELETED_RMDIR (1 << 9)
> #define REF_SKIP_OID_VERIFICATION (1 << 10)
> #define REF_SKIP_REFNAME_VERIFICATION (1 << 11)
> #define REF_SKIP_CREATE_REFLOG (1 << 12)
> #define REF_LOG_USE_PROVIDED_OIDS (1 << 13)
> #define REF_LOG_VIA_SPLIT (1 << 14)
> #define REF_HAVE_PEELED (1 << 15)
>
> So I guess it makes sense to use `(1 << 15)`.
I wish I has this command, I manually went through the individual files.
I think it is messy though, hopefully something to cleanup next.
#leftoverbits
> --
> Cheers,
> Toon
Thanks for the review, appreciate it..
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [GSoC PATCH v3 0/1] graph: add indentation for commits preceded by a parentless commit
From: Pablo Sabater @ 2026-04-27 10:28 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260404092425.550346-1-pabloosabaterr@gmail.com>
When having a history with multiple root commits or commits
that act like roots (they have excluded parents), let's call
them parentless, and drawing the history near them, the
graphing engine renders the commits one below the other, seeming
that they are related:
* parentless A
* child B
* parentless B
This issue has been attempted multiple times:
https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
This happens because the engine prints left to right from the first free
column and their column of these parentless commits for the next row
becomes empty and the engine fills that gap with the next commit (child B)
seeming that parentless A and child B are related when they are not.
The actual implementation is very minimal.
This patch makes the parentless commits to be kept alive at least one more row
to avoid that, indenting the next commit to the next column and then clean
the mapping letting the indented commit to naturally collapse to the column
where the parentless commit was:
* parentless A
* child B
/
* parentless B
This is done by adding a is_placeholder flag to the columns, the parentless
commit is actually there but marked as a placeholder:
* parentless A
(A) * child B
/
* parentless B
(A) would be "parentless A" column with the placeholder flag active.
By teaching the rendering function to print a padding ' ' when meeting a
placeholder column hides them, printing the second example.
There could also be the case where there are multiple parentless commits
without the patch:
* parentless A
* parentless B
* parentless C
* child D
* parentless D
with the patch, the indentation cascades:
* parentless A
* parentless B
* parentless C
* child D
_ /
/
/
* parentless D
the _ / might look weird but that's how the collapsing rendering does it
for big gaps, this case being from the 4th column to the 0th column.
A follow-up could change the collapsing rendering for placeholders?
I haven't done it to keep it minimal, but a follow up could make it
to be straight '/'. This would make it bigger but easier for the eye to follow.
Is not worth it IMO, but opinions are welcome.
The patch also adds tests for different cases like a parentless commit
preceding multiple parents merges and the examples above.
PSA: the tests are on t4215-log-skewed-merges.sh, which is not very related,
but other graph related tests have +140 tests, and this one has less than
20 and some of them are also not very related and differ in style.
A cleanup patch before this renaming the file and style of the tests is fine?
Changes from v2:
- Removed trailing whitespace.
- Added more comments to make it more clear an reviewable.
- Changed is_root to is_parentless to follow the name at the cover letter and
commit.
- simplified cover letter and commit ascii graphs.
Pablo Sabater (1):
graph: add indentation for commits preceded by a parentless commit
graph.c | 115 ++++++++++++++++++++++++++++++--
t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
2 files changed, 233 insertions(+), 6 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
2.43.0
^ permalink raw reply
* [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
From: Pablo Sabater @ 2026-04-27 10:28 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260427102838.44867-1-pabloosabaterr@gmail.com>
When having a history with multiple root commits or commits
that act like roots (they have excluded parents), let's call
them parentless, and drawing the history near them, the
graphing engine renders the commits one below the other, seeming
that they are related.
This issue has been attempted multiple times:
https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
This happens because for these parentless commits, in the next
row the column becomes empty and the engine prints from left
to right from the first empty column, filling the gap below
these parentless commits.
Keep a parentless commit for at least one row more to avoid
having the column empty but hide it as indentation,
therefore making the next unrelated commit live in
the next column (column means even positions where edges live:
0, 2, 4), then clean that "placeholder" column and let
the unrelated commit to naturally collapse to the column
where the parentless commit was.
Add is_placeholder to the struct column to mark if a column
is acting as a placeholder for the padding.
When a column is parentless, add a column with the parentless
commit data to prevent segfaults when 'column->commit' and
mark it as a placeholder.
Teach rendering functions to print a padding ' ' instead of
an edge when a placeholder column is met.
Then, unless the next commit is also parentless (then we
need to keep cascading the indentation) clean the mapping
and columns from the placeholder to allow it to
collapse naturally.
Add tests for different cases.
before this patch:
* parentless A
* child B
* parentless B
after this patch:
* parentless A
* child B
/
* parentless B
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
graph.c | 115 ++++++++++++++++++++++++++++++--
t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
2 files changed, 233 insertions(+), 6 deletions(-)
diff --git a/graph.c b/graph.c
index 26f6fbf000..97292df998 100644
--- a/graph.c
+++ b/graph.c
@@ -60,6 +60,12 @@ struct column {
* index into column_colors.
*/
unsigned short color;
+ /*
+ * A placeholder column keeps the column of a parentless commit filled
+ * for one extra row, avoiding a next unrelated commit to be printed
+ * in the same column.
+ */
+ unsigned is_placeholder:1;
};
enum graph_state {
@@ -563,6 +569,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
i = graph->num_new_columns++;
graph->new_columns[i].commit = commit;
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+ graph->new_columns[i].is_placeholder = 0;
}
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -607,7 +614,7 @@ static void graph_update_columns(struct git_graph *graph)
{
struct commit_list *parent;
int max_new_columns;
- int i, seen_this, is_commit_in_columns;
+ int i, seen_this, is_commit_in_columns, is_parentless;
/*
* Swap graph->columns with graph->new_columns
@@ -654,6 +661,26 @@ static void graph_update_columns(struct git_graph *graph)
*/
seen_this = 0;
is_commit_in_columns = 1;
+ /*
+ * A commit is "parentless" (is a visual root that starts a new column)
+ * only if has no visible parents AND it's not a boundary commit.
+ *
+ * Boundary commits also have no visible parents, but they are
+ * NOT a visual root:
+ *
+ * 1. A boundary only appears in the output because an included commit
+ * is its child. Children are always above, and the renderer draws an
+ * edge down to the boundary from that child. Rather than starting
+ * a column like a visual root would do, it "inherits" its child
+ * column.
+ *
+ * 2. Included commit CAN'T appear below a boundary. Boundaries are
+ * ancestors of the exclusion point; if an included commit were an
+ * ancestor of the boundary it would be excluded and not rendered.
+ * Boundaries therefore always sink to the bottom.
+ */
+ is_parentless = graph->num_parents == 0 &&
+ !(graph->commit->object.flags & BOUNDARY);
for (i = 0; i <= graph->num_columns; i++) {
struct commit *col_commit;
if (i == graph->num_columns) {
@@ -688,11 +715,46 @@ static void graph_update_columns(struct git_graph *graph)
* least 2, even if it has no interesting parents.
* The current commit always takes up at least 2
* spaces.
+ *
+ * Check for the commit to seem like a root, no parents
+ * rendered and that it is not a boundary commit. If so,
+ * add a placeholder to keep that column filled for
+ * at least one row.
+ *
+ * Prevents the next commit from being inserted
+ * just below and making the graph confusing.
*/
- if (graph->num_parents == 0)
+ if (is_parentless) {
+ graph_insert_into_new_columns(graph, graph->commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (graph->num_parents == 0) {
graph->width += 2;
+ }
} else {
- graph_insert_into_new_columns(graph, col_commit, -1);
+ if (graph->columns[i].is_placeholder) {
+ /*
+ * Keep the placeholders if the next commit is
+ * parentless also, making the indentation cascade.
+ */
+ if (!seen_this && is_parentless) {
+ graph_insert_into_new_columns(graph,
+ graph->columns[i].commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (!seen_this) {
+ graph->mapping[graph->width] = -1;
+ graph->width += 2;
+ }
+ /*
+ * seen_this && is_placeholder means that this
+ * line is the one after the indented one, the
+ * placeholder is no longer needed, gets
+ * dropped and the columns collapses naturally.
+ */
+ } else {
+ graph_insert_into_new_columns(graph, col_commit, -1);
+ }
}
}
@@ -846,7 +908,10 @@ static void graph_output_padding_line(struct git_graph *graph,
* Output a padding row, that leaves all branch lines unchanged
*/
for (i = 0; i < graph->num_new_columns; i++) {
- graph_line_write_column(line, &graph->new_columns[i], '|');
+ if (graph->new_columns[i].is_placeholder)
+ graph_line_write_column(line, &graph->new_columns[i], ' ');
+ else
+ graph_line_write_column(line, &graph->new_columns[i], '|');
graph_line_addch(line, ' ');
}
}
@@ -1058,7 +1123,34 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
graph->mapping[2 * i] < i) {
graph_line_write_column(line, col, '/');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ /*
+ * When the indented commit is a merge commit,
+ * the placeholder column adds unwanted padding
+ * between the commit and its subject.
+ *
+ * * parentless commit
+ * * merge commit
+ * /|
+ * | * parent A
+ * * parent B
+ * ^^ unwanted padding
+ *
+ * Once the current commit has been seen, don't
+ * let placeholder columns to be rendered:
+ *
+ * * parentless commit
+ * * merge commit
+ * /|
+ * | * parent A
+ * * parent B
+ */
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
}
graph_line_addch(line, ' ');
}
@@ -1135,7 +1227,18 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
graph_line_write_column(line, col, '|');
graph_line_addch(line, ' ');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ /*
+ * Same placeholder handling as in
+ * graph_output_commit_line().
+ */
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
+
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
if (parent_col)
graph_line_write_column(
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..0f6f95a6b5 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,128 @@ test_expect_success 'log --graph with multiple tips' '
EOF
'
+test_expect_success 'log --graph with root commit' '
+ git checkout --orphan 8_1 && test_commit 8_A && test_commit 8_A1 &&
+ git checkout --orphan 8_2 && test_commit 8_B &&
+
+ check_graph 8_2 8_1 <<-\EOF
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph with multiple root commits' '
+ test_commit 8_B1 &&
+ git checkout --orphan 8_3 && test_commit 8_C &&
+
+ check_graph 8_3 8_2 8_1 <<-\EOF
+ * 8_C
+ * 8_B1
+ /
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph commit from a two parent merge shifted' '
+ git checkout --orphan 9_1 && test_commit 9_B &&
+ git checkout --orphan 9_2 && test_commit 9_C &&
+ git checkout 9_1 &&
+ git merge 9_2 --allow-unrelated-histories -m 9_M &&
+ git checkout --orphan 9_3 &&
+ test_commit 9_A && test_commit 9_A1 && test_commit 9_A2 &&
+
+ check_graph 9_3 9_1 <<-\EOF
+ * 9_A2
+ * 9_A1
+ * 9_A
+ * 9_M
+ /|
+ | * 9_C
+ * 9_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a three parent merge shifted' '
+ git checkout --orphan 10_1 && test_commit 10_B &&
+ git checkout --orphan 10_2 && test_commit 10_C &&
+ git checkout --orphan 10_3 && test_commit 10_D &&
+ git checkout 10_1 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 10_1 -p 10_2 -p 10_3 -m 10_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 10_4 &&
+ test_commit 10_A && test_commit 10_A1 && test_commit 10_A2 &&
+
+ check_graph 10_4 10_1 <<-\EOF
+ * 10_A2
+ * 10_A1
+ * 10_A
+ * 10_M
+ /|\
+ | | * 10_D
+ | * 10_C
+ * 10_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a four parent merge shifted' '
+ git checkout --orphan 11_1 && test_commit 11_B &&
+ git checkout --orphan 11_2 && test_commit 11_C &&
+ git checkout --orphan 11_3 && test_commit 11_D &&
+ git checkout --orphan 11_4 && test_commit 11_E &&
+ git checkout 11_1 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 11_1 -p 11_2 -p 11_3 -p 11_4 -m 11_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 11_5 &&
+ test_commit 11_A && test_commit 11_A1 && test_commit 11_A2 &&
+
+ check_graph 11_5 11_1 <<-\EOF
+ * 11_A2
+ * 11_A1
+ * 11_A
+ *-. 11_M
+ /|\ \
+ | | | * 11_E
+ | | * 11_D
+ | * 11_C
+ * 11_B
+ EOF
+'
+
+test_expect_success 'log --graph disconnected three roots cascading' '
+ git checkout --orphan 12_1 && test_commit 12_D && test_commit 12_D1 &&
+ git checkout --orphan 12_2 && test_commit 12_C &&
+ git checkout --orphan 12_3 && test_commit 12_B &&
+ git checkout --orphan 12_4 && test_commit 12_A &&
+
+ check_graph 12_4 12_3 12_2 12_1 <<-\EOF
+ * 12_A
+ * 12_B
+ * 12_C
+ * 12_D1
+ _ /
+ /
+ /
+ * 12_D
+ EOF
+'
+
+test_expect_success 'log --graph with excluded parent (not a root)' '
+ git checkout --orphan 13_1 && test_commit 13_X && test_commit 13_Y &&
+ git checkout --orphan 13_2 && test_commit 13_O && test_commit 13_A &&
+
+ check_graph 13_O..13_A 13_1 <<-\EOF
+ * 13_A
+ * 13_Y
+ /
+ * 13_X
+ EOF
+'
+
test_done
--
2.43.0
^ permalink raw reply related
* Re: [GSoC PATCH v3 0/1] graph: add indentation for commits preceded by a parentless commit
From: Pablo @ 2026-04-27 10:35 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Christian Couder, karthik nayak, jltobler,
Ayush Chandekar, Siddharth Asthana, Chandra Pratap
In-Reply-To: <20260427102838.44867-1-pabloosabaterr@gmail.com>
El lun, 27 abr 2026 a las 12:28, Pablo Sabater
(<pabloosabaterr@gmail.com>) escribió:
>
> Pablo Sabater (1):
> graph: add indentation for commits preceded by a parentless commit
>
> graph.c | 115 ++++++++++++++++++++++++++++++--
> t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
> 2 files changed, 233 insertions(+), 6 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Hi!
This patch seems to have a problem to get reviewed, I improved the comments
at the code and simplified the example graphs at the cover letter and
patch to try
to make it easier to review.
Let me know if any clarification is needed,
--
Pablo
^ permalink raw reply
* [PATCH v3 0/9] refs: move some of the generic logic out of the backends
From: Karthik Nayak @ 2026-04-27 10:42 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, toon
In-Reply-To: <20260420-refs-move-to-generic-layer-v1-0-513e354f376b@gmail.com>
This series came together while I was working on other reference related
code and realized that some of the individual logic implemented with the
reference backends can be moved to the generic layer.
Moving code to the generic layer, simplifies the responsibility of
individual backends and avoids deviation in logic between the backends.
The biggest changes are related to moving out usage of `parse_object()`
and `peel_object()` from reference transactions. The former is used to
validate that the OID provided points to a commit object. The latter is
an optimization technique where the packed/reftable backend store the
peeled OID whenever available, so reading such references provides the
peeled OID without having to call the ODB.
Moving object parsing to the generic layout involves moving it out of
the prepare stage of the transaction and into `ref_transaction_update()`
where every added update is checked. As such, this also involves
modifying update-ref(1) and receive-pack(1) to follow this paradigm.
---
Changes in v3:
- Remove an unwanted change which creeped up during a rebase.
- Add information in the commit message around how the order of errors
in git-update-ref(1) will change while maintaining functionality.
- Change up the order of an `if..else` to make it clearer.
- Other small typos and fixes.
- Link to v2: https://patch.msgid.link/20260423-refs-move-to-generic-layer-v2-0-ae5a4f146d7d@gmail.com
Changes in v2:
- Split the second commit into two: one introducing
`ref_store_init_options` and the second to use it for reflog config.
- Use opts as the variable name consistently.
- A bunch of grammar fixes.
- Link to v1: https://patch.msgid.link/20260420-refs-move-to-generic-layer-v1-0-513e354f376b@gmail.com
---
builtin/receive-pack.c | 22 ++++---
builtin/update-ref.c | 151 +++++++++++++++++++++++++++++++-----------------
refs.c | 60 ++++++++++++++-----
refs.h | 16 ++---
refs/files-backend.c | 58 +++++++------------
refs/packed-backend.c | 10 ++--
refs/packed-backend.h | 3 +-
refs/refs-internal.h | 35 +++++++++--
refs/reftable-backend.c | 40 +++----------
9 files changed, 231 insertions(+), 164 deletions(-)
Karthik Nayak (9):
refs: remove unused typedef 'ref_transaction_commit_fn'
refs: introduce `ref_store_init_options`
refs: extract out reflog config to generic layer
refs: return `ref_transaction_error` from `ref_transaction_update()`
update-ref: move `print_rejected_refs()` up
update-ref: handle rejections while adding updates
refs: move object parsing to the generic layer
refs: add peeled object ID to the `ref_update` struct
refs: use peeled tag values in reference backends
Range-diff versus v2:
1: b8ab8a6c8b = 1: 704a218bce refs: remove unused typedef 'ref_transaction_commit_fn'
2: dd419614e0 ! 2: f3e0caa8e4 refs: introduce `ref_store_init_options`
@@ refs/files-backend.c: static struct ref_store *files_ref_store_init(struct repos
refs->gitcommondir = strbuf_detach(&ref_common_dir, NULL);
refs->packed_ref_store =
- packed_ref_store_init(repo, NULL, refs->gitcommondir, flags);
-+ packed_ref_store_init(repo, payload, refs->gitcommondir, opts);
++ packed_ref_store_init(repo, NULL, refs->gitcommondir, opts);
+ refs->store_flags = opts->access_flags;
refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
+
3: 337e5c8c5b ! 3: 48ebcc2438 refs: extract out reflog config to generic layer
@@ refs.c: static struct ref_store *ref_store_init(struct repository *repo,
## refs/files-backend.c ##
@@ refs/files-backend.c: static struct ref_store *files_ref_store_init(struct repository *repo,
refs->packed_ref_store =
- packed_ref_store_init(repo, payload, refs->gitcommondir, opts);
+ packed_ref_store_init(repo, NULL, refs->gitcommondir, opts);
refs->store_flags = opts->access_flags;
- refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
+ refs->log_all_ref_updates = opts->log_all_ref_updates;
4: cdb4aad11e = 4: d4c120ab28 refs: return `ref_transaction_error` from `ref_transaction_update()`
5: 454549240a = 5: 6e3b16258c update-ref: move `print_rejected_refs()` up
6: 1f224fb868 ! 6: 505c5b8edb update-ref: handle rejections while adding updates
@@ Commit message
rejected, and also checking for rejections and only dying for generic
failures.
+ Errors encountered during updates will be shown to the user immediately
+ unlike other errors encountered only when the transaction is
+ prepared/committed. As the verification of object IDs and peeled tag
+ objects will move into `ref_transaction_update()` in the following
+ commit, this means that those errors will be shown to the user before
+ other errors, this changes the order of errors, but the functionality
+ remains the same.
+
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
## builtin/update-ref.c ##
@@ builtin/update-ref.c: static void parse_cmd_update(struct ref_transaction *trans
+ update_flags | create_reflog_flag,
+ msg, &err);
+
-+ if (tx_err && tx_err != REF_TRANSACTION_ERROR_GENERIC &&
-+ opts->allow_update_failures) {
++ /*
++ * Generic errors are non-recoverable, so we cannot skip the update
++ * or mark it as rejected.
++ */
++ if (tx_err == REF_TRANSACTION_ERROR_GENERIC)
+ die("%s", err.buf);
+
++ if (tx_err && opts->allow_update_failures)
+ print_rejected_refs(refname, have_old ? &old_oid : NULL,
+ &new_oid, NULL, NULL, tx_err, err.buf,
+ NULL);
-+ } else if (tx_err) {
- die("%s", err.buf);
-+ }
-
++
update_flags = default_flags;
free(refname);
-@@ builtin/update-ref.c: static void parse_cmd_update(struct ref_transaction *transaction,
+ strbuf_release(&err);
}
static void parse_cmd_symref_update(struct ref_transaction *transaction,
@@ builtin/update-ref.c: static void parse_cmd_symref_update(struct ref_transaction
+ update_flags | create_reflog_flag,
+ msg, &err);
+
-+ if (tx_err && tx_err != REF_TRANSACTION_ERROR_GENERIC &&
-+ opts->allow_update_failures) {
++ /*
++ * Generic errors are non-recoverable, so we cannot skip the update
++ * or mark it as rejected.
++ */
++ if (tx_err == REF_TRANSACTION_ERROR_GENERIC)
+ die("%s", err.buf);
+
++ if (tx_err && opts->allow_update_failures)
+ print_rejected_refs(refname, have_old_oid ? &old_oid : NULL,
+ NULL, have_old_oid ? NULL : old_target,
+ new_target, tx_err, err.buf, NULL);
-+ } else if (tx_err) {
- die("%s", err.buf);
-+ }
-
++
update_flags = default_flags;
free(refname);
+ free(old_arg);
@@ builtin/update-ref.c: static void parse_cmd_symref_update(struct ref_transaction *transaction,
}
7: a52b019afa ! 7: ad1aae6b35 refs: move object parsing to the generic layer
@@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh
- }
- else {
+ } else {
-+ enum ref_transaction_error err_type;
++ enum ref_transaction_error tx_err;
struct strbuf err = STRBUF_INIT;
if (shallow_update && si->shallow_ref[cmd->index] &&
update_shallow_ref(cmd, si)) {
@@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh
- NULL, NULL,
- 0, "push",
- &err)) {
-+ err_type = ref_transaction_update(transaction,
++ tx_err = ref_transaction_update(transaction,
+ namespaced_name,
+ new_oid, old_oid,
+ NULL, NULL,
+ 0, "push",
+ &err);
-+ if (err_type) {
++ if (tx_err) {
rp_error("%s", err.buf);
- ret = "failed to update ref";
-+ if (err_type == REF_TRANSACTION_ERROR_GENERIC)
++ if (tx_err == REF_TRANSACTION_ERROR_GENERIC)
+ ret = "failed to update ref";
+ else
-+ ret = ref_transaction_error_msg(err_type);
++ ret = ref_transaction_error_msg(tx_err);
} else {
ret = NULL; /* good */
}
8: aa5fd09831 ! 8: 5cb7ee9853 refs: add peeled object ID to the `ref_update` struct
@@ Metadata
## Commit message ##
refs: add peeled object ID to the `ref_update` struct
- Certain reference backend {packed, reftable}, have the ability to also
+ Certain reference backends {packed, reftable}, have the ability to also
store the peeled object ID for a reference pointing to a tag object.
This has the added benefit that during retrieval of such references, we
also obtain the peeled object ID without having to use the ODB.
9: 96146b5083 = 9: ba34e10548 refs: use peeled tag values in reference backends
base-commit: f65aba1e87db64413b6d1ed5ae5a45b5a84a0997
change-id: 20260417-refs-move-to-generic-layer-f7525c5e8764
Thanks
- Karthik
^ permalink raw reply
* [PATCH v3 1/9] refs: remove unused typedef 'ref_transaction_commit_fn'
From: Karthik Nayak @ 2026-04-27 10:42 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, toon
In-Reply-To: <20260427-refs-move-to-generic-layer-v3-0-e4638dfb7897@gmail.com>
The typedef 'ref_transaction_commit_fn' is not used anywhere in our
code, let's remove it.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs/refs-internal.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d79e35fd26..2d963cc4f4 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -421,10 +421,6 @@ typedef int ref_transaction_abort_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
-typedef int ref_transaction_commit_fn(struct ref_store *refs,
- struct ref_transaction *transaction,
- struct strbuf *err);
-
typedef int optimize_fn(struct ref_store *ref_store,
struct refs_optimize_opts *opts);
--
2.53.GIT
^ permalink raw reply related
* [PATCH v3 2/9] refs: introduce `ref_store_init_options`
From: Karthik Nayak @ 2026-04-27 10:42 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, toon
In-Reply-To: <20260427-refs-move-to-generic-layer-v3-0-e4638dfb7897@gmail.com>
Reference backends are initiated via the `init()` function. When
initiating the function, the backend is also provided flags which denote
the access levels of the initiator. Create a new structure
`ref_store_init_options` to house such options and move the access flags
to this structure.
This allows easier extension of providing further options to the
backends. In the following commit, we'll also provide config around
reflog creation to the backends via the same structure.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 6 +++++-
refs/files-backend.c | 8 +++++---
refs/packed-backend.c | 4 ++--
refs/packed-backend.h | 3 ++-
refs/refs-internal.h | 11 ++++++++++-
refs/reftable-backend.c | 4 ++--
6 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index bfcb9c7ac3..8992dd6ae8 100644
--- a/refs.c
+++ b/refs.c
@@ -2295,6 +2295,9 @@ static struct ref_store *ref_store_init(struct repository *repo,
{
const struct ref_storage_be *be;
struct ref_store *refs;
+ struct ref_store_init_options opts = {
+ .access_flags = flags,
+ };
be = find_ref_storage_backend(format);
if (!be)
@@ -2304,7 +2307,8 @@ static struct ref_store *ref_store_init(struct repository *repo,
* TODO Send in a 'struct worktree' instead of a 'gitdir', and
* allow the backend to handle how it wants to deal with worktrees.
*/
- refs = be->init(repo, repo->ref_storage_payload, gitdir, flags);
+ refs = be->init(repo, repo->ref_storage_payload, gitdir, &opts);
+
return refs;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b3b0c25f84..72afe62cee 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -108,7 +108,7 @@ static void clear_loose_ref_cache(struct files_ref_store *refs)
static struct ref_store *files_ref_store_init(struct repository *repo,
const char *payload,
const char *gitdir,
- unsigned int flags)
+ const struct ref_store_init_options *opts)
{
struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
struct ref_store *ref_store = (struct ref_store *)refs;
@@ -120,11 +120,13 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
&ref_common_dir);
base_ref_store_init(ref_store, repo, refdir.buf, &refs_be_files);
- refs->store_flags = flags;
+
refs->gitcommondir = strbuf_detach(&ref_common_dir, NULL);
refs->packed_ref_store =
- packed_ref_store_init(repo, NULL, refs->gitcommondir, flags);
+ packed_ref_store_init(repo, NULL, refs->gitcommondir, opts);
+ refs->store_flags = opts->access_flags;
refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
+
repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs);
chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 23ed62984b..35a0f32e1c 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -218,14 +218,14 @@ static size_t snapshot_hexsz(const struct snapshot *snapshot)
struct ref_store *packed_ref_store_init(struct repository *repo,
const char *payload UNUSED,
const char *gitdir,
- unsigned int store_flags)
+ const struct ref_store_init_options *opts)
{
struct packed_ref_store *refs = xcalloc(1, sizeof(*refs));
struct ref_store *ref_store = (struct ref_store *)refs;
struct strbuf sb = STRBUF_INIT;
base_ref_store_init(ref_store, repo, gitdir, &refs_be_packed);
- refs->store_flags = store_flags;
+ refs->store_flags = opts->access_flags;
strbuf_addf(&sb, "%s/packed-refs", gitdir);
refs->path = strbuf_detach(&sb, NULL);
diff --git a/refs/packed-backend.h b/refs/packed-backend.h
index 2c2377a356..1db48e801d 100644
--- a/refs/packed-backend.h
+++ b/refs/packed-backend.h
@@ -3,6 +3,7 @@
struct repository;
struct ref_transaction;
+struct ref_store_init_options;
/*
* Support for storing references in a `packed-refs` file.
@@ -16,7 +17,7 @@ struct ref_transaction;
struct ref_store *packed_ref_store_init(struct repository *repo,
const char *payload,
const char *gitdir,
- unsigned int store_flags);
+ const struct ref_store_init_options *options);
/*
* Lock the packed-refs file for writing. Flags is passed to
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 2d963cc4f4..f49b3807bf 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -385,6 +385,15 @@ struct ref_store;
REF_STORE_ODB | \
REF_STORE_MAIN)
+/*
+ * Options for initializing the ref backend. All backend-agnostic information
+ * which backends required will be held here.
+ */
+struct ref_store_init_options {
+ /* The kind of operations that the ref_store is allowed to perform. */
+ unsigned int access_flags;
+};
+
/*
* Initialize the ref_store for the specified gitdir. These functions
* should call base_ref_store_init() to initialize the shared part of
@@ -393,7 +402,7 @@ struct ref_store;
typedef struct ref_store *ref_store_init_fn(struct repository *repo,
const char *payload,
const char *gitdir,
- unsigned int flags);
+ const struct ref_store_init_options *opts);
/*
* Release all memory and resources associated with the ref store.
*/
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index daea30a5b4..ad4ee2627c 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -369,7 +369,7 @@ static int reftable_be_config(const char *var, const char *value,
static struct ref_store *reftable_be_init(struct repository *repo,
const char *payload,
const char *gitdir,
- unsigned int store_flags)
+ const struct ref_store_init_options *opts)
{
struct reftable_ref_store *refs = xcalloc(1, sizeof(*refs));
struct strbuf ref_common_dir = STRBUF_INIT;
@@ -386,8 +386,8 @@ static struct ref_store *reftable_be_init(struct repository *repo,
base_ref_store_init(&refs->base, repo, refdir.buf, &refs_be_reftable);
strmap_init(&refs->worktree_backends);
- refs->store_flags = store_flags;
refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
+ refs->store_flags = opts->access_flags;
switch (repo->hash_algo->format_id) {
case GIT_SHA1_FORMAT_ID:
--
2.53.GIT
^ permalink raw reply related
* [PATCH v3 3/9] refs: extract out reflog config to generic layer
From: Karthik Nayak @ 2026-04-27 10:42 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, toon
In-Reply-To: <20260427-refs-move-to-generic-layer-v3-0-e4638dfb7897@gmail.com>
The reference backends need to know when to create reflog entries, this
is dictated by the 'core.logallrefupdates' config. Instead of relying on
the backends to call `repo_settings_get_log_all_ref_updates()` to obtain
this config value, let's do this in the generic layer and pass down the
value to the backends.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 1 +
refs/files-backend.c | 2 +-
refs/refs-internal.h | 6 ++++++
refs/reftable-backend.c | 2 +-
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 8992dd6ae8..6b506aeea3 100644
--- a/refs.c
+++ b/refs.c
@@ -2297,6 +2297,7 @@ static struct ref_store *ref_store_init(struct repository *repo,
struct ref_store *refs;
struct ref_store_init_options opts = {
.access_flags = flags,
+ .log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo),
};
be = find_ref_storage_backend(format);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 72afe62cee..4b2faf4777 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -125,7 +125,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
refs->packed_ref_store =
packed_ref_store_init(repo, NULL, refs->gitcommondir, opts);
refs->store_flags = opts->access_flags;
- refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
+ refs->log_all_ref_updates = opts->log_all_ref_updates;
repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index f49b3807bf..d103387ebf 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -392,6 +392,12 @@ struct ref_store;
struct ref_store_init_options {
/* The kind of operations that the ref_store is allowed to perform. */
unsigned int access_flags;
+
+ /*
+ * Denotes under what conditions reflogs should be created when updating
+ * references.
+ */
+ enum log_refs_config log_all_ref_updates;
};
/*
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index ad4ee2627c..93374d25c2 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -386,7 +386,7 @@ static struct ref_store *reftable_be_init(struct repository *repo,
base_ref_store_init(&refs->base, repo, refdir.buf, &refs_be_reftable);
strmap_init(&refs->worktree_backends);
- refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
+ refs->log_all_ref_updates = opts->log_all_ref_updates;
refs->store_flags = opts->access_flags;
switch (repo->hash_algo->format_id) {
--
2.53.GIT
^ permalink raw reply related
* [PATCH v3 4/9] refs: return `ref_transaction_error` from `ref_transaction_update()`
From: Karthik Nayak @ 2026-04-27 10:42 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, toon
In-Reply-To: <20260427-refs-move-to-generic-layer-v3-0-e4638dfb7897@gmail.com>
The `ref_transaction_update()` function is used to add updates to a
given reference transactions. In the following commit, we'll add more
validation to this function. As such, it would be beneficial if the
function returns specific error types, so callers can differentiate
between different errors.
To facilitate this, return `enum ref_transaction_error` from the
function and covert the existing '-1' returns to
'REF_TRANSACTION_ERROR_GENERIC'. Since this retains the existing
behavior, no changes are made to any of the callers but this sets the
necessary infrastructure for introduction of other errors.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 20 ++++++++++----------
refs.h | 16 ++++++++--------
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index 6b506aeea3..efa16b739d 100644
--- a/refs.c
+++ b/refs.c
@@ -1383,25 +1383,25 @@ static int transaction_refname_valid(const char *refname,
return 1;
}
-int ref_transaction_update(struct ref_transaction *transaction,
- const char *refname,
- const struct object_id *new_oid,
- const struct object_id *old_oid,
- const char *new_target,
- const char *old_target,
- unsigned int flags, const char *msg,
- struct strbuf *err)
+enum ref_transaction_error ref_transaction_update(struct ref_transaction *transaction,
+ const char *refname,
+ const struct object_id *new_oid,
+ const struct object_id *old_oid,
+ const char *new_target,
+ const char *old_target,
+ unsigned int flags, const char *msg,
+ struct strbuf *err)
{
assert(err);
if ((flags & REF_FORCE_CREATE_REFLOG) &&
(flags & REF_SKIP_CREATE_REFLOG)) {
strbuf_addstr(err, _("refusing to force and skip creation of reflog"));
- return -1;
+ return REF_TRANSACTION_ERROR_GENERIC;
}
if (!transaction_refname_valid(refname, new_oid, flags, err))
- return -1;
+ return REF_TRANSACTION_ERROR_GENERIC;
if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS)
BUG("illegal flags 0x%x passed to ref_transaction_update()", flags);
diff --git a/refs.h b/refs.h
index d65de6ab5f..71d5c186d0 100644
--- a/refs.h
+++ b/refs.h
@@ -905,14 +905,14 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
* See the above comment "Reference transaction updates" for more
* information.
*/
-int ref_transaction_update(struct ref_transaction *transaction,
- const char *refname,
- const struct object_id *new_oid,
- const struct object_id *old_oid,
- const char *new_target,
- const char *old_target,
- unsigned int flags, const char *msg,
- struct strbuf *err);
+enum ref_transaction_error ref_transaction_update(struct ref_transaction *transaction,
+ const char *refname,
+ const struct object_id *new_oid,
+ const struct object_id *old_oid,
+ const char *new_target,
+ const char *old_target,
+ unsigned int flags, const char *msg,
+ struct strbuf *err);
/*
* Similar to `ref_transaction_update`, but this function is only for adding
--
2.53.GIT
^ permalink raw reply related
* [PATCH v3 5/9] update-ref: move `print_rejected_refs()` up
From: Karthik Nayak @ 2026-04-27 10:42 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, toon
In-Reply-To: <20260427-refs-move-to-generic-layer-v3-0-e4638dfb7897@gmail.com>
The `print_rejected_refs()` function is used to print any rejected refs
when using git-updated-ref(1) with the '--batch-updates' option. In the
following commit, we'll need to use this function in another place, so
move the function up to avoid a separate forward declaration.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/update-ref.c | 45 ++++++++++++++++++++++-----------------------
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 2d68c40ecb..5259cc7226 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -234,6 +234,28 @@ static int parse_next_oid(const char **next, const char *end,
command, refname);
}
+static void print_rejected_refs(const char *refname,
+ const struct object_id *old_oid,
+ const struct object_id *new_oid,
+ const char *old_target,
+ const char *new_target,
+ enum ref_transaction_error err,
+ const char *details,
+ void *cb_data UNUSED)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ if (details && *details)
+ error("%s", details);
+
+ strbuf_addf(&sb, "rejected %s %s %s %s\n", refname,
+ new_oid ? oid_to_hex(new_oid) : new_target,
+ old_oid ? oid_to_hex(old_oid) : old_target,
+ ref_transaction_error_msg(err));
+
+ fwrite(sb.buf, sb.len, 1, stdout);
+ strbuf_release(&sb);
+}
/*
* The following five parse_cmd_*() functions parse the corresponding
@@ -567,29 +589,6 @@ static void parse_cmd_abort(struct ref_transaction *transaction,
report_ok("abort");
}
-static void print_rejected_refs(const char *refname,
- const struct object_id *old_oid,
- const struct object_id *new_oid,
- const char *old_target,
- const char *new_target,
- enum ref_transaction_error err,
- const char *details,
- void *cb_data UNUSED)
-{
- struct strbuf sb = STRBUF_INIT;
-
- if (details && *details)
- error("%s", details);
-
- strbuf_addf(&sb, "rejected %s %s %s %s\n", refname,
- new_oid ? oid_to_hex(new_oid) : new_target,
- old_oid ? oid_to_hex(old_oid) : old_target,
- ref_transaction_error_msg(err));
-
- fwrite(sb.buf, sb.len, 1, stdout);
- strbuf_release(&sb);
-}
-
static void parse_cmd_commit(struct ref_transaction *transaction,
const char *next, const char *end UNUSED)
{
--
2.53.GIT
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox