* Re: [PATCH v2 01/18] setup: replace use of `the_repository` in static functions
From: Karthik Nayak @ 2026-05-19 8:03 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-1-6933c0f1d568@pks.im>
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Replace the use of `the_repository` in "setup.c" for all static
> functions. For now, we simply add `the_repository` to invocations of
> these functions. This will be addressed in subsequent commits, where
> we'll move up `the_repository` one more layer to callers of "setup.c".
>
This commit is straight forward. We simply pass forward the
`the_repository` variable to static functions, so they have an incoming
`strict repository *`. I like this approach of:
1. Fixing up all static functions in a file to no longer use
`the_repository`.
2. Fixing up the other external functions one-by-one to receive a
`struct repository *`.
3. Fixing up any other global variables / config used.
Makes it easy to follow.
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH v11] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren via GitGitGadget @ 2026-05-19 7:58 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.v10.git.git.1779091483321.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
If you want to fork your topic branch from the very latest of the
tip of a branch your remote has, you would do:
git fetch origin some-branch
git checkout -b new_branch --track origin/some-branch
Extend the "--track" option of "git checkout" and allow users to
write
git checkout -b new_branch --track=fetch origin/some-branch
to (1) fetch 'some-branch' from the remote 'origin', updating the
remote-tracking branch 'origin/some-branch', (2) arrange subsequent
'git pull' on 'new_branch' to interact with 'origin/some-branch' and
(3) fork 'new_branch' from it.
In the value of the '--track' option, 'fetch' can be combined with
the existing 'direct' (default) and 'inherit' modes via a
comma-separated list. Examples:
git checkout -b new_branch --track=fetch,inherit some_local_branch
git switch -c new_branch --track=fetch origin/some-branch
When "fetch" is requested and <start-point> is in <remote>/<branch>
form, run "git fetch <remote> <branch>" before resolving the ref, so
that other remote-tracking branches are left untouched. If
<start-point> is a bare remote name like "origin" (which resolves to
that remote's default branch), "git fetch <remote>" is run instead,
since the target branch is not known up front. Abort the checkout if
the fetch fails.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
checkout: --track=fetch
* Find the right remote by checking which remote's fetch refspec maps
to the user's start-point, instead of assuming the start-point begins
with the remote's name. This fixes cases where the user has a custom
refspec mapping into a namespace whose name differs from the remote
(e.g. fetching from origin into refs/remotes/upstream/*).
* For a bare namespace name, follow <namespace>/HEAD first to figure
out which branch to fetch.
* Strengthen the custom-refspec test so it actually exercises the fetch
(no prior git fetch).
* New test for the bare-namespace case.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2281%2FHaraldNordgren%2Fcheckout-fetch-start-point-v11
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2281/HaraldNordgren/checkout-fetch-start-point-v11
Pull-Request: https://github.com/git/git/pull/2281
Range-diff vs v10:
1: a773fb6bdf ! 1: d0c9e3e879 checkout: extend --track with a "fetch" mode to refresh start-point
@@ builtin/checkout.c: struct branch_info {
char *checkout;
};
++struct fetch_target_cb {
++ struct refspec_item query;
++ const char *remote_name;
++ int matches;
++};
++
++static int match_fetch_target(struct remote *remote, void *priv)
++{
++ struct fetch_target_cb *cb = priv;
++ struct refspec_item q = { .dst = cb->query.dst };
++
++ if (!remote_find_tracking(remote, &q) && q.src) {
++ if (++cb->matches == 1) {
++ cb->remote_name = remote->name;
++ free(cb->query.src);
++ cb->query.src = q.src;
++ } else {
++ free(q.src);
++ }
++ }
++ return 0;
++}
++
+static int resolve_fetch_target(const char *arg, char **remote_out,
+ char **src_ref_out, char **existing_ref_out)
+{
-+ const char *slash;
-+ char *remote_name = NULL;
-+ struct remote *remote = NULL;
-+ struct refspec_item query = { 0 };
+ struct strbuf dst = STRBUF_INIT;
++ struct strbuf head_path = STRBUF_INIT;
++ struct fetch_target_cb cb = { 0 };
+ struct object_id oid;
-+ const char *rest = NULL;
-+ const char *head_target = NULL;
-+ const char *short_target;
++ const char *head_target;
+
+ *remote_out = NULL;
+ *src_ref_out = NULL;
+ *existing_ref_out = NULL;
+
-+ if (!arg || !*arg || *arg == '/')
++ if (!arg || !*arg)
+ return -1;
+
-+ slash = arg + strlen(arg);
-+ while (1) {
-+ free(remote_name);
-+ remote_name = xstrndup(arg, slash - arg);
-+ remote = remote_get(remote_name);
-+ if (remote && remote_is_configured(remote, 1))
-+ break;
-+ while (slash > arg && *--slash != '/')
-+ ;
-+ if (slash == arg) {
-+ free(remote_name);
-+ return -1;
-+ }
-+ }
-+
-+ if (*slash == '/' && slash[1])
-+ rest = slash + 1;
-+ if (!rest) {
-+ strbuf_addf(&dst, "refs/remotes/%s/HEAD", remote_name);
-+ head_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-+ dst.buf,
-+ RESOLVE_REF_READING |
-+ RESOLVE_REF_NO_RECURSE,
-+ &oid, NULL);
-+ if (head_target) {
-+ *existing_ref_out = xstrdup(dst.buf);
-+ if (skip_prefix(head_target, "refs/remotes/", &short_target) &&
-+ skip_prefix(short_target, remote_name, &short_target) &&
-+ *short_target == '/')
-+ rest = short_target + 1;
-+ }
-+ strbuf_reset(&dst);
++ strbuf_addf(&head_path, "refs/remotes/%s/HEAD", arg);
++ head_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
++ head_path.buf,
++ RESOLVE_REF_READING |
++ RESOLVE_REF_NO_RECURSE,
++ &oid, NULL);
++ if (head_target)
++ strbuf_addstr(&dst, head_target);
++ else
++ strbuf_addf(&dst, "refs/remotes/%s", arg);
++
++ cb.query.dst = dst.buf;
++ for_each_remote(match_fetch_target, &cb);
++
++ if (cb.matches != 1) {
++ free(cb.query.src);
++ strbuf_release(&dst);
++ strbuf_release(&head_path);
++ return -1;
+ }
+
-+ if (rest) {
-+ strbuf_addf(&dst, "refs/remotes/%s/%s", remote_name, rest);
-+ query.dst = dst.buf;
-+ if (!remote_find_tracking(remote, &query) && query.src) {
-+ *src_ref_out = xstrdup(query.src);
-+ free(query.src);
-+ } else {
-+ *src_ref_out = xstrdup(rest);
-+ }
-+ if (!*existing_ref_out) {
-+ strbuf_reset(&dst);
-+ strbuf_addf(&dst, "refs/remotes/%s", arg);
-+ if (!refs_read_ref(get_main_ref_store(the_repository),
-+ dst.buf, &oid))
-+ *existing_ref_out = xstrdup(dst.buf);
-+ }
-+ }
++ *remote_out = xstrdup(cb.remote_name);
++ *src_ref_out = cb.query.src;
++ if (head_target)
++ *existing_ref_out = strbuf_detach(&head_path, NULL);
++ else if (!refs_read_ref(get_main_ref_store(the_repository),
++ dst.buf, &oid))
++ *existing_ref_out = strbuf_detach(&dst, NULL);
+
+ strbuf_release(&dst);
-+ *remote_out = remote_name;
++ strbuf_release(&head_path);
+ return 0;
+}
+
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+
+test_expect_success 'checkout --track=fetch resolves through configured fetch refspec' '
+ git checkout main &&
-+ git -C fetch_upstream checkout -b fetch_refspec &&
-+ test_commit -C fetch_upstream u_refspec &&
-+ git fetch fetch_upstream fetch_refspec &&
+ 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 fetch fetch_custom &&
-+ test_commit -C fetch_upstream u_refspec_post &&
++ 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 namespace bare name follows <ns>/HEAD' '
++ git checkout main &&
++ git remote add fetch_ns ./fetch_upstream &&
++ test_when_finished "git remote remove fetch_ns" &&
++ 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 handles hierarchical remote name' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_hier &&
Documentation/git-checkout.adoc | 13 ++-
Documentation/git-switch.adoc | 13 ++-
builtin/checkout.c | 164 +++++++++++++++++++++++++++++++-
t/t7201-co.sh | 158 ++++++++++++++++++++++++++++++
4 files changed, 342 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
index a8b3b8c2e2..ec63434159 100644
--- a/Documentation/git-checkout.adoc
+++ b/Documentation/git-checkout.adoc
@@ -158,11 +158,22 @@ 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 name (e.g. `origin`),
+only the remote's default branch is updated. If the fetch 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..b5e79435cd 100644
--- a/Documentation/git-switch.adoc
+++ b/Documentation/git-switch.adoc
@@ -155,11 +155,22 @@ 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.
+
+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 name (e.g. `origin`),
+only the remote's default branch is updated. If the fetch fails and the
+corresponding remote-tracking ref already exists, a warning is printed
+and the switch proceeds from the existing tip; otherwise the switch is
+aborted.
++
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
refspec configured for the corresponding remote, and then stripping
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1345e8574a..5b3a06c0cd 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 "strvec.h"
@@ -62,6 +64,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;
@@ -115,6 +118,154 @@ struct branch_info {
char *checkout;
};
+struct fetch_target_cb {
+ struct refspec_item query;
+ const char *remote_name;
+ int matches;
+};
+
+static int match_fetch_target(struct remote *remote, void *priv)
+{
+ struct fetch_target_cb *cb = priv;
+ struct refspec_item q = { .dst = cb->query.dst };
+
+ if (!remote_find_tracking(remote, &q) && q.src) {
+ if (++cb->matches == 1) {
+ cb->remote_name = remote->name;
+ free(cb->query.src);
+ cb->query.src = q.src;
+ } else {
+ free(q.src);
+ }
+ }
+ return 0;
+}
+
+static int resolve_fetch_target(const char *arg, char **remote_out,
+ char **src_ref_out, char **existing_ref_out)
+{
+ struct strbuf dst = STRBUF_INIT;
+ struct strbuf head_path = STRBUF_INIT;
+ struct fetch_target_cb cb = { 0 };
+ struct object_id oid;
+ const char *head_target;
+
+ *remote_out = NULL;
+ *src_ref_out = NULL;
+ *existing_ref_out = NULL;
+
+ if (!arg || !*arg)
+ return -1;
+
+ strbuf_addf(&head_path, "refs/remotes/%s/HEAD", arg);
+ head_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ head_path.buf,
+ RESOLVE_REF_READING |
+ RESOLVE_REF_NO_RECURSE,
+ &oid, NULL);
+ if (head_target)
+ strbuf_addstr(&dst, head_target);
+ else
+ strbuf_addf(&dst, "refs/remotes/%s", arg);
+
+ cb.query.dst = dst.buf;
+ for_each_remote(match_fetch_target, &cb);
+
+ if (cb.matches != 1) {
+ free(cb.query.src);
+ strbuf_release(&dst);
+ strbuf_release(&head_path);
+ return -1;
+ }
+
+ *remote_out = xstrdup(cb.remote_name);
+ *src_ref_out = cb.query.src;
+ if (head_target)
+ *existing_ref_out = strbuf_detach(&head_path, NULL);
+ else if (!refs_read_ref(get_main_ref_store(the_repository),
+ dst.buf, &oid))
+ *existing_ref_out = strbuf_detach(&dst, NULL);
+
+ strbuf_release(&dst);
+ strbuf_release(&head_path);
+ return 0;
+}
+
+static void fetch_remote_for_start_point(const char *arg)
+{
+ char *remote_name = NULL;
+ char *src_ref = NULL;
+ char *existing_ref = NULL;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+
+ if (resolve_fetch_target(arg, &remote_name, &src_ref, &existing_ref))
+ return;
+
+ strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
+ if (src_ref)
+ strvec_push(&cmd.args, src_ref);
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
+ if (existing_ref)
+ warning(_("failed to fetch start-point '%s'; "
+ "using existing '%s'"),
+ arg, existing_ref);
+ else
+ die(_("failed to fetch start-point '%s'"), arg);
+ }
+
+ free(remote_name);
+ free(src_ref);
+ free(existing_ref);
+}
+
+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, saw_inherit = 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;
+ opts->track = BRANCH_TRACK_EXPLICIT;
+ } else if (!strcmp(item->string, "inherit")) {
+ saw_inherit = 1;
+ opts->track = BRANCH_TRACK_INHERIT;
+ } else {
+ ret = error(_("option `%s' expects \"%s\", \"%s\", "
+ "or \"%s\""),
+ "--track", "direct", "inherit", "fetch");
+ goto out;
+ }
+ }
+
+ if (saw_direct && saw_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);
@@ -1733,10 +1884,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")),
@@ -1941,8 +2092,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]);
+
+ 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..d4f5467903 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -870,4 +870,162 @@ 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 &&
+ other_before=$(git rev-parse 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_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 &&
+ unrelated_before=$(git rev-parse 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_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 namespace bare name follows <ns>/HEAD' '
+ git checkout main &&
+ git remote add fetch_ns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_ns" &&
+ 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 handles hierarchical remote name' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_hier &&
+ test_commit -C fetch_upstream u_hier &&
+ git remote add nested/remote ./fetch_upstream &&
+ test_when_finished "git remote remove nested/remote" &&
+ git fetch nested/remote fetch_hier &&
+ test_commit -C fetch_upstream u_hier_post &&
+ 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=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 and inherits' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_inherit &&
+ test_commit -C fetch_upstream u_inherit &&
+ git fetch fetch_upstream fetch_inherit &&
+ git checkout -b base_inherit fetch_upstream/fetch_inherit &&
+ test_commit -C fetch_upstream u_inherit2 &&
+ git checkout main &&
+ git checkout --track=fetch,inherit -b local_inherit base_inherit &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD &&
+ test_cmp_config fetch_upstream branch.local_inherit.remote &&
+ test_cmp_config refs/heads/fetch_inherit branch.local_inherit.merge
+'
+
+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 '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
base-commit: 7bcaabddcf68bd0702697da5904c3b68c52f94cf
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v10] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren @ 2026-05-19 7:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Harald Nordgren via GitGitGadget, git, Ramsay Jones,
D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud, Phillip Wood
In-Reply-To: <xmqq8q9f9b5w.fsf@gitster.g>
> > + strbuf_release(&dst);
> > + *remote_out = remote_name;
> > + return 0;
> > +}
> > +
> > +static void fetch_remote_for_start_point(const char *arg)
> > +{
> > + char *remote_name = NULL;
> > + char *src_ref = NULL;
> > + char *existing_ref = NULL;
> > + struct child_process cmd = CHILD_PROCESS_INIT;
> > +
> > + if (resolve_fetch_target(arg, &remote_name, &src_ref, &existing_ref))
> > + return;
> > +
> > + strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
> > + if (src_ref)
> > + strvec_push(&cmd.args, src_ref);
>
> What should happen with this configuration
>
> [remote "origin"]
> fetch = refs/heads/*:refs/upstream/*
>
> and the user says either of these two:
>
> $ git checkout --track=fetch upstream
> $ git checkout --track=fetch upstream/master
>
> We fail to find in "where does the remote name ends and branch name
> start?" loop that this request is about remote "origin" at all, no?
> We may see in the former case that there is
> refs/remotes/upstream/HEAD >that points at "master" in the same
> hierarchy, but the code thinks "upstream" is the remote name, which
> would mean you would "git fetch upstream", when the remote you need
> to fetch from is "origin".
>
> > + cmd.git_cmd = 1;
> > + if (run_command(&cmd)) {
> > + if (existing_ref)
> > + warning(_("failed to fetch start-point '%s'; "
> > + "using existing '%s'"),
> > + arg, existing_ref);
> > + else
> > + die(_("failed to fetch start-point '%s'"), arg);
>
> If we failed to set *existing_ref_out, shouldn't we fail without
> even attempting to call run_command() here, as we will have to die()
> anyway even if "git fetch" succeeds. For that matter, it may be
> simpler and more correct for resolve_fetch_target() to fail (return
> -1) when it happens, by making the lat "if (rest) {...}" to have a
> corresponding "else { return -1 }" after it.
Yeah, good point. I will try to address this and send a new patch.
Harald
^ permalink raw reply
* Re: [PATCH] log: let --follow follow renames in merge commits
From: Junio C Hamano @ 2026-05-19 6:37 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Patrick Steinhardt, brian m. carlson
In-Reply-To: <agwAkHzjrJQPVtCS@collabora.com>
Miklos Vajna <vmiklos@collabora.com> writes:
> Hi Junio,
>
> On Tue, May 12, 2026 at 09:21:17AM +0200, Miklos Vajna <vmiklos@collabora.com> wrote:
>> I sent this out a week ago at
>> <https://lore.kernel.org/git/afmfSa-p-9vuDL3E@collabora.com/T/#u>, I
>> didn't get any reply to it -- so I'm somewhat optimistic that the patch
>> itself is a good idea, seeing no negative comments.
The patch collecting no comments is just that--nobody so far is
interested enough to drop other things they were doing to give
supporting code reviews---and "no news" does not mean a good news.
>> So this is a resend, this time to you, CC'ing the list, rather than the
>> other way around.
>>
>> Could you please review this?
>
> I'm a bit confused regarding what can be a next step here. I
> understanding you were away for 3 weeks, so there is a lot to process.
> :-) Should I just wait more or should I resend this?
Rather, ask other reviewers; when I do not comment on a patch, I
often am not interested, or too busy and the change does not look
interesting enough to me to make me drop what I am doing.
^ permalink raw reply
* Re: [PATCH v7] revision.c: implement --max-count-oldest
From: Junio C Hamano @ 2026-05-19 6:27 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Johannes Sixt, Chris Torek
In-Reply-To: <463cc8e2764edb7de3d379f615f5cfbd0919bfa3.1778887662.git.mroik@delayed.space>
Mirko Faina <mroik@delayed.space> writes:
> + * graph_update() as it doesn't do the actualy printing, we'd
"actually"?
^ permalink raw reply
* Re: [PATCH] commit: fall back to full read when maybe_tree is NULL
From: Rasmus Villemoes @ 2026-05-19 6:25 UTC (permalink / raw)
To: Jeff King; +Cc: git, Daniel Mach
In-Reply-To: <20260519050513.GA1635924@coredump.intra.peff.net>
On Tue, May 19 2026, Jeff King <peff@peff.net> wrote:
> When we load a commit object from the commit graph (rather than reading
> the object contents), we don't fill in its "maybe_tree" entry, but
> rather wait to lazy-load it. This goes back to 7b8a21dba1 (commit-graph:
> lazy-load trees for commits, 2018-04-06), and saves the work of
> instantiating tree objects that nobody cares about.
>
> But it creates a data dependency: now the commit struct depends on the
> graph file to do that lazy load. This is a problem if we close the graph
> file; now we have a commit struct that claims to be parsed but is
> missing some of its data.
>
> It's rare for this to be a problem in practice, because we don't tend to
> close the graph files at all, and if we do we don't tend to look at
> their commits afterward. But there is one case that is easy to trigger:
> git-clone's --dissociate option will close the object database before
> running the dissociate repack, and then afterwards still try to check
> out the working tree. This will yield an error like:
>
> fatal: unable to parse commit b29edc0babef41810f7b1c9ee1d74058f22e4080
> warning: Clone succeeded, but checkout failed.
>
> What happens is that we expect repo_get_commit_tree() to lazy-load the
> tree, but commit_graph_position() returns COMMIT_NOT_FROM_GRAPH because
> the position slab has gone away (and even if it hadn't, we don't have
> the graph file itself available anymore).
>
> Let's try harder to find the tree in repo_get_commit_tree() by actually
> opening the commit object and parsing the tree line. This is extra work,
> but no more than we'd have to go to if we hadn't done the initial graph
> load in the first place.
I can confirm that this, applied on top of v2.54.0, fixes the problem
for the instance I had.
Tested-by: Rasmus Villemoes <ravi@prevas.dk>
Thanks,
Rasmus
^ permalink raw reply
* Re: [PATCH] log: let --follow follow renames in merge commits
From: Miklos Vajna @ 2026-05-19 6:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Patrick Steinhardt, brian m. carlson
In-Reply-To: <agLU58gbG1y7KLz-@collabora.com>
Hi Junio,
On Tue, May 12, 2026 at 09:21:17AM +0200, Miklos Vajna <vmiklos@collabora.com> wrote:
> I sent this out a week ago at
> <https://lore.kernel.org/git/afmfSa-p-9vuDL3E@collabora.com/T/#u>, I
> didn't get any reply to it -- so I'm somewhat optimistic that the patch
> itself is a good idea, seeing no negative comments.
>
> So this is a resend, this time to you, CC'ing the list, rather than the
> other way around.
>
> Could you please review this?
I'm a bit confused regarding what can be a next step here. I
understanding you were away for 3 weeks, so there is a lot to process.
:-) Should I just wait more or should I resend this?
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH v10] checkout: extend --track with a "fetch" mode to refresh start-point
From: Junio C Hamano @ 2026-05-19 6:16 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk,
Marc Branchaud, Phillip Wood, Harald Nordgren
In-Reply-To: <pull.2281.v10.git.git.1779091483321.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> If you want to fork your topic branch from the very latest of the
> tip of a branch your remote has, you would do:
>
> git fetch origin some-branch
> git checkout -b new_branch --track origin/some-branch
>
> Extend the "--track" option of "git checkout" and allow users to
> write
>
> git checkout -b new_branch --track=fetch origin/some-branch
>
> to (1) fetch 'some-branch' from the remote 'origin', updating the
> remote-tracking branch 'origin/some-branch', (2) arrange subsequent
> 'git pull' on 'new_branch' to interact with 'origin/some-branch' and
> (3) fork 'new_branch' from it.
>
> In the value of the '--track' option, 'fetch' can be combined with
> the existing 'direct' (default) and 'inherit' modes via a
> comma-separated list. Examples:
>
> git checkout -b new_branch --track=fetch,inherit some_local_branch
> git switch -c new_branch --track=fetch origin/some-branch
>
> When "fetch" is requested and <start-point> is in <remote>/<branch>
> form, run "git fetch <remote> <branch>" before resolving the ref, so
> that other remote-tracking branches are left untouched. If
> <start-point> is a bare remote name like "origin" (which resolves to
> that remote's default branch), "git fetch <remote>" is run instead,
> since the target branch is not known up front. Abort the checkout if
> the fetch fails.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> checkout: --track=fetch
>
> Rebased to fix merge conflict with master.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2281%2FHaraldNordgren%2Fcheckout-fetch-start-point-v10
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2281/HaraldNordgren/checkout-fetch-start-point-v10
> Pull-Request: https://github.com/git/git/pull/2281
>
> Range-diff vs v9:
>
> 1: 021375e4cc ! 1: a773fb6bdf checkout: extend --track with a "fetch" mode to refresh start-point
> @@ Documentation/git-checkout.adoc: of it").
> the refspec configured for the corresponding remote, and then stripping
>
> ## Documentation/git-switch.adoc ##
> -@@ Documentation/git-switch.adoc: should result in deletion of the path).
> +@@ Documentation/git-switch.adoc: variable.
> attached to a terminal, regardless of `--quiet`.
>
> `-t`::
> @@ builtin/checkout.c
> #include "resolve-undo.h"
> #include "revision.h"
> +#include "run-command.h"
> + #include "sequencer.h"
> #include "setup.h"
> #include "strvec.h"
> - #include "submodule.h"
> @@ builtin/checkout.c: struct checkout_opts {
> int count_checkout_paths;
> int overlay_mode;
>
>
> Documentation/git-checkout.adoc | 13 ++-
> Documentation/git-switch.adoc | 13 ++-
> builtin/checkout.c | 168 +++++++++++++++++++++++++++++++-
> t/t7201-co.sh | 144 +++++++++++++++++++++++++++
> 4 files changed, 332 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
> index a8b3b8c2e2..ec63434159 100644
> --- a/Documentation/git-checkout.adoc
> +++ b/Documentation/git-checkout.adoc
> @@ -158,11 +158,22 @@ 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 name (e.g. `origin`),
> +only the remote's default branch is updated. If the fetch 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..b5e79435cd 100644
> --- a/Documentation/git-switch.adoc
> +++ b/Documentation/git-switch.adoc
> @@ -155,11 +155,22 @@ 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.
> +
> +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 name (e.g. `origin`),
> +only the remote's default branch is updated. If the fetch fails and the
> +corresponding remote-tracking ref already exists, a warning is printed
> +and the switch proceeds from the existing tip; otherwise the switch is
> +aborted.
> ++
> 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
> refspec configured for the corresponding remote, and then stripping
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 1345e8574a..fc58456546 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 "strvec.h"
> @@ -62,6 +64,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;
> @@ -115,6 +118,158 @@ struct branch_info {
> char *checkout;
> };
>
> +static int resolve_fetch_target(const char *arg, char **remote_out,
> + char **src_ref_out, char **existing_ref_out)
> +{
> + const char *slash;
> + char *remote_name = NULL;
> + struct remote *remote = NULL;
> + struct refspec_item query = { 0 };
> + struct strbuf dst = STRBUF_INIT;
> + struct object_id oid;
> + const char *rest = NULL;
> + const char *head_target = NULL;
> + const char *short_target;
> +
> + *remote_out = NULL;
> + *src_ref_out = NULL;
> + *existing_ref_out = NULL;
> +
> + if (!arg || !*arg || *arg == '/')
> + return -1;
> +
> + slash = arg + strlen(arg);
> + while (1) {
> + free(remote_name);
> + remote_name = xstrndup(arg, slash - arg);
> + remote = remote_get(remote_name);
> + if (remote && remote_is_configured(remote, 1))
> + break;
> + while (slash > arg && *--slash != '/')
> + ;
> + if (slash == arg) {
> + free(remote_name);
> + return -1;
> + }
> + }
OK. So the caller gives "foo/bar/baz" when "foo/bar" is the name of
the remote that uses "refs/remotes/foo/bar" to store remote-tracking
branches from there. You check "foo/bar/baz", which fails to be a
remote, then "foo/bar", which is a configured remote and break.
> +
> + if (*slash == '/' && slash[1])
> + rest = slash + 1;
And "baz" becomes the "rest"; if the user gave us "foo/bar" and the
abobve loop found it as the name of the remote, we would want to use
"refs/remotes/foo/bar/HEAD", which is the "if (!rest)" below is
about.
> + if (!rest) {
> + strbuf_addf(&dst, "refs/remotes/%s/HEAD", remote_name);
> + head_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> + dst.buf,
> + RESOLVE_REF_READING |
> + RESOLVE_REF_NO_RECURSE,
> + &oid, NULL);
> + if (head_target) {
> + *existing_ref_out = xstrdup(dst.buf);
> + if (skip_prefix(head_target, "refs/remotes/", &short_target) &&
> + skip_prefix(short_target, remote_name, &short_target) &&
> + *short_target == '/')
> + rest = short_target + 1;
> + }
> + strbuf_reset(&dst);
> + }
So we may have been given "foo/bar" and after resolving HEAD there,
have "baz" in rest. Or "foo/bar/baz" was given and we may have
figured out that that is "baz" in "foo/bar".
> + if (rest) {
> + strbuf_addf(&dst, "refs/remotes/%s/%s", remote_name, rest);
> + query.dst = dst.buf;
> + if (!remote_find_tracking(remote, &query) && query.src) {
> + *src_ref_out = xstrdup(query.src);
> + free(query.src);
> + } else {
> + *src_ref_out = xstrdup(rest);
> + }
> + if (!*existing_ref_out) {
> + strbuf_reset(&dst);
> + strbuf_addf(&dst, "refs/remotes/%s", arg);
> + if (!refs_read_ref(get_main_ref_store(the_repository),
> + dst.buf, &oid))
> + *existing_ref_out = xstrdup(dst.buf);
> + }
> + }
What happens if "HEAD" was not there, though. If "refs/remotes/foo/bar/"
did not exist, we would have already returned -1 after trying to
find which part in arg is the remote name. But if "refs/remotes/foo/bar"
is valid, the user gave us "foo/bar", and we cannot find HEAD, then
the above "if (rest)" is skipped. We give "foo/bar" to *remote_out,
and return 0 without touching *src_ref_out or *existing_ref_out at
all.
> + strbuf_release(&dst);
> + *remote_out = remote_name;
> + return 0;
> +}
> +
> +static void fetch_remote_for_start_point(const char *arg)
> +{
> + char *remote_name = NULL;
> + char *src_ref = NULL;
> + char *existing_ref = NULL;
> + struct child_process cmd = CHILD_PROCESS_INIT;
> +
> + if (resolve_fetch_target(arg, &remote_name, &src_ref, &existing_ref))
> + return;
> +
> + strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
> + if (src_ref)
> + strvec_push(&cmd.args, src_ref);
What should happen with this configuration
[remote "origin"]
fetch = refs/heads/*:refs/upstream/*
and the user says either of these two:
$ git checkout --track=fetch upstream
$ git checkout --track=fetch upstream/master
We fail to find in "where does the remote name ends and branch name
start?" loop that this request is about remote "origin" at all, no?
We may see in the former case that there is
refs/remotes/upstream/HEAD >that points at "master" in the same
hierarchy, but the code thinks "upstream" is the remote name, which
would mean you would "git fetch upstream", when the remote you need
to fetch from is "origin".
> + cmd.git_cmd = 1;
> + if (run_command(&cmd)) {
> + if (existing_ref)
> + warning(_("failed to fetch start-point '%s'; "
> + "using existing '%s'"),
> + arg, existing_ref);
> + else
> + die(_("failed to fetch start-point '%s'"), arg);
If we failed to set *existing_ref_out, shouldn't we fail without
even attempting to call run_command() here, as we will have to die()
anyway even if "git fetch" succeeds. For that matter, it may be
simpler and more correct for resolve_fetch_target() to fail (return
-1) when it happens, by making the lat "if (rest) {...}" to have a
corresponding "else { return -1 }" after it.
> + }
> +
> + free(remote_name);
> + free(src_ref);
> + free(existing_ref);
> +}
I'll stop here.
^ permalink raw reply
* Re: [PATCH] commit: fall back to full read when maybe_tree is NULL
From: Jeff King @ 2026-05-19 6:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rasmus Villemoes, Daniel Mach
In-Reply-To: <xmqqcxys7xi4.fsf@gitster.g>
On Tue, May 19, 2026 at 02:56:51PM +0900, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > It also means we have to reimplement a bit of the commit parsing. We
> > can't just use parse_commit_buffer() here, because it expects an
> > unparsed struct and wants to load everything, including parent links.
> > But we don't know if the parent list has been munged during traversal,
> > so it's not safe for us to touch it. Fortunately, it's quite easy to
> > load just the tree, as it is always the first line of the commit object.
>
> I was hoping that existing code to parse out the tree in
> parse_commit_buffer() will become a call into this new helper
> function, so that we avoid duplicating the logic.
Yeah, I would like to have shared more code, but I think the amount that
can actually be shared gets overwhelmed by boilerplate. In particular,
parse_commit_buffer() wants to keep advancing the pointer afterwards,
since it actually reads the other lines.
> > Moreover, this strategy does nothing if we lose access to the graph file
> > unexpectedly (e.g., due to a system error).
>
> Or simultaneous repack may lose the file from the filesystem,
> perhaps?
I don't think so, because our mmap would hold onto the contents until
the process ends. You'd really need some case where we actually drop the
mmap. I could see us doing that if we found that it was corrupted or
something, but I don't think that happens currently. We close it only
for odb_close(), or when writing a new graph file (and so it would only
affect the "commit-graph write" process itself).
> Looks quite straight-forward. Don't you need to pay attention to
> r->hash_algo and call parse_oid_hex_algop() instead?
>
> Or are we pretty much sure that "r" is always "the_repository" here,
> in which case parse_oid_hex() that uses "the_hash_algo" would be
> sufficient?
No, I didn't even think about it, since the use of the_hash_algo is
hidden behind the function. We definitely should use the hash algo from
"r", since we have access to it. I'm not even sure if you can have repos
of two different hashes loaded in the same process at this point, but
certainly it is the correct long-term direction.
Here's a re-roll with the one-line fixup:
diff --git a/commit.c b/commit.c
index cfc87ad185..499a9602ad 100644
--- a/commit.c
+++ b/commit.c
@@ -448,7 +448,7 @@ static void load_tree_from_commit_contents(struct repository *r, struct commit *
if (type == OBJ_COMMIT &&
skip_prefix(buf, "tree ", &p) &&
- !parse_oid_hex(p, &tree_oid, &p) &&
+ !parse_oid_hex_algop(p, &tree_oid, &p, r->hash_algo) &&
*p == '\n')
set_commit_tree(commit, lookup_tree(r, &tree_oid));
-- >8 --
Subject: commit: fall back to full read when maybe_tree is NULL
When we load a commit object from the commit graph (rather than reading
the object contents), we don't fill in its "maybe_tree" entry, but
rather wait to lazy-load it. This goes back to 7b8a21dba1 (commit-graph:
lazy-load trees for commits, 2018-04-06), and saves the work of
instantiating tree objects that nobody cares about.
But it creates a data dependency: now the commit struct depends on the
graph file to do that lazy load. This is a problem if we close the graph
file; now we have a commit struct that claims to be parsed but is
missing some of its data.
It's rare for this to be a problem in practice, because we don't tend to
close the graph files at all, and if we do we don't tend to look at
their commits afterward. But there is one case that is easy to trigger:
git-clone's --dissociate option will close the object database before
running the dissociate repack, and then afterwards still try to check
out the working tree. This will yield an error like:
fatal: unable to parse commit b29edc0babef41810f7b1c9ee1d74058f22e4080
warning: Clone succeeded, but checkout failed.
What happens is that we expect repo_get_commit_tree() to lazy-load the
tree, but commit_graph_position() returns COMMIT_NOT_FROM_GRAPH because
the position slab has gone away (and even if it hadn't, we don't have
the graph file itself available anymore).
Let's try harder to find the tree in repo_get_commit_tree() by actually
opening the commit object and parsing the tree line. This is extra work,
but no more than we'd have to go to if we hadn't done the initial graph
load in the first place.
It does mean that a corrupt commit (e.g., one that points to a non-tree
object for which we couldn't instantiate a struct) will repeatedly load
the object from disk, once for each call to repo_get_commit_tree(). But
such corruptions should be rare, and we don't tend to perform such calls
repeatedly (usually we'd abort the operation upon seeing corruption).
It also means we have to reimplement a bit of the commit parsing. We
can't just use parse_commit_buffer() here, because it expects an
unparsed struct and wants to load everything, including parent links.
But we don't know if the parent list has been munged during traversal,
so it's not safe for us to touch it. Fortunately, it's quite easy to
load just the tree, as it is always the first line of the commit object.
There is an alternative approach which I considered but rejected:
"complete" each graph-loaded commit struct when we close the graph file
by looking up and instantiating their trees at close time. This is the
most elegant solution in some sense, as it resolves the data dependency
at the moment it goes away. And it avoids ever opening the commit
objects at all, which can be more efficient.
But not always. The resolving effort scales with the number of
graph-loaded commits, even though we may only later access one or a few.
So the tradeoff depends on how many were loaded in total versus how many
will be later accessed.
And in most cases, we will not access any at all! Programs which close
the object database before exiting will then do a bunch of work for no
reason. This could be mitigated by requiring a separate function to
resolve the graph structs before closing the file. But now each close
call has to consider whether to call that resolving function. So we'd
fix this case in git-clone, but we don't know what other cases (if any)
are lurking.
Moreover, this strategy does nothing if we lose access to the graph file
unexpectedly (e.g., due to a system error). I'm not entirely sure this
is possible now (we mmap it, so I'd guess any error would turn into
SIGBUS anyway). But it feels like making the lazy-load more robust
(which this patch does) is the best way to handle a wide variety of
possible failure modes.
Signed-off-by: Jeff King <peff@peff.net>
---
commit.c | 33 ++++++++++++++++++++++++++++++++-
t/t5604-clone-reference.sh | 23 +++++++++++++++++++++++
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index 4385ae4329..499a9602ad 100644
--- a/commit.c
+++ b/commit.c
@@ -434,6 +434,27 @@ static inline void set_commit_tree(struct commit *c, struct tree *t)
c->maybe_tree = t;
}
+static void load_tree_from_commit_contents(struct repository *r, struct commit *commit)
+{
+ enum object_type type;
+ unsigned long size;
+ char *buf;
+ const char *p;
+ struct object_id tree_oid;
+
+ buf = odb_read_object(r->objects, &commit->object.oid, &type, &size);
+ if (!buf)
+ return;
+
+ if (type == OBJ_COMMIT &&
+ skip_prefix(buf, "tree ", &p) &&
+ !parse_oid_hex_algop(p, &tree_oid, &p, r->hash_algo) &&
+ *p == '\n')
+ set_commit_tree(commit, lookup_tree(r, &tree_oid));
+
+ free(buf);
+}
+
struct tree *repo_get_commit_tree(struct repository *r,
const struct commit *commit)
{
@@ -443,7 +464,17 @@ struct tree *repo_get_commit_tree(struct repository *r,
if (commit_graph_position(commit) != COMMIT_NOT_FROM_GRAPH)
return get_commit_tree_in_graph(r, commit);
- return NULL;
+ /*
+ * This is either a corrupt commit, or one which we partially loaded
+ * from a graph file but then subsequently threw away the graph data.
+ *
+ * Optimistically assume it's the latter and try to reload from
+ * scratch. This gives a performance penalty if it really is a corrupt
+ * commit, but presumably that happens rarely (and only once per
+ * process).
+ */
+ load_tree_from_commit_contents(r, (struct commit *)commit);
+ return commit->maybe_tree;
}
struct object_id *get_commit_tree_oid(const struct commit *commit)
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index 470bfb610c..c232ab8c15 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -360,4 +360,27 @@ test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '
grep "is a symlink, refusing to clone with --local" err
'
+test_expect_success 'dissociate from repo with commit graph' '
+ git init orig &&
+ # We are trying to make sure the dissociated repo can
+ # find the tree of the tip commit, so the test could still
+ # serve its purpose with an empty tree. But having actual
+ # content future-proofs us against any kind of internal
+ # empty-tree optimizations.
+ echo content >orig/file &&
+ git -C orig add . &&
+ git -C orig commit -m foo &&
+
+ # We will use graph.git as our "local" source to dissociate
+ # from.
+ git clone --bare orig graph.git &&
+ git -C graph.git commit-graph write --reachable &&
+
+ # And then finally clone orig, using graph.git to get our objects. This
+ # must be non-bare so that we perform the checkout step, which will
+ # need to access the tree of HEAD, which we will have originally loaded
+ # via the commit graph.
+ git clone --no-local --reference graph.git --dissociate orig clone
+'
+
test_done
--
2.54.0.547.gb3b6f86dd6
^ permalink raw reply related
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Pablo Sabater @ 2026-05-19 5:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Chandra Pratap, phillip.wood, git, christian.couder, karthik.188,
jltobler, ayu.chandekar, siddharthasthana31
In-Reply-To: <xmqq8q9gb704.fsf@gitster.g>
El mar, 19 may 2026 a las 2:03, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > By having is_parentless as a flag in 'git_graph' that every stage can
> > access we could modify the rendering and maybe completely drop the
> > commit placeholders, working on it for v4 but currently renders like
> > this
> >
> > * A parentless
> > * B parentless
> > * C parentless
> > * D1 child
> > * D parentless
> >
> > (A has indentation when it could not have, but that would require a
> > lookahead if the next commit is also parentless)
> > But definitely a step forward.
> >
> > Do we want cascading or just a fixed indentation?
> >
> > * A parentless
> > * B parentless
> > * C parentless
> > * D1 child
> > * D parentless
>
> I am late to the party, but I cannot get how the latter is viable.
> If "A" had parent "B" whose parent was "C" that is root, wouldn't we
> see the same output? Or are we adding " parentless" at the end of
> the one-liner log message?
We wouldn't see the same output because A and B wouldn't get padded in
that case. Vertical adjacency between indented commits doesn't imply
relation because indentation means that they are "parentless",
ambiguity happens when there's no indentation, you can't know whether
they are related or not, but knowing that every indented commit is a
"parentless" eliminates the ambiguity.
* A child
* B child
\
* C parentless
* D1 child
* D parentless
Some different cases:
A child
\
B parentless
C parentless
A parentless
B parentless
C parentless
C has no indentation because if there's nothing to render below,
indentation is disabled.
A parentless
B child
C parentless
Anyways, having more than 2 "parentless" commits one after the other
is strange. Cascading is just having a depth counter and printing the
padding depth times, so I'll keep it as it is more intuitive.
>
> The former, with the understanding that "two '*' commit marks
> vertically adjacent have parent-child relationship, otherwise we
> draw line between '*' to connect them if they have parent-child
> relationship", does not have such a problem.
^ permalink raw reply
* Re: [PATCH] commit: fall back to full read when maybe_tree is NULL
From: Junio C Hamano @ 2026-05-19 5:56 UTC (permalink / raw)
To: Jeff King; +Cc: git, Rasmus Villemoes, Daniel Mach
In-Reply-To: <20260519050513.GA1635924@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It also means we have to reimplement a bit of the commit parsing. We
> can't just use parse_commit_buffer() here, because it expects an
> unparsed struct and wants to load everything, including parent links.
> But we don't know if the parent list has been munged during traversal,
> so it's not safe for us to touch it. Fortunately, it's quite easy to
> load just the tree, as it is always the first line of the commit object.
I was hoping that existing code to parse out the tree in
parse_commit_buffer() will become a call into this new helper
function, so that we avoid duplicating the logic.
> Moreover, this strategy does nothing if we lose access to the graph file
> unexpectedly (e.g., due to a system error).
Or simultaneous repack may lose the file from the filesystem,
perhaps?
> +static void load_tree_from_commit_contents(struct repository *r, struct commit *commit)
> +{
> + enum object_type type;
> + unsigned long size;
> + char *buf;
> + const char *p;
> + struct object_id tree_oid;
> +
> + buf = odb_read_object(r->objects, &commit->object.oid, &type, &size);
> + if (!buf)
> + return;
> +
> + if (type == OBJ_COMMIT &&
> + skip_prefix(buf, "tree ", &p) &&
> + !parse_oid_hex(p, &tree_oid, &p) &&
> + *p == '\n')
> + set_commit_tree(commit, lookup_tree(r, &tree_oid));
> +
> + free(buf);
> +}
Looks quite straight-forward. Don't you need to pay attention to
r->hash_algo and call parse_oid_hex_algop() instead?
Or are we pretty much sure that "r" is always "the_repository" here,
in which case parse_oid_hex() that uses "the_hash_algo" would be
sufficient?
Thanks.
^ permalink raw reply
* [PATCH] connect: use "service" enum for "name" argument
From: Jeff King @ 2026-05-19 5:22 UTC (permalink / raw)
To: git
The git_connect() function takes a "name" argument which is a bit
confusing. It is _not_ the program to run on the remote repo, which is
specified by the "prog" argument. It should instead be one of a few
well-known strings specifying the type of operation (e.g.,
"git-upload-pack"). But to add to the confusion, unless otherwise
configured, those well-known strings will also be the same as the
programs we run, making it easy to mistake which variable is which.
This confusion comes from eaa0fd6584 (git_connect(): fix corner cases in
downgrading v2 to v0, 2023-03-17), though in its defense, the term
"name" and the use of a string are found in other connect code, going
all the way back to b236752a87 (Support remote archive from all smart
transports, 2009-12-09).
But let's see if we can clean things up a bit. The term "name" is overly
vague. We use "service" in other places, including in the smart-http
protocol, so let's use it here, too.
Using a string invites the notion that it can be anything, not one of a
defined set. Let's instead introduce an enum, which has the added bonus
that the compiler can catch typos for us, rather than quietly choosing
the wrong service from an unexpected strcmp() result.
We do still have to turn our enum into those well-known strings to pass
along in the remote-helper protocol (e.g., for a stateless-connect
directive). But now we do so explicitly and in a way that I think is
much more obvious to follow.
This is a pure cleanup; there should be no behavior change.
Signed-off-by: Jeff King <peff@peff.net>
---
This was a cleanup that come out of discussion on another patch a month
or two ago:
https://lore.kernel.org/git/20260327213308.GA598533@coredump.intra.peff.net/
builtin/archive.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/send-pack.c | 5 +++--
connect.c | 4 ++--
connect.h | 7 ++++++-
transport-helper.c | 43 ++++++++++++++++++++++++++++++-------------
transport-internal.h | 5 ++++-
transport.c | 14 ++++++++------
transport.h | 4 +++-
9 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index 13ea7308c8..3c1288a123 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -31,7 +31,7 @@ static int run_remote_archiver(int argc, const char **argv,
_remote = remote_get(remote);
transport = transport_get(_remote, _remote->url.v[0]);
- transport_connect(transport, "git-upload-archive", exec, fd);
+ transport_connect(transport, GIT_CONNECT_UPLOAD_ARCHIVE, exec, fd);
/*
* Inject a fake --format field at the beginning of the
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index d9e42bad58..316badd969 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -223,7 +223,7 @@ int cmd_fetch_pack(int argc,
int flags = args.verbose ? CONNECT_VERBOSE : 0;
if (args.diag_url)
flags |= CONNECT_DIAG_URL;
- conn = git_connect(fd, dest, "git-upload-pack",
+ conn = git_connect(fd, dest, GIT_CONNECT_UPLOAD_PACK,
args.uploadpack, flags);
if (!conn)
return args.diag_url ? 0 : 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 8b81c8a848..1412b49bc8 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -273,8 +273,9 @@ int cmd_send_pack(int argc,
fd[0] = 0;
fd[1] = 1;
} else {
- conn = git_connect(fd, dest, "git-receive-pack", receivepack,
- args.verbose ? CONNECT_VERBOSE : 0);
+ conn = git_connect(fd, dest, GIT_CONNECT_RECEIVE_PACK,
+ receivepack,
+ args.verbose ? CONNECT_VERBOSE : 0);
}
packet_reader_init(&reader, fd[0], NULL, 0,
diff --git a/connect.c b/connect.c
index a02583a102..9af277bed6 100644
--- a/connect.c
+++ b/connect.c
@@ -1427,7 +1427,7 @@ static void fill_ssh_args(struct child_process *conn, const char *ssh_host,
* the connection failed).
*/
struct child_process *git_connect(int fd[2], const char *url,
- const char *name,
+ enum git_connect_service service,
const char *prog, int flags)
{
char *hostandport, *path;
@@ -1441,7 +1441,7 @@ struct child_process *git_connect(int fd[2], const char *url,
* fetch, ls-remote, etc), then fallback to v0 since we don't know how
* to do anything else (like push or remote archive) via v2.
*/
- if (version == protocol_v2 && strcmp("git-upload-pack", name))
+ if (version == protocol_v2 && service != GIT_CONNECT_UPLOAD_PACK)
version = protocol_v0;
/* Without this we cannot rely on waitpid() to tell
diff --git a/connect.h b/connect.h
index 1645126c17..c56ecddc0e 100644
--- a/connect.h
+++ b/connect.h
@@ -7,7 +7,12 @@
#define CONNECT_DIAG_URL (1u << 1)
#define CONNECT_IPV4 (1u << 2)
#define CONNECT_IPV6 (1u << 3)
-struct child_process *git_connect(int fd[2], const char *url, const char *name, const char *prog, int flags);
+enum git_connect_service {
+ GIT_CONNECT_UPLOAD_PACK,
+ GIT_CONNECT_RECEIVE_PACK,
+ GIT_CONNECT_UPLOAD_ARCHIVE,
+};
+struct child_process *git_connect(int fd[2], const char *url, enum git_connect_service, const char *prog, int flags);
int finish_connect(struct child_process *conn);
int git_connection_is_socket(struct child_process *conn);
int server_supports(const char *feature);
diff --git a/transport-helper.c b/transport-helper.c
index 4614036c99..bf37c5280c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -620,8 +620,22 @@ static int run_connect(struct transport *transport, struct strbuf *cmdbuf)
return ret;
}
+static const char *connect_service_cmd(enum git_connect_service service)
+{
+ switch (service) {
+ case GIT_CONNECT_UPLOAD_PACK:
+ return "git-upload-pack";
+ case GIT_CONNECT_RECEIVE_PACK:
+ return "git-receive-pack";
+ case GIT_CONNECT_UPLOAD_ARCHIVE:
+ return "git-upload-archive";
+ }
+ BUG("unknown git_connect_type: %d", service);
+}
+
static int process_connect_service(struct transport *transport,
- const char *name, const char *exec)
+ enum git_connect_service service,
+ const char *exec)
{
struct helper_data *data = transport->data;
struct strbuf cmdbuf = STRBUF_INIT;
@@ -631,7 +645,7 @@ static int process_connect_service(struct transport *transport,
* Handle --upload-pack and friends. This is fire and forget...
* just warn if it fails.
*/
- if (strcmp(name, exec)) {
+ if (strcmp(connect_service_cmd(service), exec)) {
int r = set_helper_option(transport, "servpath", exec);
if (r > 0)
warning(_("setting remote service path not supported by protocol"));
@@ -640,13 +654,15 @@ static int process_connect_service(struct transport *transport,
}
if (data->connect) {
- strbuf_addf(&cmdbuf, "connect %s\n", name);
+ strbuf_addf(&cmdbuf, "connect %s\n",
+ connect_service_cmd(service));
ret = run_connect(transport, &cmdbuf);
} else if (data->stateless_connect &&
(get_protocol_version_config() == protocol_v2) &&
- (!strcmp("git-upload-pack", name) ||
- !strcmp("git-upload-archive", name))) {
- strbuf_addf(&cmdbuf, "stateless-connect %s\n", name);
+ (service == GIT_CONNECT_UPLOAD_PACK ||
+ service == GIT_CONNECT_UPLOAD_ARCHIVE)) {
+ strbuf_addf(&cmdbuf, "stateless-connect %s\n",
+ connect_service_cmd(service));
ret = run_connect(transport, &cmdbuf);
if (ret)
transport->stateless_rpc = 1;
@@ -660,32 +676,33 @@ static int process_connect(struct transport *transport,
int for_push)
{
struct helper_data *data = transport->data;
- const char *name;
+ enum git_connect_service service;
const char *exec;
int ret;
- name = for_push ? "git-receive-pack" : "git-upload-pack";
+ service = for_push ? GIT_CONNECT_RECEIVE_PACK : GIT_CONNECT_UPLOAD_PACK;
if (for_push)
exec = data->transport_options.receivepack;
else
exec = data->transport_options.uploadpack;
- ret = process_connect_service(transport, name, exec);
+ ret = process_connect_service(transport, service, exec);
if (ret)
do_take_over(transport);
return ret;
}
-static int connect_helper(struct transport *transport, const char *name,
- const char *exec, int fd[2])
+static int connect_helper(struct transport *transport, enum git_connect_service service,
+ const char *exec, int fd[2])
{
struct helper_data *data = transport->data;
/* Get_helper so connect is inited. */
get_helper(transport);
- if (!process_connect_service(transport, name, exec))
- die(_("can't connect to subservice %s"), name);
+ if (!process_connect_service(transport, service, exec))
+ die(_("can't connect to subservice %s"),
+ connect_service_cmd(service));
fd[0] = data->helper->out;
fd[1] = data->helper->in;
diff --git a/transport-internal.h b/transport-internal.h
index 90ea749e5c..051f3ab0dc 100644
--- a/transport-internal.h
+++ b/transport-internal.h
@@ -1,6 +1,8 @@
#ifndef TRANSPORT_INTERNAL_H
#define TRANSPORT_INTERNAL_H
+#include "connect.h"
+
struct ref;
struct transport;
struct strvec;
@@ -58,7 +60,8 @@ struct transport_vtable {
* process involved generating new commits.
**/
int (*push_refs)(struct transport *transport, struct ref *refs, int flags);
- int (*connect)(struct transport *connection, const char *name,
+ int (*connect)(struct transport *connection,
+ enum git_connect_service service,
const char *executable, int fd[2]);
/** get_refs_list(), fetch(), and push_refs() can keep
diff --git a/transport.c b/transport.c
index 9cde4a4e43..132c93e665 100644
--- a/transport.c
+++ b/transport.c
@@ -309,8 +309,8 @@ static int connect_setup(struct transport *transport, int for_push)
data->conn = git_connect(data->fd, transport->url,
for_push ?
- "git-receive-pack" :
- "git-upload-pack",
+ GIT_CONNECT_RECEIVE_PACK :
+ GIT_CONNECT_UPLOAD_PACK,
for_push ?
data->options.receivepack :
data->options.uploadpack,
@@ -957,12 +957,13 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
return ret;
}
-static int connect_git(struct transport *transport, const char *name,
+static int connect_git(struct transport *transport,
+ enum git_connect_service service,
const char *executable, int fd[2])
{
struct git_transport_data *data = transport->data;
data->conn = git_connect(data->fd, transport->url,
- name, executable, 0);
+ service, executable, 0);
fd[0] = data->fd[0];
fd[1] = data->fd[1];
return 0;
@@ -1656,11 +1657,12 @@ void transport_unlock_pack(struct transport *transport, unsigned int flags)
string_list_clear(&transport->pack_lockfiles, 0);
}
-int transport_connect(struct transport *transport, const char *name,
+int transport_connect(struct transport *transport,
+ enum git_connect_service service,
const char *exec, int fd[2])
{
if (transport->vtable->connect)
- return transport->vtable->connect(transport, name, exec, fd);
+ return transport->vtable->connect(transport, service, exec, fd);
else
die(_("operation not supported by protocol"));
}
diff --git a/transport.h b/transport.h
index 892f19454a..78e9ea8ad1 100644
--- a/transport.h
+++ b/transport.h
@@ -5,6 +5,7 @@
#include "remote.h"
#include "list-objects-filter-options.h"
#include "string-list.h"
+#include "connect.h"
struct git_transport_options {
unsigned thin : 1;
@@ -324,7 +325,8 @@ char *transport_anonymize_url(const char *url);
void transport_take_over(struct transport *transport,
struct child_process *child);
-int transport_connect(struct transport *transport, const char *name,
+int transport_connect(struct transport *transport,
+ enum git_connect_service service,
const char *exec, int fd[2]);
/* Transport methods defined outside transport.c */
--
2.54.0.524.g198262df96
^ permalink raw reply related
* (no subject)
From: Lucy Phere @ 2026-05-19 5:05 UTC (permalink / raw)
To: Junio C Hamano, git+subscribe,
git+confsub-2780373d217fb5d3-lphere34=gmail.com, git
[-- Attachment #1.1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #1.2: Type: text/html, Size: 23 bytes --]
[-- Attachment #2: Lucy_Phere.vcf --]
[-- Type: text/x-vcard, Size: 19683 bytes --]
BEGIN:VCARD
VERSION:2.1
N:Phere;Lucy;;;
FN:Lucy Phere
TEL;CELL:+48691639992
ORG:
TITLE:MASTER DEVELOPER
PHOTO;ENCODING=BASE64;PNG:iVBORw0KGgoAAAANSUhEUgAAAGAAAABgCAIAAABt+uBvAAAA
AXNSR0IArs4c6QAAAANzQklUCAgI2+FP4AAAIABJREFUeJycvXm8bVlVHvp9Y8619t6nvV3dW
3WrbjVAUQ1F0YilAkZEiTH6nolgE8UYFBRQAY3Js0miIQEhECMojXQGwRhag0YjErrQKUJVUV
B9FdXcqtvfc0+/915rzvG9P+Za+5xbRZKXt37nV3c3a60955ij+cY3xlzFf7UuEwA4UV6U1wB
MuHbzw1et/gkdTpgZ6wtQz4VYY7gf1aL7pk2O53BxCK2Ha9+P7yxXPfKgGwjAIQDl9f/ioB75
Kuy6hLvvTy9fEQ4Z4bOTKZC0/nyDU/2HAvtxGgjlAJZPDKRg6P7owYo4ikTKC+8H48RFzY10S
CIqMAAOuEjPDdQGiGFQT+8JmIZ0xzPyjWE2fHr5M2omEQP/j6RDkmQ4/xKxFw27paUAGXeJ8h
HSAXzXuHYETAFSpPGR6yoJEEG3//Vw5/2sWImVWSQDOTVPlCJby2NCJC0GtOPgGxfnWy9o7ze
qCMVAM5MerVE746N2XszellcmSJ0I0M/KKAo07QgCThNNgM8+NID97xrOmyRJwq38nKkXRjlz
pj4MxOxW3/gwISjLjZSZmZwkPFMt1KptLDWWm4AgVuar1pypsPIsfXJ/ex8ws1WR/ycGRQWia
M15umPcJT6fSa27uROu3bpTLgqOHRUrs1W5o81+MEA75tYvgySou4uZztOu8ra7L7wTOYMsUE
YGaQxvmadKY08TedOdTFk6Znl6fTy6L9//v5bIzmzPl1OZ5CO82Mx30GS75l8+KT7lfCclkgZ
6pwWG3vGVa00+U59yOMFeJgaiv5uIOJPLo4+gtJ6WD+Q1IlkYuNFypmpykzYwZCYoBGM2DjJa
qjW/8+B08zpu3AactSvUD7uIQ3yUXM4XzaMPA2EJMKAzt+D9deWErJmOGgiCcIoo7oYEuhBRP
HEvJg9OkuW2BkICYYCkcmbRuPgNh9UPwe8fPWf/6lv6syOD0Rs4oRbeuAWDQWXUkaioljh6eD
A9luYRcSZcQRURELusqZPUNxLNbgnSBKhYxI4zKjqD7m1Rit76ivcpKuNgF9fK5cWUSBkJGeC
QipDQK6aB1g+j+Kn/qYBIyOvVhcfbiuDJ4SFUiLWZyUEmhKlZJKNJno2YMlSAoNab4/Nxhckp
nImXQ7bjlQCAEeqi8/mi2dE4Ex5ta3AYuEsNbZfcSRqcMvReuUjHQEkEAchEAS46QJCki2APB
YrD3vGaNEQrw/pGCp5ZOawJc1W77nnKmIMgCyHQrQT0RiFlq5izCPkUcMBoXvl0gK0FHCPttF
1WBNAri3ZjrkfIaKYdu0+gAPpMWUiW0OPsLM4A0ekmesFE5hBhlDlJgwS4aWZlYm9N6J02hd6
4xO4MmPfS6X5cgECoLHkblm7e9xJlmEtt69MtNBNv3NqWqWVOlqZot5SnaFu0Gak1b81zoGqN
R3ltsXngAn8g8DxTmk3+vABPnynOedIpgVwFP+zcITiCg2QgSRVJBSEqGOiQgVaUSSIVFThTN
2AWJSmQiEAEjZ2YglA5omjspCNAxVcUtQNBoglz5+Ye9/V9z0MyNJmTFtNttlvWTpFaZFd2dz
DBXEGGXEnuMACOCICeq7yylE/M8GeBGyyw1VTkUqDNTgBVj3HgJKliBj4TH/vDgIIDOxPrtSy
wj+VAKA6euXiWIJAMDpMiGMggQGQnnW4lWYAYIe5yEIRQJtCLbDvuv+Wi563VF8vpydUmNeOU
WiZnTsoeMpFdKXvKzO7Eg+EZjiAEB2Gki3m71pg7DgBFNPhGob1oU7GpIp3ZETp/4YT3MLeTD
qWZ6DsVAyMUoE5lxAAaGcDgIBRgAEK58Dyg1L2Ivc/+3xxE/TdXv9KHC5nmmZ6kZprblJOYoJ
Q9wzOUHaE9NfzmjCDErBIEDFIAQp4SMJJAQTQzXdiN8bhLO3bDpYD+tPOg50x3eoeN3E2PDHI
nuh8t4BMyyQASRToFcEuYQc7yW9b/TgeiHvFXgNbupdsKBz5x7e9NDj4zM3pmbpO322qmSlJ2
VyLJmE4ufvMW5jPrlOkKgLkCGAAGgp7PA6LcmeyOdEzsE8uZXIIDriAn3ODlDsFLTtBJp7tEo
Rga4c5+qkIUYxGcCDDIi3SsGwYoFL8TQPgOvI7fMHucBU50Lh1ObNoFn73khXuXnn7hys2XrH
7KNEFuYFSIsLA2d/l6fUnygdsgybIFIWSZMziiGEr4nAWgHdEUqMKSbZaE30123gnFltCPykV
2F6DTHS/uyOAGM4kKndMp3sUd3doX8FgZvQyic4jqdWZXekQhrn/kfXt+4Ed6dFp+r6QYfa4M
qkexk+rQib3Lorbr/SNfidwecJpZoZpDqJOqTMuKTpOHzIA4hCowClEMNEMPc6GSYRaPE8gEm
IR/uhQB/NsHzyzs2TuDediFJynAZD1ksuIrgaIUARScMDBTNMhAzTK1cgcHkWcYagdGzfS6rF
D3FhAw+v4f+pf/6f1AlvjrywHAq9d3C63XLBLAXHPy4NpNc9sPR62NMJZVHoeOKscaNmhZe6x
a1JnDbHOtDZuw0HKxDcs51Dv3kYFeIkV/ZzfZR1/7qv/2qn/1hrVcXHiZ888v7QK0RIjxTWen
ZQKxjzA92FNBhgZjl2CruHaSKMCnKGARSoFRQvcV3GQBAowCXfayT34RwOS/fuDBu+8Cwm5ZE
Dx+7z333HxLnk5u+evPfeIDf+y5vfemm75667EvPGC33Lv2tdtXtrXn5q8du+WrD25sumPUen
XrV++79eZ7nMNbb7n71q/dlVm/69/97qf/8mNCSCm94zf/5Znjx9nrdpqM3/3K3/yj177aVHJ
0f8z3fO/P/Y+/LU5t5pjLf35/Lb9jtYGQW19ITRSLdN716le99/WvMygQBp4+evTOr9w03li7
78473/yv/gVy58jvvPHLd9x8oySSd9x845033VhYFLo++Adv/w+/+au33fS3BYh3muX62i034
jXr+tbXvxkAQvWa9fPg7WvWdcMPPBfAL7zjveWTV5/YmA23HB+6/X3P+vvfAeCGZ3zzh+790O
9+8M0AlvYvv/fhz+xWOgCv+uzfzi5840b6nfX8uO98zu6fe/HHPv2GjVRev+74+ps28ps22jd
ttG9db8uHb1tPb1tryz3euda+cy398ItevPsO//aP3v+ec+lHf/4XAHvZq1/TDZX4yOnxB87m
cs5/OZ0/cqq7yZ+d8j87nnq9LCeHjx9LnzimTz2sTx3LBaXgH7z4JQCQWwC/teaEAXjthli+B
n73hc//1yfWfvP4ajU//7oNf/2av/DVryEwOnTh5tzBF73h1wB88XN/23LxV1/wqwDecdNfZV
so0vndh+79uU9+EsCvf/u3/Pu1VN/wrQBe+aM/TPKJP/xj//Atf/DGjfT6U+sA3vr3vnMm+Qg
ZFVSQSPfxzyzFn1muRHvT3UeLjbz/7W8F8O619J7VBsC/+PEf7llEf8drfut9K3lp734Ib3jJ
C8KOJXoBhgBMeNe/eyWAX33b+z56Mr/5Y195y1/dCJkVWC888Sk3mAHWe5vAnXBh2Hn9/T/3s
rn5xfmFpXLaieMPv+PXfkW0f3P3sa2la9Ngf/k82cLmxhqArWrOwwAl2ZlfuuqpNwBAqMzw87
/7VgDwTPh3/Mg/+uQ73vqyvaNfvmgfAGSvegmJOwzx7Phn73rvP3/nH0L+c1ceic10pptWYE2
Zf//hK/71qwj/8Vf8cwCnHrxvJ/9k2EnxqRf84q8A+K2f+ZG/eyi89DlPW1xeCiDolBn4669/
swF49Q/9AADUwwIHOuaAO6zl8kUX7uAU4fVXHQHw2+vZAsbx4ObSlc/47u8CsO3zAI7c8NQU5
9uwWK71MGIYlvFVtNiBPVWwV+wbrn35b/742Ol3PXSmW6F+qSohAMFYCRVZFusJz/tH1/3Qj7
/4tb8N4EVH9kd50a0gt37O1E5s2Y1grOdDenq/OwaDwcdO5Ne9849Ho3mgff4NV7BInDLgyFV
Pjh95/vNWP/qnIF71wBnrOBoD/MQf/cdL/tE/6ZFkoIkSiF9arAD80Mt+6YG3v5Hk4376pdkW
/8F7P/i5C/f+3FVPA/BLH/iA4hI4W9JR7IUee7hgha8BBJxLeuvzn9cJqB95B68FGOFdFnb3+
98b5L//K/8UwNOf96NkyZHs7ve/Z/X40e4OZL8GuynDHiUIr/7BZ6MazUzse668EOun/583vO
vP79t49oUlr3CAPZsa7LN/+iEAL/vCVxcX56NUkweuugbA63/2BdH69J+sgYrID3y9XPmBN/7
2G37p5b/zi68IhmiYX1gG0LYtgLk9lyos5bg4U+PdDEaxmaKP8aonAHjpxXu/+fkvmE1jJ9ss
+I2a2d2rXviTr3zRT0s6/MSnvPBN7wDw7pNjwn/jZ37qP/zGb4D2/tVUeA10WrCD1AV7zZ9+g
sAXPvM/jj7YMcIEPnbP8VhXr335TxXpfNtzf6rAzp3s5/c2d+oOMwFaX12xPk/JRFCBpJAcHe
5VeUHhJfsGaFuE6vdXp1li6Ni8HWajx9DmBnbg2DJUUiIqipJesBQgvO3U1mBuMLt8h4ovY5X
BFOSU9Qy04BLNIJuRZCXlUIYiuzk7iw/u0lcUdo1CoednxMsMLsUZ2dc5IPbrLLIk9xJIc7Ho
PNTlLkVyAskXL3YL9ba1CSUa0CU7ls3LzEkVi+nAWT/TguIEAnzBogE4cunl9fwg9IC7hNI+O
SIpsqMK0aPyYlwdZ+EqNm3dMAKZTVY4AANmPriUrajO2xoIZCv3K2majG/Z8B7YaHeipB5o7y
5s0WQ9lMKuYuzayko1HM6P5gpNtZNvU90UoN6bIIAZO1Wa2Z0BBJ+xrs7iSrlT+Sx598y5dD6
4g8XdemAXT4hO5Tv0bAApyqSulmkd5x96Fs0BVI4sBZZFZQy7GNhOJLveAaAVYXkhLtjbF3fx
fnv3dZE+0kDBZ7fhLIUyOWCgA4r9CgQhGzpuvhOpzfICekmTZ5Q8Anem15kw4XR6iQkO2C6Ci
SjMmbLR4JkIVLdY2KEQnDSpcwgO0ESn0SRF7ixyJ8WeMyprxo6B7VKZzocC2EWxF8F6od8ERC
Gfz7E6wdxxQZ0+FSEYwmyp6UDoCB3vvi10cqmFAhHwMuFeBDBEk4x9qq9uVNYvsUkqhR2LRel
LOkrBuiKYA9lQqhEAYbRCPNMVQ7/Im2vrgBYWFhm4ubZGcX5PhwzHm1s5Z5LzS4tbq+u7xAIL
WFhczm0ab20BmNu7HDvPAsJBM0cxS+uMmzMgKvTEQuFfSy1cJGlGdy8Lxq44Y0A2kt77hELi0
ItOqa/h0IjsZT37xSPkveB85pLL/ftre0fIHiUUPX3XVhfSfuHwvvW1c+/4yu3hyqtfMEcA/+
aDf3rp3/8+yH5yqUJO3/vbb/rRn33pT86fxx8NhnNvP7N17rZbXn7DkwC8Z/s8DuC8mlcp7Hb
FiVKfKYJyE5wedle4dsUvdJlzcS4obglikDSrc3RGKHOCKlhid4V6RoB1PksdHzLjZ9WR3+c5
OAoxnmcoHb56wrc9/ba//sLrfvonf+/4igDkBOLHfvals9PeO+7mUdog6sXl8yUyw7WclWvKJ
MqMTDPaQKBoNFjnTWflMXb1hv7OgIjgpX0hANhhcGYtBt3yl5pBcZW9u2GZtPUsYBFLuX0gHU
bS8uwneykbEIEwGwcVgV/+4w9KWl87R8Oit+XEQLeeAJ8xpwE0+fzePd0AiwJLRhq72jlnxXX
SZIFiT06XPpCOEqWh9wABMDcjAhAAK+ZjoMzU5ZodG10qI+qI48KQkDQzsvCYpDqD2Q3Kyn8D
GTjzSz2IVSEJDZQF7l6BbkEGBy8qr9Pm+J89/ZsBvPG/fbxgBwACfnyOPzbHH5vjx177SjOrl
pdAzC8sGWhUoM1+JjgCFQ3lzwIMjAQps55yJkpxxQhCHYkV1E2VLJGpnGz0YAjo6mTlcgtuQk
QvdDKYenIagTtEeI9C3KiCBQHAcymHWVmGclJWAKMxFwhdhBrISEi69Hu+78GP/vmf/e5/uPe
rXwFw6NuftRsQfefL/1mJ00dueMYszdl74IIuFd7F6RagvONZuvoTAmYl+07hTaU0LLAr45Rv
rXCq6gpkJbB2PsI7WzI3soTa4rCLxAvYIQgp936q9zKFLKPTC+pxWgEVpJc6GgDwg9uiSc6fP
rx/bXXlvV+7Y/jYK8tknjcKoBVM/KGJA6L4gyMrb4uv2E1HdCLQjicojTadD1D/7650rJda77
M4qwlrx8GzFDlsh8x3CZkkZNZn8JQVoGul/N2FhC5QKvtuV939dM+9ApBUSpVQ1C5mNj5vjgD
+ZNJNNIBmOxXeEh0veexjrctdui+eO4oAQMTh8ENnt07dd++Lrn0caB8ZJ/YwsiAJsDSKsVOk
/tZGdIlA59bK/WiAk7FPZQhAgfCibyx5toElTnfGYv0tZK4CMEyQddGvSMRmEK4U9eUM7IlvB
NEhyExeuh3Lj3dkeNdxBgR2cCoI+y+9/OyD9wt4x1fvaEr6OpOcunwqyI1YjgYgBgs6Pyh21g
uHIijyERoXhRk5byjNol1puCuq96WrrhpDzeJ3kQjP64RCgb+lx0Vy9DpeiJSZhCpBKNIhACk
DZlRAUFeCF0uu91+n3mt1h614/hTVcRSPmDg62wNnSVzPQM5Uvbcg9Vkbd0xpd2vPblN9ZD+h
3N3ZVV+tr332YMAZCsxTSVYdXaeCF4ubtWz2acGuwXjfwuEysgeSkBRm4MsRq47DKLnwo+XQ5
xW72bn+i9DNn74Di0NpjOyvAoo+dgDExEQZIBqKAVOw3nFmiL0DLqM1MFjs2ZduGGW9gkqqJ3
Q5c5czyRC7pjJ5gLm6tBMsfUKxIBGW0O59dJKkLpD2KTqJaHDlrkgEeJZmEijCRjcyoURN7xA
EANAdcKmP2n0qUTQWkgTE0KVuUrcIJS032ykzdeHWvKQl/QJa7hQepaSOIDjdQCH3bWG2ow7K
lJHuLkOwvke1RCHsolqLoVICM1kBkDu0W31dgmQx+MRtLpdWNxm5I5cSJzsegijTKfm19T9jA
Ism9T0iAvrWx5JhibvyWu4ahBvNRdIRVGKZcQYmKMoD+kZSUHCRFouX8ThrJlJBBjuBzEhA5n
2bORRYYj5JVwHV5YcCunqHo0+Deg0o7jRGtW3PzsAU1IGUnYMoGtgLn1U/YeuVuuRWu11JH9x
7DA102Kck18US1CUCASgKPgu6AEhlIpR0qcd4xt7MeumwEw6kLqsIFGDWeeAOUvRjEwwhy7vu
DgdtllX0a9rndejscdc+AQHI5zvK/phFCkrn55AAShIWWS7u0Fv5wsq9MYvELElphxhmmI3e+
+DuwtixAJhxgCACPankMOVqkewxjrqTZz/dwYCdTs1uKiXgF/REdf2tss4Cu/UTRZhiWTQHYh
djztOe/9nRt2LsHEF9PyqMlKlzQuw8/yyacEeiu8Tf00ehnGtdC0pXNTeya6boy83qUTJVEsq
uBbOgbRVtLdG0B6ozpSvqpBksKnZXzIGODihQcAix9Nf0jUP9eM/vUOnZ4bJEOxMjdkL1+eGP
ZZ7sOrR2fTdLF3efbZ1edl8XLZLMrCQq/aUdQhEoU/AOYxTzZ/YZZCR2dGZm4Oo7e4skY0cRO
b14867jzaidtQYiUatf/9kmn5lBlsP7ST+yNbXvjvjGBx3ajcp3UsPdgiZnGH3HgRtgxl3S7I
Juj2dhYuU6/Ef3DeeG6f4tu7D2+6eS5q7cd/dzD1zyhXPxwOiBq6qyh2EWsztf1Zljp1KAyjp
2OtN56K6fKBpr9D7K0Cn2I45v2LJbPt89/0dLCH3aVY7QZaqc+fASg7oPAapEyk6KhLpKqQXm
HIqK09nmC99819LaaEVbwyOhuWndT0zl5F75JeHSD014SsLqpbfWR597CDMyRwZk0AxeVK/fO
ATvarTdsvW2IoARFmbrJinYeWWM/+1B8hHnP0Ji5MztzDoi+4SjS9DKyHbcOLqxzjJyUxYZSk
figXffH+HDe8PkKcBnJiufOFtNmJL2/JPL/bBNjo91+6oy4jB05a7ZMJSkECggSNlAl5gdZuo
6+M9XDoJSPC9idbrXRxScJyz2lZBOxtYR/I9s4ivTKsDEMROB+kQ1q3N2s7vNyFf0MWS2w4uF
tTdS2vfe+0eb2Y55uHpuvV3hO8fazsG576euGh9O0/u3/fMTkyHAIhUttLlSVp/VgWRwyqREU
fS5//T1xTDgWA9eP4jXHypsEuAGm3nhGIsgShli1+rNpFIcw07VoFRuShYTeg/9iKMvjOhRXj
xoR5wz1LNrkTQLRh1VSlJ+6X88YSFt37w5uOFAe8fq9vsfDutp89zm/EuuWDw0Pz0+8S9swb2
1PDcNeeQIVHaxUA6auQh2OEYwzU3iAkeQq7bDX9rmTQ9Yy42fuGx7OLN6h8hb2oLcdrTAd5bx
GxydA9nlm7/xHp6d4HP+h6V4Wy5U9wm7sFtEOeuhdIl733r3BfX82b89tvDUg37LVnpwK61NG
m+W/8kVtljzgWnz8LannI8MBhcMyoXb/+1kffFCOBitrs68/BIAUu4D4Y4dH/jPp2RAFibCti
tC2RksT9LoyL6T33ugDW4Cb0uSpFlr5U5EP09k/7+PR4HOLpzPinHsNMtLtlQoAQowDn77q6N
TKd6bh8/eN/78Ch5qIIVrlueesafxVvc2+dS0HkQ8YZ6SG21QykIKFXPrPJNP/8JB66geI1wS
+5xu74dXAChLmymdHA8uWmqOrdue2g1mZo4Tcdt+7GoeT9oo3Igjl+oHrAOThSj+P/HZjz4o5
n4j3E4iMlOTXe0PJZ+ekeeH3nZ/e+ea3TYeft+FzX8/rS0HffScQ03leaOp2+BBdvE8SW02ON
uiIoD4+PmOkZ5krOZzL9zvgV1CDaC0wUpLH17p9yrY9PNnF158mWW6q17h2o0nGAxOWLYYIol
lYCw4kEwuy1DszSOrw9z/349Hgkwo7nQY9FKbCauHJIVyMWQqEDDHaBX5wWb09P26ZcvHGYbq
+mVNcthwMvgg2jVz1TlPx7e9dYRuBzCOJ14+BIABABtstpM9XcXf+ownbqMeVpMz29aaL9riS
y6X5KH0bMFigFFIItQqAhA4otoCaA2t+hqfUMqkkhx8tCYV76Mu5e7ePiJY7jjpnU/KP0BvgG
XgFMQQIEEX3j5p797kBOlQTF9aUw3KeLDOq5k1YKguqy2EtD5JWylUEXCQItO0jWEIOCyEaYO
NZMulWlWCbwaw/Ik0eXBj4R9eDEDZMd0Z2+bxNQbIEKwG4FutSZAgsKLmoCjUYMWdFqFC1pXa
Gc//K9kgcylvwbybZwE13Ydl8qbZVTuV0LIVoKvzSLvo6vDgVtpofJqmd64zJ8D2XnvIT0wwA
CJYGZfrfO+WrJiqK1CmMjeYGMxC0KbrgbEhG9zQGtrARjetNk0z930XzYSS2dXsSMa66rq4KF
lGNWMXHMVPD4khMYBqokY/JSBQwVSVNirrRQAZFJgDaH0PaLT+c7CHfx1gK+fMamFl/xNVeJO
uHF5oADvXYoqcs226G82wdWETq4q1IYbRtXs9om1bDIwHB4qGSFYBlVVLtYIhGiukZjq3VRMt
MSVaKM3dafvOLi5ftN/qssGFahDUs46u+uI5X88MQEVtqXrC0nmpQiZLHjwCh0RNVFam3eXBB
lWBtRQkSqGrDnYZJSX2DqhHj6JUtmfb+YF/9rvFHxVMVEQsUAru3vpUJAa2aptpJfscUla8bi
HXkmTLNQch7A1WB0ZjdEUPl40sQtE82vbhQmBMgImpmR+H+qGgs016LBkCC6br6iZmZgjm8xp
++76pNwlqFjQcV7GviwBWumM6/D8omywcLZGM2mltKkYHI6TuYQw7TWr96xmD5z3qmyFp9DB9
5611rJiAhZU893t3pzpwIbYTLMxHnU0XPOmSNE0c1MMnLSKYAiwZanCg+JTPDi79InxNeYrsz
CFvXtM+9CIoLjz+AMjoU4dVbYh/W03WtxaefbB4QM04UOOM4VK06cntOgx0djr3bXvP/enRWN
a6gEWJHccEtGRFDUgjTEgG7zimndyBkM6jH7scJe/wcsV1zq7qagfqymHd0dEv2edffduSDXJ
lksJijKsRSzAzbxNrZx1ohkgGE3282ez9kXcQZxwbRCYFZjCFwed45Cu+8ou+fZXEUbtnMN4v
MV6JKs4DVJvSNHMQbBCJAKaSpbo7ziZuiBHzl+xpz7hVFneF29JG2DPIZBJNGBkqKYHJkAk5M
mYT3sHcQreBEbtaIQAE2kxHg5C5EyJnh0HVq762qMrCKMO7qlHrIXBQzTd1mmxP64NzVpkiQg
wyseKeF/4OdFKejQEW5S3KXlvLxtNY+FWGn8C+51eTRdBI93kYgjLy3WMGo2cfTcOeofdknaQ
wQYBNjm+GZy+2Hz1te6pYKPpCwJb6GPrYLMKBJiMYR0DrSqADqQeQuYd7Zd91f5WgjslTr23l
rPJDpf+1VO6qpD2vvtdDBmpaaaWjJEjaX9ltOnfi9Nz8COuTcGlQECsqwqLFq3+d7UMwwUbgI
hgRtzDdAlp6ljKRZO8wu0r2LejXFIDR6ifuabemenia11K9kXRZkGcn0j1beS1rLY2evG/8qd
M6sD34e5+Iu8sY1HmOtOiSAy44UJNVsb6iCLM9cOr6GHNPaDq7dPURTGJXtncI2Rnmvro+/Mg
xDzs8gwmCyiLZ/iocHp67+ezCMx/Tntmcq4hgMLKy6sg7NX3A2TDOwxZQX0QZm1PZJiEBckny
bJ710C/xms8D5rMnflAAqsUaV9ebD69vfvzs6PJDikq3bXHDA6y5zKrHfHrvt34E2FBULI28u
0pg54O6/nUGpvLaLAKlpzQLiXAhEIlg2cI+o257imN2JwCOREQhAx6owZ8cZ1V6NQFYgMpRDF
xAPDCw+7PqFOoYEVHbKCKwAAAWlElEQVSZJ8Wls1mfs2YTdUWrERcRF6GItEpZkgd3lkVDFiZ
rd7136dp/zL6c1ns9ARguj/iD80jKd45TSrGysP9LC1e9B77m45aDil5Fdt4H3d6MQtnYeWIq
h8OaDKMqsAICUREtFMXV9WY0rFLsnieivvDRk2XKcsCdckyK8Rpoc0TTK45JLnM5d7TYDtSL8
/PcYhzUabOJCwO0Oe7/NRryJDOYM9IG4DI0ITI8mYuZ8OA5mwfmMNr8/XTvD+YlGx4cna/QsI
Lba9VPmBuufQD7vqCR0YJvRHACDNwWYyWk4ob6J8KobKItQnlUFuZiA1SlmuwI5CdP4vYTNYA
nHcaTDsKFHAAVlkAJAtQ/4MjL0gnusBOPyYcfqtNWDoXcA7wvfZbuCVuK8cK5c3ecWqoWNhe2
9oZRPPhBLtQ5tj76O2H7r8EIWwQjPSlN6RkpIbdQsBzgGc7gTRze2uTrm7vX62s3oduJ00wn4
VGDc85GplBPtecUtCEMUV0sTcwi4jLrgzEAgZzsrDzQFc+BvqNx9zOpyobyVBg2Q5rizlMYJ1
ngV47h2Iq+//FMPnVkJ1ypUALMimADz5bpILh8uw+/pKRmcO1Cc2Zq22UzvUlShnW7t0M4PGx
uW8WlF6S7Vv0pC9WRe8RN2DJ5ASzKRmZzGdnyBtstpYY5yTMaQ/Y+UOTxg78zvOoAL17xyYKF
ZaESGqsy6hS05bmRJ3CAeJHFK7B1j1XrGOzz4WGrLu3aX2ogA3mnVFRIm67Vw2a7egF11UEIc
OmLDzdbzTC5IpDIU2P91zv9+gvqK/ZPp8wW4gJG0wnOTrCWMR+ghdMtJal+3xpclrB187nRU/
fl09O80TCboe8vKY0GB0Z79y7X++LgbDX/9Deg3hDqUB1ojv2lzdewBYQhcmZaV56obTylkEo
bgpgoQc48fVB+AmlAm8rHiHvJebQNbEvmrJagwGpPOx3xxKdj3XKwiOFhixcyzu88PygQDaBH
RLRSE+/jUZbKU7eyRMNc1o1n6o0kE9tABGTnWtJfn+YtZwejqFGwaBgEVIZhhSxeHw/e6kcNC
MOARmo9tmw/e4ZXz89dvjS5Zz2XDh+jKCPjKMYje7Y1PvSG/+KLZnDFixWuroc3eXUgVAcyor
UraFbZjpVatklNhgzZ6O4epKyUqQSMiIFsCB+DGwKUxZSYTiqvylkF19yC4qUcHUQ8JM4jjXd
2E0uoDQ0RtAMbiuuYPYYnkKUNQubDXL3rdpxukIVoiK6GjMY92v7m/ZhM81e291QGA2QIRASq
AMCDnNT9L8Pj3lhP9winp3Dmu7YmF+Xq+qXprWswk9FKuDfkS6d7//EHfC5bPufxgGnBT/whh
kuoDsHmQ5qiOYHpWUzGaKfWtspoFT0uxuYcGYwmA1QRix4vYXUANpCvMZ+RNimiqt3maQPYAu
Ih1AcVFqRsaQWTB+KOJEQ5KiCxg8Ilsch962/JRiK41YTPHg9fPonNpgRyuVh6jf/lN8GxuJb
w/lsnP/UUbEzxpZMwgQYzLERIzDQTOIeTrxgc/DNoZE1s+RDytnDz2uhZF2zeeG4w8dIRHp75
6cFTz0au07cUlm39qDC16oDiPqv2wbOak5g8zGYdqbFp0sHrUW9XFlEv065DfXXz6f/MdhuKy
UPAkGGfBhcZkuezzOv0LfkGapAV47w4gsRmhc0ZtKfRrHcCom84FwqbHqFU1s6Qe6ry3Mr4i6
fDfRt2dsy1zJSZes0LfXX5mY9BK1QVLvzhfz+/et+/CO3qx37/mj3p6GakFEUzQCaETF+4Ix6
8FWyUK3Id9ZP38GhKa5PmM2fnv+uAvrSpje3Fn/iC9t4pJWorNwy+qhDJCtUy632yOUxOa3Ic
k1VMx5okLV0emodzQLARtIi8mrb+pvqW5/hNN1KnLcvbidk5MLrNkbWHJaCikRJkuW3Nt5C3v
V1F2rRmU802J7nkFq0Qz9+egoZdb90X7pvetTVYbXh6GxstmoTGZ615qANGRIh8y7MA4ajjtX
9+7D1v/LeWmgOPfcI97/zFjz+EpQrzAcMKFy/gNjt51cdD+spaRjaZgZ6ybSsfqeYW5ybHxz5
Jg28f1le9leYwggl5Az4xtGKFuIdxv2yE5gy37svTszZewdYU9UHaFkbB50YcXsj6SiGzOYnp
2Nslrt2hap6DZdZzCjWsEiKQzKLn04BDCWlqPkaaZCW0UyZXyrErq3m1WzQSFHKEuVzI6y2qw
Cfs8eM1j27oxNjGGePGX/d0u/qAO21lit/85BqwTOJi4tNf+BvQ2sWLTl/8LABDtS2ikxLWWg
wGw5PX+YFPtYO9A5mnreQuDgNPpenG5vD6vc1D50ZXvgZhihpeU5sN3YiAsOi2h9UeMKDd4vh
+jE/beAOTMXkYzYoGQayAOcWDqI7Ix9Sm8jbT2ex7LE/RTOBGmzqNlulZniwdU3ZJpgTPyq15
drfGl2mRbZ8OlJZmKXtpH5QAd2RHbsb2wXurYQzX7vM7zupLZ3hsk3/0PRwE3Hpi/J1vvf30X
72FzebGX/9xiYoSnvumz375jvu/+Mr/+9DepRtPtA2ruQq1YRixGFFX48U4cvfPfNAuWT9z0d
aGL9G3HEnIvvR9/wZzKxpWHC1q8UKlRWqRrJUrekbeRrOuyQlOTmjjKKZTTCsioM4+CJib4/w
BLV5rw2vdk21/DVunPe4TBuHELaqGCHOq9iBUlsfuE2w/xHSWSvKG2T2lbHsZRrRGtRTndtdQ
C+Ue+q59d6iVt8g+mjzhgK23aJM/bg8WKr7o6lSbAPzaH378q798nQ1Hgh1+1o8XOAniwz//z
Ad+7/mH9i2BON6ENiM7MuCOjRbx+KA57eM7dcPTm+lVBz64ePmdowvyPAaXLtQX1Ng6jtSQAY
NF4rDFy1FdrngYGCJtYXzKJyc03vSJaXuKaZvzotzlFUA6lVGSQ6KG5sAK1aJVDZYuUZPgMlW
GRXCZaajJtk9S3mow9mZlKl0Spi2bdeWpOxj37X4KXpeJiRCykLPnBk1GO0W+5mA4Pl58eBtP
2W/zUc+8JJQU/N0v//7v/nDj4wkG9fgx346+vWTHkaV2u4kDpLkY5FyaQxWw/XXbOI1jt7UiR
RxJvrivOvf4xzzw0PrVez+EybamIywa6wgcFDKx6ZN1jk9hsorpOtMESWwcmkurq6GeqgYB5g
rZkB15G+02qkM+3GN5nblhtZT3HwjTsdpG2EaKRKPUYtyglVofr7bV8kHfXEWEApFNGuW8aLu
kM3uVHblFbqxxaxs2GZNT4eT1B7AxCVvOaDi20UGlPTV+68nr4bq/Gx/zjF/5oW8H8OJPs2/b
x02b+MO7QiYnHjdbNhn7aixGDeY0mG/TgLefqD59B770dbv17nr7ZhxqloZXvXR177+Tln3Jg
AX3v0Ochu5hPsrxaU020ZC5RktkOi7YXqmRRQ9KRDbmwDYzbSOdo4bkIa/3IyTUFzPQL//u6c
I1aNfYnMb4DKYbmGRtsTlTW32IOUkkA1KQ17LLkvbE0qyrniHLaB1q1GSlZGmCaUKbmJPyFfO
qKhnpGb/++fz+/6sq4f051xxIb3/ebgf/K5/DUoX56HM1B0Z3eWAWGsdtq5gzXHGAaONFlwFD
HVjBdutnJ37uQe4dhivWePnhZ3Dh6XaEGCQzA85gusFthyIxDxB5qnSWKXhrYzu06JvysmkqM
JsyMd2inUQ9ZTwEQHyIzVHYImIcXHRl3vP48MCfKHve2vJtSxv16ZPjSy9vPUO0NDUPkcND2R
dSe8h2d7S4J5cnNRlNy3aCaat2Cm9dGWxc86HLZ+NwuLIxxqOOXGD3jAwozbUqDw0tlAcAoAI
C6zkuLfHQQV58MFxysFreG32E+zZx1ymc3pBuFR6IeeJp/Tdx7PvbEzVWgemipsEbR0ZqJY9h
uIAUkINaU4ucQBfaVpN1TM6CFapDrB+Heg8DkBpwEEaLvPInlNVubLfbtnHGLjyylBhSDqm11
DI1nrmw2lz+udWXx5nDcOTMlDhOmjRospDkqYQ+Coy3nbXL9sBdrZQd//C/j5576fRlTx10XB
gsJf+hj9rekSpjK7TOOYNY6tdqEgYRwWABqoDoqhEGjMZ2ojoSA4wbKOPoGMfuw/KZfNkZXHD
ON87Vx04/ZzJ9zsVHcj339cHkLcvDLW9bOrKTqNq5AyFtwo1u8KApRYcmnNzjpNWXqTqMuA/5
nPk5+QmkCXzKK56dHnj/dDxYX9fSIReiE7JWHiaLjz1z8E2/+PblX/4BxDI5R87M2ZusbdcqO
HaGjDqpkixbdRWOfHRT33SI961ofRomGRL+6N7B2+/KV14Q/s4V9pd3Qa0t1xCYhJLNFuvNgG
cydg9cG5UNo+I0Q4GphQLbJA9QZJaSANr2up3ZRn08n1jDtLHlJZyRbWw+3uLvXPNYWP76peF
tA93NAC4s+comFZACqsAMNqKA8VlWBKbEQcSLEBbBi5FXgXuRz1pz3I5cv/3QbZhrpIFydmIr
HT7+Te963YfnntGE734S/uDPy8P8mDKUMG059ny2Sto7uPZk/h/GRXIJGFWovvCAXbqAu9bwl
bN2ssFWwiQhAXUMD5/zP92wQAwjZpxkKclJcCAATdEjR2WsIyFkc3logByQXF4x564InhokKB
qnrui2JayP8dA5jRsqcnkRx7cQwxUWXvPYS7A0WL98490XLn3et7ZAWQpCYDC1ySaN5lepBn4
c8SHaHufQ0jSnDcvb2Nqw1LYK+w6NWmWh/ov5d965cXHzqZXveVr4848DgBKio81SRutos2+b
n5uLTwpcuCA8NaUvudVuoyPNobtbW5nirnP+0CZXtn27DSUXi8AgmAOVgUQMCt0zSuBZMn7rB
bh5Be5oXLnr+QKA1DK5JgmtM4MJaLMcyGJLNSIChkPmJhtCdiSHDGY4u6qVbVqtwchXWyzWS1
8ZvOSKC154tb3zoupzlSeIzNEEpZY+FRK0Jd+0cFIaSM68AW/a9Sa1ibGKIdxlf//Qd/3sH/w
2XvE8vO8vFu87WhyoIEZJLm9tknyNfpY4E+NeADUvvLT6u9s4c6ZtPn+U953zhybh1DZXxthM
lr0rjZYn6JfdWJEYBFYRcxGLFRYqXjxITz0YH9gQoGClEROatKqq1tlAOVjO2JYn0Y1tQgO0R
o9AxNZWPr3iGw03GmS3NBVaaWQxQIGThqfEkwJDvOt0/sqel9aDl15+CNfEDzxu6S8MHlJwb0
ALMM9TVwXu4f6fRjrlJ96Wpjm1tD1X8wdf/xuvzv/gAK7cs/6HH1qEVUrlES60AG74dqOUuSE
/pXT6YP3s3D/6B1ox7j2bVz9z39Lt59RkkvLEcfKzTbx3A417FWwUSNOA3DPAYvT9Q87XWK45
iv7U5cl1F81J+vzDzQ0XVlW0u1dxQcCicPSoTp7kxjbGCamVA9OMtkGTKcGFlbM48QDGQHYoo
mtbDqgHCjXMiOAMUDZUZT8rWs8hhIv3YjDCwSXcsP+vr7zw3TbflKIdOICCVZf69LRPT2/dEY
7ve8X7bn/C6ZNnDh4+8NXbQIdaKEuEMkOAO7jim42ajK15H86FfX1bNwC4dDJ9rbHlex/iHZt
HqkLrlF2KGVtZt5xJD25VJQwuVbhwHpfOqzbODTAftVDxW/ZPDy0NZrvN2u3Vu6Z7LhkhbuPo
gzp2QtNkkynajJyVyexoE1rXdMybb8d4ipy7qlCsEWrM7/HBCKEiyeRltwMSkGfPZOm7vSYtD
uzlQq1LDqbvveBLhy7/j2ADmLI14yP3r/3gn339ui9/rf2Op1Wf+RukaVdb9J2njsCTXIrZ4c
wOrfLkuXxvrfG+8EyzCGC7PZ1tjojXHqmezBODnMbt6Oub+z9xFOsb7WV7wpMPxfYkjo1RNg7
PVxjVGAUMIgYRpEIg4OXxcWCYbK8n7XFo3HBromnGJGFqcBdNKXHaZKtJmCqc3pZLwUym4QCZ
nFvE3JLFquApBWMpalqpKBHyjiYeRMyPkIGHNnn/Kdy6fMPgtm89PLd23WNxwVJ1yz2Tz319/
+OOYCFO/+oz0RKYSUMGkBEiJKS2PGCUMVtOUovgtuA+yGxO5Fv26ZoqDNfiKrEwRBWZh56Dp6
V09inh1JMvTmz0NVz5N6ujp16g7ZNcn9AChhG1sbSwrG/n9RweM0z7RwbPzA3gKaUUkR0bY6y
POU5shOwQ+KPfTXIjhHM+2f7yFy7+yxOLW86cUdWIEYyIBGtYBS8bXUK3qagQUwVtIYCQGd3k
gcGwdx4aVsmwsYUbzyzfcUZVtL0L82ub6eOfjaYFE7K756JBlOANGMveO7kjZrkIBzMqt5i1G
EJ7SicvwmXUQmSogejO7GrlmUzGNir71X7PY5aGn01XHt3CbQ2yl9YptMIDq/nQKFy6gJU8pB
zKgKNtB8gDU+u+thW2G05aTFr8wHdt7T84j3aMlOUDm+PTvuPM07a+/pFPPb4aDd3BIVvCKqQ
MB0KAs2OAVR6y5AJggd2Wyp0dH2ULlSogjDgaQm5rmzp6DPBoghwSPLO45L5pFZbhLgmxomUo
gZkRqomhWDvmwaWTeSViVMNCTpZlLZRgTUCq5MFZKQ5DCND0mr06OEJFDAPMcHxDB+bCUpW+9
eD6Nfsn8BZywREwmK4u15obcnuM7VaLg+kLfiTtPzBUs4k0FhLrJQz3rB8NDPri79z1M0++WY
Zxg2nCpMU0wwUJsdS2y5MopCyqtIwTNInwvr9EFAO7hzKJ2eCabW2VETmX56FY42wc04xJRuo
e3aOcEHNX5DGQThhqI0yqmAPchErMGTmLObgnSLJIhGymEG6Yz19e44WLOLuJkWFljOUhHrcf
T9s7pVWysC4tttvMAp2DamneAEwSvu16XnxwzIbwBCVIuQn/+QNY37uY8uLTLxg/7cn+ghcff
vOLYUFtoAlTYZpV1RTBCOv+V0GsDcmJzomQAcquSOsbb0SRzKVxxL10Rg7FRrLI3AJZAJvUWV
MOGEWYOAheWnQjIXkVSCIEIdKjI0LmkGgKOXt0ZlakEJgsZhs4ApxNm69dDrc0WmsYiR99fBs
CGWoBwSwjjGVza0dLuWjh5k9uX/8dz3riKXNh0sgz2u22bX//w5eP9+4NI+UpKHzqoaue9k33
od0aNDauF5g1MUym3NrWYIAy81y49gjl2f92yGkkEWnsHvRXDK7suGFOFOguMyZ1PU2Fbs0J4
9KMIU4aDJew3/Cea/8kQmWvWCACRRMDUJcmSwec2YkEIKZSkiAVLaOWDTKCI1y3376+iYvnfb
UN//i6xiJptrOlDhgvHpzOH6juuC2ePD5Y21yc/IUozC+AuOfhb/nj+6p6WGkZ2VV2i5cWJLQ
ZGP+9fXf+yeazcoICxwlbDedL0ksOgGzIgluZl8xMdMlaeMgmQ3CmoOD00hbW9WpSqRS66HIL
yA6HkCQ3EdNW37fvzM/e8PH2wvkoGOlBwbunyHgUiUSHnPDATJWHPpXn2xndqsyKEVCg23yFJ
8/huv02rCSLtO6RZOo3wABwC+naJ+LaJ/6/PZyxCQJREETf7D8wUwM5MLEMc3swE+zOSLAAI0
2tQcECxMTg9LjdNfhoEzNvgjcv/iYR6exOj8u9XbTMJtkUmeUQkpDhQ1/Eajk6HEWDOx+j63h
3Kg322yxGBlI9TXEyJMNMXrNWyJVGOJZ4KDwr7NRSB4skgyHDVZT0zn57nT9vm/N6Oi5fZgBU
45Tpxc0AAAAASUVORK5CYII=
END:VCARD
^ permalink raw reply
* [PATCH] commit: fall back to full read when maybe_tree is NULL
From: Jeff King @ 2026-05-19 5:05 UTC (permalink / raw)
To: git; +Cc: Rasmus Villemoes, Daniel Mach
When we load a commit object from the commit graph (rather than reading
the object contents), we don't fill in its "maybe_tree" entry, but
rather wait to lazy-load it. This goes back to 7b8a21dba1 (commit-graph:
lazy-load trees for commits, 2018-04-06), and saves the work of
instantiating tree objects that nobody cares about.
But it creates a data dependency: now the commit struct depends on the
graph file to do that lazy load. This is a problem if we close the graph
file; now we have a commit struct that claims to be parsed but is
missing some of its data.
It's rare for this to be a problem in practice, because we don't tend to
close the graph files at all, and if we do we don't tend to look at
their commits afterward. But there is one case that is easy to trigger:
git-clone's --dissociate option will close the object database before
running the dissociate repack, and then afterwards still try to check
out the working tree. This will yield an error like:
fatal: unable to parse commit b29edc0babef41810f7b1c9ee1d74058f22e4080
warning: Clone succeeded, but checkout failed.
What happens is that we expect repo_get_commit_tree() to lazy-load the
tree, but commit_graph_position() returns COMMIT_NOT_FROM_GRAPH because
the position slab has gone away (and even if it hadn't, we don't have
the graph file itself available anymore).
Let's try harder to find the tree in repo_get_commit_tree() by actually
opening the commit object and parsing the tree line. This is extra work,
but no more than we'd have to go to if we hadn't done the initial graph
load in the first place.
It does mean that a corrupt commit (e.g., one that points to a non-tree
object for which we couldn't instantiate a struct) will repeatedly load
the object from disk, once for each call to repo_get_commit_tree(). But
such corruptions should be rare, and we don't tend to perform such calls
repeatedly (usually we'd abort the operation upon seeing corruption).
It also means we have to reimplement a bit of the commit parsing. We
can't just use parse_commit_buffer() here, because it expects an
unparsed struct and wants to load everything, including parent links.
But we don't know if the parent list has been munged during traversal,
so it's not safe for us to touch it. Fortunately, it's quite easy to
load just the tree, as it is always the first line of the commit object.
There is an alternative approach which I considered but rejected:
"complete" each graph-loaded commit struct when we close the graph file
by looking up and instantiating their trees at close time. This is the
most elegant solution in some sense, as it resolves the data dependency
at the moment it goes away. And it avoids ever opening the commit
objects at all, which can be more efficient.
But not always. The resolving effort scales with the number of
graph-loaded commits, even though we may only later access one or a few.
So the tradeoff depends on how many were loaded in total versus how many
will be later accessed.
And in most cases, we will not access any at all! Programs which close
the object database before exiting will then do a bunch of work for no
reason. This could be mitigated by requiring a separate function to
resolve the graph structs before closing the file. But now each close
call has to consider whether to call that resolving function. So we'd
fix this case in git-clone, but we don't know what other cases (if any)
are lurking.
Moreover, this strategy does nothing if we lose access to the graph file
unexpectedly (e.g., due to a system error). I'm not entirely sure this
is possible now (we mmap it, so I'd guess any error would turn into
SIGBUS anyway). But it feels like making the lazy-load more robust
(which this patch does) is the best way to handle a wide variety of
possible failure modes.
Signed-off-by: Jeff King <peff@peff.net>
---
Reported twice recently:
- https://lore.kernel.org/git/87h5onsi0f.fsf@prevas.dk/
- https://lore.kernel.org/git/6ae85515-9373-4c9e-90d2-5e4176590c5b@suse.com/
I don't why we suddenly got two reports. AFAICT the bug goes back to
2018, though it would become more prominent as use of commit graphs
increased.
commit.c | 33 ++++++++++++++++++++++++++++++++-
t/t5604-clone-reference.sh | 23 +++++++++++++++++++++++
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index 4385ae4329..cfc87ad185 100644
--- a/commit.c
+++ b/commit.c
@@ -434,6 +434,27 @@ static inline void set_commit_tree(struct commit *c, struct tree *t)
c->maybe_tree = t;
}
+static void load_tree_from_commit_contents(struct repository *r, struct commit *commit)
+{
+ enum object_type type;
+ unsigned long size;
+ char *buf;
+ const char *p;
+ struct object_id tree_oid;
+
+ buf = odb_read_object(r->objects, &commit->object.oid, &type, &size);
+ if (!buf)
+ return;
+
+ if (type == OBJ_COMMIT &&
+ skip_prefix(buf, "tree ", &p) &&
+ !parse_oid_hex(p, &tree_oid, &p) &&
+ *p == '\n')
+ set_commit_tree(commit, lookup_tree(r, &tree_oid));
+
+ free(buf);
+}
+
struct tree *repo_get_commit_tree(struct repository *r,
const struct commit *commit)
{
@@ -443,7 +464,17 @@ struct tree *repo_get_commit_tree(struct repository *r,
if (commit_graph_position(commit) != COMMIT_NOT_FROM_GRAPH)
return get_commit_tree_in_graph(r, commit);
- return NULL;
+ /*
+ * This is either a corrupt commit, or one which we partially loaded
+ * from a graph file but then subsequently threw away the graph data.
+ *
+ * Optimistically assume it's the latter and try to reload from
+ * scratch. This gives a performance penalty if it really is a corrupt
+ * commit, but presumably that happens rarely (and only once per
+ * process).
+ */
+ load_tree_from_commit_contents(r, (struct commit *)commit);
+ return commit->maybe_tree;
}
struct object_id *get_commit_tree_oid(const struct commit *commit)
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index 470bfb610c..c232ab8c15 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -360,4 +360,27 @@ test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '
grep "is a symlink, refusing to clone with --local" err
'
+test_expect_success 'dissociate from repo with commit graph' '
+ git init orig &&
+ # We are trying to make sure the dissociated repo can
+ # find the tree of the tip commit, so the test could still
+ # serve its purpose with an empty tree. But having actual
+ # content future-proofs us against any kind of internal
+ # empty-tree optimizations.
+ echo content >orig/file &&
+ git -C orig add . &&
+ git -C orig commit -m foo &&
+
+ # We will use graph.git as our "local" source to dissociate
+ # from.
+ git clone --bare orig graph.git &&
+ git -C graph.git commit-graph write --reachable &&
+
+ # And then finally clone orig, using graph.git to get our objects. This
+ # must be non-bare so that we perform the checkout step, which will
+ # need to access the tree of HEAD, which we will have originally loaded
+ # via the commit graph.
+ git clone --no-local --reference graph.git --dissociate orig clone
+'
+
test_done
--
2.54.0.524.g198262df96
^ permalink raw reply related
* Re: [PATCH 0/3] small quote.[ch] cleanup
From: Junio C Hamano @ 2026-05-19 3:19 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I noticed some unused code while looking at an unrelated topic. So
> here's a small cleanup.
>
> [1/3]: quote.h: bump strvec forward declaration to the top
> [2/3]: quote: drop sq_dequote_to_argv()
> [3/3]: quote: simplify internals of dequoting
>
> quote.c | 21 ++-------------------
> quote.h | 14 ++++----------
> 2 files changed, 6 insertions(+), 29 deletions(-)
>
> -Peff
These were very straight-forward and pleasant to read. Queued.
Thanks.
^ permalink raw reply
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Junio C Hamano @ 2026-05-19 3:11 UTC (permalink / raw)
To: Chris Torek; +Cc: Michael Montalbo, Johannes Sixt, vincent, git
In-Reply-To: <CAPx1Gvd_FqnsjCkpAA5uy7aDz9oQnWx7WTvKk-kLWemkqF9PsQ@mail.gmail.com>
Chris Torek <chris.torek@gmail.com> writes:
> Call it an "implementation note" (or, if you like, a "practical
> consideration"?).
> Something along these lines might work...
>
> Implementation Note
>
> The --word-diff option currently operates by taking the same
> line by line diff that you get without the option, then massaging
> the result into a word-by-word difference. This may cause an
> unnecessarily-larger diff than you would see with a more-clever
> implementation. If and when Git acquires a more-clever
> implementation, the output may change. Note that this is
> similar to the --diff-algorithm option, which may change the
> output.
>
> Regardless of which algorithm is used, _any_ diff simply shows
> _a_ way to achieve some particular change. It's impossible for
> any algorithm to tell whether someone deleted two lines and
> then put one back exactly as it appeared earlier, saving the
> resulting text, vs deleting a single line, for instance. Only a
> keystroke-by-keystroke logger would be able to tell what the
> human operator actually typed into some editor. Git does
> not have that information, and having it is not desired.
>
> Chris
I understand your frustration in the second paragraph ;-) but let's
not go there. The first paragraph is excellent. It gives readers a
clear enough explanation to understand what is happening and stop
complaining where there is nothing to complain about (which is
already hinted by the "Note that" at the end).
^ permalink raw reply
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Chris Torek @ 2026-05-19 2:31 UTC (permalink / raw)
To: Michael Montalbo; +Cc: Johannes Sixt, vincent, git, Junio C Hamano
In-Reply-To: <CAC2Qwm+BLNf-2kvePKNF-FKQX3raOBzSRmwd0ZEdzmo8TqkMGA@mail.gmail.com>
On Mon, May 18, 2026 at 7:11 PM Michael Montalbo <mmontalbo@gmail.com> wrote:
> Yeah, I was trying to explain the difference Vincent saw compared to wdiff,
> but I agree with your criticism. In "beating around the bush" regarding
> implementation details / making a direct comparison to wdiff, it has been
> hard to craft a meaningful message.
My opinion is: don't do that, just get right to it.
> > If we document the algorithm in such detail, we cast it in stone. I
> > wouldn't want to paint ourselves into that corner.
> I also agree with this sentiment. I haven't been able to come up with a
> message that threads the needle appropriately, so I'm open to dropping
> the patch or reworking it if others have suggestions.
Call it an "implementation note" (or, if you like, a "practical
consideration"?).
Something along these lines might work...
Implementation Note
The --word-diff option currently operates by taking the same
line by line diff that you get without the option, then massaging
the result into a word-by-word difference. This may cause an
unnecessarily-larger diff than you would see with a more-clever
implementation. If and when Git acquires a more-clever
implementation, the output may change. Note that this is
similar to the --diff-algorithm option, which may change the
output.
Regardless of which algorithm is used, _any_ diff simply shows
_a_ way to achieve some particular change. It's impossible for
any algorithm to tell whether someone deleted two lines and
then put one back exactly as it appeared earlier, saving the
resulting text, vs deleting a single line, for instance. Only a
keystroke-by-keystroke logger would be able to tell what the
human operator actually typed into some editor. Git does
not have that information, and having it is not desired.
Chris
^ permalink raw reply
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Michael Montalbo @ 2026-05-19 2:07 UTC (permalink / raw)
To: Johannes Sixt; +Cc: vincent, git, Junio C Hamano
In-Reply-To: <89224cb5-27b1-45b6-93d8-a0ad5e2447a2@kdbg.org>
On Mon, May 18, 2026 at 12:30 AM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 18.05.26 um 05:30 schrieb Michael Montalbo:
> > On Thu, May 14, 2026 at 12:37 AM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Michael Montalbo <mmontalbo@gmail.com> writes:
> >>
> >>> @@ -457,6 +457,11 @@ endif::git-diff[]
> >>> +
> >>> Note that despite the name of the first mode, color is used to
> >>> highlight the changed parts in all modes if enabled.
> >>> ++
> >>> +Word diff works by finding word-level changes within each hunk of
> >>> +the line-level diff. The line-level alignment determines which
> >>> +changed lines are compared to each other, which can affect the
> >>> +word-level output.
> >>
> >> The added text may not say anything wrong, but I am not sure how it
> >> helps the end user to know the way machinery works internally.
> >>
> >
> > I see what you mean. Maybe the doc should focus more on calling out
> > the user-facing implication:
> >
> > `--word-diff` finds word-level changes within each hunk of the
> > line-level diff, so changes that only affect whitespace may still
> > appear in the output.
> I don't know what this paragraph is trying to explain. I don't see how
> this would explain Vincent's observed word-diff.
>
Yeah, I was trying to explain the difference Vincent saw compared to wdiff,
but I agree with your criticism. In "beating around the bush" regarding
implementation details / making a direct comparison to wdiff, it has been
hard to craft a meaningful message.
> The thing is, "word-diff" is such a descriptive name for the operation
> that it is difficult to find a description that is even better. The
> manual page doesn't even give it a try. It defers to --word-diff-regex
> right away, which then only talks about low-level details and doesn't
> attempt to give a higher-level description what a word-diff is.
>
> I don't think you can summarize the algorithm in a single sentence. But
> then I have to ask: why write it down anyway? How does it help the
> reader? Only so that they are able to derive an explanation for a
> particular observed output? Would it have saved Vincent to write a bug
> report?
>
> If we document the algorithm in such detail, we cast it in stone. I
> wouldn't want to paint ourselves into that corner.
>
I also agree with this sentiment. I haven't been able to come up with a
message that threads the needle appropriately, so I'm open to dropping
the patch or reworking it if others have suggestions.
> -- Hannes
>
^ permalink raw reply
* Re: [PATCH 02/18] setup: stop using `the_repository` in `is_inside_worktree()`
From: Junio C Hamano @ 2026-05-19 1:22 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Tian Yuchen, git, Karthik Nayak, Elijah Newren
In-Reply-To: <agrD4p1AIPtwa5gW@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
>> I do not offhand know if other code paths that are called from this
>> function are thread-safe, but yeah, this use of file-scope static is
>> not a safe thing to do.
>
> In the current status quo this is a safe conversion to do, as we still
> have the assumption ingrained that this only works for a single repo.
> We were using static variables before, and we're still using static
> variables now.
>
> That being said, I wouldn't mind dropping the static variable if this is
> something we'd rather want to get rid of. It's a smell that I'm not
> particularly happy about myself.
>
> I'll send a revised version in a bit, thanks!
OK. In the v2 (perhaps v3?) series, I can see the removal of two
static strbuf based "optimizations" is the only change since the
previous round (after merging the previous one to the base of the
new iteration and comparing the result with the new iteration).
Looking very good. Shall we mark the topic for 'next' now?
Thanks.
diff --git w/setup.c c/setup.c
index 1847111616..6aee839d8c 100644
--- w/setup.c
+++ c/setup.c
@@ -471,17 +471,26 @@ int is_nonbare_repository_dir(struct strbuf *path)
int is_inside_git_dir(struct repository *repo)
{
- static struct strbuf buf = STRBUF_INIT;
- return is_inside_dir(strbuf_realpath(&buf, repo_get_git_dir(repo), 1));
+ struct strbuf buf = STRBUF_INIT;
+ int ret = is_inside_dir(strbuf_realpath(&buf, repo_get_git_dir(repo), 1));
+ strbuf_release(&buf);
+ return ret;
}
int is_inside_work_tree(struct repository *repo)
{
- static struct strbuf buf = STRBUF_INIT;
- const char *worktree = repo_get_work_tree(repo);
+ struct strbuf buf = STRBUF_INIT;
+ const char *worktree;
+ int ret;
+
+ worktree = repo_get_work_tree(repo);
if (!worktree)
return 0;
- return is_inside_dir(strbuf_realpath(&buf, worktree, 1));
+
+ ret = is_inside_dir(strbuf_realpath(&buf, worktree, 1));
+
+ strbuf_release(&buf);
+ return ret;
}
void setup_work_tree(struct repository *repo)
^ permalink raw reply related
* [PATCH 3/3] quote: simplify internals of dequoting
From: Jeff King @ 2026-05-19 1:20 UTC (permalink / raw)
To: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
Our sq_dequote_to_argv_internal() helper was wrapped by the to_argv()
and to_strvec() forms. Now that we have only the latter, we can stop
wrapping it and drop the argv-only bits.
Note that in theory sq_dequote_to_strvec() could take a const input
string, which would be friendlier to its callers. We couldn't do that
with the to_argv() form because it reused the input string to hold the
output elements. But since we're built on sq_dequote_step(), which
munges the input, we'd have to rework the parser. Since no callers care
about it currently, we'll leave that for another day.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/quote.c b/quote.c
index cff78af3a4..235fac8e47 100644
--- a/quote.c
+++ b/quote.c
@@ -171,9 +171,7 @@ char *sq_dequote(char *arg)
return sq_dequote_step(arg, NULL);
}
-static int sq_dequote_to_argv_internal(char *arg,
- const char ***argv, int *nr, int *alloc,
- struct strvec *array)
+int sq_dequote_to_strvec(char *arg, struct strvec *array)
{
char *next = arg;
@@ -191,22 +189,12 @@ static int sq_dequote_to_argv_internal(char *arg,
c = *++next;
} while (isspace(c));
}
- if (argv) {
- ALLOC_GROW(*argv, *nr + 1, *alloc);
- (*argv)[(*nr)++] = dequoted;
- }
- if (array)
- strvec_push(array, dequoted);
+ strvec_push(array, dequoted);
} while (next);
return 0;
}
-int sq_dequote_to_strvec(char *arg, struct strvec *array)
-{
- return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array);
-}
-
/* 1 means: quote as octal
* 0 means: quote as octal if (quote_path_fully)
* -1 means: never quote
--
2.54.0.524.g198262df96
^ permalink raw reply related
* [PATCH 2/3] quote: drop sq_dequote_to_argv()
From: Jeff King @ 2026-05-19 1:19 UTC (permalink / raw)
To: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
The last caller went away in f9dbb64fad (config: parse more robust
format in GIT_CONFIG_PARAMETERS, 2021-01-12), when we switched to using
sq_dequote_step().
The "to_argv()" form is not a great interface. If you care about raw
speed, then sq_dequote_step() lets you work incrementally without extra
allocations. If you care about simplicity, then sq_dequote_to_strvec()
puts the result in an encapsulated data structure. With sq_dequote_to_argv(),
you have a data dependency on the original string but still have to
remember to manually free the argv array itself (but not its elements).
So it's sort of a worst-of-both-worlds middle ground. Let's get rid of
it.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.c | 5 -----
quote.h | 12 +++---------
2 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/quote.c b/quote.c
index b9f6bdc775..cff78af3a4 100644
--- a/quote.c
+++ b/quote.c
@@ -202,11 +202,6 @@ static int sq_dequote_to_argv_internal(char *arg,
return 0;
}
-int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)
-{
- return sq_dequote_to_argv_internal(arg, argv, nr, alloc, NULL);
-}
-
int sq_dequote_to_strvec(char *arg, struct strvec *array)
{
return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array);
diff --git a/quote.h b/quote.h
index 400397b11a..989f2388c0 100644
--- a/quote.h
+++ b/quote.h
@@ -68,15 +68,9 @@ char *sq_dequote_step(char *src, char **next);
/*
* Same as the above, but can be used to unwrap many arguments in the
- * same string separated by space. Like sq_quote, it works in place,
- * modifying arg and appending pointers into it to argv.
- */
-int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);
-
-/*
- * Same as above, but store the unquoted strings in a strvec. We will
- * still modify arg in place, but unlike sq_dequote_to_argv, the strvec
- * will duplicate and take ownership of the strings.
+ * same string separated by space. The strvec will duplicate and take
+ * ownership of the strings, but note that "arg" is still modified in-place
+ * during parsing.
*/
int sq_dequote_to_strvec(char *arg, struct strvec *);
--
2.54.0.524.g198262df96
^ permalink raw reply related
* [PATCH 1/3] quote.h: bump strvec forward declaration to the top
From: Jeff King @ 2026-05-19 1:19 UTC (permalink / raw)
To: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
We usually put forward declarations at the top of header files, rather
than next to the functions that need them. In theory placing it next to
the function has some explanatory value, but it's also just as likely to
become stale if other uses are added.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/quote.h b/quote.h
index 0300c29104..400397b11a 100644
--- a/quote.h
+++ b/quote.h
@@ -2,6 +2,7 @@
#define QUOTE_H
struct strbuf;
+struct strvec;
extern int quote_path_fully;
@@ -77,7 +78,6 @@ int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);
* still modify arg in place, but unlike sq_dequote_to_argv, the strvec
* will duplicate and take ownership of the strings.
*/
-struct strvec;
int sq_dequote_to_strvec(char *arg, struct strvec *);
int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);
--
2.54.0.524.g198262df96
^ permalink raw reply related
* [PATCH 0/3] small quote.[ch] cleanup
From: Jeff King @ 2026-05-19 1:18 UTC (permalink / raw)
To: git
I noticed some unused code while looking at an unrelated topic. So
here's a small cleanup.
[1/3]: quote.h: bump strvec forward declaration to the top
[2/3]: quote: drop sq_dequote_to_argv()
[3/3]: quote: simplify internals of dequoting
quote.c | 21 ++-------------------
quote.h | 14 ++++----------
2 files changed, 6 insertions(+), 29 deletions(-)
-Peff
^ permalink raw reply
* [PATCH v8] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-19 0:55 UTC (permalink / raw)
To: git
Cc: Mirko Faina, Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Johannes Sixt,
Chris Torek
In-Reply-To: <463cc8e2764edb7de3d379f615f5cfbd0919bfa3.1778887662.git.mroik@delayed.space>
--max-count is a commit limiting option sets a maximum amount of commits
to be shown. If a user wants to see only the first N commits of the
history (the oldest commits) they'd have to do something like
git log $(git rev-list HEAD | tail -n N | head -n 1)
This is not very user-friendly.
Teach get_revision() the --max-count-oldest option.
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Documentation/rev-list-options.adoc | 5 +-
revision.c | 111 +++++++++++++++++++++++++++-
revision.h | 2 +
t/t4202-log.sh | 41 ++++++++++
4 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..e8c88d0f1c 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -16,7 +16,10 @@ ordering and formatting options, such as `--reverse`.
`-<number>`::
`-n <number>`::
`--max-count=<number>`::
- Limit the output to _<number>_ commits.
+ Limit the output to the first _<number>_ commits that would be shown.
+
+`--max-count-oldest=<number>`::
+ Limit the output to the last _<number>_ commits that would be shown.
`--skip=<number>`::
Skip _<number>_ commits before starting to show the commit output.
diff --git a/revision.c b/revision.c
index 599b3a66c3..5d53db3152 100644
--- a/revision.c
+++ b/revision.c
@@ -2339,10 +2339,28 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
}
if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
+ if (revs->max_count_type == 1)
+ die_for_incompatible_opt2(1, "--max-count", 1,
+ "--max-count-oldest");
revs->max_count = parse_count(optarg);
revs->no_walk = 0;
+ revs->max_count_type = 0;
return argcount;
+ } else if ((argcount = parse_long_opt("max-count-oldest", argv, &optarg))) {
+ if (revs->max_count_type == 0 && revs->max_count != -1)
+ die_for_incompatible_opt2(1, "--max-count", 1,
+ "--max-count-oldest");
+ if (revs->skip_count > 0)
+ die_for_incompatible_opt2(1, "--skip", 1,
+ "--max-count-oldest");
+ revs->max_count = parse_count(optarg);
+ revs->no_walk = 0;
+ revs->max_count_type = 1;
+ revs->max_count_stage = 0;
} else if ((argcount = parse_long_opt("skip", argv, &optarg))) {
+ if (revs->max_count_type == 1)
+ die_for_incompatible_opt2(1, "--skip", 1,
+ "--max-count-oldest");
revs->skip_count = parse_count(optarg);
return argcount;
} else if ((*arg == '-') && isdigit(arg[1])) {
@@ -4521,15 +4539,91 @@ static struct commit *get_revision_internal(struct rev_info *revs)
return c;
}
+static void retrieve_oldest_commits(struct rev_info *revs,
+ struct commit_list **queue)
+{
+ struct commit *c;
+ int max_count = revs->max_count;
+ int queuei_count = 0;
+ int queueo_count = 0;
+ struct commit_list *queueo = NULL;
+ struct commit_list *queuei = NULL;
+ struct commit_list *reversed_queue = NULL;
+ struct commit_list *p;
+
+ revs->max_count = -1;
+ while ((c = get_revision_internal(revs))) {
+ /*
+ * We need to reset SHOWN status otherwise --graph breaks.
+ * It is fine to do, get_revision_internal() doesn't consider
+ * children commits as they have been already processed and the
+ * traversal happens only child to parent.
+ *
+ * We do this because the --graph machinery relies on the status
+ * of the parents to decide how the printing will happen.
+ *
+ * We can't simply replace this instruction with a
+ * graph_update() as it doesn't do the actualy printing, we'd
+ * have to remove any commit that goes over the
+ * --max-count-oldest limit from revs->graph.
+ */
+ c->object.flags &= ~(SHOWN | CHILD_SHOWN);
+ commit_list_insert(c, &queuei);
+ if (!(c->object.flags & BOUNDARY))
+ queuei_count++;
+ while (queuei_count + queueo_count > max_count) {
+ if (!queueo_count) {
+ while ((c = pop_commit(&queuei))) {
+ commit_list_insert(c, &queueo);
+ queueo_count++;
+ }
+ queuei_count = 0;
+ }
+ c = pop_commit(&queueo);
+ queueo_count--;
+ /* We need to do this otherwise we'll discard the
+ * commits that go over the --max-count-oldest limit but
+ * not their respective boundaries. This matters only if
+ * we're discarding the commit right before the boundary.
+ */
+ for (p = c->parents; p; p = p->next)
+ p->item->object.flags &= ~CHILD_SHOWN;
+ }
+ }
+
+ while ((c = pop_commit(&queueo)))
+ commit_list_insert(c, &reversed_queue);
+ while ((c = pop_commit(&queuei)))
+ commit_list_insert(c, &queueo);
+ while ((c = pop_commit(&queueo)))
+ commit_list_insert(c, &reversed_queue);
+
+ while ((c = pop_commit(&reversed_queue)))
+ commit_list_insert(c, queue);
+}
+
struct commit *get_revision(struct rev_info *revs)
{
struct commit *c;
struct commit_list *reversed;
+ struct commit_list *queue = NULL;
+ struct commit_list *p;
+
+ if (revs->max_count_type == 1 && !revs->max_count_stage) {
+ retrieve_oldest_commits(revs, &queue);
+ commit_list_free(revs->commits);
+ revs->commits = queue;
+ revs->max_count_stage = 1;
+ }
if (revs->reverse) {
reversed = NULL;
- while ((c = get_revision_internal(revs)))
- commit_list_insert(c, &reversed);
+ if (revs->max_count_type == 1)
+ while ((c = pop_commit(&revs->commits)))
+ commit_list_insert(c, &reversed);
+ else
+ while ((c = get_revision_internal(revs)))
+ commit_list_insert(c, &reversed);
commit_list_free(revs->commits);
revs->commits = reversed;
revs->reverse = 0;
@@ -4543,7 +4637,18 @@ struct commit *get_revision(struct rev_info *revs)
return c;
}
- c = get_revision_internal(revs);
+ if (revs->max_count_stage) {
+ c = pop_commit(&revs->commits);
+ if (c) {
+ c->object.flags |= SHOWN;
+ if (!(c->object.flags & BOUNDARY))
+ for (p = c->parents; p; p = p->next)
+ p->item->object.flags |= CHILD_SHOWN;
+ }
+ } else {
+ c = get_revision_internal(revs);
+ }
+
if (c && revs->graph)
graph_update(revs->graph, c);
if (!c) {
diff --git a/revision.h b/revision.h
index 584f1338b5..e157463cb1 100644
--- a/revision.h
+++ b/revision.h
@@ -309,6 +309,8 @@ struct rev_info {
/* special limits */
int skip_count;
int max_count;
+ unsigned int max_count_type:1;
+ unsigned int max_count_stage:1;
timestamp_t max_age;
timestamp_t max_age_as_filter;
timestamp_t min_age;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 05cee9e41b..c3c1b862d3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1882,6 +1882,47 @@ test_expect_success 'log --graph with --name-status' '
test_cmp_graph --name-status tangle..reach
'
+test_expect_success 'log --max-count-oldest=3 --oneline' '
+ test_when_finished rm expect &&
+ git log --oneline | tail -n3 >expect &&
+ git log --oneline --max-count-oldest=3 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '
+ test_when_finished rm expect &&
+ git log --oneline --reverse | head -n3 >expect &&
+ git log --oneline --max-count-oldest=3 --reverse >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --max-count-oldest with --max-count' '
+ test_when_finished rm stderr &&
+ test_must_fail git log --max-count-oldest=3 --max-count=3 2>stderr &&
+ test_grep "cannot be used together" stderr
+'
+
+test_expect_success 'log --max-count-oldest with --skip' '
+ test_when_finished rm stderr &&
+ test_must_fail git log --max-count-oldest=3 --skip=1 2>stderr &&
+ test_grep "cannot be used together" stderr
+'
+
+test_expect_success 'log --max-count-oldest=1000 --graph --boundary' '
+ test_when_finished rm expect actual &&
+ git log --graph --boundary >expect &&
+ git log --max-count-oldest=1000 --graph --boundary >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --oneline --graph --boundary --max-count-oldest=1' '
+ test_when_finished rm actual &&
+ echo 2 >expect &&
+ git log --oneline --graph --boundary --max-count-oldest=1 HEAD~1..HEAD \
+ | wc -l >actual &&
+ test_cmp expect actual
+'
+
cat >expect <<-\EOF
* reach
|
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v8] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-19 1:04 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Johannes Sixt,
Chris Torek, Mirko Faina
In-Reply-To: <8210d60832b9a58aa4d71fc3790e44d8989564ce.1779152064.git.mroik@delayed.space>
On Tue, May 19, 2026 at 02:55:22AM +0200, Mirko Faina wrote:
> --max-count is a commit limiting option sets a maximum amount of commits
> to be shown. If a user wants to see only the first N commits of the
> history (the oldest commits) they'd have to do something like
>
> git log $(git rev-list HEAD | tail -n N | head -n 1)
>
> This is not very user-friendly.
>
> Teach get_revision() the --max-count-oldest option.
>
> Signed-off-by: Mirko Faina <mroik@delayed.space>
> ---
> Documentation/rev-list-options.adoc | 5 +-
> revision.c | 111 +++++++++++++++++++++++++++-
> revision.h | 2 +
> t/t4202-log.sh | 41 ++++++++++
> 4 files changed, 155 insertions(+), 4 deletions(-)
Sorry, forgot to write down what changed since v7. There was an issue
with the counting as --max-count-oldest counted boundary commits too.
That is simply solved by only adding on non boundaries.
That left another issue, there are now some "orphaned" boundaries when
printing the graph. In addition to that, because of how the graph
machinery works, the graph is now trying to include the parents of the
orphaned boundaries. To fix this we just flip the CHILD_SHOWN flag on
the parents of the commit we're discarding.
Hopefully this is the last version.
^ 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