* [PATCH v10 3/5] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-07-20 8:27 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v10.git.git.1784536024.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Folding a series of commits into one required either an interactive
rebase where each commit after the first was hand-edited to "fixup", or
a "git reset --soft" to the merge base followed by "git commit --amend".
Add "git history squash <revision-range>" to do this directly. It folds
every commit in the range into the oldest one, keeping that commit's
authorship and taking the tree of the newest commit, then replays the
commits above the range on top. The squashed message comes from the
oldest commit by default, or from the body of the last amend! commit
targeting it. An editor opens with the selected message when
--reedit-message is given. A fixup!, squash! or amend! commit is refused
unless the commit it targets is also in the range, so the fold does not
silently absorb a marker meant for a commit outside it. The check runs
the range through todo_list_rearrange_squash(), which leaves such a
marker as a plain pick. Markers whose target is in the range fold in as
usual. As an exception, a range made up entirely of markers for one
target is combined anyway, taking its message from the last amend! if
there is one, so a batch of fixups for the same commit can be collapsed.
The range is read like the arguments to "git rev-list", so several
revisions such as "HEAD~3..HEAD ^topic" may be given, and rev-list
options are accepted too. As "git replay" does, the walk options the fold
relies on are forced after setup_revisions() and a warning is printed if
an option changed them, so the first commit returned is the range's
oldest and its parent is the base regardless of what the user passed
(including after a "--"). A merge inside the range is folded when its
other parent is reachable from the base, otherwise the range has more
than one base and is rejected. By default the command also refuses when a
ref points at a commit that the fold would discard. Use --update-refs=head
to rewrite only the current branch instead.
Inspired-by: Sergey Chernov <serega.morph@gmail.com>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/advice.adoc | 4 +
Documentation/git-history.adoc | 45 ++-
advice.c | 1 +
advice.h | 1 +
builtin/history.c | 374 ++++++++++++++++++++
t/meson.build | 1 +
t/t3455-history-squash.sh | 569 +++++++++++++++++++++++++++++++
7 files changed, 992 insertions(+), 3 deletions(-)
create mode 100755 t/t3455-history-squash.sh
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 81f80a9274..e2a3487778 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -59,6 +59,10 @@ all advice messages.
forceDeleteBranch::
Shown when the user tries to delete a not fully merged
branch without the force option set.
+ historyUpdateRefs::
+ Shown when `git history squash` refuses because a ref points
+ into the range being folded, to tell the user about
+ `--update-refs=head`.
ignoredHook::
Shown when a hook is ignored because the hook is not
set as executable.
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 28b477cd37..e1e930f355 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -12,6 +12,7 @@ git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(
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>...]
+git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>
DESCRIPTION
-----------
@@ -43,8 +44,11 @@ at once.
LIMITATIONS
-----------
-This command does not (yet) work with histories that contain merges. You
-should use linkgit:git-rebase[1] with the `--rebase-merges` flag instead.
+This command does not (yet) replay merge commits onto the rewritten
+history: if a commit that would be replayed is a merge, the operation is
+rejected, and you should use linkgit:git-rebase[1] with the
+`--rebase-merges` flag instead. The `squash` subcommand can still fold a
+merge that lies inside the range, as long as the range has a single base.
Furthermore, the command does not support operations that can result in merge
conflicts. This limitation is by design as history rewrites are not intended to
@@ -113,6 +117,41 @@ linkgit:gitglossary[7].
It is invalid to select either all or no hunks, as that would lead to
one of the commits becoming empty.
+`squash <revision-range>`::
+ Fold all commits in _<revision-range>_ into the oldest commit of that
+ range. The resulting commit keeps the oldest commit's authorship and
+ takes the tree of the range's newest commit, so the whole range
+ collapses into a single commit. Commits above the range are replayed
+ on top of the result.
++
+The range is given in the usual `<base>..<tip>` form, where _<base>_ is
+the commit just below the oldest commit to squash. For example, `git
+history squash HEAD~3..HEAD` folds the three most recent commits into
+one, and `git history squash HEAD~5..HEAD~2` squashes an interior range
+while leaving the two newest commits in place. Several revisions may be
+given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
+already on `topic`. Rev-list options may also be given, but any that would
+change how the range is walked are overridden with a warning.
++
+The oldest commit's message is preserved by default, except that an `amend!`
+commit targeting it replaces its message. Specify `--reedit-message` to edit
+the resulting message. A merge commit inside the range is folded like any
+other, but the range must have a single base, so a range that reaches more
+than one entry point (for example a side branch that forked before the range
+and was later merged into it) is rejected.
++
+A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
+targets is also in the range, so the fold does not silently absorb a
+marker meant for a commit outside it. As an exception, a range made up entirely
+of markers for one target is combined into a single commit, keeping the last
+`amend!` message if there is one.
++
+A branch or tag that points at a commit inside the range would be left
+dangling once those commits are folded away, so with the default
+`--update-refs=branches` the command refuses. Rerun with
+`--update-refs=head` to rewrite only the current branch and leave such
+refs pointing at the old commits.
+
OPTIONS
-------
@@ -123,7 +162,7 @@ OPTIONS
ref updates is generally safe.
`--reedit-message`::
- Open an editor to modify the target commit's message.
+ Open an editor to modify the rewritten commit's message.
`--empty=(drop|keep|abort)`::
Control what happens when a commit becomes empty as a result of the
diff --git a/advice.c b/advice.c
index 63bf8b0c5f..401d047391 100644
--- a/advice.c
+++ b/advice.c
@@ -58,6 +58,7 @@ static struct {
[ADVICE_FETCH_SHOW_FORCED_UPDATES] = { "fetchShowForcedUpdates" },
[ADVICE_FORCE_DELETE_BRANCH] = { "forceDeleteBranch" },
[ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated" },
+ [ADVICE_HISTORY_UPDATE_REFS] = { "historyUpdateRefs" },
[ADVICE_IGNORED_HOOK] = { "ignoredHook" },
[ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity" },
[ADVICE_MERGE_CONFLICT] = { "mergeConflict" },
diff --git a/advice.h b/advice.h
index 66f6cd6a77..3f0b4f0485 100644
--- a/advice.h
+++ b/advice.h
@@ -25,6 +25,7 @@ enum advice_type {
ADVICE_FETCH_SHOW_FORCED_UPDATES,
ADVICE_FORCE_DELETE_BRANCH,
ADVICE_GRAFT_FILE_DEPRECATED,
+ ADVICE_HISTORY_UPDATE_REFS,
ADVICE_IGNORED_HOOK,
ADVICE_IMPLICIT_IDENTITY,
ADVICE_MERGE_CONFLICT,
diff --git a/builtin/history.c b/builtin/history.c
index b592b98393..423c8beaaf 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
+#include "advice.h"
#include "cache-tree.h"
#include "commit.h"
#include "commit-reach.h"
@@ -34,6 +35,8 @@
N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
#define GIT_HISTORY_SPLIT_USAGE \
N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
+#define GIT_HISTORY_SQUASH_USAGE \
+ N_("git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>")
static void change_data_free(void *util, const char *str UNUSED)
{
@@ -1004,6 +1007,375 @@ out:
return ret;
}
+/*
+ * Resolve a "<base>..<tip>" revision range into the base commit just outside
+ * the range (which becomes the parent of the squashed commit), the oldest
+ * commit contained in the range (whose message the squash reuses), and the
+ * range tip (whose tree becomes the result). A merge inside the range is fine,
+ * but the range must have a single base and must not reach a root commit.
+ */
+static int resolve_squash_range(struct repository *repo,
+ const char **argv,
+ struct commit **base_out,
+ struct commit **oldest_out,
+ struct commit **tip_out,
+ struct oidset *interior_out)
+{
+ struct rev_info revs;
+ struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
+ struct commit_list *boundaries = NULL, *b;
+ struct strvec args = STRVEC_INIT;
+ size_t i;
+ int ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ revs.reverse = 1;
+ revs.topo_order = 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.simplify_history = 0;
+ revs.boundary = 1;
+
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--ancestry-path");
+ strvec_pushv(&args, argv);
+ setup_revisions_from_strvec(&args, &revs, NULL);
+ if (args.nr != 1) {
+ ret = error(_("unrecognized argument: %s"), args.v[1]);
+ goto out;
+ }
+
+ if (revs.reverse != 1 || revs.topo_order != 1 ||
+ revs.sort_order != REV_SORT_IN_GRAPH_ORDER ||
+ revs.simplify_history != 0 || revs.boundary != 1) {
+ warning(_("ignoring rev-list options that would change how the "
+ "range is walked"));
+ revs.reverse = 1;
+ revs.topo_order = 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.simplify_history = 0;
+ revs.boundary = 1;
+ }
+
+ /*
+ * A squash needs a base to reparent onto, so the range has to exclude
+ * something, as in "<base>..<tip>". A revision range with no such
+ * bottom commit cannot be squashed.
+ */
+ for (i = 0; i < revs.cmdline.nr; i++)
+ if (revs.cmdline.rev[i].flags & UNINTERESTING)
+ break;
+ if (i == revs.cmdline.nr) {
+ ret = error(_("not a '<base>..<tip>' revision range"));
+ goto out;
+ }
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ /*
+ * Set boundary commits aside for the base check below, and put every
+ * in-range commit but the tip into the interior set. A ref pointing
+ * at an interior commit would dangle once the range is folded away.
+ */
+ while ((commit = get_revision(&revs))) {
+ if (commit->object.flags & BOUNDARY) {
+ commit_list_insert(commit, &boundaries);
+ continue;
+ }
+ if (!oldest)
+ oldest = commit;
+ if (tip)
+ oidset_insert(interior_out, &tip->object.oid);
+ tip = commit;
+ }
+
+ if (!oldest) {
+ ret = error(_("the revision range is empty"));
+ goto out;
+ } else if (oldest == tip) {
+ ret = error(_("the revision range holds a single commit; "
+ "nothing to squash"));
+ goto out;
+ } else if (!oldest->parents) {
+ BUG("an in-range commit must have a parent");
+ }
+ base = oldest->parents->item;
+
+ /*
+ * A boundary other than the base is an in-range commit reaching a
+ * commit outside the range, so the range has more than one base.
+ */
+ for (b = boundaries; b; b = b->next) {
+ if (b->item != base) {
+ ret = error(_("the revision range has more than one base; "
+ "cannot squash"));
+ goto out;
+ }
+ }
+
+ *base_out = base;
+ *oldest_out = oldest;
+ *tip_out = tip;
+ ret = 0;
+
+out:
+ commit_list_free(boundaries);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
+static const char *autosquash_target(const char *subject)
+{
+ const char *rest;
+
+ while (skip_prefix(subject, "fixup! ", &rest) ||
+ skip_prefix(subject, "squash! ", &rest) ||
+ skip_prefix(subject, "amend! ", &rest))
+ subject = rest;
+ return subject;
+}
+
+static int reject_dangling_fixups(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct commit *oldest,
+ struct commit **msg_source,
+ struct commit **amend_source)
+{
+ struct todo_list todo = TODO_LIST_INIT;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
+ struct rev_info revs;
+ struct commit *commit, *last_amend = NULL;
+ struct strvec args = STRVEC_INIT;
+ char *dangling_subject = NULL, *dangling_target = NULL;
+ bool mixed_target = false, all_fixups_one_target;
+ bool past_oldest_group = false;
+ int i, ret, nr_dangling = 0;
+
+ *msg_source = oldest;
+ *amend_source = NULL;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+ while ((commit = get_revision(&revs)))
+ strbuf_addf(&todo.buf, "pick %s\n",
+ oid_to_hex(&commit->object.oid));
+
+ if (todo_list_parse_insn_buffer(repo, &opts, todo.buf.buf, &todo) < 0 ||
+ todo_list_rearrange_squash(&todo) < 0) {
+ ret = error(_("could not check the range for fixups"));
+ goto out;
+ }
+
+ for (i = 0; i < todo.nr; i++) {
+ const char *message, *subject_start, *target;
+ char *subject;
+ size_t sublen;
+
+ message = repo_logmsg_reencode(repo, todo.items[i].commit,
+ NULL, NULL);
+ sublen = find_commit_subject(message, &subject_start);
+
+ if (todo.items[i].command != TODO_PICK) {
+ if (!past_oldest_group &&
+ starts_with(subject_start, "amend! "))
+ *amend_source = todo.items[i].commit;
+ repo_unuse_commit_buffer(repo, todo.items[i].commit, message);
+ continue;
+ }
+ if (i)
+ past_oldest_group = true;
+
+ subject = xmemdupz(subject_start, sublen);
+ target = autosquash_target(subject);
+ if (target != subject) {
+ nr_dangling++;
+ if (!dangling_target) {
+ dangling_target = xstrdup(target);
+ dangling_subject = xstrdup(subject);
+ } else if (strcmp(dangling_target, target)) {
+ mixed_target = true;
+ }
+ if (starts_with(subject, "amend! "))
+ last_amend = todo.items[i].commit;
+ }
+ free(subject);
+ repo_unuse_commit_buffer(repo, todo.items[i].commit, message);
+ }
+
+ all_fixups_one_target = nr_dangling == todo.nr && !mixed_target;
+ if (nr_dangling && !all_fixups_one_target) {
+ ret = error(_("cannot squash '%s': its target is not in the "
+ "range"), dangling_subject);
+ } else {
+ if (last_amend)
+ *msg_source = last_amend;
+ ret = 0;
+ }
+
+out:
+ free(dangling_subject);
+ free(dangling_target);
+ todo_list_release(&todo);
+ replay_opts_release(&opts);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
+struct interior_ref_cb {
+ const struct oidset *interior;
+ const char *name;
+};
+
+static int find_interior_ref(const struct reference *ref, void *cb_data)
+{
+ struct interior_ref_cb *data = cb_data;
+
+ if (oidset_contains(data->interior, ref->oid)) {
+ data->name = xstrdup(ref->name);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int cmd_history_squash(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo)
+{
+ const char * const usage[] = {
+ GIT_HISTORY_SQUASH_USAGE,
+ NULL,
+ };
+ 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_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+ struct strbuf message = STRBUF_INIT;
+ struct oidset interior = OIDSET_INIT;
+ struct commit *base, *oldest, *tip, *rewritten, *msg_source,
+ *amend_source;
+ const struct object_id *base_tree_oid, *tip_tree_oid;
+ const char *message_template = NULL;
+ struct commit_list *parents = NULL;
+ struct rev_info revs = { 0 };
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage,
+ PARSE_OPT_KEEP_UNKNOWN_OPT);
+ if (!argc) {
+ ret = error(_("command expects a revision range"));
+ goto out;
+ }
+ repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+
+ ret = resolve_squash_range(repo, argv, &base, &oldest, &tip,
+ &interior);
+ if (ret < 0)
+ goto out;
+
+ ret = reject_dangling_fixups(repo, base, tip, oldest, &msg_source,
+ &amend_source);
+ if (ret < 0)
+ goto out;
+ if (amend_source) {
+ const char *amend_message, *body;
+
+ amend_message = repo_logmsg_reencode(repo, amend_source,
+ NULL, NULL);
+ find_commit_subject(amend_message, &body);
+ body = skip_blank_lines(body + commit_subject_length(body));
+ strbuf_addstr(&message, body);
+ message_template = message.buf;
+ repo_unuse_commit_buffer(repo, amend_source, amend_message);
+ }
+
+ if (action == REF_ACTION_BRANCHES) {
+ struct interior_ref_cb cb = { .interior = &interior };
+
+ refs_for_each_ref(get_main_ref_store(repo),
+ find_interior_ref, &cb);
+ if (cb.name) {
+ ret = error(_("'%s' points into the squashed range"),
+ cb.name);
+ advise_if_enabled(ADVICE_HISTORY_UPDATE_REFS,
+ _("Use --update-refs=head to rewrite only "
+ "the current branch and leave such refs "
+ "untouched."));
+ free((char *)cb.name);
+ goto out;
+ }
+ }
+
+ ret = setup_revwalk(repo, action, tip, &revs);
+ if (ret < 0)
+ goto out;
+
+ base_tree_oid = &repo_get_commit_tree(repo, base)->object.oid;
+ tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
+ commit_list_append(base, &parents);
+
+ ret = commit_tree_ext(repo, "squash", msg_source, message_template,
+ parents,
+ base_tree_oid, tip_tree_oid, &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing squashed commit"));
+ goto out;
+ }
+
+ strbuf_addstr(&reflog_msg, "squash: updating ");
+ strbuf_join_argv(&reflog_msg, argc, argv, ' ');
+
+ ret = handle_reference_updates(&revs, action, tip, rewritten,
+ reflog_msg.buf, dry_run,
+ REPLAY_EMPTY_COMMIT_ABORT);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ strbuf_release(&reflog_msg);
+ strbuf_release(&message);
+ oidset_clear(&interior);
+ commit_list_free(parents);
+ release_revisions(&revs);
+ return ret;
+}
+
static int update_worktree(struct repository *repo,
const struct commit *old_head,
const struct commit *new_head,
@@ -1192,6 +1564,7 @@ int cmd_history(int argc,
GIT_HISTORY_FIXUP_USAGE,
GIT_HISTORY_REWORD_USAGE,
GIT_HISTORY_SPLIT_USAGE,
+ GIT_HISTORY_SQUASH_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -1200,6 +1573,7 @@ int cmd_history(int argc,
OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
OPT_SUBCOMMAND("split", &fn, cmd_history_split),
+ OPT_SUBCOMMAND("squash", &fn, cmd_history_squash),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index 8ae6ab6c5f..89cff16405 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -405,6 +405,7 @@ integration_tests = [
't3452-history-split.sh',
't3453-history-fixup.sh',
't3454-history-drop.sh',
+ 't3455-history-squash.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
new file mode 100755
index 0000000000..9c362f3094
--- /dev/null
+++ b/t/t3455-history-squash.sh
@@ -0,0 +1,569 @@
+#!/bin/sh
+
+test_description='tests for git-history squash subcommand'
+
+. ./test-lib.sh
+
+stage_file () {
+ printf "%s\n" "$1" >file &&
+ git add file
+}
+
+commit_with_message () {
+ printf "%b" "$1" >msg &&
+ git commit --allow-empty -qF msg
+}
+
+check_commit_count () {
+ git rev-list --count "$1" >actual &&
+ echo "$2" >expect &&
+ test_cmp expect actual
+}
+
+check_log_subjects () {
+ git log --format="%s" "$1" >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+check_log_messages () {
+ git log --format="%B" "$1" >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+test_expect_success 'setup linear history touching two files' '
+ test_commit base file a &&
+ git tag start &&
+ test_commit --no-tag one other x &&
+ test_commit --no-tag two file c &&
+ test_commit three file d
+'
+
+test_expect_success 'errors on missing range argument' '
+ test_must_fail git history squash 2>err &&
+ test_grep "expects a revision range" err
+'
+
+test_expect_success 'errors on an empty range' '
+ test_must_fail git history squash HEAD..HEAD 2>err &&
+ test_grep "the revision range is empty" err
+'
+
+test_expect_success 'errors on a single revision that is not a range' '
+ test_must_fail git history squash HEAD 2>err &&
+ test_grep "not a .*range" err &&
+ test_must_fail git history squash HEAD~1 2>err &&
+ test_grep "not a .*range" err
+'
+
+test_expect_success 'errors on a range holding a single commit' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash "HEAD^!" 2>err &&
+ test_grep "single commit; nothing to squash" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'accepts multiple revision arguments with an exclusion' '
+ git reset --hard three &&
+ git branch -f keep HEAD~2 &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..HEAD ^keep &&
+
+ git reflog -1 --format=%gs >actual &&
+ echo "squash: updating start..HEAD ^keep" >expect &&
+ test_cmp expect actual &&
+
+ check_log_subjects start..HEAD <<-\EOF &&
+ two
+ one
+ EOF
+ test_cmp_rev keep HEAD~1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+
+ git branch -D keep
+'
+
+test_expect_success 'squashes a branch the current branch is not on' '
+ git reset --hard three &&
+ main=$(git symbolic-ref --short HEAD) &&
+ head_before=$(git rev-parse HEAD) &&
+ git checkout -b off-history start &&
+ test_commit --no-tag off-one off a &&
+ test_commit --no-tag off-two off b &&
+ git checkout "$main" &&
+
+ git history squash start..off-history &&
+
+ check_commit_count start..off-history 1 &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D off-history
+'
+
+test_expect_success 'squashes a range into a single commit without changing the tree' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash --dry-run start.. >out &&
+ predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git history squash start.. &&
+
+ test "$predicted" = "$(git rev-parse HEAD)" &&
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ check_log_subjects -1 <<-\EOF &&
+ one
+ EOF
+ git reflog >reflog &&
+ test_grep "squash: updating" reflog
+'
+
+test_expect_success 'sanitizes rev-list walk options, before and after --' '
+ git reset --hard three &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash --date-order start.. 2>err &&
+ test_grep "ignoring rev-list options" err &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+
+ git reset --hard three &&
+ git history squash -- --reverse start.. 2>err &&
+ test_grep "ignoring rev-list options" err &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashes an interior range and replays descendants verbatim' '
+ git reset --hard three &&
+ final_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..@~1 &&
+
+ check_log_subjects start..HEAD <<-\EOF &&
+ three
+ one
+ EOF
+
+ test_cmp_rev start HEAD~2 &&
+ test "$final_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashes when the base is the root commit' '
+ git reset --hard three &&
+ root=$(git rev-list --max-parents=0 HEAD) &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash "$root.." &&
+
+ check_commit_count "$root..HEAD" 1 &&
+ test_cmp_rev "$root" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+
+test_expect_success 'folds fixups whose target is in the range' '
+ git reset --hard start &&
+ test_commit --no-tag target file b &&
+ git commit --allow-empty -m "fixup! target" &&
+ git commit --allow-empty -m "fixup! target" &&
+ test_commit --no-tag later file c &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_subjects -1 <<-\EOF
+ target
+ EOF
+'
+
+test_expect_success 'refuses a below-range fixup! after an in-range commit' '
+ git reset --hard start &&
+ test_commit --no-tag inside file b &&
+ test_commit --no-tag "fixup! outside" file c &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "target is not in the range" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'combines a run of fixups for one commit below the range' '
+ git reset --hard start &&
+ stage_file b && git commit -m "fixup! base" &&
+ stage_file c && git commit -m "fixup! base" &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_subjects -1 <<-\EOF
+ fixup! base
+ EOF
+'
+
+test_expect_success 'combining below-range fixups keeps the last amend! message' '
+ git reset --hard start &&
+ stage_file b && git commit -m "fixup! base" &&
+ stage_file c &&
+ commit_with_message "amend! base\n\namended body\n" &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_messages -1 <<-\EOF
+ amend! base
+
+ amended body
+
+ EOF
+'
+
+test_expect_success 'refuses fixups for two different commits below the range' '
+ git reset --hard start &&
+ stage_file b && git commit -m "fixup! aaa" &&
+ stage_file c && git commit -m "fixup! bbb" &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "target is not in the range" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'the last amend! for the oldest commit replaces its message' '
+ git reset --hard start &&
+ test_commit --no-tag marker-oldest file b &&
+ git commit --allow-empty -m "squash! marker-oldest" &&
+ commit_with_message "amend! marker-oldest\n\nearlier message\n" &&
+ commit_with_message \
+ "amend! marker-oldest\n\namended subject\n\namended body\n" &&
+ test_commit --no-tag marker-later file c &&
+ commit_with_message "amend! marker-later\n\nwrong message\n" &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_messages -1 <<-\EOF
+ amended subject
+
+ amended body
+
+ EOF
+'
+
+test_expect_success 'preserves authorship of the oldest commit' '
+ git reset --hard start &&
+ GIT_AUTHOR_NAME=Squasher GIT_AUTHOR_EMAIL=squash@example.com \
+ test_commit --no-tag oldest file b &&
+ test_commit newest file c &&
+
+ git history squash start.. &&
+
+ git log -1 --format="%an <%ae>" >actual &&
+ echo "Squasher <squash@example.com>" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--update-refs=head only moves HEAD' '
+ git reset --hard three &&
+ git branch -f other HEAD &&
+ other_before=$(git rev-parse other) &&
+
+ git history squash --update-refs=head start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev "$other_before" other
+'
+
+test_expect_success 'refuses to fold a range a ref points into' '
+ git reset --hard three &&
+ git branch -f mid HEAD~1 &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "error: .* points into the squashed range" err &&
+ test_grep "hint: .*--update-refs=head" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D mid
+'
+
+test_expect_success 'advice.historyUpdateRefs silences the hint' '
+ git reset --hard three &&
+ git branch -f mid HEAD~1 &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git -c advice.historyUpdateRefs=false \
+ history squash start.. 2>err &&
+ test_grep "points into the squashed range" err &&
+ test_grep ! "hint:" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D mid
+'
+
+test_expect_success '--update-refs=head folds past a ref pointing into the range' '
+ git reset --hard three &&
+ git branch -f mid HEAD~1 &&
+ mid_before=$(git rev-parse mid) &&
+
+ git history squash --update-refs=head start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev "$mid_before" mid &&
+
+ git branch -D mid
+'
+
+test_expect_success 'refuses to fold a range a tag points into' '
+ git reset --hard three &&
+ git tag -f mark HEAD~1 &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "refs/tags/mark" err &&
+ test_grep "points into the squashed range" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git tag -d mark
+'
+
+test_expect_success 'squashes a range whose internal merge has a single base' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag before-side file b &&
+ git checkout -b inner-side &&
+ test_commit --no-tag on-inner-side inner x &&
+ git checkout "$main" &&
+ test_commit --no-tag after-side file c &&
+ git merge --no-ff -m merge inner-side &&
+ git branch -D inner-side &&
+ test_commit --no-tag after-merge file d &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_subjects -1 <<-\EOF &&
+ before-side
+ EOF
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file inner
+'
+
+test_expect_success 'folds a merge of a branch that forked at the base' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b base-fork-side &&
+ test_commit --no-tag base-fork-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag base-fork-main file b &&
+ git merge --no-ff -m "merge base-fork-side" base-fork-side &&
+ git branch -D base-fork-side &&
+ test_commit --no-tag base-fork-tail file c &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
+'
+
+test_expect_success 'refuses a merge whose other parent is outside the range' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b outside-parent &&
+ test_commit --no-tag outside-parent outside x &&
+ git checkout "$main" &&
+ test_commit --no-tag outside-main file b &&
+ base=$(git rev-parse HEAD) &&
+ test_commit --no-tag outside-mid file c &&
+ git merge --no-ff -m "merge outside-parent" outside-parent &&
+ git branch -D outside-parent &&
+ merged=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash "$base.." 2>err &&
+ test_grep "more than one base" err &&
+ test_cmp_rev "$merged" HEAD
+'
+
+test_expect_success 'folds a range whose tip is a merge commit' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag tipmerge-base file b &&
+ git checkout -b tipmerge-side &&
+ test_commit --no-tag tipmerge-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag tipmerge-main file c &&
+ git merge --no-ff -m "merge tipmerge-side" tipmerge-side &&
+ git branch -D tipmerge-side &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
+'
+
+test_expect_success 'folds a range whose base is a merge commit' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b basemerge-side &&
+ test_commit --no-tag basemerge-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag basemerge-main file b &&
+ git merge --no-ff -m "merge basemerge-side" basemerge-side &&
+ git branch -D basemerge-side &&
+ base=$(git rev-parse HEAD) &&
+ test_commit --no-tag basemerge-one file c &&
+ test_commit --no-tag basemerge-two file d &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash "$base.." &&
+
+ check_commit_count "$base..HEAD" 1 &&
+ test_cmp_rev "$base" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'folds a range with two interior merges' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag two-merge-a file a1 &&
+ git checkout -b two-merge-s1 &&
+ test_commit --no-tag two-merge-s1 s1 x &&
+ git checkout "$main" &&
+ git merge --no-ff -m "merge s1" two-merge-s1 &&
+ test_commit --no-tag two-merge-b file b1 &&
+ git checkout -b two-merge-s2 &&
+ test_commit --no-tag two-merge-s2 s2 y &&
+ git checkout "$main" &&
+ git merge --no-ff -m "merge s2" two-merge-s2 &&
+ git branch -D two-merge-s1 two-merge-s2 &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file s1 &&
+ test_path_is_file s2
+'
+
+test_expect_success 'folds a range with a nested merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b nested-outer &&
+ test_commit --no-tag nested-outer outer x &&
+ git checkout -b nested-inner &&
+ test_commit --no-tag nested-inner inner y &&
+ git checkout nested-outer &&
+ git merge --no-ff -m "merge inner" nested-inner &&
+ git checkout "$main" &&
+ test_commit --no-tag nested-main file b1 &&
+ git merge --no-ff -m "merge outer" nested-outer &&
+ git branch -D nested-outer nested-inner &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file outer &&
+ test_path_is_file inner
+'
+
+test_expect_success 'folds a range with an octopus merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag octo-base file a1 &&
+ git checkout -b octo-1 &&
+ test_commit --no-tag octo-1 o1 x &&
+ git checkout "$main" &&
+ git checkout -b octo-2 &&
+ test_commit --no-tag octo-2 o2 y &&
+ git checkout "$main" &&
+ git merge --no-ff -m octopus octo-1 octo-2 &&
+ git branch -D octo-1 octo-2 &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file o1 &&
+ test_path_is_file o2
+'
+
+test_expect_success 'refuses an octopus merge with an arm forked before the base' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b octo-pre &&
+ test_commit octo-pre-side pside x &&
+ git checkout "$main" &&
+ test_commit octo-pre-main file b1 &&
+ octo_base=$(git rev-parse HEAD) &&
+ git checkout -b octo-within &&
+ test_commit --no-tag octo-within wside y &&
+ git checkout "$main" &&
+ git merge --no-ff -m octopus octo-pre octo-within &&
+ merged=$(git rev-parse HEAD) &&
+ git branch -D octo-pre octo-within &&
+
+ test_must_fail git history squash "$octo_base.." 2>err &&
+ test_grep "more than one base" err &&
+ test_cmp_rev "$merged" HEAD
+'
+
+test_expect_success 'refuses when a descendant above the range is a merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag desc-one file b &&
+ test_commit --no-tag desc-two file c &&
+ git tag desc-tip &&
+ git checkout -b desc-above &&
+ test_commit --no-tag desc-above above x &&
+ git checkout "$main" &&
+ test_commit --no-tag desc-main file d &&
+ git merge --no-ff -m "merge desc-above" desc-above &&
+ git branch -D desc-above &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start..desc-tip 2>err &&
+ test_grep "merge commits is not supported" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'refuses to fold a range a ref points into at a merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag refmerge-base file b &&
+ git checkout -b refmerge-side &&
+ test_commit --no-tag refmerge-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag refmerge-main file c &&
+ git merge --no-ff -m "interior merge" refmerge-side &&
+ git branch -D refmerge-side &&
+ git branch at-merge HEAD &&
+ test_commit --no-tag refmerge-tail file d &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "at-merge" err &&
+ test_grep "points into the squashed range" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D at-merge
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v10 4/5] sequencer: share the squash message marker helpers and flags
From: Harald Nordgren via GitGitGadget @ 2026-07-20 8:27 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v10.git.git.1784536024.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When "git rebase -i" squashes commits it builds an editor template with a
"This is a combination of N commits." banner, a "This is the 1st/Nth
commit message:" header above each kept message (or a "will be skipped"
header for a dropped one), and a commented-out subject for any fixup!,
squash! or amend! commit. The banner, the headers and the
subject-commenting all live in static helpers in sequencer.c wired to the
rebase state, so no other command can present a squash the same way.
Pull the three pieces out into add_squash_combination_header(),
add_squash_message_header() (which takes a flag for the "will be skipped"
variant) and squash_subject_comment_len(), and use them from
update_squash_messages() and append_squash_message(). Also move the
todo_item_flags enum to the header, so a caller reading the output of
todo_list_rearrange_squash() can tell an amend! (TODO_REPLACE_FIXUP_MSG)
from a plain fixup!. A later change reuses all of this to give "git
history squash --reedit-message" the same template.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
sequencer.c | 70 +++++++++++++++++++++++++++++------------------------
sequencer.h | 30 +++++++++++++++++++++++
2 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1355a99a09..3c704fd5ab 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1880,18 +1880,38 @@ static int is_pick_or_similar(enum todo_command command)
}
}
-enum todo_item_flags {
- TODO_EDIT_MERGE_MSG = (1 << 0),
- TODO_REPLACE_FIXUP_MSG = (1 << 1),
- TODO_EDIT_FIXUP_MSG = (1 << 2),
-};
-
static const char first_commit_msg_str[] = N_("This is the 1st commit message:");
static const char nth_commit_msg_fmt[] = N_("This is the commit message #%d:");
static const char skip_first_commit_msg_str[] = N_("The 1st commit message will be skipped:");
static const char skip_nth_commit_msg_fmt[] = N_("The commit message #%d will be skipped:");
static const char combined_commit_msg_fmt[] = N_("This is a combination of %d commits.");
+void add_squash_combination_header(struct strbuf *buf, int n)
+{
+ strbuf_addf(buf, "%s ", comment_line_str);
+ strbuf_addf(buf, _(combined_commit_msg_fmt), n);
+}
+
+void add_squash_message_header(struct strbuf *buf, int n, int skip)
+{
+ strbuf_addf(buf, "%s ", comment_line_str);
+ if (n == 1)
+ strbuf_addstr(buf, skip ? _(skip_first_commit_msg_str) :
+ _(first_commit_msg_str));
+ else
+ strbuf_addf(buf, skip ? _(skip_nth_commit_msg_fmt) :
+ _(nth_commit_msg_fmt), n);
+}
+
+size_t squash_subject_comment_len(const char *body, int squashing)
+{
+ if (starts_with(body, "amend!") ||
+ (squashing && (starts_with(body, "squash!") ||
+ starts_with(body, "fixup!"))))
+ return commit_subject_length(body);
+ return 0;
+}
+
static int is_fixup_flag(enum todo_command command, unsigned flag)
{
return command == TODO_FIXUP && ((flag & TODO_REPLACE_FIXUP_MSG) ||
@@ -2005,20 +2025,13 @@ static int append_squash_message(struct strbuf *buf, const char *body,
{
struct replay_ctx *ctx = opts->ctx;
const char *fixup_msg;
- size_t commented_len = 0, fixup_off;
- /*
- * amend is non-interactive and not normally used with fixup!
- * or squash! commits, so only comment out those subjects when
- * squashing commit messages.
- */
- if (starts_with(body, "amend!") ||
- ((command == TODO_SQUASH || seen_squash(ctx)) &&
- (starts_with(body, "squash!") || starts_with(body, "fixup!"))))
- commented_len = commit_subject_length(body);
+ size_t commented_len, fixup_off;
+
+ commented_len = squash_subject_comment_len(body,
+ command == TODO_SQUASH || seen_squash(ctx));
- strbuf_addf(buf, "\n%s ", comment_line_str);
- strbuf_addf(buf, _(nth_commit_msg_fmt),
- ++ctx->current_fixup_count + 1);
+ strbuf_addch(buf, '\n');
+ add_squash_message_header(buf, ++ctx->current_fixup_count + 1, 0);
strbuf_addstr(buf, "\n\n");
strbuf_add_commented_lines(buf, body, commented_len, comment_line_str);
/* buf->buf may be reallocated so store an offset into the buffer */
@@ -2083,9 +2096,8 @@ static int update_squash_messages(struct repository *r,
eol = !starts_with(buf.buf, comment_line_str) ?
buf.buf : strchrnul(buf.buf, '\n');
- strbuf_addf(&header, "%s ", comment_line_str);
- strbuf_addf(&header, _(combined_commit_msg_fmt),
- ctx->current_fixup_count + 2);
+ add_squash_combination_header(&header,
+ ctx->current_fixup_count + 2);
strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len);
strbuf_release(&header);
if (is_fixup_flag(command, flag) && !seen_squash(ctx))
@@ -2109,12 +2121,9 @@ static int update_squash_messages(struct repository *r,
repo_unuse_commit_buffer(r, head_commit, head_message);
return error(_("cannot write '%s'"), rebase_path_fixup_msg());
}
- strbuf_addf(&buf, "%s ", comment_line_str);
- strbuf_addf(&buf, _(combined_commit_msg_fmt), 2);
- strbuf_addf(&buf, "\n%s ", comment_line_str);
- strbuf_addstr(&buf, is_fixup_flag(command, flag) ?
- _(skip_first_commit_msg_str) :
- _(first_commit_msg_str));
+ add_squash_combination_header(&buf, 2);
+ strbuf_addch(&buf, '\n');
+ add_squash_message_header(&buf, 1, is_fixup_flag(command, flag));
strbuf_addstr(&buf, "\n\n");
if (is_fixup_flag(command, flag))
strbuf_add_commented_lines(&buf, body, strlen(body),
@@ -2133,9 +2142,8 @@ static int update_squash_messages(struct repository *r,
if (command == TODO_SQUASH || is_fixup_flag(command, flag)) {
res = append_squash_message(&buf, body, command, opts, flag);
} else if (command == TODO_FIXUP) {
- strbuf_addf(&buf, "\n%s ", comment_line_str);
- strbuf_addf(&buf, _(skip_nth_commit_msg_fmt),
- ++ctx->current_fixup_count + 1);
+ strbuf_addch(&buf, '\n');
+ add_squash_message_header(&buf, ++ctx->current_fixup_count + 1, 1);
strbuf_addstr(&buf, "\n\n");
strbuf_add_commented_lines(&buf, body, strlen(body),
comment_line_str);
diff --git a/sequencer.h b/sequencer.h
index 64a9c7fb1b..b01f897020 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -119,6 +119,13 @@ enum todo_command {
TODO_COMMENT
};
+/* Bits for the "flags" member of struct todo_item */
+enum todo_item_flags {
+ TODO_EDIT_MERGE_MSG = (1 << 0),
+ TODO_REPLACE_FIXUP_MSG = (1 << 1),
+ TODO_EDIT_FIXUP_MSG = (1 << 2),
+};
+
struct todo_item {
enum todo_command command;
struct commit *commit;
@@ -208,6 +215,29 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);
*/
void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
+/*
+ * Append the "This is a combination of N commits." banner that "git rebase
+ * -i" writes at the top of a squashed commit's message, commented out with
+ * the comment character.
+ */
+void add_squash_combination_header(struct strbuf *buf, int n);
+
+/*
+ * Append the header (1-based N) that "git rebase -i" writes above each message
+ * when squashing, commented out with the comment character. With SKIP it reads
+ * "The ... commit message will be skipped" for a message that is dropped (a
+ * fixup), otherwise "This is the ... commit message".
+ */
+void add_squash_message_header(struct strbuf *buf, int n, int skip);
+
+/*
+ * Return the length of the leading subject of BODY when it should be commented
+ * out in a squash message, or 0 otherwise. An "amend!" subject always
+ * qualifies; "squash!" and "fixup!" subjects only when SQUASHING, since a
+ * plain fixup chain keeps them.
+ */
+size_t squash_subject_comment_len(const char *body, int squashing);
+
void append_conflicts_hint(struct index_state *istate,
struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);
enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v10 5/5] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-07-20 8:27 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v10.git.git.1784536024.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
By default "git history squash" reuses the oldest commit's message, or
the replacement body from an amend! commit targeting it. When
--reedit-message is given it only reopened that selected message, so the
messages of the other commits in the range were lost.
Gather the message of every commit in the range and build the same editor
template that "git rebase -i --autosquash" shows for a squash, reusing
add_squash_combination_header(), add_squash_message_header() and
squash_subject_comment_len(). Feed the range through
todo_list_rearrange_squash() so that each fixup!, squash! or amend! is
grouped under the commit it targets rather than shown in commit order,
exactly as autosquash would arrange them.
Only the message text differs, the changes are always folded in. A fixup!
message is commented out in full under a "will be skipped" header, a
squash! keeps its body with only the marker subject commented, and an
amend! replaces its target's message unless a squash! already folded into
that target, in which case it behaves like a squash!.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 22 +++-
builtin/history.c | 104 +++++++++++++++++
t/t3455-history-squash.sh | 201 +++++++++++++++++++++++++++++++++
3 files changed, 321 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index e1e930f355..6f3b031d2a 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -134,11 +134,12 @@ already on `topic`. Rev-list options may also be given, but any that would
change how the range is walked are overridden with a warning.
+
The oldest commit's message is preserved by default, except that an `amend!`
-commit targeting it replaces its message. Specify `--reedit-message` to edit
-the resulting message. A merge commit inside the range is folded like any
-other, but the range must have a single base, so a range that reaches more
-than one entry point (for example a side branch that forked before the range
-and was later merged into it) is rejected.
+commit targeting it replaces its message. With `--reedit-message`, an editor
+opens pre-filled with the messages of all the folded commits so you can
+combine them. A merge commit inside the range is folded like any other, but
+the range must have a single base, so a range that reaches more than one entry
+point (for example a side branch that forked before the range and was later
+merged into it) is rejected.
+
A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
targets is also in the range, so the fold does not silently absorb a
@@ -146,6 +147,14 @@ marker meant for a commit outside it. As an exception, a range made up entirely
of markers for one target is combined into a single commit, keeping the last
`amend!` message if there is one.
+
+With `--reedit-message` the template mirrors `git rebase -i --autosquash`:
+each `fixup!`, `squash!`, or `amend!` is grouped under the commit it
+targets rather than shown in commit order. A `fixup!` message is dropped
+(commented out in full), a `squash!` keeps its body with only the marker
+subject commented, and an `amend!` replaces its target's message, unless
+a `squash!` folded into that target first, in which case it keeps its
+body like a `squash!`.
++
A branch or tag that points at a commit inside the range would be left
dangling once those commits are folded away, so with the default
`--update-refs=branches` the command refuses. Rerun with
@@ -162,7 +171,8 @@ OPTIONS
ref updates is generally safe.
`--reedit-message`::
- Open an editor to modify the rewritten commit's message.
+ Open an editor to modify the rewritten commit's message. For `squash`
+ the editor is pre-filled with the messages of all the folded commits.
`--empty=(drop|keep|abort)`::
Control what happens when a commit becomes empty as a result of the
diff --git a/builtin/history.c b/builtin/history.c
index 423c8beaaf..2542ea33a0 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1255,6 +1255,102 @@ static int find_interior_ref(const struct reference *ref, void *cb_data)
return 0;
}
+static bool amend_replaces_target(struct todo_list *todo, int target)
+{
+ int i;
+
+ for (i = target + 1; i < todo->nr &&
+ todo->items[i].command != TODO_PICK; i++) {
+ if (todo->items[i].command == TODO_SQUASH)
+ return false;
+ if (todo->items[i].flags & TODO_REPLACE_FIXUP_MSG)
+ return true;
+ }
+ return false;
+}
+
+static int build_squash_message(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct strbuf *out)
+{
+ struct rev_info revs;
+ struct commit *commit;
+ struct strvec args = STRVEC_INIT;
+ struct todo_list todo = TODO_LIST_INIT;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
+ int i, nr_commits, ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs)))
+ strbuf_addf(&todo.buf, "pick %s\n",
+ oid_to_hex(&commit->object.oid));
+
+ if (todo_list_parse_insn_buffer(repo, &opts, todo.buf.buf, &todo) < 0 ||
+ todo_list_rearrange_squash(&todo) < 0) {
+ ret = error(_("could not prepare the squash message"));
+ goto out;
+ }
+
+ nr_commits = todo.nr;
+ for (i = 0; i < nr_commits; i++) {
+ struct todo_item *item = &todo.items[i];
+ const char *message, *body;
+ size_t commented_len;
+ bool skip, squashing;
+
+ squashing = item->command == TODO_SQUASH ||
+ (item->flags & TODO_REPLACE_FIXUP_MSG);
+ if (item->command == TODO_PICK)
+ skip = amend_replaces_target(&todo, i);
+ else
+ skip = !squashing;
+
+ message = repo_logmsg_reencode(repo, item->commit, NULL, NULL);
+ find_commit_subject(message, &body);
+
+ if (skip)
+ commented_len = strlen(body);
+ else if (squashing)
+ commented_len = squash_subject_comment_len(body, 1);
+ else
+ commented_len = 0;
+
+ if (!i)
+ add_squash_combination_header(out, nr_commits);
+ strbuf_addch(out, '\n');
+ add_squash_message_header(out, i + 1, skip);
+ strbuf_addstr(out, "\n\n");
+ strbuf_add_commented_lines(out, body, commented_len, comment_line_str);
+ strbuf_addstr(out, body + commented_len);
+ strbuf_complete_line(out);
+
+ repo_unuse_commit_buffer(repo, item->commit, message);
+ }
+
+ ret = 0;
+
+out:
+ todo_list_release(&todo);
+ replay_opts_release(&opts);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
@@ -1338,6 +1434,14 @@ static int cmd_history_squash(int argc,
}
}
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ strbuf_reset(&message);
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
+ message_template = message.buf;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
index 9c362f3094..6fce924327 100755
--- a/t/t3455-history-squash.sh
+++ b/t/t3455-history-squash.sh
@@ -271,6 +271,207 @@ test_expect_success 'preserves authorship of the oldest commit' '
test_cmp expect actual
'
+test_expect_success '--reedit-message offers every folded-in message' '
+ git reset --hard start &&
+ stage_file b &&
+ git commit -m "re-one subject" -m "re-one body line" &&
+ test_commit --no-tag re-two file c &&
+ test_commit re-three file d &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited &&
+ echo combined >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 3 commits.
+ # This is the 1st commit message:
+
+ re-one subject
+
+ re-one body line
+
+ # This is the commit message #2:
+
+ re-two
+
+ # This is the commit message #3:
+
+ re-three
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_subjects -1 <<-\EOF
+ combined
+ EOF
+'
+
+test_expect_success '--reedit-message handles fixup!, squash! and amend! like rebase' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
+ stage_file c &&
+ commit_with_message "fixup! mark-base\n\nfixup body\n" &&
+ stage_file d &&
+ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+ stage_file e &&
+ commit_with_message "amend! mark-base\n\namended message\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ mark-base
+
+ # The commit message #2 will be skipped:
+
+ # fixup! mark-base
+ #
+ # fixup body
+
+ # This is the commit message #3:
+
+ # squash! mark-base
+
+ squash remark
+
+ # This is the commit message #4:
+
+ # amend! mark-base
+
+ amended message
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_messages -1 <<-\EOF
+ mark-base
+
+ squash remark
+
+ amended message
+
+ EOF
+'
+
+test_expect_success '--reedit-message groups fixups under their targets' '
+ git reset --hard start &&
+ test_commit --no-tag alpha file a1 &&
+ test_commit --no-tag beta file b1 &&
+ stage_file a2 &&
+ commit_with_message "fixup! alpha\n" &&
+ stage_file b2 &&
+ commit_with_message "fixup! beta\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ alpha
+
+ # The commit message #2 will be skipped:
+
+ # fixup! alpha
+
+ # This is the commit message #3:
+
+ beta
+
+ # The commit message #4 will be skipped:
+
+ # fixup! beta
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited
+'
+
+test_expect_success '--reedit-message lets amend! replace its target message' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
+ stage_file c &&
+ commit_with_message "amend! mark-base\n\namended message\n" &&
+ stage_file d &&
+ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 3 commits.
+ # The 1st commit message will be skipped:
+
+ # mark-base
+
+ # This is the commit message #2:
+
+ # amend! mark-base
+
+ amended message
+
+ # This is the commit message #3:
+
+ # squash! mark-base
+
+ squash remark
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_messages -1 <<-\EOF
+ amended message
+
+ squash remark
+
+ EOF
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ write_script editor <<-\EOF &&
+ >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ test_must_fail git history squash --reedit-message start.. &&
+
+ test_cmp_rev "$head_before" HEAD
+'
+
test_expect_success '--update-refs=head only moves HEAD' '
git reset --hard three &&
git branch -f other HEAD &&
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] userdiff: add support for Swift
From: Johannes Sixt @ 2026-07-20 8:49 UTC (permalink / raw)
To: Shlok Kulshreshtha
Cc: D. Ben Knoble, Junio C Hamano, René Scharfe, Eric Sunshine,
Scott L. Burson, git
In-Reply-To: <2a3a73c5-5e90-44a3-bf6a-6e98ce5e5a59@kdbg.org>
Am 18.07.26 um 20:11 schrieb Johannes Sixt:
> Am 17.07.26 um 16:02 schrieb Shlok Kulshreshtha:
>> +PATTERNS("swift",
>> + "^[ \t]*((@[A-Za-z_][A-Za-z0-9_]*(\\([^()]*\\))?[ \t]+)*([a-z]+[ \t]+)*(func|init|deinit|subscript|class|struct|enum|protocol|extension|actor)[ \t(?!<].*)$",
>
> This looks good.
>
> Notice, however, how the regular expression matcher has to backtrack on
> even simple lines such as
[...]
> It may be worth considering to enumerate all keywords and permit any run
> of them:
Let me back-paddle on this one. As I said, the original RE is good. I am
making up a problem here without providing evidence. Modern RE matchers
may be clever enough that there is no problem. If it turns out there is
a problem, we can improve later something that already works.
-- Hannes
^ permalink raw reply
* [PATCH 0/2] remote: resolve url push tracking
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
After renaming remotes, git status may stop showing the push branch even
though Git still pushes to the right URL, use the remote with the same URL
to find it.
Harald Nordgren (2):
remote: pass repository to push tracking helper
remote: resolve URL-valued push tracking remotes
Documentation/revisions.adoc | 3 +
remote.c | 36 ++++++++++--
remote.h | 2 +
t/t5505-remote.sh | 104 +++++++++++++++++++++++++++++++++++
transport.c | 5 +-
5 files changed, 144 insertions(+), 6 deletions(-)
base-commit: 41365c2a9ba347870b80881c0d67454edd22fd49
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2358%2FHaraldNordgren%2Fremote-resolve-url-push-tracking-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2358/HaraldNordgren/remote-resolve-url-push-tracking-v1
Pull-Request: https://github.com/git/git/pull/2358
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/2] remote: pass repository to push tracking helper
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2358.git.git.1784538618.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The push tracking helper currently only needs the push remote. However,
resolving a URL-valued remote requires access to the repository's list
of configured remotes.
Pass the repository through the existing callers and mark the parameter
as unused for now. This prepares the helper for that lookup without
changing its behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
remote.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/remote.c b/remote.c
index e6c52c850c..89d0f9e2d8 100644
--- a/remote.c
+++ b/remote.c
@@ -1887,7 +1887,8 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
-static char *tracking_for_push_dest(struct remote *remote,
+static char *tracking_for_push_dest(struct repository *repo UNUSED,
+ struct remote *remote,
const char *refname,
struct strbuf *err)
{
@@ -1925,13 +1926,13 @@ static char *branch_get_push_1(struct repository *repo,
_("push refspecs for '%s' do not include '%s'"),
remote->name, branch->name);
- ret = tracking_for_push_dest(remote, dst, err);
+ ret = tracking_for_push_dest(repo, remote, dst, err);
free(dst);
return ret;
}
if (remote->mirror)
- return tracking_for_push_dest(remote, branch->refname, err);
+ return tracking_for_push_dest(repo, remote, branch->refname, err);
switch (push_default) {
case PUSH_DEFAULT_NOTHING:
@@ -1939,7 +1940,7 @@ static char *branch_get_push_1(struct repository *repo,
case PUSH_DEFAULT_MATCHING:
case PUSH_DEFAULT_CURRENT:
- return tracking_for_push_dest(remote, branch->refname, err);
+ return tracking_for_push_dest(repo, remote, branch->refname, err);
case PUSH_DEFAULT_UPSTREAM:
return xstrdup_or_null(branch_get_upstream(branch, err));
@@ -1953,7 +1954,7 @@ static char *branch_get_push_1(struct repository *repo,
up = branch_get_upstream(branch, err);
if (!up)
return NULL;
- cur = tracking_for_push_dest(remote, branch->refname, err);
+ cur = tracking_for_push_dest(repo, remote, branch->refname, err);
if (!cur)
return NULL;
if (strcmp(cur, up)) {
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] bisect: add --auto-reset to leave when done
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2335.v2.git.git.1784312854.gitgitgadget@gmail.com>
Add a --reset-when-found option to git bisect that resets the bisect session
when culprit is found.
Changes in v3:
* Rename --auto-reset to --reset-when-found, including internal names.
* Defer git bisect run cleanup until captured output is printed and
BISECT_RUN is closed. Drop the open-descriptor preparatory change,
retaining the existing filename-based output handling.
Changes in v2:
* Add option --auto-reset[=<where>] with option to go to final commit as
well as original.
* Refactored tests.
Harald Nordgren (2):
bisect: let bisect_reset() optionally check out quietly
bisect: add --reset-when-found to leave when done
Documentation/git-bisect.adoc | 14 +++-
bisect.c | 2 +
builtin/bisect.c | 146 +++++++++++++++++++++++++++++-----
t/t6030-bisect-porcelain.sh | 109 +++++++++++++++++++++++++
4 files changed, 250 insertions(+), 21 deletions(-)
base-commit: 41365c2a9ba347870b80881c0d67454edd22fd49
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2335%2FHaraldNordgren%2Fbisect-auto-reset-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2335/HaraldNordgren/bisect-auto-reset-v3
Pull-Request: https://github.com/git/git/pull/2335
Range-diff vs v2:
1: 0de8b12f65 < -: ---------- bisect: read run output from the open descriptor
2: 8a2dcdf305 = 1: 59920c51ae bisect: let bisect_reset() optionally check out quietly
3: 5b3704fbd4 ! 2: 542f4b2c80 bisect: add --auto-reset to leave when done
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- bisect: add --auto-reset to leave when done
+ bisect: add --reset-when-found to leave when done
When a bisection finishes, "git bisect" reports the first bad commit
but leaves the session active until "git bisect reset" is run by hand.
- Add an "--auto-reset[=<where>]" option, accepted by both "git bisect
- start" and "git bisect run", that resets as soon as the first bad commit
- is found. The "original" value returns to the commit checked out before
- "git bisect start", while "found" leaves the first bad commit checked
- out; omitting the value defaults to "original".
+ Add a "--reset-when-found[=<where>]" option, accepted by both "git
+ bisect start" and "git bisect run", that resets as soon as the first
+ bad commit is found. The "original" value returns to the commit checked
+ out before "git bisect start", while "found" leaves the first bad commit
+ checked out; omitting the value defaults to "original".
- Persist the selected target in a BISECT_AUTO_RESET state file and perform
- the reset quietly. Reject this option together with "--no-checkout",
- since that mode must not check out either target.
+ Persist the selected target in a BISECT_RESET_WHEN_FOUND state file
+ and perform the reset quietly.
+
+ For "git bisect run", defer the reset until after the captured output
+ is printed and BISECT_RUN is closed. This lets cleanup remove the file
+ on systems that cannot unlink an open file.
+
+ Reject this option together with "--no-checkout", since that mode must
+ not check out either target.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/git-bisect.adoc: SYNOPSIS
[synopsis]
git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
-+ [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
++ [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-(good|old) | --term-(bad|new)]
@@ Documentation/git-bisect.adoc: git bisect reset [<commit>]
git bisect replay <logfile>
git bisect log
-git bisect run <cmd> [<arg>...]
-+git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]
++git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]
git bisect help
DESCRIPTION
@@ Documentation/git-bisect.adoc: ignored.
This option is particularly useful in avoiding false positives when a merged
branch contained broken or non-buildable commits, but the merge itself was OK.
-+`--auto-reset[=<where>]`::
++`--reset-when-found[=<where>]`::
+ Once the first bad commit is found, report it and clean up the
+ bisection state. `<where>` may be `original` to return to the commit
+ checked out before `git bisect start`, or `found` to leave the first
@@ bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
-+static GIT_PATH_FUNC(git_path_bisect_auto_reset, "BISECT_AUTO_RESET")
++static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static void read_bisect_paths(struct strvec *array)
{
@@ bisect.c: int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
-+ unlink_or_warn(git_path_bisect_auto_reset());
++ unlink_or_warn(git_path_bisect_reset_when_found());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
-+static GIT_PATH_FUNC(git_path_bisect_auto_reset, "BISECT_AUTO_RESET")
++static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_START_USAGE \
N_("git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]\n" \
- " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]")
-+ " [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
++ " [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
#define BUILTIN_GIT_BISECT_BAD_USAGE \
N_("git bisect (bad|new|<term-new>) [<rev>]")
#define BUILTIN_GIT_BISECT_GOOD_USAGE \
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
"git bisect log"
#define BUILTIN_GIT_BISECT_RUN_USAGE \
- N_("git bisect run <cmd> [<arg>...]")
-+ N_("git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]")
++ N_("git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]")
#define BUILTIN_GIT_BISECT_HELP_USAGE \
"git bisect help"
@@ builtin/bisect.c: static const char * const git_bisect_usage[] = {
NULL
};
-+enum auto_reset_mode {
-+ AUTO_RESET_NONE,
-+ AUTO_RESET_ORIGINAL,
-+ AUTO_RESET_FOUND,
++enum reset_when_found_mode {
++ RESET_WHEN_FOUND_NONE,
++ RESET_WHEN_FOUND_TO_ORIGINAL,
++ RESET_WHEN_FOUND_TO_FOUND,
+};
+
struct add_bisect_ref_data {
@@ builtin/bisect.c: static int bisect_reset(const char *commit, int quiet)
return bisect_clean_state();
}
-+static int parse_auto_reset(const char *value, enum auto_reset_mode *mode)
++static int parse_reset_when_found(const char *value,
++ enum reset_when_found_mode *mode)
+{
+ if (!strcmp(value, "original"))
-+ *mode = AUTO_RESET_ORIGINAL;
++ *mode = RESET_WHEN_FOUND_TO_ORIGINAL;
+ else if (!strcmp(value, "found"))
-+ *mode = AUTO_RESET_FOUND;
++ *mode = RESET_WHEN_FOUND_TO_FOUND;
+ else
-+ return error(_("invalid value for '--auto-reset': '%s'"), value);
++ return error(_("invalid value for '--reset-when-found': '%s'"),
++ value);
+
+ return 0;
+}
+
-+static const char *auto_reset_mode_name(enum auto_reset_mode mode)
++static const char *reset_when_found_mode_name(enum reset_when_found_mode mode)
+{
+ switch (mode) {
-+ case AUTO_RESET_ORIGINAL:
++ case RESET_WHEN_FOUND_TO_ORIGINAL:
+ return "original";
-+ case AUTO_RESET_FOUND:
++ case RESET_WHEN_FOUND_TO_FOUND:
+ return "found";
-+ case AUTO_RESET_NONE:
-+ BUG("no name for unset auto-reset mode");
++ case RESET_WHEN_FOUND_NONE:
++ BUG("no name for unset reset-when-found mode");
+ }
-+ BUG("unknown auto-reset mode %d", mode);
++ BUG("unknown reset-when-found mode %d", mode);
+}
+
-+static int bisect_auto_reset(struct bisect_terms *terms)
++static int bisect_reset_when_found(struct bisect_terms *terms)
+{
+ struct strbuf value = STRBUF_INIT;
-+ enum auto_reset_mode mode;
++ enum reset_when_found_mode mode;
+ char *commit = NULL;
+ int res;
+
-+ if (strbuf_read_file(&value, git_path_bisect_auto_reset(), 0) < 0) {
++ if (strbuf_read_file(&value, git_path_bisect_reset_when_found(), 0) < 0) {
+ res = error_errno(_("could not read '%s'"),
-+ git_path_bisect_auto_reset());
++ git_path_bisect_reset_when_found());
+ goto cleanup;
+ }
+ strbuf_trim(&value);
-+ if (parse_auto_reset(value.buf, &mode)) {
++ if (parse_reset_when_found(value.buf, &mode)) {
+ res = -1;
+ goto cleanup;
+ }
+
-+ if (mode == AUTO_RESET_FOUND)
++ if (mode == RESET_WHEN_FOUND_TO_FOUND)
+ commit = xstrfmt("refs/bisect/%s", terms->term_bad);
+ res = bisect_reset(commit, 1);
+
@@ builtin/bisect.c: static int bisect_reset(const char *commit, int quiet)
static void log_commit(FILE *fp,
const char *fmt, const char *state,
struct commit *commit)
+@@ builtin/bisect.c: static int bisect_successful(struct bisect_terms *terms)
+ return res;
+ }
+
+-static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
++static enum bisect_error bisect_next(struct bisect_terms *terms,
++ const char *prefix, bool defer_reset)
+ {
+ enum bisect_error res;
+
@@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
-+ if (!res && !is_empty_or_missing_file(git_path_bisect_auto_reset()))
-+ res = bisect_auto_reset(terms);
++ if (!res && !defer_reset &&
++ !is_empty_or_missing_file(git_path_bisect_reset_when_found()))
++ res = bisect_reset_when_found(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
+@@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
+ return res;
+ }
+
+-static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
++static enum bisect_error bisect_auto_next(struct bisect_terms *terms,
++ const char *prefix, bool defer_reset)
+ {
+ if (bisect_next_check(terms, NULL)) {
+ bisect_print_status(terms);
+ return BISECT_OK;
+ }
+
+- return bisect_next(terms, prefix);
++ return bisect_next(terms, prefix, defer_reset);
+ }
+
+ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
{
int no_checkout = 0;
int first_parent_only = 0;
-+ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
++ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos;
enum bisect_error res = BISECT_OK;
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
no_checkout = 1;
} else if (!strcmp(arg, "--first-parent")) {
first_parent_only = 1;
-+ } else if (!strcmp(arg, "--auto-reset")) {
-+ auto_reset = AUTO_RESET_ORIGINAL;
-+ } else if (skip_prefix(arg, "--auto-reset=", &arg)) {
-+ if (parse_auto_reset(arg, &auto_reset)) {
++ } else if (!strcmp(arg, "--reset-when-found")) {
++ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
++ } else if (skip_prefix(arg, "--reset-when-found=", &arg)) {
++ if (parse_reset_when_found(arg, &reset_when_found)) {
+ res = BISECT_FAILED;
+ goto finish;
+ }
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
break;
}
}
-+ if (auto_reset != AUTO_RESET_NONE && no_checkout) {
-+ res = error(_("'--auto-reset' cannot be used with '--no-checkout'"));
++ if (reset_when_found != RESET_WHEN_FOUND_NONE && no_checkout) {
++ res = error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
+ goto finish;
+ }
pathspec_pos = i;
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
if (first_parent_only)
write_file(git_path_bisect_first_parent(), "\n");
-+ if (auto_reset != AUTO_RESET_NONE)
-+ write_file(git_path_bisect_auto_reset(), "%s\n",
-+ auto_reset_mode_name(auto_reset));
++ if (reset_when_found != RESET_WHEN_FOUND_NONE)
++ write_file(git_path_bisect_reset_when_found(), "%s\n",
++ reset_when_found_mode_name(reset_when_found));
+
if (no_checkout) {
if (repo_get_oid(the_repository, start_head.buf, &oid) < 0) {
res = error(_("invalid ref: '%s'"), start_head.buf);
+@@ builtin/bisect.c: finish:
+ if (res)
+ return res;
+
+- res = bisect_auto_next(terms, NULL);
++ res = bisect_auto_next(terms, NULL, false);
+ if (!is_bisect_success(res))
+ bisect_clean_state();
+ return res;
+@@ builtin/bisect.c: static int bisect_autostart(struct bisect_terms *terms)
+ }
+
+ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
+- const char **argv)
++ const char **argv, bool defer_reset)
+ {
+ const char *state;
+ int i, verify_expected = 1;
+@@ builtin/bisect.c: static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
+ }
+
+ oid_array_clear(&revs);
+- return bisect_auto_next(terms, NULL);
++ return bisect_auto_next(terms, NULL, defer_reset);
+ }
+
+ static enum bisect_error bisect_log(void)
+@@ builtin/bisect.c: static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
+ if (res)
+ return BISECT_FAILED;
+
+- return bisect_auto_next(terms, NULL);
++ return bisect_auto_next(terms, NULL, false);
+ }
+
+ static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
+@@ builtin/bisect.c: static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
+ strvec_push(&argv_state, argv[i]);
+ }
+ }
+- res = bisect_state(terms, argv_state.nr, argv_state.v);
++ res = bisect_state(terms, argv_state.nr, argv_state.v, false);
+
+ strvec_clear(&argv_state);
+ return res;
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
-+ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
-+ const char *auto_reset_arg;
++ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
++ const char *reset_when_found_arg;
const char *new_state;
int temporary_stdout_fd, saved_stdout;
int is_first_run = 1;
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
if (bisect_next_check(terms, NULL))
return BISECT_FAILED;
-+ if (argc && !strcmp(argv[0], "--auto-reset"))
-+ auto_reset = AUTO_RESET_ORIGINAL;
-+ else if (argc && skip_prefix(argv[0], "--auto-reset=", &auto_reset_arg)) {
-+ if (parse_auto_reset(auto_reset_arg, &auto_reset))
++ if (argc && !strcmp(argv[0], "--reset-when-found"))
++ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
++ else if (argc && skip_prefix(argv[0], "--reset-when-found=",
++ &reset_when_found_arg)) {
++ if (parse_reset_when_found(reset_when_found_arg, &reset_when_found))
+ return BISECT_FAILED;
+ }
+
-+ if (auto_reset != AUTO_RESET_NONE) {
++ if (reset_when_found != RESET_WHEN_FOUND_NONE) {
+ if (refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"))
-+ return error(_("'--auto-reset' cannot be used with '--no-checkout'"));
-+ write_file(git_path_bisect_auto_reset(), "%s\n",
-+ auto_reset_mode_name(auto_reset));
++ return error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
++ write_file(git_path_bisect_reset_when_found(), "%s\n",
++ reset_when_found_mode_name(reset_when_found));
+ argc--;
+ argv++;
+ }
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
if (!argc) {
error(_("bisect run failed: no command provided."));
return BISECT_FAILED;
+@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
+ saved_stdout = dup(1);
+ dup2(temporary_stdout_fd, 1);
+
+- res = bisect_state(terms, 1, &new_state);
++ res = bisect_state(terms, 1, &new_state, true);
+
+ fflush(stdout);
+ dup2(saved_stdout, 1);
+@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
+ res = BISECT_OK;
+ } else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
+ printf(_("bisect found first '%s' commit\n"), terms->term_bad);
+- res = BISECT_OK;
++ if (!is_empty_or_missing_file(git_path_bisect_reset_when_found()) &&
++ bisect_reset_when_found(terms))
++ res = BISECT_FAILED;
++ else
++ res = BISECT_OK;
+ } else if (res) {
+ error(_("bisect run failed: 'git bisect %s'"
+ " exited with error code %d"), new_state, res);
+@@ builtin/bisect.c: static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref
+ return error(_("'%s' requires 0 arguments"),
+ "git bisect next");
+ get_terms(&terms);
+- res = bisect_next(&terms, prefix);
++ res = bisect_next(&terms, prefix, false);
+ free_terms(&terms);
+ return res;
+ }
+@@ builtin/bisect.c: int cmd_bisect(int argc,
+ !one_of(argv[0], terms.term_good, terms.term_bad, NULL))
+ usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
+ options, argv[0]);
+- res = bisect_state(&terms, argc, argv);
++ res = bisect_state(&terms, argc, argv, false);
+ free_terms(&terms);
+ } else {
+ argc--;
## t/t6030-bisect-porcelain.sh ##
@@ t/t6030-bisect-porcelain.sh: test_bisect_usage () {
@@ t/t6030-bisect-porcelain.sh: test_bisect_usage () {
+ git bisect bad
+}
+
-+bisect_run_auto_reset () {
++bisect_run_reset_when_found () {
+ write_script test_script.sh <<-\EOF &&
+ ! grep Another hello >/dev/null
+ EOF
+ git bisect start $HASH4 $HASH2 &&
+ git bisect run "$1" ./test_script.sh >my_bisect_log.txt &&
-+ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt
++ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt &&
++ test_bisect_state_missing BISECT_RUN
+}
+
-+test_auto_reset_fails () {
++test_reset_when_found_fails () {
+ local pattern="$1" &&
+ local state_file="$2" &&
+ shift 2 &&
@@ t/t6030-bisect-porcelain.sh: test_expect_success '"git bisect run" simple case'
git bisect reset
'
-+test_expect_success '"git bisect start --auto-reset" defaults to original' '
++test_expect_success '"git bisect start --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ git checkout main &&
-+ bisect_start_and_finish --auto-reset &&
++ bisect_start_and_finish --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START &&
+
-+ bisect_start_and_finish --auto-reset=original &&
++ bisect_start_and_finish --reset-when-found=original &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect start --auto-reset=found" leaves first bad checked out' '
++test_expect_success '"git bisect start --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_start_and_finish --auto-reset=found &&
++ bisect_start_and_finish --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect run --auto-reset" defaults to original' '
++test_expect_success '"git bisect run --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_run_auto_reset --auto-reset &&
++ bisect_run_reset_when_found --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect run --auto-reset=found" leaves first bad checked out' '
++test_expect_success '"git bisect run --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_run_auto_reset --auto-reset=found &&
++ bisect_run_reset_when_found --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '--auto-reset rejects an unknown reset target' '
++test_expect_success '--reset-when-found rejects an unknown reset target' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ test_auto_reset_fails \
-+ "invalid value for.*--auto-reset.*unknown" BISECT_START \
-+ git bisect start --auto-reset=unknown $HASH4 $HASH2 &&
++ test_reset_when_found_fails \
++ "invalid value for.*--reset-when-found.*unknown" BISECT_START \
++ git bisect start --reset-when-found=unknown $HASH4 $HASH2 &&
+
+ git bisect start $HASH4 $HASH2 &&
-+ test_auto_reset_fails \
-+ "invalid value for.*--auto-reset.*unknown" BISECT_AUTO_RESET \
-+ git bisect run --auto-reset=unknown true
++ test_reset_when_found_fails \
++ "invalid value for.*--reset-when-found.*unknown" \
++ BISECT_RESET_WHEN_FOUND \
++ git bisect run --reset-when-found=unknown true
+'
+
-+test_expect_success '--auto-reset cannot be used with --no-checkout' '
++test_expect_success '--reset-when-found cannot be used with --no-checkout' '
+ test_when_finished "git bisect reset" &&
-+ test_auto_reset_fails \
++ test_reset_when_found_fails \
+ "cannot be used with.*--no-checkout" BISECT_START \
-+ git bisect start --auto-reset=original --no-checkout $HASH4 $HASH2 &&
++ git bisect start --reset-when-found=original --no-checkout $HASH4 $HASH2 &&
+
+ git bisect start --no-checkout $HASH4 $HASH2 &&
-+ test_auto_reset_fails \
-+ "cannot be used with.*--no-checkout" BISECT_AUTO_RESET \
-+ git bisect run --auto-reset=found true
++ test_reset_when_found_fails \
++ "cannot be used with.*--no-checkout" BISECT_RESET_WHEN_FOUND \
++ git bisect run --reset-when-found=found true
+'
+
-+test_expect_success 'without --auto-reset the bisection state is kept' '
++test_expect_success 'without --reset-when-found the bisection state is kept' '
+ test_when_finished "git bisect reset" &&
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
-+test_expect_success '--auto-reset does not leak into a later bisection' '
++test_expect_success '--reset-when-found does not leak into a later bisection' '
+ test_when_finished "git bisect reset; git checkout main" &&
-+ bisect_start_and_finish --auto-reset &&
++ bisect_start_and_finish --reset-when-found &&
+
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/2] bisect: let bisect_reset() optionally check out quietly
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v3.git.git.1784538619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a "quiet" parameter to bisect_reset() that passes "--quiet" to the
checkout restoring the original HEAD, suppressing its progress and
branch-status output.
No caller sets the flag yet, so behavior is unchanged.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/bisect.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 798e28f501..0e49ca23ae 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -234,7 +234,7 @@ static int write_terms(const char *bad, const char *good)
return res;
}
-static int bisect_reset(const char *commit)
+static int bisect_reset(const char *commit, int quiet)
{
struct strbuf branch = STRBUF_INIT;
@@ -255,8 +255,10 @@ static int bisect_reset(const char *commit)
struct child_process cmd = CHILD_PROCESS_INIT;
cmd.git_cmd = 1;
- strvec_pushl(&cmd.args, "checkout", "--ignore-other-worktrees",
- branch.buf, "--", NULL);
+ strvec_pushl(&cmd.args, "checkout", "--ignore-other-worktrees", NULL);
+ if (quiet)
+ strvec_push(&cmd.args, "--quiet");
+ strvec_pushl(&cmd.args, branch.buf, "--", NULL);
if (run_command(&cmd)) {
error(_("could not check out original"
" HEAD '%s'. Try 'git bisect"
@@ -1089,7 +1091,7 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
if (is_empty_or_missing_file(filename))
return error(_("cannot read file '%s' for replaying"), filename);
- if (bisect_reset(NULL))
+ if (bisect_reset(NULL, 0))
return BISECT_FAILED;
fp = fopen(filename, "r");
@@ -1338,7 +1340,7 @@ static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNU
if (argc > 1)
return error(_("'%s' requires either no argument or a commit"),
"git bisect reset");
- return bisect_reset(argc ? argv[0] : NULL);
+ return bisect_reset(argc ? argv[0] : NULL, 0);
}
static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNUSED,
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] remote: resolve URL-valued push tracking remotes
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2358.git.git.1784538618.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
A branch may name its push destination with a URL instead of a
configured remote. This is useful in fork workflows, where the original
remote is renamed to "upstream", the fork is added as "origin", and an
existing branch.<name>.pushRemote continues to contain the fork URL.
Git can still push through the anonymous remote created for that URL.
However, the anonymous remote has no fetch refspec. Git therefore cannot
resolve @{push} to origin/<branch> or update that remote-tracking branch
after a push. The push can succeed, or report that everything is up to
date, while status continues to compare against a stale tracking ref or
cannot show the push branch at all.
A uniquely matching configured remote already provides the missing
mapping. Use its fetch refspec when resolving the push tracking branch
and when updating tracking refs after a push. This changes neither the
push destination nor configuration. Keep the existing behavior when no
remote matches or multiple remotes share the URL, since either case is
ambiguous.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/revisions.adoc | 3 +
remote.c | 27 ++++++++-
remote.h | 2 +
t/t5505-remote.sh | 104 +++++++++++++++++++++++++++++++++++
transport.c | 5 +-
5 files changed, 139 insertions(+), 2 deletions(-)
diff --git a/Documentation/revisions.adoc b/Documentation/revisions.adoc
index 6ea6c7cead..b691691c8c 100644
--- a/Documentation/revisions.adoc
+++ b/Documentation/revisions.adoc
@@ -127,6 +127,9 @@ some output processing may assume ref names in UTF-8.
`git push` were run while `branchname` was checked out (or the current
`HEAD` if no branchname is specified). Like for '@\{upstream\}', we report
the remote-tracking branch that corresponds to that branch at the remote.
+ If the push remote is specified as a URL, the fetch refspec of a uniquely
+ matching configured remote is used to find and update the remote-tracking
+ branch.
+
Here's an example to make it more clear:
+
diff --git a/remote.c b/remote.c
index 89d0f9e2d8..03908dfe8d 100644
--- a/remote.c
+++ b/remote.c
@@ -1887,13 +1887,38 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
-static char *tracking_for_push_dest(struct repository *repo UNUSED,
+struct remote *repo_remote_for_push_tracking(struct repository *repo,
+ struct remote *remote)
+{
+ struct remote *first_match = NULL;
+ struct remote_state *remote_state = repo->remote_state;
+
+ if (remote->origin != REMOTE_UNCONFIGURED || remote->url.nr != 1)
+ return remote;
+
+ for (int i = 0; i < remote_state->remotes_nr; i++) {
+ struct remote *candidate = remote_state->remotes[i];
+
+ if (!candidate || candidate == remote ||
+ !remote_is_configured(candidate, 0) ||
+ !remote_has_url(candidate, remote->url.v[0]))
+ continue;
+ if (first_match)
+ return remote;
+ first_match = candidate;
+ }
+
+ return first_match ? first_match : remote;
+}
+
+static char *tracking_for_push_dest(struct repository *repo,
struct remote *remote,
const char *refname,
struct strbuf *err)
{
char *ret;
+ remote = repo_remote_for_push_tracking(repo, remote);
ret = apply_refspecs(&remote->fetch, refname);
if (!ret)
return error_buf(err,
diff --git a/remote.h b/remote.h
index 72a54d84ad..cca02033b9 100644
--- a/remote.h
+++ b/remote.h
@@ -345,6 +345,8 @@ char *remote_ref_for_branch(struct branch *branch, int for_push);
const char *repo_default_remote(struct repository *repo);
const char *repo_remote_from_url(struct repository *repo, const char *url);
+struct remote *repo_remote_for_push_tracking(struct repository *repo,
+ struct remote *remote);
/* returns true if the given branch has merge configuration given. */
int branch_has_merge_config(struct branch *branch);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index e592c0bcde..e16b3f320a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -24,6 +24,28 @@ setup_repository () {
)
}
+setup_url_pushremote () {
+ rm -rf fork.git client &&
+ git clone --bare one fork.git &&
+ git clone one client &&
+ fork_url="$TRASH_DIRECTORY/fork.git" &&
+ (
+ cd client &&
+ git checkout -b topic --track origin/main &&
+ git commit --allow-empty -m topic-change &&
+ git config push.default current &&
+ git config status.compareBranches "@{upstream} @{push}" &&
+ git config branch.topic.pushRemote "$fork_url" &&
+ git push
+ )
+}
+
+check_status () {
+ git -C client status >actual &&
+ cat >expected &&
+ test_cmp expected actual
+}
+
tokens_match () {
echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
@@ -1018,6 +1040,88 @@ test_expect_success 'rename a remote renames repo remote.pushDefault but keeps g
)
'
+test_expect_success 'URL-valued pushRemote without matching remote is not trackable' '
+ setup_url_pushremote &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}origin/main${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'adding fork remote makes URL-valued pushRemote trackable' '
+ setup_url_pushremote &&
+
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'up-to-date URL push refreshes stale tracking branch' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git commit --allow-empty -m another-topic-change &&
+ git -C ../fork.git fetch ../client topic:topic
+ ) &&
+
+ check_status <<-EOF &&
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 2 commits.
+
+ Your branch is ahead of ${SQ}origin/topic${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+
+ git -C client push >actual 2>&1 &&
+ test_grep "Everything up-to-date" actual &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 2 commits.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'duplicate remote URL leaves URL-valued pushRemote ambiguous' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git remote add duplicate "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+'
+
test_expect_success 'rename handles remote without fetch refspec' '
git clone --bare one no-refspec.git &&
# confirm assumption that bare clone does not create refspec
diff --git a/transport.c b/transport.c
index fc144f0aed..30a4ab2cd5 100644
--- a/transport.c
+++ b/transport.c
@@ -1553,8 +1553,11 @@ int transport_push(struct repository *r,
if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
struct ref *ref;
+ struct remote *tracking_remote = repo_remote_for_push_tracking(
+ r, transport->remote);
+
for (ref = remote_refs; ref; ref = ref->next)
- transport_update_tracking_ref(transport->remote, ref, verbose);
+ transport_update_tracking_ref(tracking_remote, ref, verbose);
}
if (porcelain && !push_ret)
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/2] bisect: add --reset-when-found to leave when done
From: Harald Nordgren via GitGitGadget @ 2026-07-20 9:10 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v3.git.git.1784538619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When a bisection finishes, "git bisect" reports the first bad commit
but leaves the session active until "git bisect reset" is run by hand.
Add a "--reset-when-found[=<where>]" option, accepted by both "git
bisect start" and "git bisect run", that resets as soon as the first
bad commit is found. The "original" value returns to the commit checked
out before "git bisect start", while "found" leaves the first bad commit
checked out; omitting the value defaults to "original".
Persist the selected target in a BISECT_RESET_WHEN_FOUND state file
and perform the reset quietly.
For "git bisect run", defer the reset until after the captured output
is printed and BISECT_RUN is closed. This lets cleanup remove the file
on systems that cannot unlink an open file.
Reject this option together with "--no-checkout", since that mode must
not check out either target.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-bisect.adoc | 14 +++-
bisect.c | 2 +
builtin/bisect.c | 134 ++++++++++++++++++++++++++++++----
t/t6030-bisect-porcelain.sh | 109 +++++++++++++++++++++++++++
4 files changed, 243 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-bisect.adoc b/Documentation/git-bisect.adoc
index d2115b2990..aabddd42ca 100644
--- a/Documentation/git-bisect.adoc
+++ b/Documentation/git-bisect.adoc
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[synopsis]
git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
+ [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-(good|old) | --term-(bad|new)]
@@ -20,7 +20,7 @@ git bisect reset [<commit>]
git bisect (visualize|view)
git bisect replay <logfile>
git bisect log
-git bisect run <cmd> [<arg>...]
+git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]
git bisect help
DESCRIPTION
@@ -385,6 +385,16 @@ ignored.
This option is particularly useful in avoiding false positives when a merged
branch contained broken or non-buildable commits, but the merge itself was OK.
+`--reset-when-found[=<where>]`::
+ Once the first bad commit is found, report it and clean up the
+ bisection state. `<where>` may be `original` to return to the commit
+ checked out before `git bisect start`, or `found` to leave the first
+ bad commit checked out. If `<where>` is omitted, it defaults to
+ `original`.
++
+This option may be given to `git bisect start` or to `git bisect run`. It
+cannot be used for a bisection started with `--no-checkout`.
+
EXAMPLES
--------
diff --git a/bisect.c b/bisect.c
index 94c7028d2a..d426fcd5a9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -488,6 +488,7 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static void read_bisect_paths(struct strvec *array)
{
@@ -1211,6 +1212,7 @@ int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
+ unlink_or_warn(git_path_bisect_reset_when_found());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 0e49ca23ae..de13f22f8a 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -24,11 +24,12 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_reset_when_found, "BISECT_RESET_WHEN_FOUND")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_START_USAGE \
N_("git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]\n" \
- " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]")
+ " [--no-checkout] [--first-parent] [--reset-when-found[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
#define BUILTIN_GIT_BISECT_BAD_USAGE \
N_("git bisect (bad|new|<term-new>) [<rev>]")
#define BUILTIN_GIT_BISECT_GOOD_USAGE \
@@ -48,7 +49,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_LOG_USAGE \
"git bisect log"
#define BUILTIN_GIT_BISECT_RUN_USAGE \
- N_("git bisect run <cmd> [<arg>...]")
+ N_("git bisect run [--reset-when-found[=<where>]] <cmd> [<arg>...]")
#define BUILTIN_GIT_BISECT_HELP_USAGE \
"git bisect help"
@@ -68,6 +69,12 @@ static const char * const git_bisect_usage[] = {
NULL
};
+enum reset_when_found_mode {
+ RESET_WHEN_FOUND_NONE,
+ RESET_WHEN_FOUND_TO_ORIGINAL,
+ RESET_WHEN_FOUND_TO_FOUND,
+};
+
struct add_bisect_ref_data {
struct rev_info *revs;
unsigned int object_flags;
@@ -272,6 +279,61 @@ static int bisect_reset(const char *commit, int quiet)
return bisect_clean_state();
}
+static int parse_reset_when_found(const char *value,
+ enum reset_when_found_mode *mode)
+{
+ if (!strcmp(value, "original"))
+ *mode = RESET_WHEN_FOUND_TO_ORIGINAL;
+ else if (!strcmp(value, "found"))
+ *mode = RESET_WHEN_FOUND_TO_FOUND;
+ else
+ return error(_("invalid value for '--reset-when-found': '%s'"),
+ value);
+
+ return 0;
+}
+
+static const char *reset_when_found_mode_name(enum reset_when_found_mode mode)
+{
+ switch (mode) {
+ case RESET_WHEN_FOUND_TO_ORIGINAL:
+ return "original";
+ case RESET_WHEN_FOUND_TO_FOUND:
+ return "found";
+ case RESET_WHEN_FOUND_NONE:
+ BUG("no name for unset reset-when-found mode");
+ }
+ BUG("unknown reset-when-found mode %d", mode);
+}
+
+static int bisect_reset_when_found(struct bisect_terms *terms)
+{
+ struct strbuf value = STRBUF_INIT;
+ enum reset_when_found_mode mode;
+ char *commit = NULL;
+ int res;
+
+ if (strbuf_read_file(&value, git_path_bisect_reset_when_found(), 0) < 0) {
+ res = error_errno(_("could not read '%s'"),
+ git_path_bisect_reset_when_found());
+ goto cleanup;
+ }
+ strbuf_trim(&value);
+ if (parse_reset_when_found(value.buf, &mode)) {
+ res = -1;
+ goto cleanup;
+ }
+
+ if (mode == RESET_WHEN_FOUND_TO_FOUND)
+ commit = xstrfmt("refs/bisect/%s", terms->term_bad);
+ res = bisect_reset(commit, 1);
+
+cleanup:
+ free(commit);
+ strbuf_release(&value);
+ return res;
+}
+
static void log_commit(FILE *fp,
const char *fmt, const char *state,
struct commit *commit)
@@ -677,7 +739,8 @@ static int bisect_successful(struct bisect_terms *terms)
return res;
}
-static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
+static enum bisect_error bisect_next(struct bisect_terms *terms,
+ const char *prefix, bool defer_reset)
{
enum bisect_error res;
@@ -692,6 +755,9 @@ static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
+ if (!res && !defer_reset &&
+ !is_empty_or_missing_file(git_path_bisect_reset_when_found()))
+ res = bisect_reset_when_found(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
@@ -700,14 +766,15 @@ static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
return res;
}
-static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
+static enum bisect_error bisect_auto_next(struct bisect_terms *terms,
+ const char *prefix, bool defer_reset)
{
if (bisect_next_check(terms, NULL)) {
bisect_print_status(terms);
return BISECT_OK;
}
- return bisect_next(terms, prefix);
+ return bisect_next(terms, prefix, defer_reset);
}
static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
@@ -715,6 +782,7 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
{
int no_checkout = 0;
int first_parent_only = 0;
+ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos;
enum bisect_error res = BISECT_OK;
@@ -747,6 +815,13 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
no_checkout = 1;
} else if (!strcmp(arg, "--first-parent")) {
first_parent_only = 1;
+ } else if (!strcmp(arg, "--reset-when-found")) {
+ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
+ } else if (skip_prefix(arg, "--reset-when-found=", &arg)) {
+ if (parse_reset_when_found(arg, &reset_when_found)) {
+ res = BISECT_FAILED;
+ goto finish;
+ }
} else if (!strcmp(arg, "--term-good") ||
!strcmp(arg, "--term-old")) {
i++;
@@ -784,6 +859,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
break;
}
}
+ if (reset_when_found != RESET_WHEN_FOUND_NONE && no_checkout) {
+ res = error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
+ goto finish;
+ }
pathspec_pos = i;
/*
@@ -861,6 +940,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
if (first_parent_only)
write_file(git_path_bisect_first_parent(), "\n");
+ if (reset_when_found != RESET_WHEN_FOUND_NONE)
+ write_file(git_path_bisect_reset_when_found(), "%s\n",
+ reset_when_found_mode_name(reset_when_found));
+
if (no_checkout) {
if (repo_get_oid(the_repository, start_head.buf, &oid) < 0) {
res = error(_("invalid ref: '%s'"), start_head.buf);
@@ -902,7 +985,7 @@ finish:
if (res)
return res;
- res = bisect_auto_next(terms, NULL);
+ res = bisect_auto_next(terms, NULL, false);
if (!is_bisect_success(res))
bisect_clean_state();
return res;
@@ -941,7 +1024,7 @@ static int bisect_autostart(struct bisect_terms *terms)
}
static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
- const char **argv)
+ const char **argv, bool defer_reset)
{
const char *state;
int i, verify_expected = 1;
@@ -1018,7 +1101,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
}
oid_array_clear(&revs);
- return bisect_auto_next(terms, NULL);
+ return bisect_auto_next(terms, NULL, defer_reset);
}
static enum bisect_error bisect_log(void)
@@ -1107,7 +1190,7 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
if (res)
return BISECT_FAILED;
- return bisect_auto_next(terms, NULL);
+ return bisect_auto_next(terms, NULL, false);
}
static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
@@ -1141,7 +1224,7 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
strvec_push(&argv_state, argv[i]);
}
}
- res = bisect_state(terms, argv_state.nr, argv_state.v);
+ res = bisect_state(terms, argv_state.nr, argv_state.v, false);
strvec_clear(&argv_state);
return res;
@@ -1239,6 +1322,8 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
+ enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
+ const char *reset_when_found_arg;
const char *new_state;
int temporary_stdout_fd, saved_stdout;
int is_first_run = 1;
@@ -1246,6 +1331,23 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
if (bisect_next_check(terms, NULL))
return BISECT_FAILED;
+ if (argc && !strcmp(argv[0], "--reset-when-found"))
+ reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
+ else if (argc && skip_prefix(argv[0], "--reset-when-found=",
+ &reset_when_found_arg)) {
+ if (parse_reset_when_found(reset_when_found_arg, &reset_when_found))
+ return BISECT_FAILED;
+ }
+
+ if (reset_when_found != RESET_WHEN_FOUND_NONE) {
+ if (refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"))
+ return error(_("'--reset-when-found' cannot be used with '--no-checkout'"));
+ write_file(git_path_bisect_reset_when_found(), "%s\n",
+ reset_when_found_mode_name(reset_when_found));
+ argc--;
+ argv++;
+ }
+
if (!argc) {
error(_("bisect run failed: no command provided."));
return BISECT_FAILED;
@@ -1304,7 +1406,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
saved_stdout = dup(1);
dup2(temporary_stdout_fd, 1);
- res = bisect_state(terms, 1, &new_state);
+ res = bisect_state(terms, 1, &new_state, true);
fflush(stdout);
dup2(saved_stdout, 1);
@@ -1320,7 +1422,11 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
res = BISECT_OK;
} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
printf(_("bisect found first '%s' commit\n"), terms->term_bad);
- res = BISECT_OK;
+ if (!is_empty_or_missing_file(git_path_bisect_reset_when_found()) &&
+ bisect_reset_when_found(terms))
+ res = BISECT_FAILED;
+ else
+ res = BISECT_OK;
} else if (res) {
error(_("bisect run failed: 'git bisect %s'"
" exited with error code %d"), new_state, res);
@@ -1379,7 +1485,7 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref
return error(_("'%s' requires 0 arguments"),
"git bisect next");
get_terms(&terms);
- res = bisect_next(&terms, prefix);
+ res = bisect_next(&terms, prefix, false);
free_terms(&terms);
return res;
}
@@ -1482,7 +1588,7 @@ int cmd_bisect(int argc,
!one_of(argv[0], terms.term_good, terms.term_bad, NULL))
usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
options, argv[0]);
- res = bisect_state(&terms, argc, argv);
+ res = bisect_state(&terms, argc, argv, false);
free_terms(&terms);
} else {
argc--;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 081116220a..7dfb871ab9 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -43,6 +43,38 @@ test_bisect_usage () {
test_cmp expect actual
}
+test_bisect_state_file () {
+ test_path_is_file "$(git rev-parse --git-path "$1")"
+}
+
+test_bisect_state_missing () {
+ test_path_is_missing "$(git rev-parse --git-path "$1")"
+}
+
+bisect_start_and_finish () {
+ git bisect start "$1" $HASH4 $HASH2 &&
+ git bisect bad
+}
+
+bisect_run_reset_when_found () {
+ write_script test_script.sh <<-\EOF &&
+ ! grep Another hello >/dev/null
+ EOF
+ git bisect start $HASH4 $HASH2 &&
+ git bisect run "$1" ./test_script.sh >my_bisect_log.txt &&
+ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt &&
+ test_bisect_state_missing BISECT_RUN
+}
+
+test_reset_when_found_fails () {
+ local pattern="$1" &&
+ local state_file="$2" &&
+ shift 2 &&
+ test_must_fail "$@" 2>err &&
+ test_grep -- "$pattern" err &&
+ test_bisect_state_missing "$state_file"
+}
+
test_expect_success 'bisect usage' "
test_bisect_usage 1 git bisect reset extra1 extra2 <<-\EOF &&
error: 'git bisect reset' requires either no argument or a commit
@@ -453,6 +485,83 @@ test_expect_success '"git bisect run" simple case' '
git bisect reset
'
+test_expect_success '"git bisect start --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ git checkout main &&
+ bisect_start_and_finish --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START &&
+
+ bisect_start_and_finish --reset-when-found=original &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect start --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_start_and_finish --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect run --reset-when-found" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_run_reset_when_found --reset-when-found &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect run --reset-when-found=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_run_reset_when_found --reset-when-found=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '--reset-when-found rejects an unknown reset target' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ test_reset_when_found_fails \
+ "invalid value for.*--reset-when-found.*unknown" BISECT_START \
+ git bisect start --reset-when-found=unknown $HASH4 $HASH2 &&
+
+ git bisect start $HASH4 $HASH2 &&
+ test_reset_when_found_fails \
+ "invalid value for.*--reset-when-found.*unknown" \
+ BISECT_RESET_WHEN_FOUND \
+ git bisect run --reset-when-found=unknown true
+'
+
+test_expect_success '--reset-when-found cannot be used with --no-checkout' '
+ test_when_finished "git bisect reset" &&
+ test_reset_when_found_fails \
+ "cannot be used with.*--no-checkout" BISECT_START \
+ git bisect start --reset-when-found=original --no-checkout $HASH4 $HASH2 &&
+
+ git bisect start --no-checkout $HASH4 $HASH2 &&
+ test_reset_when_found_fails \
+ "cannot be used with.*--no-checkout" BISECT_RESET_WHEN_FOUND \
+ git bisect run --reset-when-found=found true
+'
+
+test_expect_success 'without --reset-when-found the bisection state is kept' '
+ test_when_finished "git bisect reset" &&
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
+test_expect_success '--reset-when-found does not leak into a later bisection' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_start_and_finish --reset-when-found &&
+
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
# We want to automatically find the commit that
# added "Ciao" into hello.
test_expect_success '"git bisect run" with more complex "git bisect start"' '
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: Tian Yuchen @ 2026-07-20 9:13 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <alvNq8rXF/jofqUc@szeder.dev>
On 7/19/26 03:02, SZEDER Gábor wrote:
> On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
>> The ce_mode_from_stat() function is a performance-critical static
>> inline helper in 'read-cache.h'. As we migrate configuration
>> variables into the repository struct, this helper needs access
>> to the repository context.
>>
>> Update the signature of ce_mode_from_stat() to take a 'struct
>> repository *' parameter, and update all callers to pass the
>> appropriate repository instance.
>>
>> To prepare for the overhead of replacing cheap global variable
>> accesses with getter functions, the boolean expressions are
>> reordered to evaluate 'S_ISREG(mode)' first.
>>
>> While at it, add a comment for ce_mode_from_stat().
>>
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
>> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
>> Signed-off-by: Tian Yuchen <cat@malon.dev>
>> ---
>
>> diff --git a/read-cache.h b/read-cache.h
>> index 043da1f1aa..94b8d3e547 100644
>> --- a/read-cache.h
>> +++ b/read-cache.h
>> @@ -4,15 +4,24 @@
>> #include "read-cache-ll.h"
>> #include "object.h"
>> #include "pathspec.h"
>> +#include "environment.h"
>>
>> -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
>> +/*
>> + * Determine the appropriate index mode for a file based on its stat()
>> + * information and the existing cache entry (if any).
>> + *
>> + * This function handles degradation for filesystems that lack
>> + * symlink support or reliable executable bits.
>> + */
>> +static inline unsigned int ce_mode_from_stat(struct repository *repo,
>
> This new parameter is not yet used in this function, which causes
> compilation errors in all source files which include "read-cache.h"
> when trying to build this commit using DEVELOPER=1, e.g.:
>
> CC pathspec.o
> In file included from pathspec.c:11:
> read-cache.h: In function ‘ce_mode_from_stat’:
> read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
> 16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
> | ~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:2921: pathspec.o] Error 1
> CC preload-index.o
> In file included from preload-index.c:16:
> read-cache.h: In function ‘ce_mode_from_stat’:
> read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
> 16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
> | ~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:2921: preload-index.o] Error 1
> CC read-cache.o
> In file included from read-cache.c:34:
> read-cache.h: In function ‘ce_mode_from_stat’:
> read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
> 16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
> | ~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:2921: read-cache.o] Error 1
>
Nice catch.
> I think the new parameter should be marked as UNUSED in this patch,
> and then the UNUSED should be dropped in the next, where you start
> using the parameter.
>
>> + const struct cache_entry *ce,
>> unsigned int mode)
>> {
>> extern int trust_executable_bit, has_symlinks;
>> - if (!has_symlinks && S_ISREG(mode) &&
>> + if (S_ISREG(mode) && !has_symlinks &&
>> ce && S_ISLNK(ce->ce_mode))
>> return ce->ce_mode;
>> - if (!trust_executable_bit && S_ISREG(mode)) {
>> + if (S_ISREG(mode) && !trust_executable_bit) {
>> if (ce && S_ISREG(ce->ce_mode))
>> return ce->ce_mode;
>> return create_ce_mode(0666);
>> --
>> 2.43.0
>>
But 'USUSED' cannot be used here since the corresponding header
(git-compat-util.h, or more specifically compat/posix.h) is not included.
Can we write..
(void)repo; /* TODO: use this parameter in the next patch */
..to keep it simple?
Regards, yuchen
^ permalink raw reply
* Re: [PATCH 3/4] last-modified: check pathspec against Bloom filter first
From: Jeff King @ 2026-07-20 9:42 UTC (permalink / raw)
To: Taylor Blau; +Cc: Toon Claes, git, Gusted
In-Reply-To: <alvulw2fk67duo8n@com-79390>
On Sat, Jul 18, 2026 at 04:22:31PM -0500, Taylor Blau wrote:
> I think that we could feasibly get rid of "d" in the output in this
> particular case within last-modified. As you note, the command is marked
> EXPERIMENTAL for a reason, after all ;-).
>
> If we wanted to do that, it should be straightforward to do. I think the
> following (untested) patch would be sufficient:
>
> --- 8< ---
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index adc7cd8c74..0f0c1d1d17 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -103,7 +103,7 @@ struct last_modified_callback_data {
> };
>
> static void add_path_from_diff(struct diff_queue_struct *q,
> - struct diff_options *opt UNUSED, void *data)
> + struct diff_options *opt, void *data)
> {
> struct last_modified *lm = data;
>
> @@ -112,6 +112,11 @@ static void add_path_from_diff(struct diff_queue_struct *q,
> struct last_modified_entry *ent;
> const char *path = p->two->path;
>
> + if (!match_pathspec(opt->repo->index, &opt->pathspec, path,
> + strlen(path), 0, NULL,
> + S_ISDIR(p->two->mode)))
> + continue;
> +
Yeah, that was exactly what I was thinking, but I wasn't sure if
match_pathspec() was the right tool. I mean, obviously it sounds like it
should be from the name, but I don't think it is actually what is used
in tree-diffs! There we have tree-walk.c:do_match() which does some
magic. And match_pathspec() is used more for dir.c callers.
I guess the two are supposed to be equivalent, or else we'd have weird
discrepancies between commands. So maybe just a weird existing oddity
that we don't need to worry about here.
There is one other interesting corner case here. If I do this in
git.git, for example:
git last-modified -t Documentation/technical/
it shows an entry for Documentation/, which we both find weird. And the
patch above would remove that. But it also shows an entry for
Documentation/technical/, which _is_ within the pathspec and would still
be shown after the patch above. That's OK for the optimization we're
talking about (it would be part of the filter key), but I do find it
still a little funny. The invocation above, at least as we used to use
it as blame-tree at GitHub, is really about asking for the entries
inside that directory, not the directory itself.
Perhaps not worth worrying too much about, though. The caller can easily
ignore the extra entry.
> If, on the other hand, we wanted to retain "d" in the output (which I am
> inclined to suggest is a bad idea), we could keep a list of paths which
> are not covered by the given pathspec.
Yeah, your analysis here makes sense, but I agree that it is not worth
retaining "d". Besides reducing our ability to optimize, it is IMHO just
plain confusing to have in the output.
-Peff
^ permalink raw reply
* Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: SZEDER Gábor @ 2026-07-20 9:52 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <8b9528b8-198b-489f-8f0e-fbd0c7d07b64@malon.dev>
On Mon, Jul 20, 2026 at 05:13:17PM +0800, Tian Yuchen wrote:
> On 7/19/26 03:02, SZEDER Gábor wrote:
> > On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
> > > diff --git a/read-cache.h b/read-cache.h
> > > index 043da1f1aa..94b8d3e547 100644
> > > --- a/read-cache.h
> > > +++ b/read-cache.h
> > > @@ -4,15 +4,24 @@
> > > #include "read-cache-ll.h"
> > > #include "object.h"
> > > #include "pathspec.h"
> > > +#include "environment.h"
> > > -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
> > > +/*
> > > + * Determine the appropriate index mode for a file based on its stat()
> > > + * information and the existing cache entry (if any).
> > > + *
> > > + * This function handles degradation for filesystems that lack
> > > + * symlink support or reliable executable bits.
> > > + */
> > > +static inline unsigned int ce_mode_from_stat(struct repository *repo,
> >
> > This new parameter is not yet used in this function, which causes
> > compilation errors in all source files which include "read-cache.h"
> > when trying to build this commit using DEVELOPER=1, e.g.:
> > I think the new parameter should be marked as UNUSED in this patch,
> > and then the UNUSED should be dropped in the next, where you start
> > using the parameter.
> >
> > > + const struct cache_entry *ce,
> > > unsigned int mode)
> > > {
> > > extern int trust_executable_bit, has_symlinks;
> > > - if (!has_symlinks && S_ISREG(mode) &&
> > > + if (S_ISREG(mode) && !has_symlinks &&
> > > ce && S_ISLNK(ce->ce_mode))
> > > return ce->ce_mode;
> > > - if (!trust_executable_bit && S_ISREG(mode)) {
> > > + if (S_ISREG(mode) && !trust_executable_bit) {
> > > if (ce && S_ISREG(ce->ce_mode))
> > > return ce->ce_mode;
> > > return create_ce_mode(0666);
> > > --
> > > 2.43.0
> > >
>
> But 'USUSED' cannot be used here since the corresponding header
> (git-compat-util.h, or more specifically compat/posix.h) is not included.
UNUSED _can_ be used here, because:
- This is a header file, so it's not supposed to be compiled on its
own.
- All C source files including this header file must start with
including "git-compat-util.h", so by the time they include
"read-cache.h", the UNUSED macro is already defined.
^ permalink raw reply
* Re: [PATCH] userdiff: add support for Swift
From: Shlok Kulshreshtha @ 2026-07-20 9:52 UTC (permalink / raw)
To: j6t
Cc: Shlok Kulshreshtha, D. Ben Knoble, Junio C Hamano,
René Scharfe, Eric Sunshine, Scott L. Burson, git
In-Reply-To: <2a3a73c5-5e90-44a3-bf6a-6e98ce5e5a59@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
>> - attributes, with or without arguments, whether on their own line
>> ("@objc" above a "func") or inline with the declaration
> AFAIC, the regular expression does not match attributes on their own
> line. What relevance does this statement have?
You are right. The pattern only matches attributes that are inline with
the declaration. An attribute on its own line is not matched, and does
not need to be, because the declaration line below it matches on its own.
I have reworded this.
> This test contains "RIGHT" twice. This is not good, because we do not
> know which one is picked.
[...]
> Again "RIGHT" twice in a harmful way.
Fixed in a coming v2: swift-init, swift-failable-init and
swift-generic-subscript now contain "RIGHT" only once, on the
declaration line.
> It may be worth considering to enumerate all keywords and permit any
> run of them:
> (public|final|etc.|func|init|...|actor)[ \t(?!<]+)+
Noted, and thanks for the follow-up on this one. I did check it anyway
out of curiosity: with that shape, a line that is only modifiers and
never reaches a real declaration keyword, such as
public var counter = 0
would still match, because it merges modifiers and declaration keywords
into one interchangeable run. The current pattern requires a real
keyword at the end, so that line correctly gets no header. I will keep
the current form for now, and can revisit if the backtracking turns out
to matter in practice.
> You could just throw all of them into a single pattern like this:
> 0[xXoObB][0-9a-fA-F_]+
> except when, for example, 0b1_abc
Right -- that is why I kept them as three separate patterns, so the
digit ranges stay correct (binary [01], octal [0-7]); merging would
mis-tokenize "0b1_abc".
> Is ".5" a correct floating-point number?
No -- Swift requires a leading digit, so ".5" is a syntax error (one must
write "0.5"). Tokenizing it as "." and "5" is therefore fine, and it
does not occur in valid Swift.
> You do not have to account for single-character operators; they are
> automatic. Drop the "?" from the first "=?".
Done in a coming v2, thanks; I had not realized PATTERNS appends
"|[^[:space:]]". It is a nice simplification, and it only touches the
word regex, not the funcname pattern.
Since neither of us speaks Swift, for your ease of judgement I have also
put together some coverage numbers, which the coming v2 cover note will
include:
- Grammar: I went through every declaration form listed in the "Summary
of the Grammar" in Swift's own language reference (func, init incl.
failable/generic, deinit, subscript incl. generic, class, struct,
enum, protocol, extension, actor, operator methods, stacked
modifiers, attributes with and without arguments, "where" clauses,
multi-line signatures -- 26 forms total) and wrote a case for each.
All 26 get the correct header.
- Real-world code: I ran the driver over the last 200 commits touching
*.swift in seven different Swift projects -- Alamofire,
apple/swift-argument-parser, vapor, Kingfisher, RxSwift, SnapKit, and
pointfreeco/swift-composable-architecture -- and checked every hunk
header by hand. Out of 20454 hunks, 15310 got a header, and 15296 of
those (99.9%) named a real declaration. None of the empty-header
hunks turned out to be a real miss (they were things like file
comment blocks, imports, or Package.swift, which have nothing to
attach a header to).
These numbers are unaffected by the changes in this reply: the funcname
pattern is identical in v1 and v2 (only the word regex and the test
files changed), and both measurements are of hunk headers, which come
from the funcname pattern alone. So the coverage above still holds for
v2.
Besides the fixes above, v2 will also carry the reworded attribute
description and the changelog explaining what changed since v1, so the
full picture is in one place when you look at it.
Thanks for the careful review.
Shlok
^ permalink raw reply
* Re: [PATCH 0/4] send-pack: introduce a `no-ref-delta` capability
From: Jeff King @ 2026-07-20 9:57 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano
In-Reply-To: <alvp8KsZPKqCyqma@com-79390>
On Sat, Jul 18, 2026 at 04:02:40PM -0500, Taylor Blau wrote:
> > The problem there is not REF_DELTA itself, but the fact that REF_DELTA
> > allows you to place a base after the delta which depends on it.
>
> Exactly.
>
> > If _that_ is your main concern, would it be worth a tighter capability
> > advertisement that insists that bases come before their deltas (if they
> > are in the pack at all)? We already generate packs that way by default,
> > and it would really just give the server a license to reject these
> > non-standard packs.
>
> That would address the ordering problem, but is weaker than the format
> restriction this receiver wants. Even a backward REF_DELTA requires an
> OID-to-entry lookup, whereas the retained pack's reconstruction metadata
> is addressed by offset alone. Supporting that is possible, of course,
> but adds another way to locate a base.
Yeah, but I don't think it's that much more complicated. You are
collecting the oids of the stuff you index anyway (since that is the
point of indexing), so it is just a matter of storing that in a
searchable data structure.
But what _is_ more complicated is the data dependency. Imagine you have
a pool of workers waiting to do delta resolution and hash computations.
You want to hand off each new object entry you parse to one of the
workers. With OFS_DELTA you know where the base is immediately, and if
its resolution is still pending, you know which worker you handed it off
to. But with REF_DELTA, you don't know which worker is processing your
base until it has finished (since that's when it reports back the oid).
That might or might not matter depending on your caching strategy for
intermediate states. For example, if you're trying to maintain locality
in what you hand to a worker (so if you have a delta chain A-B-C, when
you find C you want to give it to the worker who computed B, because
they may have that intermediate result at hand).
I do think in general that the intermediate-state caching is going to be
the trickiest part of a streaming resolution, though. Even with just
OFS_DELTA, there is no way to know in a single pass that the object at
position N is worth keeping around because N+K is going to reference it,
and K can be arbitrarily large. With Git's pack-objects implementation,
you will generally see delta families grouped together, but that's not
guaranteed.
So I'm growing more convinced that asking the client not to send
REF_DELTAs might open up some more opportunities for optimizations, but:
1. It would be neat if we could see those optimizations in git.git's
receiving code. ;)
2. Without seeing the whole scheme, I am still unclear on how some of
these other complications aren't much worse/harder.
I don't think you're asking for a lot of support on the git.git side, so
in that sense I'm not really opposed. I'm just not excited about
carrying protocol additions that wouldn't be used by git.git itself,
especially if we later find that the capability isn't a perfect match
for the optimizations we want to make.
-Peff
^ permalink raw reply
* git config: unintuitive behaviour with --global and --no-includes
From: Hendrik Jaeger @ 2026-07-20 9:34 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 4788 bytes --]
Hi
I ran into a problem working with lbmk (https://codeberg.org/libreboot/lbmk).
To see whether git is correctly configured, it runs `git config --global user.name` and that failed for my setup.
The reason is that I have user.name and user.email not directly in the normal git config file but in an included file and when given a scope like --global `git config` does not by default check included files.
I tried to create a commented minimal example showing the problem with that:
```
# no config exists
~ % cat .gitconfig
cat: .gitconfig: No such file or directory
~ % cat .gitconfig_personal
cat: .gitconfig_personal: No such file or directory
~ % ls ~/.config/git
ls: cannot access '/home/resu/.config/git': No such file or directory
# config var is not set
~ % git config --show-scope --show-origin user.name
# set user.name
~ % git config --global user.name "Hendrik Jäger"
# check if it is set
~ % cat .gitconfig
[user]
name = Hendrik Jäger
# check where it is set
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig Hendrik Jäger
# check whether we can still retrieve it when explicitly giving the scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Hendrik Jäger
# move setting to non-standard file
~ % cat .gitconfig > .gitconfig_personal
# include that non-standard file
~ % echo '[include]\npath = .gitconfig_personal' >| .gitconfig
# check config status
~ % cat .gitconfig
[include]
path = .gitconfig_personal
~ % cat .gitconfig_personal
[user]
name = Hendrik Jäger
# check whether git still finds that setting
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
# check whether git still finds that setting in the scope it is in
~ % git config --show-scope --show-origin --global user.name
# set it again in the global scope
~ % git config --global user.name "Henk Hunter"
# check again
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig Henk Hunter
# check again with specific scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Henk Hunter
# reset git config and check if the setting is really gone
~ % rm .gitconfig
~ % git config --show-scope --show-origin user.name
# set it again in global scope with different value
~ % git config --global user.name "Henk Hunter"
# check whether setting it was successful
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig Henk Hunter
# check again with specific scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Henk Hunter
# add the include back
~ % echo '[include]\npath = .gitconfig_personal' >> .gitconfig
# check config status
~ % cat .gitconfig
[user]
name = Henk Hunter
[include]
path = .gitconfig_personal
# check the value and from which scope it comes
~ % git config --show-scope --show-origin user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
# check again with specific scope
~ % git config --show-scope --show-origin --global user.name
global file:/home/resu/.gitconfig Henk Hunter
# check again while allowing includes
~ % git config --show-scope --show-origin --includes user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
~ % git config --show-scope --show-origin --global --includes user.name
global file:/home/resu/.gitconfig_personal Hendrik Jäger
```
The manpage says:
> Respect include.* directives in config files when looking up values. Defaults to off when a specific file is given (e.g., using --file, --global, etc) and on when searching all config files.
IMHO it makes sense the way it is phrased “when a specific file is given” but then seems to turn into non-sense when --global is given as an example. Giving --global is not “giving a specific file” but “restricting to a specific scope”, which may `include` other files.
The results seem inconsistent and counterintuitive to me.
Am I misunderstanding anything here?
Is this behaviour intended?
If it is intended, can someone please explain the rationale behind it? I don’t get it, it seems wrong to me.
Regarding the initial issue: I just added --includes to the call in lbmk and it works just fine, so there is no need to address this. I only mentioned it for context to how I got to looking into this behaviour.
If any relevant information is missing in this bugreport, I’ll be happy to add it, please let me know!
Thank you very much
henk
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v7 3/4] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-07-20 10:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <xmqq8q7961xe.fsf@gitster.g>
On 7/18/26 00:01, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
>
>> diff --git a/environment.c b/environment.c
>> index fc3ed8bb1c..75069a884d 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -41,7 +41,6 @@
>> static int pack_compression_seen;
>> static int zlib_compression_seen;
>>
>> -int trust_executable_bit = 1;
>> int trust_ctime = 1;
>> int check_stat = 1;
>> int has_symlinks = 1;
>> @@ -142,6 +141,13 @@ int is_bare_repository(void)
>> return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>> }
>>
>> +int repo_trust_executable_bit(struct repository *repo)
>> +{
>> + return repo->gitdir?
>> + repo_config_values(repo)->trust_executable_bit :
>> + 1;
>> +}
>> +
>> int have_git_dir(void)
>> {
>> return startup_info->have_repository
>
> Two comments.
>
> * Missing SP before '?'. It may be easier to read if it is written
> like this:
>
> return repo->gitdir
> ? repo_config_values(repo)->trust_executable_bit
> : 1;
>
> which more clearly highlights the ternary structure. If you tilt
> your head 90 degrees to the left, you can almost see the parse
> tree of the expression.
>
Okay.
> * Does it make sense to protect against a NULL 'repo' case, as
> repo_protect_ntfs() and repo_protect_hfs() helpers do? Or is it
> better to crash loudly with a segfault to let the developer know
> they have a bug to fix? I lean toward the latter myself, and if
> we go that route, we should probably stop using 'repo &&
> repo->gitdir' elsewhere, rather than sweeping the problem under
> the rug with defensive checks.
Sounds sensible. I will drop the checks and adjust the calls themselves
instead.
Note that repo_protect_ntfs() looks like this:
int repo_protect_ntfs(struct repository *repo)
{
return (repo && repo->initialized) ?
repo_config_values(repo)->protect_ntfs :
PROTECT_NTFS_DEFAULT;
}
Do we need another fixup commit for it?
Regards, yuchen
^ permalink raw reply
* Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: Tian Yuchen @ 2026-07-20 10:12 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <al3v0NVZJYS9SVZF@szeder.dev>
On 7/20/26 17:52, SZEDER Gábor wrote:
> On Mon, Jul 20, 2026 at 05:13:17PM +0800, Tian Yuchen wrote:
>> On 7/19/26 03:02, SZEDER Gábor wrote:
>>> On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
>>>> diff --git a/read-cache.h b/read-cache.h
>>>> index 043da1f1aa..94b8d3e547 100644
>>>> --- a/read-cache.h
>>>> +++ b/read-cache.h
>>>> @@ -4,15 +4,24 @@
>>>> #include "read-cache-ll.h"
>>>> #include "object.h"
>>>> #include "pathspec.h"
>>>> +#include "environment.h"
>>>> -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
>>>> +/*
>>>> + * Determine the appropriate index mode for a file based on its stat()
>>>> + * information and the existing cache entry (if any).
>>>> + *
>>>> + * This function handles degradation for filesystems that lack
>>>> + * symlink support or reliable executable bits.
>>>> + */
>>>> +static inline unsigned int ce_mode_from_stat(struct repository *repo,
>>>
>>> This new parameter is not yet used in this function, which causes
>>> compilation errors in all source files which include "read-cache.h"
>>> when trying to build this commit using DEVELOPER=1, e.g.:
>
>>> I think the new parameter should be marked as UNUSED in this patch,
>>> and then the UNUSED should be dropped in the next, where you start
>>> using the parameter.
>>>
>>>> + const struct cache_entry *ce,
>>>> unsigned int mode)
>>>> {
>>>> extern int trust_executable_bit, has_symlinks;
>>>> - if (!has_symlinks && S_ISREG(mode) &&
>>>> + if (S_ISREG(mode) && !has_symlinks &&
>>>> ce && S_ISLNK(ce->ce_mode))
>>>> return ce->ce_mode;
>>>> - if (!trust_executable_bit && S_ISREG(mode)) {
>>>> + if (S_ISREG(mode) && !trust_executable_bit) {
>>>> if (ce && S_ISREG(ce->ce_mode))
>>>> return ce->ce_mode;
>>>> return create_ce_mode(0666);
>>>> --
>>>> 2.43.0
>>>>
>>
>> But 'USUSED' cannot be used here since the corresponding header
>> (git-compat-util.h, or more specifically compat/posix.h) is not included.
>
> UNUSED _can_ be used here, because:
>
> - This is a header file, so it's not supposed to be compiled on its
> own.
> - All C source files including this header file must start with
> including "git-compat-util.h", so by the time they include
> "read-cache.h", the UNUSED macro is already defined.
>
I see.
Thanks, yuchen
^ permalink raw reply
* Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options
From: Siddharth Shrimali @ 2026-07-20 10:19 UTC (permalink / raw)
To: Christian Couder
Cc: git, gitster, siddharthasthana31, me, ps, johannes.schindelin,
l.s.r, karthik nayak
In-Reply-To: <CAP8UFD2ZNmWh4fjh+vFvKCihfebg2yif9=xLjqpKZFgF-O0RSg@mail.gmail.com>
Hey Christian,
On Sat, 18 Jul 2026 at 18:01, Christian Couder
<christian.couder@gmail.com> wrote:
> An alternative would be `--drop-filtered[=dry-run]`, which might be
> extended with other `--drop-filtered` specific options later.
>
> I think separating `--dry-run` from `--drop-filtered` like this patch
> does makes sense though if we think that `--dry-run` could be useful
> later without `--drop-filtered`. The fact that a number of other
> commands already have a `--dry-run` option might be a good sign.
Agreed. I chose a separate --dry-run mainly for consistency with the many other
Git commands that already use it that way, and because it leaves room
for --dry-run
to apply to other repack behavior later rather than being tied to
--drop-filtered.
That said, I do like the --drop-filtered[=dry-run] form for keeping future
--drop-filtered-specific options together, and it might end up being the tidier
choice.
Since this is an RFC, I'd genuinely welcome others' thoughts on which one
holds up better in the long run, before settling on any one command-line.
>
> Anyway it would be nice if the commit message explained a bit the
> choice to have a separate `--dry-run` option.
Either way, for v2 I'll add a note to the commit message explaining
the final choice.
Thanks!
^ permalink raw reply
* [PATCH v8 0/4] environment: migrate 'trust_executable_bit' and 'has_symlinks' into 'repo_config_values'
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen
In-Reply-To: <20260717063559.1633567-1-cat@malon.dev>
This series moves 'trust_executable_bit' and 'has_symlinks' into
'struct repo_config_values' to tie them to the specific repository
instance they were read from. Eager parsing is maintained because
these two flags are heavily consulted in hot paths.
Note: 'repo_config_values()' still does not support any struct
repository other than the_repository due to how deeply these flags
are accessed. In other words, this series of patches is laying
the groundwork for the eventual elimination of the_repository.
Previous related work:
[PATCH 2/6] config: add trust_executable_bit to global config [1]
[PATCH] Refactor 'trust_executable_bit' to repository-scoped setting [2]
(This previous attempt was unsuccessful because the target location
selected was 'struct repo_settings', which our analysis indicated
was not the optimal choice. For further details, please see: [3])
[PATCH 5/6] config: move has_symlinks [4]
RFC:
- Is the locations of the newly introduced definitions/macros
appropriate?
Changes since V7:
- In commit 2/4, mark the 'struct repository' parameter of
ce_mode_from_stat() UNUSED. In commit 3/4, drop it.
- Don't check '!repo' in the getters, which lets the developers know
there is a bug to fix when NULL is passed in. Callers should be
responsible of passing non-null repos. Therefore, adjust the call in
write_entry().
Change back to check 'repo->initiaized' instead of 'repo->gitdir'.
Thanks!
[1] https://lore.kernel.org/git/837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/20260301190017.53539-1-dronarajgyawali@gmail.com/
[3] https://lore.kernel.org/git/xmqq1pht6nyx.fsf@gitster.g/
[4] https://lore.kernel.org/git/a154008619790f7a60f2bba91db7b0fe29e67e1a.1685716420.git.gitgitgadget@gmail.com/
[5] https://lore.kernel.org/git/xmqq7bokebct.fsf@gitster.g/
Tian Yuchen (4):
read-cache: remove redundant extern declarations
read-cache: pass 'repo' to 'ce_mode_from_stat()'
environment: move trust_executable_bit into repo_config_values
environment: move has_symlinks into repo_config_values
apply.c | 6 +++---
builtin/update-index.c | 2 +-
combine-diff.c | 2 +-
compat/mingw.c | 17 +++++++++++++----
compat/mingw.h | 3 +++
diff-lib.c | 10 +++++-----
entry.c | 3 ++-
environment.c | 23 +++++++++++++++++++----
environment.h | 8 ++++++--
git-compat-util.h | 4 ++++
read-cache.c | 15 +++++++--------
read-cache.h | 16 ++++++++++++----
12 files changed, 76 insertions(+), 33 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v8 1/4] read-cache: remove redundant extern declarations
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-1-cat@malon.dev>
The 'read-cache.c' file already includes 'environment.h', which provides
the extern declarations for variables like 'trust_executable_bit' and
'has_symlinks'.
Remove the redundant extern declarations inside 'st_mode_from_ce()' to
clean up the code.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
read-cache.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 38a04b8de3..c44e4d128f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,8 +204,6 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
- extern int trust_executable_bit, has_symlinks;
-
switch (ce->ce_mode & S_IFMT) {
case S_IFLNK:
return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
--
2.43.0
^ permalink raw reply related
* [PATCH v8 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-1-cat@malon.dev>
The ce_mode_from_stat() function is a performance-critical static
inline helper in 'read-cache.h'. As we migrate configuration
variables into the repository struct, this helper needs access
to the repository context.
Update the signature of ce_mode_from_stat() to take a 'struct
repository *' parameter, and update all callers to pass the
appropriate repository instance.
To prepare for the overhead of replacing cheap global variable
accesses with getter functions, the boolean expressions are
reordered to evaluate 'S_ISREG(mode)' first.
While at it, add a comment for ce_mode_from_stat().
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 2 +-
builtin/update-index.c | 2 +-
diff-lib.c | 10 +++++-----
read-cache.c | 2 +-
read-cache.h | 15 ++++++++++++---
5 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..26286eb57b 100644
--- a/apply.c
+++ b/apply.c
@@ -3894,7 +3894,7 @@ static int check_preimage(struct apply_state *state,
BUG("ce_mode == 0 for path '%s'", old_name);
if (trust_executable_bit || !S_ISREG(st->st_mode))
- st_mode = ce_mode_from_stat(*ce, st->st_mode);
+ st_mode = ce_mode_from_stat(state->repo, *ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
else
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8a5907767b..7917bd286f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -293,7 +293,7 @@ static int add_one_path(const struct cache_entry *old, const char *path, int len
ce->ce_flags = create_ce_flags(0);
ce->ce_namelen = len;
fill_stat_cache_info(the_repository->index, ce, st);
- ce->ce_mode = ce_mode_from_stat(old, st->st_mode);
+ ce->ce_mode = ce_mode_from_stat(the_repository, old, st->st_mode);
if (index_path(the_repository->index, &ce->oid, path, st,
info_only ? 0 : INDEX_WRITE_OBJECT)) {
diff --git a/diff-lib.c b/diff-lib.c
index ae91027a02..46cae637ec 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -160,7 +160,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
changed = check_removed(ce, &st);
if (!changed)
- wt_mode = ce_mode_from_stat(ce, st.st_mode);
+ wt_mode = ce_mode_from_stat(revs->repo, ce, st.st_mode);
else {
if (changed < 0) {
perror(ce->name);
@@ -193,7 +193,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
num_compare_stages++;
oidcpy(&dpath->parent[stage - 2].oid,
&nce->oid);
- dpath->parent[stage-2].mode = ce_mode_from_stat(nce, mode);
+ dpath->parent[stage-2].mode = ce_mode_from_stat(revs->repo, nce, mode);
dpath->parent[stage-2].status =
DIFF_STATUS_MODIFIED;
}
@@ -262,7 +262,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
continue;
} else if (revs->diffopt.ita_invisible_in_index &&
ce_intent_to_add(ce)) {
- newmode = ce_mode_from_stat(ce, st.st_mode);
+ newmode = ce_mode_from_stat(revs->repo, ce, st.st_mode);
diff_addremove(&revs->diffopt, '+', newmode,
null_oid(the_hash_algo), 0, ce->name, 0);
continue;
@@ -270,7 +270,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
ce_option, &dirty_submodule);
- newmode = ce_mode_from_stat(ce, st.st_mode);
+ newmode = ce_mode_from_stat(revs->repo, ce, st.st_mode);
}
if (!changed && !dirty_submodule) {
@@ -338,7 +338,7 @@ static int get_stat_data(const struct cache_entry *ce,
changed = match_stat_with_submodule(diffopt, ce, &st,
0, dirty_submodule);
if (changed) {
- mode = ce_mode_from_stat(ce, st.st_mode);
+ mode = ce_mode_from_stat(diffopt->repo, ce, st.st_mode);
oid = null_oid(the_hash_algo);
}
}
diff --git a/read-cache.c b/read-cache.c
index c44e4d128f..b37bf688ec 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -749,7 +749,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
int pos = index_name_pos_also_unmerged(istate, path, namelen);
ent = (0 <= pos) ? istate->cache[pos] : NULL;
- ce->ce_mode = ce_mode_from_stat(ent, st_mode);
+ ce->ce_mode = ce_mode_from_stat(istate->repo, ent, st_mode);
}
/* When core.ignorecase=true, determine if a directory of the same name but differing
diff --git a/read-cache.h b/read-cache.h
index 043da1f1aa..af8c657ecb 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -4,15 +4,24 @@
#include "read-cache-ll.h"
#include "object.h"
#include "pathspec.h"
+#include "environment.h"
-static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+/*
+ * Determine the appropriate index mode for a file based on its stat()
+ * information and the existing cache entry (if any).
+ *
+ * This function handles degradation for filesystems that lack
+ * symlink support or reliable executable bits.
+ */
+static inline unsigned int ce_mode_from_stat(struct repository *repo UNUSED,
+ const struct cache_entry *ce,
unsigned int mode)
{
extern int trust_executable_bit, has_symlinks;
- if (!has_symlinks && S_ISREG(mode) &&
+ if (S_ISREG(mode) && !has_symlinks &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
+ if (S_ISREG(mode) && !trust_executable_bit) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
--
2.43.0
^ permalink raw reply related
* [PATCH v8 3/4] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-1-cat@malon.dev>
Move the global 'trust_executable_bit' configuration
into the repository-specific 'repo_config_values'
struct.
To ensure code readability, the getter function
'repo_trust_executable_bit()' has been introduced.
Callers access this configuration by passing in 'repo'
when possible, and explicitly fall back to 'the_repository'
the rest of time.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 2 +-
environment.c | 11 +++++++++--
environment.h | 4 +++-
read-cache.c | 6 +++---
read-cache.h | 6 +++---
5 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/apply.c b/apply.c
index 26286eb57b..edb1502414 100644
--- a/apply.c
+++ b/apply.c
@@ -3893,7 +3893,7 @@ static int check_preimage(struct apply_state *state,
if (*ce && !(*ce)->ce_mode)
BUG("ce_mode == 0 for path '%s'", old_name);
- if (trust_executable_bit || !S_ISREG(st->st_mode))
+ if (repo_trust_executable_bit(state->repo) || !S_ISREG(st->st_mode))
st_mode = ce_mode_from_stat(state->repo, *ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..32b110c405 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,6 @@
static int pack_compression_seen;
static int zlib_compression_seen;
-int trust_executable_bit = 1;
int trust_ctime = 1;
int check_stat = 1;
int has_symlinks = 1;
@@ -142,6 +141,13 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+int repo_trust_executable_bit(struct repository *repo)
+{
+ return repo->initialized
+ ? repo_config_values(repo)->trust_executable_bit
+ : 1;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -305,7 +311,7 @@ int git_default_core_config(const char *var, const char *value,
/* This needs a better name */
if (!strcmp(var, "core.filemode")) {
- trust_executable_bit = git_config_bool(var, value);
+ cfg->trust_executable_bit = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.trustctime")) {
@@ -720,5 +726,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
+ cfg->trust_executable_bit = 1;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 123a71cdc8..72b59fd89c 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int trust_executable_bit;
/* section "branch" config values */
enum branch_track branch_track;
@@ -123,6 +124,8 @@ int git_default_config(const char *, const char *,
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);
+int repo_trust_executable_bit(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -160,7 +163,6 @@ int is_bare_repository(void);
extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
-extern int trust_executable_bit;
extern int trust_ctime;
extern int check_stat;
extern int has_symlinks;
diff --git a/read-cache.c b/read-cache.c
index b37bf688ec..1f8b5ed15f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -208,7 +208,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
case S_IFLNK:
return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
- return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
+ return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
return S_IFDIR | 0755;
case S_IFDIR:
@@ -318,7 +318,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
/* We consider only the owner x bit to be relevant for
* "mode changes"
*/
- if (trust_executable_bit &&
+ if (repo_trust_executable_bit(the_repository) &&
(0100 & (ce->ce_mode ^ st->st_mode)))
changed |= MODE_CHANGED;
break;
@@ -739,7 +739,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (trust_executable_bit && has_symlinks) {
+ if (repo_trust_executable_bit(istate->repo) && has_symlinks) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
diff --git a/read-cache.h b/read-cache.h
index af8c657ecb..4b54cfc57c 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -13,15 +13,15 @@
* This function handles degradation for filesystems that lack
* symlink support or reliable executable bits.
*/
-static inline unsigned int ce_mode_from_stat(struct repository *repo UNUSED,
+static inline unsigned int ce_mode_from_stat(struct repository *repo,
const struct cache_entry *ce,
unsigned int mode)
{
- extern int trust_executable_bit, has_symlinks;
+ extern int has_symlinks;
if (S_ISREG(mode) && !has_symlinks &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (S_ISREG(mode) && !trust_executable_bit) {
+ if (S_ISREG(mode) && !repo_trust_executable_bit(repo)) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
--
2.43.0
^ permalink raw reply related
* [PATCH v8 4/4] environment: move has_symlinks into repo_config_values
From: Tian Yuchen @ 2026-07-20 10:53 UTC (permalink / raw)
To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260720105335.3202013-1-cat@malon.dev>
Move the global 'has_symlinks' configuration into the
repository-specific 'repo_config_values' struct.
Introduce 'repo_has_symlinks()' getter for readability.
Callers access this configuration by passing in 'repo'
when possible, and explicitly fall back to
'the_repository' the rest of the time.
Introduce 'platform_has_symlinks()' macro to allow
platform specific-customization, primarily to help MinGW.
Platforms can override this in their respective headers.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 2 +-
combine-diff.c | 2 +-
compat/mingw.c | 17 +++++++++++++----
compat/mingw.h | 3 +++
entry.c | 3 ++-
environment.c | 12 ++++++++++--
environment.h | 4 +++-
git-compat-util.h | 4 ++++
read-cache.c | 7 ++++---
read-cache.h | 3 +--
10 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/apply.c b/apply.c
index edb1502414..b748192ee2 100644
--- a/apply.c
+++ b/apply.c
@@ -4511,7 +4511,7 @@ static int try_create_file(struct apply_state *state, const char *path,
return !!mkdir(path, 0777);
}
- if (has_symlinks && S_ISLNK(mode))
+ if (repo_has_symlinks(state->repo) && S_ISLNK(mode))
/* Although buf:size is counted string, it also is NUL
* terminated.
*/
diff --git a/combine-diff.c b/combine-diff.c
index b799862068..80e5c46e9b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1078,7 +1078,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
/* if symlinks don't work, assume symlink if all parents
* are symlinks
*/
- is_file = has_symlinks;
+ is_file = repo_has_symlinks(rev->repo);
for (i = 0; !is_file && i < num_parent; i++)
is_file = !S_ISLNK(elem->parent[i].mode);
if (!is_file)
diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..4781911929 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -7,6 +7,7 @@
#include "config.h"
#include "dir.h"
#include "environment.h"
+#include "repository.h"
#include "gettext.h"
#include "run-command.h"
#include "strbuf.h"
@@ -1043,7 +1044,7 @@ int mingw_chdir(const char *dirname)
if (xutftowcs_path(wdirname, dirname) < 0)
return -1;
- if (has_symlinks) {
+ if (repo_has_symlinks(the_repository)) {
HANDLE hnd = CreateFileW(wdirname, 0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
@@ -2903,7 +2904,7 @@ int symlink(const char *target, const char *link)
int len;
/* fail if symlinks are disabled or API is not supported (WinXP) */
- if (!has_symlinks) {
+ if (!repo_has_symlinks(the_repository)) {
errno = ENOSYS;
return -1;
}
@@ -3173,15 +3174,23 @@ static void setup_windows_environment(void)
if (!tmp && (tmp = getenv("USERPROFILE")))
setenv("HOME", tmp, 1);
}
+}
+int mingw_platform_has_symlinks(void)
+{
+ static int has_symlinks = -1;
/*
* Change 'core.symlinks' default to false, unless native symlinks are
* enabled in MSys2 (via 'MSYS=winsymlinks:nativestrict'). Thus we can
* run the test suite (which doesn't obey config files) with or without
* symlink support.
*/
- if (!(tmp = getenv("MSYS")) || !strstr(tmp, "winsymlinks:nativestrict"))
- has_symlinks = 0;
+ if (has_symlinks < 0) {
+ const char *tmp = getenv("MSYS");
+ has_symlinks = (tmp && strstr(tmp, "winsymlinks:nativestrict")) ? 1 : 0;
+ }
+
+ return has_symlinks;
}
static void get_current_user_sid(PSID *sid, HANDLE *linked_token)
diff --git a/compat/mingw.h b/compat/mingw.h
index 444daedfa5..df02aeb632 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -208,6 +208,9 @@ void open_in_gdb(void);
*/
int err_win_to_posix(DWORD winerr);
+int mingw_platform_has_symlinks(void);
+#define platform_has_symlinks() mingw_platform_has_symlinks()
+
#ifndef NO_UNIX_SOCKETS
int mingw_have_unix_sockets(void);
#undef have_unix_sockets
diff --git a/entry.c b/entry.c
index 7817aee362..5913a8b51f 100644
--- a/entry.c
+++ b/entry.c
@@ -321,7 +321,8 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca
* We can't make a real symlink; write out a regular file entry
* with the symlink destination as its contents.
*/
- if (!has_symlinks || to_tempfile)
+ if (!repo_has_symlinks(state->istate && state->istate->repo ?
+ state->istate->repo : the_repository) || to_tempfile)
goto write_file_entry;
ret = symlink(new_blob, path);
diff --git a/environment.c b/environment.c
index 32b110c405..e351043446 100644
--- a/environment.c
+++ b/environment.c
@@ -43,7 +43,6 @@ static int zlib_compression_seen;
int trust_ctime = 1;
int check_stat = 1;
-int has_symlinks = 1;
int minimum_abbrev = 4, default_abbrev = -1;
int ignore_case;
int assume_unchanged;
@@ -148,6 +147,13 @@ int repo_trust_executable_bit(struct repository *repo)
: 1;
}
+int repo_has_symlinks(struct repository *repo)
+{
+ return repo->initialized
+ ? repo_config_values(repo)->has_symlinks
+ : platform_has_symlinks();
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -336,7 +342,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.symlinks")) {
- has_symlinks = git_config_bool(var, value);
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+ cfg->has_symlinks = git_config_bool(var, value);
return 0;
}
@@ -727,5 +734,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
cfg->trust_executable_bit = 1;
+ cfg->has_symlinks = platform_has_symlinks();
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 72b59fd89c..ef64a783b0 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
char *attributes_file;
int apply_sparse_checkout;
int trust_executable_bit;
+ int has_symlinks;
/* section "branch" config values */
enum branch_track branch_track;
@@ -126,6 +127,8 @@ int git_default_core_config(const char *var, const char *value,
int repo_trust_executable_bit(struct repository *repo);
+int repo_has_symlinks(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -165,7 +168,6 @@ extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
extern int trust_ctime;
extern int check_stat;
-extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
diff --git a/git-compat-util.h b/git-compat-util.h
index 5024814bd4..333a5acf33 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -245,6 +245,10 @@ static inline int git_is_dir_sep(int c)
#define is_dir_sep git_is_dir_sep
#endif
+#ifndef platform_has_symlinks
+#define platform_has_symlinks() 1
+#endif
+
#ifndef offset_1st_component
static inline int git_offset_1st_component(const char *path)
{
diff --git a/read-cache.c b/read-cache.c
index 1f8b5ed15f..c2c3c2e6cc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -206,7 +206,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
switch (ce->ce_mode & S_IFMT) {
case S_IFLNK:
- return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
+ return repo_has_symlinks(the_repository) ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
@@ -324,7 +324,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
break;
case S_IFLNK:
if (!S_ISLNK(st->st_mode) &&
- (has_symlinks || !S_ISREG(st->st_mode)))
+ (repo_has_symlinks(the_repository) || !S_ISREG(st->st_mode)))
changed |= TYPE_CHANGED;
break;
case S_IFGITLINK:
@@ -739,7 +739,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (repo_trust_executable_bit(istate->repo) && has_symlinks) {
+ if (repo_trust_executable_bit(istate->repo) &&
+ repo_has_symlinks(istate->repo)) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
diff --git a/read-cache.h b/read-cache.h
index 4b54cfc57c..ab9d40aa81 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -17,8 +17,7 @@ static inline unsigned int ce_mode_from_stat(struct repository *repo,
const struct cache_entry *ce,
unsigned int mode)
{
- extern int has_symlinks;
- if (S_ISREG(mode) && !has_symlinks &&
+ if (S_ISREG(mode) && !repo_has_symlinks(repo) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
if (S_ISREG(mode) && !repo_trust_executable_bit(repo)) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 0/9] sequencer: do not record dropped commits as rewritten
From: Oswald Buddenhagen @ 2026-07-20 12:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqecgyn5gk.fsf@gitster.g>
On Sun, Jul 19, 2026 at 12:29:31PM -0700, Junio C Hamano wrote:
>It looks like this is now ready to go? Any further comments?
>
you can add whatever footer is appropriate for "i read it, it seems to
make sense, but i didn't double-check" for me.
(same for phillip's new 2-patch series.)
(it feels silly to "spam" the list with such low-value verdicts. i
really miss gerrit code review here, where i'd leave a +1 in passing.)
^ permalink raw reply
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