* [PATCH v5 4/4] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:55 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v5.git.git.1782338102.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
By default "git history squash" reuses the oldest commit's message.
When --reedit-message is given it only reopened that one message, so the
messages of the folded-in commits were lost.
Gather the messages of every commit in the range, oldest first, and use
them as the editor template when re-editing, mirroring how "git rebase
-i" presents a squash. The combined message is built before the
descendant walk so it is not disturbed by the flags that walk leaves on
the commits.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 5 +--
builtin/history.c | 61 +++++++++++++++++++++++++++++++++-
t/t3455-history-squash.sh | 37 +++++++++++++++++++++
3 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 6716749cde..df389015aa 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -111,8 +111,9 @@ history squash @~3..` folds the three most recent commits into one, and
`git history squash @~5..@~2` squashes an interior range while leaving
the two newest commits in place.
+
-The oldest commit's message and authorship are preserved by default,
-unless you specify `--reedit-message`. A merge commit inside the range is
+The oldest commit's message and authorship are preserved by default. 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.
diff --git a/builtin/history.c b/builtin/history.c
index 0acfabed66..e93f8398e6 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1081,6 +1081,56 @@ static int find_interior_ref(const struct reference *ref, void *cb_data)
return 0;
}
+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;
+ int n = 0, 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))) {
+ const char *message, *body;
+ struct strbuf one = STRBUF_INIT;
+
+ message = repo_logmsg_reencode(repo, commit, NULL, NULL);
+ find_commit_subject(message, &body);
+ strbuf_addstr(&one, body);
+ strbuf_trim_trailing_newline(&one);
+
+ if (n++)
+ strbuf_addch(out, '\n');
+ strbuf_addbuf(out, &one);
+ strbuf_addch(out, '\n');
+
+ strbuf_release(&one);
+ repo_unuse_commit_buffer(repo, commit, message);
+ }
+
+ ret = 0;
+
+out:
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
@@ -1105,6 +1155,7 @@ static int cmd_history_squash(int argc,
OPT_END(),
};
struct strbuf reflog_msg = STRBUF_INIT;
+ struct strbuf message = STRBUF_INIT;
struct oidset interior = OIDSET_INIT;
struct commit *base, *oldest, *tip, *rewritten;
const struct object_id *base_tree_oid, *tip_tree_oid;
@@ -1144,6 +1195,12 @@ static int cmd_history_squash(int argc,
}
}
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
@@ -1152,7 +1209,8 @@ static int cmd_history_squash(int argc,
tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
commit_list_append(base, &parents);
- ret = commit_tree_ext(repo, "squash", oldest, NULL, parents,
+ ret = commit_tree_ext(repo, "squash", oldest,
+ message.len ? message.buf : NULL, parents,
base_tree_oid, tip_tree_oid, &rewritten, flags);
if (ret < 0) {
ret = error(_("failed writing squashed commit"));
@@ -1173,6 +1231,7 @@ static int cmd_history_squash(int argc,
out:
strbuf_release(&reflog_msg);
+ strbuf_release(&message);
oidset_clear(&interior);
commit_list_free(parents);
release_revisions(&revs);
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
index 7227c5c90f..af59ddf6e3 100755
--- a/t/t3455-history-squash.sh
+++ b/t/t3455-history-squash.sh
@@ -137,6 +137,43 @@ 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 &&
+ echo b >file &&
+ git add file &&
+ 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 &&
+ cp "$1" buffer &&
+ echo combined >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ test_grep "re-one subject" buffer &&
+ test_grep "re-one body line" buffer &&
+ test_grep re-two buffer &&
+ test_grep re-three buffer &&
+ git log --format="%s" -1 >actual &&
+ echo combined >expect &&
+ test_cmp expect actual
+'
+
+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 '--dry-run predicts the rewrite without performing it' '
git reset --hard three &&
head_before=$(git rev-parse HEAD) &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 3/4] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:55 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v5.git.git.1782338102.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
message and authorship and taking the tree of the newest commit, so the
range collapses into a single commit. Commits above the range are
replayed on top of the result.
The range is given as <base>..<tip>, so "git history squash @~3.."
folds the three most recent commits and "git history squash @~5..@~2"
squashes an interior range. A merge inside the range is folded like any
other commit, but the range must have a single base, so a range with
more than one entry point is rejected.
The folded commits leave the history, so by default the command refuses
when another ref points at one of them. Use "--update-refs=head" to
rewrite only the current branch and leave those refs untouched.
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 | 25 ++
advice.c | 1 +
advice.h | 1 +
builtin/history.c | 208 ++++++++++++++
t/meson.build | 1 +
t/t3455-history-squash.sh | 460 +++++++++++++++++++++++++++++++
7 files changed, 700 insertions(+)
create mode 100755 t/t3455-history-squash.sh
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..f4d692d136 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -55,6 +55,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 2ba8121795..6716749cde 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -11,6 +11,7 @@ SYNOPSIS
git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
+git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
DESCRIPTION
-----------
@@ -97,6 +98,30 @@ 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 message and
+ 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 @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place.
++
+The oldest commit's message and authorship are preserved by default,
+unless you specify `--reedit-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.
++
+The folded commits disappear from the history, so with the default
+`--update-refs=branches` the command refuses when another ref points at
+one of them. Rerun with `--update-refs=head` to rewrite only the current
+branch and leave those refs pointing at the old commits.
+
OPTIONS
-------
diff --git a/advice.c b/advice.c
index 0018501b7b..5c6ff95e31 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 8def280688..911b4e4643 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 305bde3102..0acfabed66 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"
@@ -30,6 +31,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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]")
static void change_data_free(void *util, const char *str UNUSED)
{
@@ -973,6 +976,209 @@ 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 *range,
+ 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 strvec args = STRVEC_INIT;
+ size_t i;
+ int ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_push(&args, "--boundary");
+ strvec_push(&args, "--ancestry-path");
+ strvec_push(&args, range);
+ setup_revisions_from_strvec(&args, &revs, NULL);
+ if (args.nr != 1) {
+ ret = error(_("'%s' does not name a revision range"), range);
+ goto out;
+ }
+
+ /*
+ * A squash needs a base to reparent onto, so the argument has to
+ * exclude something, as in "<base>..<tip>". A single revision has no
+ * such bottom commit and 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(_("'%s' is not a '<base>..<tip>' range"), range);
+ goto out;
+ }
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs))) {
+ if (commit->object.flags & BOUNDARY) {
+ if (base) {
+ ret = error(_("range '%s' has more than one base; "
+ "cannot squash"), range);
+ goto out;
+ }
+ base = commit;
+ continue;
+ }
+ if (!oldest)
+ oldest = commit;
+ if (tip)
+ oidset_insert(interior_out, &tip->object.oid);
+ tip = commit;
+ }
+
+ if (!oldest) {
+ ret = error(_("the range '%s' is empty"), range);
+ goto out;
+ }
+
+ if (!base)
+ BUG("a non-empty range must have a boundary commit");
+
+ *base_out = base;
+ *oldest_out = oldest;
+ *tip_out = tip;
+ ret = 0;
+
+out:
+ 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 oidset interior = OIDSET_INIT;
+ struct commit *base, *oldest, *tip, *rewritten;
+ const struct object_id *base_tree_oid, *tip_tree_oid;
+ struct commit_list *parents = NULL;
+ struct rev_info revs = { 0 };
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single 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[0], &base, &oldest, &tip,
+ &interior);
+ if (ret < 0)
+ goto out;
+
+ 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", oldest, NULL, parents,
+ base_tree_oid, tip_tree_oid, &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing squashed commit"));
+ goto out;
+ }
+
+ strbuf_addf(&reflog_msg, "squash: updating %s", argv[0]);
+
+ 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);
+ oidset_clear(&interior);
+ commit_list_free(parents);
+ release_revisions(&revs);
+ return ret;
+}
+
int cmd_history(int argc,
const char **argv,
const char *prefix,
@@ -982,6 +1188,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;
@@ -989,6 +1196,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 3219264fe7..63ea26b8ed 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -399,6 +399,7 @@ integration_tests = [
't3451-history-reword.sh',
't3452-history-split.sh',
't3453-history-fixup.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..7227c5c90f
--- /dev/null
+++ b/t/t3455-history-squash.sh
@@ -0,0 +1,460 @@
+#!/bin/sh
+
+test_description='tests for git-history squash subcommand'
+
+. ./test-lib.sh
+
+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 "command expects a single revision range" err
+'
+
+test_expect_success 'errors on too many arguments' '
+ test_must_fail git history squash start.. HEAD 2>err &&
+ test_grep "command expects a single revision range" err
+'
+
+test_expect_success 'errors on an empty range' '
+ test_must_fail git history squash HEAD..HEAD 2>err &&
+ test_grep "the 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 "is not a .*range" err &&
+ test_must_fail git history squash HEAD~1 2>err &&
+ test_grep "is not a .*range" err
+'
+
+test_expect_success 'squashes a range into a single commit without changing the tree' '
+ git reset --hard three &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ git log --format="%s" -1 >subject &&
+ echo one >expect &&
+ test_cmp expect subject &&
+ git reflog >reflog &&
+ test_grep "squash: updating" reflog
+'
+
+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 &&
+
+ git log --format="%s" start..HEAD >actual &&
+ cat >expect <<-\EOF &&
+ three
+ one
+ EOF
+ test_cmp expect actual &&
+
+ 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.." &&
+
+ git rev-list --count "$root..HEAD" >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev "$root" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashing a single-commit range replays the rest' '
+ git reset --hard three &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..@~2 &&
+
+ git log --format="%s" start..HEAD >actual &&
+ cat >expect <<-\EOF &&
+ three
+ two
+ one
+ EOF
+ test_cmp expect actual &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'reuses the message of a fixup! commit in the range' '
+ git reset --hard start &&
+ test_commit --no-tag reg1 file b &&
+ git commit --allow-empty -m "fixup! reg1" &&
+ test_commit reg2 file c &&
+
+ git history squash start.. &&
+
+ git log --format="%s" -1 >actual &&
+ echo reg1 >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'keeps the oldest message even if it is a fixup!' '
+ git reset --hard start &&
+ test_commit --no-tag "fixup! something" file b &&
+ test_commit tail file c &&
+
+ git history squash start.. &&
+
+ git log --format="%s" -1 >actual &&
+ echo "fixup! something" >expect &&
+ test_cmp expect actual
+'
+
+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 '--dry-run predicts the rewrite without performing it' '
+ 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)" &&
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ 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 &&
+
+ 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 &&
+
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ 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 &&
+ test_commit --no-tag before-side file b &&
+ git checkout -b inner-side &&
+ test_commit --no-tag on-inner-side inner x &&
+ git checkout - &&
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ git log --format="%s" -1 >subject &&
+ echo before-side >expect &&
+ test_cmp expect subject &&
+ 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 &&
+ git checkout -b base-fork-side &&
+ test_commit --no-tag base-fork-side side x &&
+ git checkout - &&
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
+'
+
+test_expect_success 'folds a range whose tip is a merge commit' '
+ git reset --hard start &&
+ test_commit --no-tag tipmerge-base file b &&
+ git checkout -b tipmerge-side &&
+ test_commit --no-tag tipmerge-side side x &&
+ git checkout - &&
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ 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 &&
+ git checkout -b basemerge-side &&
+ test_commit --no-tag basemerge-side side x &&
+ git checkout - &&
+ 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.." &&
+
+ git rev-list --count "$base..HEAD" >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ test_cmp_rev "$base" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'refuses to squash a range with more than one base' '
+ git reset --hard start &&
+ head_before=$(git rev-parse HEAD) &&
+ git checkout -b forked-before &&
+ test_commit forked-side fside x &&
+ git checkout - &&
+ test_commit forked-main file b &&
+ git merge --no-ff -m merge forked-before &&
+ merged=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash forked-main.. 2>err &&
+ test_grep "more than one base" err &&
+ test_cmp_rev "$merged" HEAD
+'
+
+test_expect_success 'folds a range with two interior merges' '
+ git reset --hard start &&
+ 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 - &&
+ 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 - &&
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ 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.. &&
+
+ git rev-list --count start..HEAD >count &&
+ echo 1 >expect &&
+ test_cmp expect count &&
+ 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-base file b &&
+ 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 c &&
+ 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 v5 2/4] history: give commit_tree_ext a message template
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:55 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v5.git.git.1782338102.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
commit_tree_ext() reuses the message of the commit it is handed. A
caller that folds several commits together wants to seed the message
from more than that single commit, so add an optional message_template
parameter. When NULL, the behavior is unchanged.
Pass NULL from the existing fixup and split callers.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/history.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index f95f26e684..305bde3102 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -101,6 +101,7 @@ enum commit_tree_flags {
static int commit_tree_ext(struct repository *repo,
const char *action,
struct commit *commit_with_message,
+ const char *message_template,
const struct commit_list *parents,
const struct object_id *old_tree,
const struct object_id *new_tree,
@@ -130,13 +131,16 @@ static int commit_tree_ext(struct repository *repo,
original_author = xmemdupz(ptr, len);
find_commit_subject(original_message, &original_body);
+ if (!message_template)
+ message_template = original_body;
+
if (flags & COMMIT_TREE_EDIT_MESSAGE) {
ret = fill_commit_message(repo, old_tree, new_tree,
- original_body, action, &commit_message);
+ message_template, action, &commit_message);
if (ret < 0)
goto out;
} else {
- strbuf_addstr(&commit_message, original_body);
+ strbuf_addstr(&commit_message, message_template);
}
original_extra_headers = read_commit_extra_headers(commit_with_message,
@@ -189,7 +193,7 @@ static int commit_tree_with_edited_message(struct repository *repo,
if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
return -1;
- return commit_tree_ext(repo, action, original, original->parents,
+ return commit_tree_ext(repo, action, original, NULL, original->parents,
&parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
}
@@ -644,7 +648,7 @@ static int cmd_history_fixup(int argc,
goto out;
if (!skip_commit) {
- ret = commit_tree_ext(repo, "fixup", original, original->parents,
+ ret = commit_tree_ext(repo, "fixup", original, NULL, original->parents,
&original_tree->object.oid, &merge_result.tree->object.oid,
&rewritten, flags);
if (ret < 0) {
@@ -855,7 +859,7 @@ static int split_commit(struct repository *repo,
* The first commit is constructed from the split-out tree. The base
* that shall be diffed against is the parent of the original commit.
*/
- ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
+ ret = commit_tree_ext(repo, "split-out", original, NULL, original->parents, &parent_tree_oid,
&split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing first commit"));
@@ -872,7 +876,7 @@ static int split_commit(struct repository *repo,
old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid;
new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
+ ret = commit_tree_ext(repo, "split-out", original, NULL, parents, old_tree_oid,
new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing second commit"));
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 1/4] history: extract helper for a commit's parent tree
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:54 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v5.git.git.1782338102.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Three places resolve the tree of a commit's first parent, falling back
to the empty tree for a root commit, each repeating the same parse and
oidcpy dance. Extract a first_parent_tree_oid() helper and route the
existing callers through it.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/history.c | 58 +++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 32 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 091465a59e..f95f26e684 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -157,6 +157,25 @@ out:
return ret;
}
+static int first_parent_tree_oid(struct repository *repo,
+ struct commit *commit,
+ struct object_id *out)
+{
+ struct commit *parent = commit->parents ? commit->parents->item : NULL;
+
+ if (!parent) {
+ oidcpy(out, repo->hash_algo->empty_tree);
+ return 0;
+ }
+
+ if (repo_parse_commit(repo, parent))
+ return error(_("unable to parse parent commit %s"),
+ oid_to_hex(&parent->object.oid));
+
+ oidcpy(out, &repo_get_commit_tree(repo, parent)->object.oid);
+ return 0;
+}
+
static int commit_tree_with_edited_message(struct repository *repo,
const char *action,
struct commit *original,
@@ -164,21 +183,11 @@ static int commit_tree_with_edited_message(struct repository *repo,
{
struct object_id parent_tree_oid;
const struct object_id *tree_oid;
- struct commit *parent;
tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- parent = original->parents ? original->parents->item : NULL;
- if (parent) {
- if (repo_parse_commit(repo, parent)) {
- return error(_("unable to parse parent commit %s"),
- oid_to_hex(&parent->object.oid));
- }
-
- parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
- }
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+ return -1;
return commit_tree_ext(repo, action, original, original->parents,
&parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
@@ -444,18 +453,10 @@ static int commit_became_empty(struct repository *repo,
struct commit *original,
struct tree *result)
{
- struct commit *parent = original->parents ? original->parents->item : NULL;
struct object_id parent_tree_oid;
- if (parent) {
- if (repo_parse_commit(repo, parent))
- return error(_("unable to parse parent of %s"),
- oid_to_hex(&original->object.oid));
-
- parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
- }
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+ return -1;
return oideq(&result->object.oid, &parent_tree_oid);
}
@@ -799,16 +800,9 @@ static int split_commit(struct repository *repo,
struct tree *split_tree;
int ret;
- if (original->parents) {
- if (repo_parse_commit(repo, original->parents->item)) {
- ret = error(_("unable to parse parent commit %s"),
- oid_to_hex(&original->parents->item->object.oid));
- goto out;
- }
-
- parent_tree_oid = *get_commit_tree_oid(original->parents->item);
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0) {
+ ret = -1;
+ goto out;
}
original_commit_tree_oid = get_commit_tree_oid(original);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v15 2/2] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:54 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Phillip Wood, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2281.v15.git.git.1782338098.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Forking from an existing remote branch without refreshing first often
has consequences: you start work that has already been done, or you
build on an old version of the code which causes big conflicts later
when you pull. The workaround is two commands ("git fetch <remote>
<branch> && git checkout -b <topic> <remote>/<branch>"), and when
the fetch is skipped the checkout silently starts from a stale tip.
Users may already expect "<remote>/<branch>" to refer to the latest
tip on the remote. While this blurs the line between fetch and
checkout, git already does this in places where it pays off: "git
clone" fetches and checks out, and "git pull" fetches and merges.
Add a "fetch" mode to "--track" that refreshes <start-point> before
checking it out:
git checkout -b new_branch --track=fetch origin/some-branch
Only the requested branch is fetched so other remote-tracking
branches are left untouched. When <start-point> is a bare <remote>
(e.g. "origin"), follow refs/remotes/<remote>/HEAD to learn which
branch to refresh. If "git fetch" fails but the remote-tracking ref
already exists locally, warn and proceed from the existing tip,
otherwise abort.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-checkout.adoc | 17 ++-
Documentation/git-switch.adoc | 5 +-
builtin/checkout.c | 138 +++++++++++++++++++-
t/t7201-co.sh | 222 ++++++++++++++++++++++++++++++++
4 files changed, 375 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
index a8b3b8c2e2..20b6cae60e 100644
--- a/Documentation/git-checkout.adoc
+++ b/Documentation/git-checkout.adoc
@@ -158,11 +158,26 @@ of it").
resets _<branch>_ to the start point instead of failing.
`-t`::
-`--track[=(direct|inherit)]`::
+`--track[=(direct|inherit|fetch)[,...]]`::
When creating a new branch, set up "upstream" configuration. See
`--track` in linkgit:git-branch[1] for details. As a convenience,
--track without -b implies branch creation.
+
+The argument is a comma-separated list. `direct` (the default) and
+`inherit` select the tracking mode and are mutually exclusive. Adding
+`fetch` requests that the remote be fetched before _<start-point>_ is
+resolved, so the new branch starts from a fresh tip: when
+_<start-point>_ is in _<remote>/<branch>_ form, only that branch is
+updated; when _<start-point>_ is a bare _<remote>_ (e.g. `origin`), the
+branch named by _<remote>/HEAD_ is updated, and the checkout fails
+with a hint to configure that symref if it is not set. The checkout
+also fails if no configured remote's fetch refspec maps to
+_<start-point>_, or if more than one does (in which case the `fetch`
+cannot be unambiguously routed). If the fetch itself fails and the
+corresponding remote-tracking ref already exists, a warning is printed
+and the checkout proceeds from the existing tip; otherwise the checkout
+is aborted.
++
If no `-b` option is given, the name of the new branch will be
derived from the remote-tracking branch, by looking at the local part of
the refspec configured for the corresponding remote, and then stripping
diff --git a/Documentation/git-switch.adoc b/Documentation/git-switch.adoc
index d6c4f229a5..a8730b1da8 100644
--- a/Documentation/git-switch.adoc
+++ b/Documentation/git-switch.adoc
@@ -155,10 +155,11 @@ variable.
attached to a terminal, regardless of `--quiet`.
`-t`::
-`--track[ (direct|inherit)]`::
+`--track[=(direct|inherit|fetch)[,...]]`::
When creating a new branch, set up "upstream" configuration.
`-c` is implied. See `--track` in linkgit:git-branch[1] for
- details.
+ details, and `--track` in linkgit:git-checkout[1] for the
+ `fetch` mode.
+
If no `-c` option is given, the name of the new branch will be derived
from the remote-tracking branch, by looking at the local part of the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b78b3a1d16..c6154fbbb0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -25,10 +25,12 @@
#include "preload-index.h"
#include "read-cache.h"
#include "refs.h"
+#include "refspec.h"
#include "remote.h"
#include "repo-settings.h"
#include "resolve-undo.h"
#include "revision.h"
+#include "run-command.h"
#include "sequencer.h"
#include "setup.h"
#include "sparse-index.h"
@@ -63,6 +65,7 @@ struct checkout_opts {
int count_checkout_paths;
int overlay_mode;
int dwim_new_local_branch;
+ int fetch;
int discard_changes;
int accept_ref;
int accept_pathspec;
@@ -116,6 +119,128 @@ struct branch_info {
char *checkout;
};
+static void fetch_remote_for_start_point(const char *arg, int quiet)
+{
+ struct strbuf dst = STRBUF_INIT;
+ struct tracking tracking = { 0 };
+ struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
+ struct string_list ambiguous_remotes = STRING_LIST_INIT_DUP;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct remote *named_remote;
+ int bare_ns;
+
+ strbuf_addf(&dst, "refs/remotes/%s", arg);
+ if (check_refname_format(dst.buf, 0))
+ die(_("cannot fetch start-point '%s': not a valid "
+ "remote-tracking name"), arg);
+
+ named_remote = remote_get(arg);
+ bare_ns = !strchr(arg, '/') ||
+ (named_remote && remote_is_configured(named_remote, 1));
+ if (bare_ns) {
+ char *head_path = xstrfmt("refs/remotes/%s/HEAD", arg);
+ const char *head_target =
+ refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ head_path,
+ RESOLVE_REF_READING,
+ NULL, NULL);
+ if (head_target &&
+ starts_with(head_target, dst.buf) &&
+ head_target[dst.len] == '/') {
+ strbuf_reset(&dst);
+ strbuf_addstr(&dst, head_target);
+ bare_ns = 0;
+ }
+ free(head_path);
+ }
+
+ tracking.spec.dst = dst.buf;
+ tracking.srcs = &tracking_srcs;
+ find_tracking_remote_for_ref(&tracking, &ambiguous_remotes);
+
+ if (tracking.matches > 1) {
+ int status = die_message(_("cannot fetch start-point '%s': "
+ "fetch refspecs of multiple remotes "
+ "map to '%s'"), arg, dst.buf);
+ advise_ambiguous_fetch_refspec(dst.buf, &ambiguous_remotes);
+ exit(status);
+ }
+
+ if (!tracking.matches) {
+ if (bare_ns && named_remote &&
+ remote_is_configured(named_remote, 1)) {
+ int status = die_message(_("cannot fetch start-point '%s' "
+ "because 'refs/remotes/%s/HEAD' "
+ "does not exist."), arg, arg);
+ advise(_("To create it run\n"
+ "\n"
+ " git remote set-head %s --auto\n"), arg);
+ exit(status);
+ }
+ die(_("cannot fetch start-point '%s': no configured remote's "
+ "fetch refspec matches it"), arg);
+ }
+
+ strvec_push(&cmd.args, "fetch");
+ if (quiet)
+ strvec_push(&cmd.args, "--quiet");
+ strvec_pushl(&cmd.args, tracking.remote,
+ tracking_srcs.items[0].string, NULL);
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
+ if (refs_ref_exists(get_main_ref_store(the_repository), dst.buf))
+ warning(_("failed to fetch start-point '%s'; "
+ "using existing '%s'"), arg, dst.buf);
+ else
+ die(_("failed to fetch start-point '%s'"), arg);
+ }
+
+ string_list_clear(&tracking_srcs, 0);
+ string_list_clear(&ambiguous_remotes, 0);
+ strbuf_release(&dst);
+}
+
+static int parse_opt_checkout_track(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct checkout_opts *opts = opt->value;
+ struct string_list tokens = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+ int saw_direct = 0;
+ int ret = 0;
+
+ opts->fetch = 0;
+ if (unset) {
+ opts->track = BRANCH_TRACK_NEVER;
+ return 0;
+ }
+ opts->track = BRANCH_TRACK_EXPLICIT;
+ if (!arg)
+ return 0;
+
+ string_list_split(&tokens, arg, ",", -1);
+ for_each_string_list_item(item, &tokens) {
+ if (!strcmp(item->string, "fetch"))
+ opts->fetch = 1;
+ else if (!strcmp(item->string, "direct"))
+ saw_direct = 1;
+ else if (!strcmp(item->string, "inherit"))
+ opts->track = BRANCH_TRACK_INHERIT;
+ else {
+ ret = error(_("option `%s' expects \"%s\", \"%s\", "
+ "or \"%s\""),
+ "--track", "direct", "inherit", "fetch");
+ goto out;
+ }
+ }
+ if (saw_direct && opts->track == BRANCH_TRACK_INHERIT)
+ ret = error(_("option `%s' cannot combine \"%s\" and \"%s\""),
+ "--track", "direct", "inherit");
+out:
+ string_list_clear(&tokens, 0);
+ return ret;
+}
+
static void branch_info_release(struct branch_info *info)
{
free(info->name);
@@ -1786,10 +1911,10 @@ static struct option *add_common_switch_branch_options(
{
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_CALLBACK_F('t', "track", &opts->track, "(direct|inherit)",
+ OPT_CALLBACK_F('t', "track", opts, "(direct|inherit|fetch)[,...]",
N_("set branch tracking configuration"),
PARSE_OPT_OPTARG,
- parse_opt_tracking_mode),
+ parse_opt_checkout_track),
OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
PARSE_OPT_NOCOMPLETE),
OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unborn branch")),
@@ -1994,8 +2119,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
opts->dwim_new_local_branch &&
opts->track == BRANCH_TRACK_UNSPECIFIED &&
!opts->new_branch;
- int n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
- &new_branch_info, opts, &rev);
+ int n;
+
+ if (opts->fetch)
+ fetch_remote_for_start_point(argv[0], opts->quiet);
+
+ n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
+ &new_branch_info, opts, &rev);
argv += n;
argc -= n;
} else if (!opts->accept_ref && opts->from_treeish) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 7613b1d2a4..bc0c72c49d 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -870,4 +870,226 @@ test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
test_cmp_config "" --default "" branch.main2.merge
'
+test_expect_success 'setup upstream for --track=fetch tests' '
+ git checkout main &&
+ git init fetch_upstream &&
+ test_commit -C fetch_upstream u_main &&
+ git remote add fetch_upstream fetch_upstream &&
+ git fetch fetch_upstream &&
+ git -C fetch_upstream checkout -b fetch_new &&
+ test_commit -C fetch_upstream u_new
+'
+
+test_expect_success 'checkout --track=fetch -b picks up branch created upstream after clone' '
+ git checkout main &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new &&
+ git checkout --track=fetch -b local_new fetch_upstream/fetch_new &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_new HEAD &&
+ test_cmp_config fetch_upstream branch.local_new.remote &&
+ test_cmp_config refs/heads/fetch_new branch.local_new.merge
+'
+
+test_expect_success 'checkout --track=fetch <remote>/<branch> leaves other tracking branches untouched' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_target &&
+ test_commit -C fetch_upstream u_target_pre &&
+ git -C fetch_upstream checkout -b fetch_other &&
+ test_commit -C fetch_upstream u_other_pre &&
+ git fetch fetch_upstream &&
+ git update-ref refs/heads/snapshot_other refs/remotes/fetch_upstream/fetch_other &&
+ git -C fetch_upstream checkout fetch_target &&
+ test_commit -C fetch_upstream u_target_post &&
+ git -C fetch_upstream checkout fetch_other &&
+ test_commit -C fetch_upstream u_other_post &&
+ git checkout --track=fetch -b local_target fetch_upstream/fetch_target &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_target HEAD &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_other snapshot_other
+'
+
+test_expect_success 'checkout --track=fetch with bare remote name fetches only <remote>/HEAD target' '
+ git checkout main &&
+ git -C fetch_upstream checkout main &&
+ git remote set-head fetch_upstream main &&
+ git -C fetch_upstream checkout -b fetch_unrelated &&
+ test_commit -C fetch_upstream u_unrelated_pre &&
+ git fetch fetch_upstream fetch_unrelated &&
+ git update-ref refs/heads/snapshot_unrelated \
+ refs/remotes/fetch_upstream/fetch_unrelated &&
+ git -C fetch_upstream checkout main &&
+ test_commit -C fetch_upstream u_main_post &&
+ git -C fetch_upstream checkout fetch_unrelated &&
+ test_commit -C fetch_upstream u_unrelated_post &&
+ git checkout --track=fetch -b local_from_remote fetch_upstream &&
+ test_cmp_rev refs/remotes/fetch_upstream/main HEAD &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_unrelated snapshot_unrelated
+'
+
+test_expect_success 'checkout --track=fetch aborts and does not create branch when no existing ref' '
+ git checkout main &&
+ test_might_fail git branch -D bogus &&
+ test_must_fail git checkout --track=fetch -b bogus fetch_upstream/does_not_exist &&
+ test_must_fail git rev-parse --verify refs/heads/bogus
+'
+
+test_expect_success 'checkout --track=fetch warns and proceeds when fetch fails but ref exists' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_offline &&
+ test_commit -C fetch_upstream u_offline &&
+ git fetch fetch_upstream fetch_offline &&
+ saved_url=$(git config remote.fetch_upstream.url) &&
+ test_when_finished "git config remote.fetch_upstream.url \"$saved_url\"" &&
+ git config remote.fetch_upstream.url ./does-not-exist &&
+ git checkout --track=fetch -b local_offline fetch_upstream/fetch_offline 2>err &&
+ test_grep "failed to fetch" err &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_offline HEAD
+'
+
+test_expect_success 'checkout --track=fetch resolves through configured fetch refspec' '
+ git checkout main &&
+ git remote add fetch_custom ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_custom" &&
+ git config --replace-all remote.fetch_custom.fetch \
+ "+refs/heads/*:refs/remotes/custom-ns/*" &&
+ git -C fetch_upstream checkout -b fetch_refspec &&
+ test_commit -C fetch_upstream u_refspec &&
+ test_must_fail git rev-parse --verify refs/remotes/custom-ns/fetch_refspec &&
+ git checkout --track=fetch -b local_refspec custom-ns/fetch_refspec &&
+ test_cmp_rev refs/remotes/custom-ns/fetch_refspec HEAD
+'
+
+test_expect_success 'checkout --track=fetch on bare remote-tracking prefix follows <prefix>/HEAD' '
+ git checkout main &&
+ git remote add fetch_ns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_ns" &&
+ test_when_finished "git update-ref -d refs/remotes/ns_alias/HEAD" &&
+ git config --replace-all remote.fetch_ns.fetch \
+ "+refs/heads/*:refs/remotes/ns_alias/*" &&
+ git fetch fetch_ns &&
+ git symbolic-ref refs/remotes/ns_alias/HEAD refs/remotes/ns_alias/main &&
+ git -C fetch_upstream checkout main &&
+ test_commit -C fetch_upstream u_ns_post &&
+ git checkout --track=fetch -b local_ns ns_alias &&
+ test_cmp_rev refs/remotes/ns_alias/main HEAD &&
+ test_cmp_config fetch_ns branch.local_ns.remote &&
+ test_cmp_config refs/heads/main branch.local_ns.merge
+'
+
+test_expect_success 'checkout --track=fetch dies on bare remote name with no <ns>/HEAD' '
+ git checkout main &&
+ git remote add fetch_nohead ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_nohead" &&
+ test_might_fail git symbolic-ref -d refs/remotes/fetch_nohead/HEAD &&
+ test_must_fail git checkout --track=fetch -b local_nohead fetch_nohead 2>err &&
+ test_grep "refs/remotes/fetch_nohead/HEAD" err &&
+ test_grep "git remote set-head fetch_nohead --auto" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_nohead
+'
+
+test_expect_success 'checkout --track=fetch on bare unknown name does not suggest set-head' '
+ git checkout main &&
+ test_must_fail git rev-parse --verify refs/remotes/no_such_ns/HEAD &&
+ test_must_fail git config --get remote.no_such_ns.url &&
+ test_must_fail git checkout --track=fetch -b local_unknown no_such_ns 2>err &&
+ test_grep "no configured remote" err &&
+ test_grep ! "set-head" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_unknown
+'
+
+test_expect_success 'checkout --track=fetch rejects <ns>/HEAD pointing outside the tracking prefix' '
+ git checkout main &&
+ git remote add fetch_crossns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_crossns" &&
+ test_when_finished "git update-ref -d refs/remotes/fetch_crossns/HEAD" &&
+ git fetch fetch_crossns &&
+ git symbolic-ref refs/remotes/fetch_crossns/HEAD \
+ refs/remotes/fetch_upstream/u_main &&
+ test_must_fail git checkout --track=fetch -b local_crossns fetch_crossns 2>err &&
+ test_grep "refs/remotes/fetch_crossns/HEAD" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_crossns
+'
+
+test_expect_success 'checkout --track=fetch dies on ambiguous fetch refspec match' '
+ git checkout main &&
+ git remote add fetch_ambig_a ./fetch_upstream &&
+ git remote add fetch_ambig_b ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_ambig_a" &&
+ test_when_finished "git remote remove fetch_ambig_b" &&
+ git config --replace-all remote.fetch_ambig_a.fetch \
+ "+refs/heads/*:refs/remotes/ambig_ns/*" &&
+ git config --replace-all remote.fetch_ambig_b.fetch \
+ "+refs/heads/*:refs/remotes/ambig_ns/*" &&
+ git -C fetch_upstream checkout -b fetch_ambig &&
+ test_commit -C fetch_upstream u_ambig &&
+ test_must_fail git checkout --track=fetch -b local_ambig ambig_ns/fetch_ambig 2>err &&
+ test_grep "fetch_ambig_a" err &&
+ test_grep "fetch_ambig_b" err &&
+ test_grep "tracking namespaces" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_ambig
+'
+
+test_expect_success 'checkout --track=fetch rejects invalid refname components' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch -b local_invalid "foo..bar" 2>err &&
+ test_grep "valid" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_invalid
+'
+
+test_expect_success 'checkout --track=inherit,direct is rejected' '
+ test_must_fail git checkout --track=inherit,direct -b bad fetch_upstream/fetch_new 2>err &&
+ test_grep "cannot combine" err
+'
+
+test_expect_success 'checkout --track=fetch then --track=direct drops fetch (last-one-wins)' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_lastwin &&
+ test_commit -C fetch_upstream u_lastwin &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin &&
+ test_must_fail git checkout --track=fetch --track=direct \
+ -b local_lastwin fetch_upstream/fetch_lastwin &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin
+'
+
+test_expect_success 'checkout --track=fetch,inherit fetches remote-tracking start-point' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_inherit &&
+ test_commit -C fetch_upstream u_inherit &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_inherit &&
+ git checkout --track=fetch,inherit -b local_inherit \
+ fetch_upstream/fetch_inherit &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD
+'
+
+test_expect_success 'checkout --track=fetch on local start-point errors' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch -b bad main 2>err &&
+ test_grep "no configured remote" err &&
+ test_must_fail git rev-parse --verify refs/heads/bad
+'
+
+test_expect_success 'checkout --track=bogus reports an error' '
+ git checkout main &&
+ test_must_fail git checkout --track=bogus -b bogus_branch fetch_upstream/fetch_new 2>err &&
+ test_grep "expects" err
+'
+
+test_expect_success 'checkout -q --track=fetch silences the fetch output' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_quiet &&
+ test_commit -C fetch_upstream u_quiet &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_quiet &&
+ git checkout -q --track=fetch -b local_quiet \
+ fetch_upstream/fetch_quiet 2>err &&
+ test_grep ! "-> fetch_upstream/fetch_quiet" err &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_quiet HEAD
+'
+
+test_expect_success 'switch --track=fetch -c picks up branch created upstream after clone' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_switch &&
+ test_commit -C fetch_upstream u_switch &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_switch &&
+ git switch --track=fetch -c local_switch fetch_upstream/fetch_switch &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_switch HEAD
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 0/4] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:54 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
In-Reply-To: <pull.2337.v4.git.git.1782021195.gitgitgadget@gmail.com>
Adds git history squash <revision-range> to fold a range of commits.
Changes in v5:
* The range walk now uses --ancestry-path, so only commits descended from
the base are folded; a single revision such as HEAD or HEAD~1 is now
rejected as "not a <base>..<tip> range" rather than treated as a squash
down to the root.
* This adopts the --ancestry-path suggestion; the multi-base rejection is
unchanged, so a side branch that forked before the base and merged in is
still refused.
* Added tests covering more merge topologies: two interior merges, a nested
merge, an octopus merge, an octopus arm forked before the base, a merge
among the descendants replayed above the range, and a ref pointing at an
interior merge commit.
Changes in v4:
* git history squash now detects when another ref points at a commit inside
the range being folded and refuses, with an advice.historyUpdateRefs hint
to use --update-refs=head.
* A merge inside the range is folded fine as long as the range has a single
base; a range with merge commit at the tip or base also folds correctly.
Only a range with more than one base is rejected.
Changes in v3:
* Moved the feature out of git rebase and into a new git history squash
<revision-range> subcommand, per the list discussion. git rebase --squash
is dropped.
* Takes an arbitrary range (git history squash @~3.., git history squash
@~5..@~2), folding it into the oldest commit and replaying any
descendants on top.
* Implemented as a single tree operation rather than picking each commit,
so there are no repeated conflict stops (addresses Phillip's efficiency
point).
* A merge inside the range is folded fine, only a range with more than one
base is rejected.
* --reedit-message seeds the editor with every folded-in message, not just
the oldest.
Harald Nordgren (4):
history: extract helper for a commit's parent tree
history: give commit_tree_ext a message template
history: add squash subcommand to fold a range
history: re-edit a squash with every message
Documentation/config/advice.adoc | 4 +
Documentation/git-history.adoc | 26 ++
advice.c | 1 +
advice.h | 1 +
builtin/history.c | 341 ++++++++++++++++++---
t/meson.build | 1 +
t/t3455-history-squash.sh | 497 +++++++++++++++++++++++++++++++
7 files changed, 833 insertions(+), 38 deletions(-)
create mode 100755 t/t3455-history-squash.sh
base-commit: 26d8d94e94df5535eecd036f16627493506a0614
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v5
Pull-Request: https://github.com/git/git/pull/2337
Range-diff vs v4:
1: fc2801c0b1 = 1: 0f1ae9b05a history: extract helper for a commit's parent tree
2: ee591e83b4 = 2: a97ffab1e6 history: give commit_tree_ext a message template
3: 80bfea642e ! 3: 04e18ef979 history: add squash subcommand to fold a range
@@ builtin/history.c: out:
+ struct rev_info revs;
+ struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
+ struct strvec args = STRVEC_INIT;
++ size_t i;
+ int ret;
+
+ repo_init_revisions(repo, &revs, NULL);
@@ builtin/history.c: out:
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_push(&args, "--boundary");
++ strvec_push(&args, "--ancestry-path");
+ strvec_push(&args, range);
+ setup_revisions_from_strvec(&args, &revs, NULL);
+ if (args.nr != 1) {
@@ builtin/history.c: out:
+ goto out;
+ }
+
++ /*
++ * A squash needs a base to reparent onto, so the argument has to
++ * exclude something, as in "<base>..<tip>". A single revision has no
++ * such bottom commit and 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(_("'%s' is not a '<base>..<tip>' range"), range);
++ goto out;
++ }
++
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
@@ builtin/history.c: out:
+ goto out;
+ }
+
-+ if (!base) {
-+ ret = error(_("cannot squash the root commit"));
-+ goto out;
-+ }
++ if (!base)
++ BUG("a non-empty range must have a boundary commit");
+
+ *base_out = base;
+ *oldest_out = oldest;
@@ t/t3455-history-squash.sh (new)
+ test_grep "the range .* is empty" err
+'
+
-+test_expect_success 'errors when the range includes the root commit' '
++test_expect_success 'errors on a single revision that is not a range' '
+ test_must_fail git history squash HEAD 2>err &&
-+ test_grep "cannot squash the root commit" err
++ test_grep "is not a .*range" err &&
++ test_must_fail git history squash HEAD~1 2>err &&
++ test_grep "is not a .*range" err
+'
+
+test_expect_success 'squashes a range into a single commit without changing the tree' '
@@ t/t3455-history-squash.sh (new)
+ test_path_is_file inner
+'
+
++test_expect_success 'folds a merge of a branch that forked at the base' '
++ git reset --hard start &&
++ git checkout -b base-fork-side &&
++ test_commit --no-tag base-fork-side side x &&
++ git checkout - &&
++ 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.. &&
++
++ git rev-list --count start..HEAD >count &&
++ echo 1 >expect &&
++ test_cmp expect count &&
++ test_cmp_rev start HEAD^ &&
++ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
++ test_path_is_file side
++'
++
+test_expect_success 'folds a range whose tip is a merge commit' '
+ git reset --hard start &&
+ test_commit --no-tag tipmerge-base file b &&
@@ t/t3455-history-squash.sh (new)
+ test_cmp_rev "$merged" HEAD
+'
+
++test_expect_success 'folds a range with two interior merges' '
++ git reset --hard start &&
++ 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 - &&
++ 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 - &&
++ 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.. &&
++
++ git rev-list --count start..HEAD >count &&
++ echo 1 >expect &&
++ test_cmp expect count &&
++ 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.. &&
++
++ git rev-list --count start..HEAD >count &&
++ echo 1 >expect &&
++ test_cmp expect count &&
++ 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.. &&
++
++ git rev-list --count start..HEAD >count &&
++ echo 1 >expect &&
++ test_cmp expect count &&
++ 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-base file b &&
++ 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 c &&
++ 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
4: 85c7817d7e = 4: a758e1f084 history: re-edit a squash with every message
--
gitgitgadget
^ permalink raw reply
* [PATCH v15 1/2] branch: expose helpers for finding the remote owning a tracking ref
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:54 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Phillip Wood, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2281.v15.git.git.1782338098.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The remote-lookup that setup_tracking() does is useful outside
branch.c too; for example, deciding which remote to "git fetch"
from given a remote-tracking ref.
Move 'struct tracking' to branch.h and add two helpers backed by the
existing for_each_remote walk: find_tracking_remote_for_ref() and
advise_ambiguous_fetch_refspec(). setup_tracking() uses both. No
behavior change.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
branch.c | 96 ++++++++++++++++++++++++++++++--------------------------
branch.h | 16 ++++++++++
2 files changed, 68 insertions(+), 44 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..46ae7f0035 100644
--- a/branch.c
+++ b/branch.c
@@ -20,16 +20,9 @@
#include "run-command.h"
#include "strmap.h"
-struct tracking {
- struct refspec_item spec;
- struct string_list *srcs;
- const char *remote;
- int matches;
-};
-
struct find_tracked_branch_cb {
struct tracking *tracking;
- struct string_list ambiguous_remotes;
+ struct string_list *ambiguous_remotes;
};
static int find_tracked_branch(struct remote *remote, void *priv)
@@ -45,10 +38,10 @@ static int find_tracked_branch(struct remote *remote, void *priv)
break;
case 2:
/* there are at least two remotes; backfill the first one */
- string_list_append(&ftb->ambiguous_remotes, tracking->remote);
+ string_list_append(ftb->ambiguous_remotes, tracking->remote);
/* fall through */
default:
- string_list_append(&ftb->ambiguous_remotes, remote->name);
+ string_list_append(ftb->ambiguous_remotes, remote->name);
free(tracking->spec.src);
string_list_clear(tracking->srcs, 0);
break;
@@ -59,6 +52,51 @@ static int find_tracked_branch(struct remote *remote, void *priv)
return 0;
}
+void find_tracking_remote_for_ref(struct tracking *tracking,
+ struct string_list *ambiguous_remotes)
+{
+ struct find_tracked_branch_cb ftb_cb = {
+ .tracking = tracking,
+ .ambiguous_remotes = ambiguous_remotes,
+ };
+
+ for_each_remote(find_tracked_branch, &ftb_cb);
+}
+
+void advise_ambiguous_fetch_refspec(const char *dst,
+ const struct string_list *ambiguous_remotes)
+{
+ struct strbuf remotes_advice = STRBUF_INIT;
+ struct string_list_item *item;
+
+ if (!advice_enabled(ADVICE_AMBIGUOUS_FETCH_REFSPEC))
+ return;
+
+ for_each_string_list_item(item, ambiguous_remotes)
+ /*
+ * TRANSLATORS: This is a line listing a remote with duplicate
+ * refspecs in the advice message below. For RTL languages you'll
+ * probably want to swap the "%s" and leading " " space around.
+ */
+ strbuf_addf(&remotes_advice, _(" %s\n"), item->string);
+
+ /*
+ * TRANSLATORS: The second argument is a \n-delimited list of
+ * duplicate refspecs, composed above.
+ */
+ advise(_("There are multiple remotes whose fetch refspecs map to the remote\n"
+ "tracking ref '%s':\n"
+ "%s"
+ "\n"
+ "This is typically a configuration error.\n"
+ "\n"
+ "To support setting up tracking branches, ensure that\n"
+ "different remotes' fetch refspecs map into different\n"
+ "tracking namespaces."), dst,
+ remotes_advice.buf);
+ strbuf_release(&remotes_advice);
+}
+
static int should_setup_rebase(const char *origin)
{
switch (autorebase) {
@@ -254,11 +292,8 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
{
struct tracking tracking;
struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
+ struct string_list ambiguous_remotes = STRING_LIST_INIT_DUP;
int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
- struct find_tracked_branch_cb ftb_cb = {
- .tracking = &tracking,
- .ambiguous_remotes = STRING_LIST_INIT_DUP,
- };
if (!track)
BUG("asked to set up tracking, but tracking is disallowed");
@@ -267,7 +302,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
tracking.spec.dst = (char *)orig_ref;
tracking.srcs = &tracking_srcs;
if (track != BRANCH_TRACK_INHERIT)
- for_each_remote(find_tracked_branch, &ftb_cb);
+ find_tracking_remote_for_ref(&tracking, &ambiguous_remotes);
else if (inherit_tracking(&tracking, orig_ref))
goto cleanup;
@@ -293,34 +328,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
if (tracking.matches > 1) {
int status = die_message(_("not tracking: ambiguous information for ref '%s'"),
orig_ref);
- if (advice_enabled(ADVICE_AMBIGUOUS_FETCH_REFSPEC)) {
- struct strbuf remotes_advice = STRBUF_INIT;
- struct string_list_item *item;
-
- for_each_string_list_item(item, &ftb_cb.ambiguous_remotes)
- /*
- * TRANSLATORS: This is a line listing a remote with duplicate
- * refspecs in the advice message below. For RTL languages you'll
- * probably want to swap the "%s" and leading " " space around.
- */
- strbuf_addf(&remotes_advice, _(" %s\n"), item->string);
-
- /*
- * TRANSLATORS: The second argument is a \n-delimited list of
- * duplicate refspecs, composed above.
- */
- advise(_("There are multiple remotes whose fetch refspecs map to the remote\n"
- "tracking ref '%s':\n"
- "%s"
- "\n"
- "This is typically a configuration error.\n"
- "\n"
- "To support setting up tracking branches, ensure that\n"
- "different remotes' fetch refspecs map into different\n"
- "tracking namespaces."), orig_ref,
- remotes_advice.buf);
- strbuf_release(&remotes_advice);
- }
+ advise_ambiguous_fetch_refspec(orig_ref, &ambiguous_remotes);
exit(status);
}
@@ -347,7 +355,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
cleanup:
string_list_clear(&tracking_srcs, 0);
- string_list_clear(&ftb_cb.ambiguous_remotes, 0);
+ string_list_clear(&ambiguous_remotes, 0);
}
int read_branch_desc(struct strbuf *buf, const char *branch_name)
diff --git a/branch.h b/branch.h
index 3dc6e2a0ff..c2e6725491 100644
--- a/branch.h
+++ b/branch.h
@@ -1,9 +1,25 @@
#ifndef BRANCH_H
#define BRANCH_H
+#include "refspec.h"
+
+struct string_list;
struct repository;
struct strbuf;
+struct tracking {
+ struct refspec_item spec;
+ struct string_list *srcs;
+ const char *remote;
+ int matches;
+};
+
+void find_tracking_remote_for_ref(struct tracking *tracking,
+ struct string_list *ambiguous_remotes);
+
+void advise_ambiguous_fetch_refspec(const char *dst,
+ const struct string_list *ambiguous_remotes);
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v15 0/2] checkout: --track=fetch
From: Harald Nordgren via GitGitGadget @ 2026-06-24 21:54 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Phillip Wood, Harald Nordgren
In-Reply-To: <pull.2281.v14.git.git.1781786652.gitgitgadget@gmail.com>
Extend checkout --track with a fetch mode to refresh start-point.
Changes in v15:
* Reword commit message to lead with motivation.
* Drop RESOLVE_REF_NO_RECURSE so <ns>/HEAD lookup matches what git checkout
does for the same ref. Drop redundant check_refname_format on a ref we
just read. Replace memset with brace initializer. Use refs_ref_exists and
pass NULL for the OID out-params.
* Split the "set HEAD" error onto its own line via die_message and advise,
matching the suggested format.
* Remove 6 redundant tests, replace test $a = $b with test_cmp_rev, and
rename test titles to avoid "namespace".
Changes in v14:
* Handle .h files in a better way.
Changes in v13:
* Create a preparatory commit that exposes find_tracking_remote_for_ref()
and advise_ambiguous_fetch_refspec() from branch.c, so checkout can reuse
the same lookup git branch --track uses.
* Use advise_ambiguous_fetch_refspec() for the "multiple remotes match"
case, so the wording matches git branch --track.
Harald Nordgren (2):
branch: expose helpers for finding the remote owning a tracking ref
checkout: extend --track with a "fetch" mode to refresh start-point
Documentation/git-checkout.adoc | 17 ++-
Documentation/git-switch.adoc | 5 +-
branch.c | 96 +++++++-------
branch.h | 16 +++
builtin/checkout.c | 138 +++++++++++++++++++-
t/t7201-co.sh | 222 ++++++++++++++++++++++++++++++++
6 files changed, 443 insertions(+), 51 deletions(-)
base-commit: ab776a62a78576513ee121424adb19597fbb7613
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2281%2FHaraldNordgren%2Fcheckout-fetch-start-point-v15
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2281/HaraldNordgren/checkout-fetch-start-point-v15
Pull-Request: https://github.com/git/git/pull/2281
Range-diff vs v14:
1: f79689c23d = 1: 8139490c36 branch: expose helpers for finding the remote owning a tracking ref
2: 8518f090b1 ! 2: b7e387f123 checkout: extend --track with a "fetch" mode to refresh start-point
@@ Metadata
## Commit message ##
checkout: extend --track with a "fetch" mode to refresh start-point
- Add a "fetch" mode to the "--track" option of "git checkout" / "git
- switch" that refreshes <start-point> before checking it out:
+ Forking from an existing remote branch without refreshing first often
+ has consequences: you start work that has already been done, or you
+ build on an old version of the code which causes big conflicts later
+ when you pull. The workaround is two commands ("git fetch <remote>
+ <branch> && git checkout -b <topic> <remote>/<branch>"), and when
+ the fetch is skipped the checkout silently starts from a stale tip.
- git checkout -b new_branch --track=fetch origin/some-branch
+ Users may already expect "<remote>/<branch>" to refer to the latest
+ tip on the remote. While this blurs the line between fetch and
+ checkout, git already does this in places where it pays off: "git
+ clone" fetches and checks out, and "git pull" fetches and merges.
- is shorthand for
+ Add a "fetch" mode to "--track" that refreshes <start-point> before
+ checking it out:
- git fetch origin some-branch
- git checkout -b new_branch --track origin/some-branch
+ git checkout -b new_branch --track=fetch origin/some-branch
- Identify the remote whose configured fetch refspec maps to
- <start-point> using find_tracking_remote_for_ref() (the same lookup
- "--track" uses to pick which remote to record in
- branch.<name>.remote), then run "git fetch <remote> <src-ref>" for
- just that ref so other remote-tracking branches are left untouched.
- When <start-point> is a bare <remote> (e.g. "origin"), follow
- refs/remotes/<remote>/HEAD to learn which branch to refresh. If
- "git fetch" fails but the remote-tracking ref already exists locally,
- warn and proceed from the existing tip; otherwise abort.
+ Only the requested branch is fetched so other remote-tracking
+ branches are left untouched. When <start-point> is a bare <remote>
+ (e.g. "origin"), follow refs/remotes/<remote>/HEAD to learn which
+ branch to refresh. If "git fetch" fails but the remote-tracking ref
+ already exists locally, warn and proceed from the existing tip,
+ otherwise abort.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ builtin/checkout.c: struct branch_info {
+static void fetch_remote_for_start_point(const char *arg, int quiet)
+{
+ struct strbuf dst = STRBUF_INIT;
-+ struct tracking tracking;
++ struct tracking tracking = { 0 };
+ struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
+ struct string_list ambiguous_remotes = STRING_LIST_INIT_DUP;
+ struct child_process cmd = CHILD_PROCESS_INIT;
-+ struct object_id oid;
+ struct remote *named_remote;
+ int bare_ns;
+
@@ builtin/checkout.c: struct branch_info {
+ const char *head_target =
+ refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ head_path,
-+ RESOLVE_REF_READING |
-+ RESOLVE_REF_NO_RECURSE,
-+ &oid, NULL);
++ RESOLVE_REF_READING,
++ NULL, NULL);
+ if (head_target &&
+ starts_with(head_target, dst.buf) &&
-+ head_target[dst.len] == '/' &&
-+ !check_refname_format(head_target, 0)) {
++ head_target[dst.len] == '/') {
+ strbuf_reset(&dst);
+ strbuf_addstr(&dst, head_target);
+ bare_ns = 0;
@@ builtin/checkout.c: struct branch_info {
+ free(head_path);
+ }
+
-+ memset(&tracking, 0, sizeof(tracking));
+ tracking.spec.dst = dst.buf;
+ tracking.srcs = &tracking_srcs;
+ find_tracking_remote_for_ref(&tracking, &ambiguous_remotes);
@@ builtin/checkout.c: struct branch_info {
+
+ if (!tracking.matches) {
+ if (bare_ns && named_remote &&
-+ remote_is_configured(named_remote, 1))
-+ die(_("cannot fetch start-point '%s': "
-+ "'refs/remotes/%s/HEAD' is not set; run "
-+ "'git remote set-head %s --auto' to set it"),
-+ arg, arg, arg);
++ remote_is_configured(named_remote, 1)) {
++ int status = die_message(_("cannot fetch start-point '%s' "
++ "because 'refs/remotes/%s/HEAD' "
++ "does not exist."), arg, arg);
++ advise(_("To create it run\n"
++ "\n"
++ " git remote set-head %s --auto\n"), arg);
++ exit(status);
++ }
+ die(_("cannot fetch start-point '%s': no configured remote's "
+ "fetch refspec matches it"), arg);
+ }
@@ builtin/checkout.c: struct branch_info {
+ tracking_srcs.items[0].string, NULL);
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
-+ if (!refs_read_ref(get_main_ref_store(the_repository),
-+ dst.buf, &oid))
++ if (refs_ref_exists(get_main_ref_store(the_repository), dst.buf))
+ warning(_("failed to fetch start-point '%s'; "
+ "using existing '%s'"), arg, dst.buf);
+ else
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ git -C fetch_upstream checkout -b fetch_other &&
+ test_commit -C fetch_upstream u_other_pre &&
+ git fetch fetch_upstream &&
-+ other_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_other) &&
++ git update-ref refs/heads/snapshot_other refs/remotes/fetch_upstream/fetch_other &&
+ git -C fetch_upstream checkout fetch_target &&
+ test_commit -C fetch_upstream u_target_post &&
+ git -C fetch_upstream checkout fetch_other &&
+ test_commit -C fetch_upstream u_other_post &&
+ git checkout --track=fetch -b local_target fetch_upstream/fetch_target &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_target HEAD &&
-+ test "$(git rev-parse refs/remotes/fetch_upstream/fetch_other)" = "$other_before"
++ test_cmp_rev refs/remotes/fetch_upstream/fetch_other snapshot_other
+'
+
+test_expect_success 'checkout --track=fetch with bare remote name fetches only <remote>/HEAD target' '
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ git -C fetch_upstream checkout -b fetch_unrelated &&
+ test_commit -C fetch_upstream u_unrelated_pre &&
+ git fetch fetch_upstream fetch_unrelated &&
-+ unrelated_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_unrelated) &&
++ git update-ref refs/heads/snapshot_unrelated \
++ refs/remotes/fetch_upstream/fetch_unrelated &&
+ git -C fetch_upstream checkout main &&
+ test_commit -C fetch_upstream u_main_post &&
+ git -C fetch_upstream checkout fetch_unrelated &&
+ test_commit -C fetch_upstream u_unrelated_post &&
+ git checkout --track=fetch -b local_from_remote fetch_upstream &&
+ test_cmp_rev refs/remotes/fetch_upstream/main HEAD &&
-+ test "$(git rev-parse refs/remotes/fetch_upstream/fetch_unrelated)" = "$unrelated_before"
++ test_cmp_rev refs/remotes/fetch_upstream/fetch_unrelated snapshot_unrelated
+'
+
+test_expect_success 'checkout --track=fetch aborts and does not create branch when no existing ref' '
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_cmp_rev refs/remotes/custom-ns/fetch_refspec HEAD
+'
+
-+test_expect_success 'checkout --track=fetch on namespace bare name follows <ns>/HEAD' '
++test_expect_success 'checkout --track=fetch on bare remote-tracking prefix follows <prefix>/HEAD' '
+ git checkout main &&
+ git remote add fetch_ns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_ns" &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_cmp_config refs/heads/main branch.local_ns.merge
+'
+
-+test_expect_success '--track=fetch on bare hierarchical remote name follows <ns>/HEAD' '
-+ git checkout main &&
-+ git remote add nested/bare ./fetch_upstream &&
-+ test_when_finished "git remote remove nested/bare" &&
-+ test_when_finished "git update-ref -d refs/remotes/nested/bare/HEAD" &&
-+ git fetch nested/bare &&
-+ git symbolic-ref refs/remotes/nested/bare/HEAD \
-+ refs/remotes/nested/bare/main &&
-+ git -C fetch_upstream checkout main &&
-+ test_commit -C fetch_upstream u_nested_bare_post &&
-+ git checkout --track=fetch -b local_nested_bare nested/bare &&
-+ test_cmp_rev refs/remotes/nested/bare/main HEAD
-+'
-+
-+test_expect_success 'checkout --track=fetch handles hierarchical remote name' '
-+ git checkout main &&
-+ git remote add nested/remote ./fetch_upstream &&
-+ test_when_finished "git remote remove nested/remote" &&
-+ git -C fetch_upstream checkout -b fetch_hier &&
-+ test_commit -C fetch_upstream u_hier &&
-+ test_must_fail git rev-parse --verify refs/remotes/nested/remote/fetch_hier &&
-+ git checkout --track=fetch -b local_hier nested/remote/fetch_hier &&
-+ test_cmp_rev refs/remotes/nested/remote/fetch_hier HEAD
-+'
-+
+test_expect_success 'checkout --track=fetch dies on bare remote name with no <ns>/HEAD' '
+ git checkout main &&
+ git remote add fetch_nohead ./fetch_upstream &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_must_fail git rev-parse --verify refs/heads/local_unknown
+'
+
-+test_expect_success 'checkout --track=fetch rejects <ns>/HEAD pointing outside namespace' '
++test_expect_success 'checkout --track=fetch rejects <ns>/HEAD pointing outside the tracking prefix' '
+ git checkout main &&
+ git remote add fetch_crossns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_crossns" &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_must_fail git rev-parse --verify refs/heads/local_invalid
+'
+
-+test_expect_success 'checkout --track=fetch,inherit rejects invalid refname components' '
-+ git checkout main &&
-+ test_must_fail git checkout --track=fetch,inherit -b local_invalid \
-+ "foo..bar" 2>err &&
-+ test_grep "valid" err &&
-+ test_must_fail git rev-parse --verify refs/heads/local_invalid
-+'
-+
+test_expect_success 'checkout --track=inherit,direct is rejected' '
+ test_must_fail git checkout --track=inherit,direct -b bad fetch_upstream/fetch_new 2>err &&
+ test_grep "cannot combine" err
+'
+
-+test_expect_success 'checkout --track=direct,inherit is rejected' '
-+ test_must_fail git checkout --track=direct,inherit -b bad fetch_upstream/fetch_new 2>err &&
-+ test_grep "cannot combine" err
-+'
-+
+test_expect_success 'checkout --track=fetch then --track=direct drops fetch (last-one-wins)' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_lastwin &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin
+'
+
-+test_expect_success 'checkout --track=fetch then --no-track drops fetch' '
-+ git checkout main &&
-+ git -C fetch_upstream checkout -b fetch_notrack &&
-+ test_commit -C fetch_upstream u_notrack &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_notrack &&
-+ test_must_fail git checkout --track=fetch --no-track \
-+ -b local_notrack fetch_upstream/fetch_notrack &&
-+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_notrack
-+'
-+
+test_expect_success 'checkout --track=fetch,inherit fetches remote-tracking start-point' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_inherit &&
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD
+'
+
-+test_expect_success 'checkout --track=fetch,inherit errors when start-point does not map to a remote' '
-+ git checkout main &&
-+ test_must_fail git checkout --track=fetch,inherit -b bad main 2>err &&
-+ test_grep "no configured remote" err &&
-+ test_must_fail git rev-parse --verify refs/heads/bad
-+'
-+
+test_expect_success 'checkout --track=fetch on local start-point errors' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch -b bad main 2>err &&
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v5 08/11] refs/files: lazy-load configuration to fix chicken-and-egg
From: Justin Tobler @ 2026-06-24 21:36 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak, Jeff King
In-Reply-To: <20260622-b4-pks-refs-avoid-chdir-notify-reparent-v5-8-018475013dbc@pks.im>
On 26/06/22 10:28AM, Patrick Steinhardt wrote:
> When initializing the "files" reference backend we read the repository's
> config to parse "core.preferSymlinkRefs" and "core.logAllRefUpdates".
> This results in a chicken-and-egg problem though, because parsing the
> configuration may require us to have access to the reference store
> already when an "onbranch" condition exists.
Ok so both of these configuration options are currently parsed at ref
store initialization time. This is problematic because we need the ref
store to properly handle "onbranch" conditions in the config.
> Luckily, all the configuration that we honor only relates to writing
> references. Consequently, we don't strictly need that configuration to
> be readily available at initialization time, and we can easiliy defer
> parsing it to a later point in time.
That's nice. So we don't actually need this configuration during
initialization and can instead lazily load it when writing the first
references. Makes sense.
> Implement this fix and add tests that verify that we can indeed properly
> parse these config knobs via an "onbranch" condition.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> refs/files-backend.c | 37 ++++++++++++++++++++++++++-----------
> t/t0600-reffiles-backend.sh | 21 +++++++++++++++++++++
> 2 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 79fb6735e1..d0f379dcd6 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -84,12 +84,14 @@ struct files_ref_store {
> unsigned int store_flags;
>
> char *gitcommondir;
> - enum log_refs_config log_all_ref_updates;
> - int prefer_symlink_refs;
> -
> struct ref_cache *loose;
> -
> struct ref_store *packed_ref_store;
> +
> + struct files_ref_store_write_options {
> + enum log_refs_config log_all_ref_updates;
> + int prefer_symlink_refs;
> + bool initialized;
> + } write_opts_lazy_loaded;
It might be nice to leave some sort of breadcrumb comment to future
readers to explain why we lazy load this configuration.
> };
>
> static void clear_loose_ref_cache(struct files_ref_store *refs)
> @@ -121,17 +123,31 @@ static int files_ref_store_config(const char *var, const char *value,
> const struct config_context *ctx UNUSED,
> void *payload)
> {
> - struct files_ref_store *refs = payload;
> + struct files_ref_store_write_options *opts = payload;
>
> if (!strcmp(var, "core.prefersymlinkrefs")) {
> - refs->prefer_symlink_refs = git_config_bool(var, value);
> + opts->prefer_symlink_refs = git_config_bool(var, value);
> } else if (!strcmp(var, "core.logallrefupdates")) {
> - refs->log_all_ref_updates = refs_parse_log_all_ref_updates_config(value);
> + opts->log_all_ref_updates = refs_parse_log_all_ref_updates_config(value);
> }
>
> return 0;
> }
>
> +static const struct files_ref_store_write_options *files_ref_store_write_options(struct files_ref_store *refs)
> +{
> + struct files_ref_store_write_options *opts = &refs->write_opts_lazy_loaded;
> +
> + if (opts->initialized)
> + return opts;
> +
> + opts->log_all_ref_updates = LOG_REFS_UNSET;
> + repo_config(refs->base.repo, files_ref_store_config, opts);
> +
> + opts->initialized = true;
> + return opts;
> +}
> +
> /*
> * Create a new submodule ref cache and add it to the internal
> * set of caches.
> @@ -156,9 +172,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
> refs->packed_ref_store =
> packed_ref_store_init(repo, NULL, refs->gitcommondir, opts);
> refs->store_flags = opts->access_flags;
> - refs->log_all_ref_updates = LOG_REFS_UNSET;
>
> - repo_config(repo, files_ref_store_config, refs);
Configs are no longer read eagerly during initialization.
The rest of this patch looks good to me.
-Justin
^ permalink raw reply
* Re: [PATCH v5 07/11] refs: move parsing of "core.logAllRefUpdates" back into ref stores
From: Justin Tobler @ 2026-06-24 21:22 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak, Jeff King
In-Reply-To: <20260622-b4-pks-refs-avoid-chdir-notify-reparent-v5-7-018475013dbc@pks.im>
On 26/06/22 10:28AM, Patrick Steinhardt wrote:
> In cc42c88945 (refs: extract out reflog config to generic layer,
> 2026-05-04) we have refactored how we parse "core.logAllRefUpdates" so
> that it happens in the generic layer. Unfortunately, this has worsened a
> preexisting issue where we may recurse when creating the reference store
> because of a chicken-and-egg problem between parsing the configuration
> and evaluating "onbranch" conditions.
Ok so IIUC, parsing "core.logAllRefUpdates" in the generic layer forces
us to read the config earlier. This is problematic though since the
refstore has not been initialized yet which we need to evaluate
"onbranch" conditions.
> Prepare for a fix by essentially reverting that change so that we handle
> this setting in the respective backends again. The backends are already
> parsing other configuration anyway, so by moving the logic back in there
> we can ensure that all backend configuration is parsed the same way.
Makes sense.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/checkout.c | 7 +++++--
> refs.c | 10 +++++++++-
> refs.h | 9 +++++++++
> refs/files-backend.c | 20 +++++++++++++++++---
> refs/refs-internal.h | 6 ------
> refs/reftable-backend.c | 20 +++++++++++---------
> repo-settings.c | 16 ----------------
> repo-settings.h | 9 ---------
> setup.c | 7 ++++++-
> 9 files changed, 57 insertions(+), 47 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index b78b3a1d16..aee84ca897 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -952,10 +952,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
> const char *old_desc, *reflog_msg;
> if (opts->new_branch) {
> if (opts->new_orphan_branch) {
> - enum log_refs_config log_all_ref_updates =
> - repo_settings_get_log_all_ref_updates(the_repository);
> + enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
> + const char *value;
> char *refname;
>
> + if (!repo_config_get_string_tmp(the_repository, "core.logallrefupdates", &value))
> + log_all_ref_updates = refs_parse_log_all_ref_updates_config(value);
> +
> refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch);
> if (opts->new_branch_log &&
> !should_autocreate_reflog(log_all_ref_updates, refname)) {
> diff --git a/refs.c b/refs.c
> index d3caa9a633..5b773b1c15 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1053,6 +1053,15 @@ static char *normalize_reflog_message(const char *msg)
> return strbuf_detach(&sb, NULL);
> }
>
> +enum log_refs_config refs_parse_log_all_ref_updates_config(const char *value)
> +{
> + if (value && !strcasecmp(value, "always"))
> + return LOG_REFS_ALWAYS;
> + else if (git_config_bool("core.logallrefupdates", value))
> + return LOG_REFS_NORMAL;
> + return LOG_REFS_NONE;
> +}
This function replaces `repo_settings_get_log_all_ref_updates()`. I
assume we just wanted a slightly more simple function where the only
concern was parsing the `core.logallrefupdates` value.
> +
> int should_autocreate_reflog(enum log_refs_config log_all_ref_updates,
> const char *refname)
> {
> @@ -2327,7 +2336,6 @@ static struct ref_store *ref_store_init(struct repository *repo,
> struct ref_store *refs;
> struct ref_store_init_options opts = {
> .access_flags = flags,
> - .log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo),
This config is no longer handled in the generic layer.
[snip]
> diff --git a/setup.c b/setup.c
> index 79125db565..0c6efb0560 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2584,10 +2584,15 @@ static int create_default_files(struct repository *repo,
> if (is_bare_repository())
> repo_config_set(repo, "core.bare", "true");
> else {
> + const char *value;
> +
> repo_config_set(repo, "core.bare", "false");
> +
> /* allow template config file to override the default */
> - if (repo_settings_get_log_all_ref_updates(repo) == LOG_REFS_UNSET)
> + if (repo_config_get_string_tmp(repo, "core.logallrefupdates", &value) ||
> + refs_parse_log_all_ref_updates_config(value) == LOG_REFS_UNSET)
Huh, can `refs_parse_log_all_ref_updates_config()` even return
LOG_REFS_UNSET?
-Justin
^ permalink raw reply
* Re: [PATCH GSoC RFC v13 06/12] connect: refactor packet writing
From: Junio C Hamano @ 2026-06-24 20:19 UTC (permalink / raw)
To: Karthik Nayak
Cc: Pablo Sabater, peff, eric.peijian, chriscool, git, jltobler, toon,
chandrapratap3519, Jonathan Tan, Calvin Wan
In-Reply-To: <CAOLa=ZSvxXuf_bSzKMvViNQ5MuDAqxnQdo4asF9vfMhJaDQcVw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
>> +/*
>> + * Writes a command along with the requested server capabilities/features into a
>> + * request buffer.
>> + */
>> struct string_list;
>
> The comment should be above the function and not the forward
> declaration.
>
> While we're here, why not `#include "string-list.h"` and remove the
> forward declaration, is there a circular dependency?
Isn't it to avoid unnecessary include? When the header itself only
needs to know about the presence of the type, and not the concrete
shape of the type (e.g., because it only uses a pointer to that
type), it may be overkill to include the entire header file.
^ permalink raw reply
* Re: [PATCH 0/6] receive-pack: use ODB transactions to stage object writes
From: Junio C Hamano @ 2026-06-24 20:09 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, ps
In-Reply-To: <20260624041920.2601961-1-jltobler@gmail.com>
Justin Tobler <jltobler@gmail.com> writes:
> Greetings,
>
> This patch series replaces direct usage of the `tmp_objdir` interfaces
> in git-receive-pack(1) to instead use the `odb_transaction` interfaces
> to create/manage a staging area to write objects to. The purpose of this
> change is to get git-receive-pack(1) one step closer to being ODB
> backend agnostic. For now, the object writes themselves are still
> "files" backend specific due to being handled by the git-index-pack(1)
> and git-unpack-objects(1) child processes. This will be tackled in a
> separate series though.
>
> Thanks,
> -Justin
The integration cycle this morning was somehow more painful than
other cycles.
FYI, this topic had some interactions with ps/odb-source-packed and
ps/refs-avoid-chdir-notify-reparent and needed the following
evil-merge fix to make it build.
commit 721c15c1d5ef8f8aab8b9f50463e979e890b1ab6
Author: Junio C Hamano <gitster@pobox.com>
Date: Wed Jun 24 12:45:35 2026 -0700
merge-fix/jt/receive-pack-use-odb-transactions
conflict with ps/odb-source-packed and ps/refs-avoid-chdir-notify-reparent
diff --git a/odb/source-packed.c b/odb/source-packed.c
index c3c26fb53e..b9231a8da0 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -545,7 +545,8 @@ static int odb_source_packed_write_object_stream(struct odb_source *source UNUSE
}
static int odb_source_packed_begin_transaction(struct odb_source *source UNUSED,
- struct odb_transaction **out UNUSED)
+ struct odb_transaction **out UNUSED,
+ enum odb_transaction_flags flags UNUSED)
{
return error("packed backend cannot begin transactions");
}
diff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c
index a35bbc8004..dd5db06534 100644
--- a/t/unit-tests/u-reftable-table.c
+++ b/t/unit-tests/u-reftable-table.c
@@ -235,7 +235,8 @@ void test_reftable_table__seek_invalid_log_offset(void)
uint8_t *footer;
cl_reftable_write_to_buf(&buf, refs, ARRAY_SIZE(refs),
- logs, ARRAY_SIZE(logs), NULL);
+ logs, ARRAY_SIZE(logs),
+ REFTABLE_HASH_SHA1, NULL);
/*
* Corrupt the log section offset stored in the footer so that it points
@@ -278,7 +279,8 @@ void test_reftable_table__new_with_truncated_table(void)
struct reftable_table *table;
struct reftable_buf buf = REFTABLE_BUF_INIT;
- cl_reftable_write_to_buf(&buf, refs, ARRAY_SIZE(refs), NULL, 0, NULL);
+ cl_reftable_write_to_buf(&buf, refs, ARRAY_SIZE(refs), NULL, 0,
+ REFTABLE_HASH_SHA1, NULL);
/*
* Truncate the table so that it is large enough to read the header, but
^ permalink raw reply related
* Re: [PATCH v4 1/3] replay: refactor enum replay_mode into a bool
From: Toon Claes @ 2026-06-24 19:15 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt; +Cc: git, Elijah Newren
In-Reply-To: <xmqq7bnq37jm.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> That's fair, and the result is easier to write. But is it really easier
>> to read?
You're bringing up a very valid point there, and not directly in what
you're saying, but how it makes me reconsider.
So we're comparing:
pick_regular_commit(revs->repo, commit, replayed_commits,
mode == REPLAY_MODE_REVERT ? last_commit : onto,
&merge_opt, &result, mode, opts->empty);
with:
pick_regular_commit(revs->repo, commit, replayed_commits,
reverse ? last_commit : onto,
&merge_opt, &result, reverse, opts->empty);
You can argue which of both is easier to read, but the problem isn't
really whether it's a bool or an enum, but the ternary operator in this
lengthy function call is. That is the problem I was trying to solve, and
converting enum to bool isn't really the solution.
>> And what if we ever have to create a third mode going forward?
Personally I find this weak argument. As far as I know we most of the
time do not write code in a way so "it will be ready to add X in the
future". In my personal experience, I'm always wrong in predicting what
might be added in the future. Although I must say this case is
different, because we're not adding something new, no this commit was
dumbing down something existing. So I'll revisit this commit in the next
iteration.
>> I'm generally no fan of booleans as parameters as they basically give
>> you no information at all at the callsite, except if you're lucky and
>> you already have an aptly-named variable available that you can pass.
>> Which seems to be the case here, but I'm still not sure whether this
>> change really improves the code.
That's also a very valid argument, which I didn't take in mind.
> I tend to agree with you on both counts. The "what happens when
> somebody else wants a third choice?" is a quesiton I would ask the
> first thing as the maintainer of a project.
>
> Even if the boolean parameter is so obviously named, the callsite
> can only say "true" or "false", unlike some other popular languages
> that lets you say
>
> my_function(use_revert_mode=true, verbose=false);
>
> and you cannot tell what effect the author wanted out of that "true"
> if all you can write were
>
> my_function(true, false);
>
> Of course, we could go ultra verbose, like
>
> my_function(true, /* use_revert_mode */
> false, /* verbose */);
>
> but then we are often better off writing:
>
> my_function(REPLAY_MODE_REVERT, REPLAY_QUIET);
Thanks for bringing in this illustrative example. Point made, I'll
revisit.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH 2/6] object-file: propagate files transaction errors
From: Junio C Hamano @ 2026-06-24 18:35 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, ps
In-Reply-To: <20260624041920.2601961-3-jltobler@gmail.com>
Justin Tobler <jltobler@gmail.com> writes:
> The "files" transaction backend may encounter errors related to managing
> the temporary directory used to stage objects, but silently ignores
> these errors. Instead return errors encountered in the
> `odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
> callers to handle as needed.
"handle them as needed", perhaps.
> -static void odb_transaction_files_prepare(struct odb_transaction *base)
> +static int odb_transaction_files_prepare(struct odb_transaction *base)
> {
> struct odb_transaction_files *transaction =
> container_of_or_null(base, struct odb_transaction_files, base);
> @@ -511,11 +511,15 @@ static void odb_transaction_files_prepare(struct odb_transaction *base)
> * added at the time they call odb_transaction_files_begin.
> */
> if (!transaction || transaction->objdir)
> - return;
> + return 0;
>
> transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
If this fails, NULL is returned, and ...
> - if (transaction->objdir)
> - tmp_objdir_replace_primary_odb(transaction->objdir, 0);
> + if (!transaction->objdir)
> + return -1;
... we return -1 from here to signal an error now.
But callers of this function in write_loose_object(), and
odb_source_loose_write_stream() are not prepared to react to such an
error.
I guess this is nothing new. The callers ignored such an error from here
in the original and proceeded writing the primary ODB anyway, and we
continue to do so after this step.
> @@ -542,13 +546,13 @@ static void fsync_loose_object_transaction(struct odb_transaction *base,
> /*
> * Cleanup after batch-mode fsync_object_files.
> */
> -static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
> +static int flush_loose_object_transaction(struct odb_transaction_files *transaction)
> {
> struct strbuf temp_path = STRBUF_INIT;
> struct tempfile *temp;
>
> if (!transaction->objdir)
> - return;
> + return 0;
>
> /*
> * Issue a full hardware flush against a temporary file to ensure
> @@ -570,8 +574,12 @@ static void flush_loose_object_transaction(struct odb_transaction_files *transac
> * Make the object files visible in the primary ODB after their data is
> * fully durable.
> */
> - tmp_objdir_migrate(transaction->objdir);
> + if (tmp_objdir_migrate(transaction->objdir))
> + return -1;
> +
> transaction->objdir = NULL;
> +
> + return 0;
> }
The caller of this function does react to a failure of it, ...
> @@ -1670,27 +1678,34 @@ int read_loose_object(struct repository *repo,
> return ret;
> }
>
> -static void odb_transaction_files_commit(struct odb_transaction *base)
> +static int odb_transaction_files_commit(struct odb_transaction *base)
> {
> struct odb_transaction_files *transaction =
> container_of(base, struct odb_transaction_files, base);
>
> - flush_loose_object_transaction(transaction);
> + if (flush_loose_object_transaction(transaction))
> + return -1;
> flush_packfile_transaction(transaction);
> +
> + return 0;
> }
... like this, which is good. Do we need an explicit "abort-transaction",
or is that implicit?
Thanks.
^ permalink raw reply
* Re: [PATCH 1/6] object-file: rename files transaction prepare function
From: Junio C Hamano @ 2026-06-24 18:26 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, ps
In-Reply-To: <20260624041920.2601961-2-jltobler@gmail.com>
Justin Tobler <jltobler@gmail.com> writes:
> The "files" ODB transaction backend lazily creates a temporary object
> directory when the first loose object is written to the transaction via
> `prepare_loose_object_transaction()`. In a subsequent commit, the
> temporary directory is used to also write packfiles to.
>
> Rename the function to `odb_transaction_files_prepare()` accordingly.
Taken by itself this renaming does make sense, but there are many
other function that follow the historical naming convention, like
{fsync,flush}_loose_object_transaction(). Should we rename them for
consistency with the new naming scheme, not necessarily as part of
this series but with a todo comment to do so once the dust settles,
or something?
^ permalink raw reply
* Re: [GSoC Patch v8 1/3] path: extract format_path() and use in rev-parse
From: Junio C Hamano @ 2026-06-24 18:15 UTC (permalink / raw)
To: K Jayatheerth
Cc: a3205153416, git, jltobler, kumarayushjha123, lucasseikioshiro,
phillip.wood, sandals
In-Reply-To: <20260624033748.108281-2-jayatheerthkulkarni2005@gmail.com>
K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
> +void format_path(struct strbuf *dest, const char *path,
> + const char *prefix, enum path_format format)
> +{
> + strbuf_reset(dest);
> +
> + switch (format) {
> + case PATH_FORMAT_UNMODIFIED:
> + strbuf_addstr(dest, path);
> + break;
> +
> + case PATH_FORMAT_RELATIVE: {
> ...
> + strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
> +
> + strbuf_release(&relative_buf);
> + strbuf_release(&real_path);
> + strbuf_release(&real_prefix);
> + free(cwd);
> + break;
> + }
> +
> + case PATH_FORMAT_RELATIVE_IF_SHARED: {
> ...
> + strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
> + strbuf_release(&relative_buf);
> + break;
> + }
> +
> + case PATH_FORMAT_CANONICAL:
> + /*
> + * strbuf_realpath_forgiving inherently resets the destination
> + * buffer, safely aligning with our replace semantics.
> + */
> + strbuf_realpath_forgiving(dest, path, 1);
> + break;
> +
> + default:
> + BUG("unknown path_format value %d", format);
> + }
> +}
Hmph.
I was hoping that we could lose even more strbuf, but since
relative_path() does not always leave its result in the strbuf that
is passed to it as its third parameter, we do need addstr() into
dest, which is a bit unsatisfying but not a fault of this patch at
all. At least, we lost extra copy in the canonical codepath ;-)
Looking good. Thanks.
^ permalink raw reply
* Re: [PATCH 0/6] odb: refactor source-specific information in object info
From: Junio C Hamano @ 2026-06-24 17:13 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260624-b4-pks-odb-drop-whence-v1-0-8d1877b790ac@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> this patch series refactors `struct object_info` to not contain the
> `whence` field anymore.
>
> This field only gave the caller information about the type of source
> this was read from, but it didn't allow them to figure out which source
> specifically yielded the object. So instead, we replace this information
> with a new `struct object_info_source` field that both contains info
> about the source, and any backend-specific data.
Great.
^ permalink raw reply
* Re: [PATCH v2 1/7] Documentation/technical: add paint-down-to-common doc
From: Junio C Hamano @ 2026-06-24 17:09 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget
Cc: git, Derrick Stolee, Elijah Newren, Kristofer Karlsson
In-Reply-To: <19ed743bd10be5341eee040eb8070876b984773d.1782303254.git.gitgitgadget@gmail.com>
"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Kristofer Karlsson <krka@spotify.com>
>
> Add a technical document describing the paint_down_to_common()
> algorithm used for merge-base computation, covering the paint
> walk, generation number regions, and termination conditions.
>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> Documentation/Makefile | 1 +
> Documentation/technical/meson.build | 1 +
> .../technical/paint-down-to-common.adoc | 114 ++++++++++++++++++
> commit-reach.c | 6 +-
> 4 files changed, 121 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/technical/paint-down-to-common.adoc
Great write-up that very clearly and concisely explains what goes on
inside the merge-base computation. Thanks for a pleasant read.
^ permalink raw reply
* Re: [PATCH v2 2/4] odb/source-packed: support flags when iterating an object prefix
From: Christian Couder @ 2026-06-24 17:02 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Christian Couder
In-Reply-To: <20260624-pks-connected-generic-promisor-checks-v2-2-132d73ee47b9@pks.im>
On Wed, Jun 24, 2026 at 12:37 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> Callers of `odb_for_each_object()` can specify an optional object name
> prefix so that we only yield objects that match it. This is incompatible
> though with passing flags at the same time, as we don't yet know to
> handle them.
>
> Loosen this restriction by calling `should_exclude_pack()`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> odb/source-packed.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index 3afc4bf01f..6f31f0ff94 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -148,6 +148,7 @@ static int for_each_prefixed_object_in_midx(
> const struct odb_for_each_object_options *opts,
> struct odb_source_packed_for_each_object_wrapper_data *data)
> {
> + bool pack_errors = false;
> int ret;
>
> for (; m; m = m->base_midx) {
> @@ -171,6 +172,20 @@ static int for_each_prefixed_object_in_midx(
> const struct object_id *current = NULL;
> struct object_id oid;
>
> + if (opts->flags) {
> + uint32_t pack_id = nth_midxed_pack_int_id(m, i);
> + struct packed_git *pack;
> +
> + if (prepare_midx_pack(m, pack_id)) {
> + pack_errors = true;
> + continue;
> + }
> +
> + pack = nth_midxed_pack(m, pack_id);
> + if (should_exclude_pack(pack, opts->flags))
> + continue;
> + }
> +
> current = nth_midxed_object_oid(&oid, m, i);
>
> if (!match_hash(len, opts->prefix->hash, current->hash))
It looks like this is:
if (!match_hash(len, opts->prefix->hash, current->hash))
break;
and I wonder if the `if (opts->flags) { ... }` block would be better
after that prefix check rather than before it.
Putting it after the prefix check would make sure we don't continue
when the prefix doesn't match.
^ permalink raw reply
* Re: [PATCH] gpg-interface: fix strip_cr_before_lf to only remove CR before LF
From: Junio C Hamano @ 2026-06-24 16:58 UTC (permalink / raw)
To: Antonio De Stefani; +Cc: git
In-Reply-To: <20260624093618.17456-1-antonio.destefani08@gmail.com>
Antonio De Stefani <antonio.destefani08@gmail.com> writes:
> c4adea82 (Convert CR/LF to LF in tag signatures, 2008-07-11)
> introduced CR stripping for GPG output on Windows, but intentionally
> stripped all CR characters unconditionally to "keep the code simpler",
> even though only \r\n sequences (Windows line endings) needed to be
> normalized. 2f47eae2 (Split GPG interface into its own helper library,
> 2011-09-07) moved the code into gpg-interface.c, and 29b31577 (ssh
> signing: add ssh key format and signing code, 2021-09-10) extracted
> it into the remove_cr_after() helper when adding SSH signing support.
>
> The original laziness was safe at the time because lone CR characters
> are not expected in GPG signature output. However, the NEEDSWORK
> comment left by a previous reader correctly identified that only
> \r\n pairs should be stripped, not lone \r characters.
>
> Fix the loop to skip \r only when immediately followed by \n, keeping
> lone trailing CR characters intact. Rename the function to
> strip_cr_before_lf to reflect its corrected behavior, and update
> both call sites and their comments accordingly.
>
> Signed-off-by: Antonio De Stefani <antonio.destefani08@gmail.com>
> ---
> gpg-interface.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
Looking good. Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH v2 4/4] connected: search promisor objects generically
From: Junio C Hamano @ 2026-06-24 16:27 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Christian Couder
In-Reply-To: <20260624-pks-connected-generic-promisor-checks-v2-4-132d73ee47b9@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> When performing connectivity checks we have to figure out whether any of
> the new objects are promisor objects, as we cannot assume full
> connectivity if so.
>
> This check is performed by iterating through all packfiles in the
> repository and searching each of them for the given object. Of course,
> this mechanism is quite specific to implementation details of the object
> database, as we assume that it uses packfiles in the first place.
>
> Refactor the logic so that we instead use `odb_for_each_object_ext()`
> with an object prefix filter and the `ODB_FOR_EACH_OBJECT_PROMISOR_ONLY`
> flag. This will yield all objects that have the exact object name and
> that are part of a promisor pack in a generic way.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> connected.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/connected.c b/connected.c
> index d2b334173f..b557ff5db9 100644
> --- a/connected.c
> +++ b/connected.c
> @@ -11,6 +11,13 @@
> #include "packfile.h"
> #include "promisor-remote.h"
>
> +static int promised_object_cb(const struct object_id *oid UNUSED,
> + struct object_info *oi UNUSED,
> + void *payload UNUSED)
> +{
> + return 1;
> +}
> +
> /*
> * For partial clones, we don't want to have to do a regular connectivity check
> * because we have to enumerate and exclude all promisor objects (slow), and
> @@ -30,25 +37,28 @@ static int check_connected_promisor(oid_iterate_fn fn,
> void *cb_data,
> const struct object_id **oid)
> {
> + struct odb_for_each_object_options opts = {
> + .flags = ODB_FOR_EACH_OBJECT_PROMISOR_ONLY,
> + .prefix_hex_len = the_repository->hash_algo->hexsz,
> + };
> + int err;
> +
> odb_reprepare(the_repository->objects);
> do {
> - struct packed_git *p;
> + opts.prefix = *oid;
>
> - repo_for_each_pack(the_repository, p) {
> - if (!p->pack_promisor)
> - continue;
> - if (find_pack_entry_one(*oid, p))
> - goto promisor_pack_found;
> - }
> + err = odb_for_each_object_ext(the_repository->objects,
> + NULL, promised_object_cb,
> + NULL, &opts);
promised_object_cb() returns 1 without any computation since we are
only interested in learning ODB_FOR_EACH_OBJECT_PROMISOR_ONLY finds
any such object.
odb_for_each_object_ext() returns 0 (if it iterates all the sources
to the end), but if its call to odb_source_for_each_object() yields
non-zero value, the returned value comes back as "err" here,
terminating the for-each iteration immediately.
odb_source_for_each_object() is implemented differently per the
source backend, but taking an example of "packfile" backend,
packfile_loose_for_each_object() ends up calling cb (wrapped in
packfile_store_for_each_object_wrapper_data) via
for_each_object_in_pack(), which stops immediately when cb returns
non-zero and the value returned from there is the value given by cb,
i.e., 1. So we will have err==1 when we find any object.
> + if (err < 0)
> + return err;
And err presumably is 1 in such a case, so this does not trigger.
> /*
> * We have found an object that is not part of a promisor pack,
> * and thus we cannot skip the full connectivity check.
> */
> - return 0;
> -
> -promisor_pack_found:
> - ;
> + if (err > 0)
> + return 0;
And this does.
I may be misreading the patch, but as we return 0 from here, do we
cause the caller to fall back to full connectivity check? The
caller, check_connected(), sees a zero returned from here.
> } while ((*oid = fn(cb_data)) != NULL);
>
> return 1;
^ permalink raw reply
* Re: Fetching missing submodule refs unnecessarily fatal
From: Ben Knoble @ 2026-06-24 16:15 UTC (permalink / raw)
To: Mike Crowe; +Cc: Git maillinglist
In-Reply-To: <ajvouniXVAPH8nyZ@mcrowe.com>
[re adding list, woops!]
> Le 24 juin 2026 à 10:24, Mike Crowe <mac@mcrowe.com> a écrit :
>
> On Wednesday 24 June 2026 at 08:39:39 -0400, Ben Knoble wrote:
>>
>>>> Le 23 juin 2026 à 11:04, Mike Crowe <mac@mcrowe.com> a écrit :
>>>
>>> When Git fetches in a superproject with --recurse-submodules, it appears to
>>> try to fetch the corresponding submodule repository commits for every new
>>> or updated superproject branch. Presumably this is so that everthing is
>>> ready to switch to one of those branches without further fetching.
>>>
>>> Developers may create commits that contain submodules that reference
>>> commits in the submodule repository, but those commits may not be pushed to
>>> the submodule's remote repository. When the superproject commits are pushed
>>> to a personal remote branch anyone else's Git fetch cannot find the
>>> corresponding submodule commit and fails.
>>
>> This is the part that confuses me: if a (public) commit of history refers
>> to a submodule at a particular commit, and that commit is not available
>> anywhere, then we won’t be able to properly update submodules when using
>> that commit. That creates a problem!
>
> It does. But only for that user's personal branch. Even though it is
> public, a personal branch is mostly only for the use of that user and it
> doesn't matter to anyone but them. (The user is probably working
> simultaneously on both the superproject and the submodule.)
>
>> Why not instead make sure the submodule commit is also available for fetching?
>
> This relies on the user realising what they've done. They might even think
> that they've made the right submodule commit available but forgot that they
> rebased it before pushing or something changing the commit hash.
Yeah, the recurse-submodules option for pushing makes this easier to notice/act on, too.
> From a resilience point of view it shouldn't be possible for someone who
> can push changes to their own personal branch to perform a "denial of
> service" on anyone else fetching from the repository by making that fetch
> fail.
>
> I hope this makes it clearer.
That does make some sense: I wouldn’t want to get interrupted by a colleague or collaborator’s bad push of an unrelated branch.
> Thanks.
>
> Mike.
>
> (Was there a reason that you didn't reply to the list?)
Nope, mis-click.
^ permalink raw reply
* Re: [PATCH v2 7/7] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Derrick Stolee @ 2026-06-24 15:07 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Kristofer Karlsson via GitGitGadget, git, Elijah Newren
In-Reply-To: <CAL71e4N1zMz=v9umGdGPTvLP1nF-tNLVQc+vAEBnekt2L0b6zQ@mail.gmail.com>
On 6/24/2026 10:47 AM, Kristofer Karlsson wrote:
> On Wed, 24 Jun 2026 at 16:02, Derrick Stolee <stolee@gmail.com> wrote:
>>> - test_trace2_data paint_down_to_common steps 81 <trace-half.txt
>>> + test_trace2_data paint_down_to_common steps 57 <trace-half.txt
>>> '
>> I love to see these steps change. If you take my suggestion to
>> update more tests with these checks, then this diff will get bigger
>> (but in a deserved way).
>
> I will try to add them to some (but not all) tests since it's more
> closely related to performance than correctness and I want to
> avoid making too many tests overly fragile.
In this case, I think it's more about protecting all of our special-
cased termination conditions. The rigidity means that it is hard to
accidentally change the behavior. It does have the downside that
more tests need to change if there is an intentional change, but it
also gives the same _evidence_ that the change has the intended
impact.
We are definitely leaning into personal preferences, though. There
is no hard rule one way or another.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v2 7/7] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-24 14:47 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Kristofer Karlsson via GitGitGadget, git, Elijah Newren
In-Reply-To: <6b0d81e7-7617-4fb4-9e39-cdf8bc778837@gmail.com>
On Wed, 24 Jun 2026 at 16:02, Derrick Stolee <stolee@gmail.com> wrote:
>
> I see how the previous implementation has a termination condition
> before calling prio_queue_get(), which is technically more
> efficient. It does make this initial diff a bit more complicated
> because we are moving the prio_queue_get() line.
I was thinking the efficiency here does not matter in practice -
prio_queue_get() only returns NULL once, and all other times
where we keep looping we do need the value.
I agree it does get a bit complex though.
> If the introduction of the method in patch 5/7 looked like this:
>
> +static struct commit *paint_queue_get(struct paint_state *state)
> +{
> + struct commit *commit = prio_queue_get(&state->queue);
> +
> + if (!commit)
> + return NULL;
> +
> + if (!state->p1_count && !state->p2_count &&
> + !state->pending_merge_bases)
> + return NULL;
> +
> + commit->object.flags &= ~ENQUEUED;
> + paint_count_update(state, commit->object.flags, -1);
> + return commit;
> +}
>
> Then this diff would look cleaner.
>
> (This is the nittiest of nitpicks so feel free to ignore if this
> doesn't bother you at all.)
That's a good point. It doesn't technically bother me,
but it would be cleaner. The refactor commit would effectively
be looking into the future and prepare for it. I can change it for
the next version - my only thinking was that the current refactor
patch matched my original idea for how to best handle
the halt condition, but that did indeed change after this discussion.
> > - test_trace2_data paint_down_to_common steps 81 <trace-half.txt
> > + test_trace2_data paint_down_to_common steps 57 <trace-half.txt
> > '
> I love to see these steps change. If you take my suggestion to
> update more tests with these checks, then this diff will get bigger
> (but in a deserved way).
I will try to add them to some (but not all) tests since it's more
closely related to performance than correctness and I want to
avoid making too many tests overly fragile.
> Also, when I suggested that 'test_all_modes' creates the trace
> files on our behalf, I forgot to mention that this specific test
> that you added in patch 4/7 simplifies by running the merge-base
> check under 'test_all_modes' and then checking the trace2 data
> on the three well-known files afterwards.
That's a nice bonus, I will try to see if I can manage to utilize it.
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH v2 5/7] commit-reach: introduce struct paint_state with per-side counters
From: Kristofer Karlsson @ 2026-06-24 14:38 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Kristofer Karlsson via GitGitGadget, git, Elijah Newren
In-Reply-To: <19639ad3-2d16-4f3b-be79-138e00144ea3@gmail.com>
On Wed, 24 Jun 2026 at 15:54, Derrick Stolee <stolee@gmail.com> wrote:
>
> I'm grateful to see these changes happening to the doc in real-
> time. I know it was extra work, but I'm grateful right now.
>
> Hopefully future historians will also benefit from this effort.
It was honestly not bad at all, and I agree it felt quite nice to
see how the doc naturally changed along with the implementation.
> > +static struct commit *paint_queue_get(struct paint_state *state)
> > +{
>
> Since we are going to make this a more complete termination
> condition, we may want to make that very explicit with a doc-
> comment. Something along the lines of "dequeue a commit when
> possible, but also signal termination of the walk when we
> conclude that no more merge bases will be discovered due to
> internal state."
Yes, I'll make sure to clean that part up more, maybe also
rename the function to be more descriptive.
> You mentioned in your cover letter how the min_generation value
> can add extra termination conditions. It may be a good idea to
> insert min_generation into the paint_queue struct and make it a
> termination condition for paint_queue_get(). If you consider this
> direction, then I'd make it a separate patch on top of this one
> _before_ adding the one-sided change. The extra tests that cover
> the exact number of walked commits can help to guarantee the same
> behavior, assuming that some of those tests check a non-zero
> min_generation input. (It may be good to add such trace tests in
> an earlier patch to help confidence in this case.)
I think I might wait with this - the patch series already feels
quite big, and I think it has a natural progression and finish now.
But I can definitely commit to following up later -- it would be a
smaller series that is easier to reason about, likely a single commit.
Thanks,
Kristofer
^ 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