* Re: [PATCH v2 2/3] builtin/log: prefetch necessary blobs for `git cherry`
From: Derrick Stolee @ 2026-04-27 13:16 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget, git; +Cc: Elijah Newren
In-Reply-To: <a705852723fbe88e94ad3de1daba548dbce32211.1776472347.git.gitgitgadget@gmail.com>
On 4/17/2026 8:32 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
(I'm sorry that I'm reviewing out of order. This reply includes my
feelings about patch 3 after reading both.)
> +/*
> + * Enumerate blob OIDs from a single commit's diff, inserting them into blobs.
> + * Skips files whose userdiff driver explicitly declares binary status
> + * (drv->binary > 0), since patch-ID uses oid_to_hex() for those and
> + * never reads blob content. Use userdiff_find_by_path() since
> + * diff_filespec_load_driver() is static in diff.c.
> + *
> + * Clean up with diff_queue_clear() (from diffcore.h).
> + */
> +static void collect_diff_blob_oids(struct commit *commit,
> + struct diff_options *opts,
> + struct oidset *blobs)
I think that this is generally a good idea, though I worry that
having this hidden in builtin/log.c may not be the right long-
term home.
I expect that we'll find more and more examples where we want to
prefetch blobs in different operations, those that exist now and
those that may be created in the future. It would be preferred if
they could automatically take advantage of the logic already in
diff_queued_diff_prefetch() within diffcore_std() in diff.c.
Ultimately, _this_ patch cares about a diff. Could we compute a
"diff prep" computation using the core diff library instead of
inventing a second queue of results for diffing?
Patch 3 cares about a "scan prep" which cares about loading all
blobs for a given tree with respect to a pathspec. This is very
similar to what a checkout would do, though it ultimately uses
a form of diff to find out what change should be applied to the
working directory. Perhaps 'git archive' is a better matching
example.
I don't mean to make your series more complicated. I value what
you're doing and can see how your current attention can be used
to make further improvements later. By implementing things in a
common location, then we can have later integrations add to the
confidence in the feature through tests covering each user-facing
use.
I'm not sure if it makes sense to attempt to create a universal
library method that would be used by builtin/log.c _and_ diff.c,
at least not right now. I'm most interested in having this logic
be more reusable in the future without needing to move code
across files.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks
From: Chris Torek @ 2026-04-27 13:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, git, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Mirko Faina
In-Reply-To: <xmqq1pg04mbt.fsf@gitster.g>
This topic has been rattling around in my head for a while, and I need
to get it out now. :-)
First, a few notes:
* I'm going to delete most of the context because I want to go back
to first principles here. Instead, I'll list what I think are the real
issues.
* I have always had a suspicion that `--max-count` / `-n` was "done
wrong" in the first place, it's just that it's generally invisibly
wrong.
* While all the revision-walking machinery normally shows commits
"newest first", there are several fundamental issues with defining
"newest" anyway. A lot of Git newcomers find this terribly confusing
-- usually a few weeks or months into use of Git, really.
It's important to note that the rev-walk machinery uses a priority
queue, and that this is necessary because commits are in a directed
graph, which cannot be presented linearly unless you're willing to:
(a) add additional information (graph drawing, parent list, whatever),
or (b) discard information. The `git log` and `git rev-list`
documentation skimp a bit on this.
There is of course nothing wrong with discarding information when it's
irrelevant. In fact, that's the whole point of abstraction, to toss
out irrelevancies so that one can concentrate only on the relevant.
And that's what all the limiting options for revision walking are for!
Sorting options affect the order in which items go into the priority
queue. Limiting options affect which items go in, and sometimes, how
many items come out. Display options affect what we see when the items
come out, and this includes the sorting options since they come out in
the order they're in there.
Thus, `--reverse` is a *display* option (part of sorting), while
`--max-count` is a *limiting* option. It's just that, well, there's a
special case when they're combined.
Junio noted:
> That makes two of us to suspect that this is more about --max-count than --reverse.
And that's really the case here. Because `--max-count` was "done
wrong" initially, we have a slight problem. Had it been done as a
"window of items in the priority queue", we would always have gotten
the limited-to-N items remaining in the queue after the selection
process, displayed according to the display process. But when the
display is going to be "in the order of items in the queue" and the
limiting count is N items *and* the display doesn't reverse the queue,
it suffices to display the first N items and then quit entirely. This
is of course a nice space-and-time optimization.
As it turns out, the only display option that causes this optimization
to be invalid is (or might be) `--reverse`.
Unfortunately, fixing the problem by simply defeating the "keep N
items in the queue and only stop early (and maybe display as we go as
well) if we're allowed" optimization -- the one that was applied
prematurely, as it were -- will change the existing behavior of `git
log -n 10 --reverse` in any repository with more than 10 commits in
it. Had the over-optimization not been done, and someone wanted to add
a "gather only N into the queue and then stop traversing, and then
display" option, we could perhaps use `--stop-walk-(after|at)=n` as a
new option.
As far as I can tell, the gripe that this exposes the priority queue
mechanism is valid, but at best trivial, because knowing about the
priority queue is crucial anyway. Beginners can skip it for a little
while, but as soon as they find out that commits have two separate
date stamps, and learn about `--date-order`, `--author-date-order`,
and `--topo-order`, they need to learn about the queue.
If it's deemed acceptable to change the historic behavior of
`--max-count` combined with `--reverse`, I'd suggest simply adding
`--stop-walk-after` (perhaps with a slightly different name) to take
over the historic behavior of `--max-count`, and make `--max-count`
not over-optimize. If not, I'd suggest a new option, with a note in
the documentation that `--max-count` has this odd behavior when
combined with `--reverse`. Perhaps the new option could be called
`--prio-queue-size=n`. The implementation can still optimize this
(using the same code as before) when `--reverse` isn't in effect,
since the effect is only visible with `--reverse`.
Chris
^ permalink raw reply
* [RFC PATCH v3 0/2] push: add support for pushing to remote groups
From: Usman Akinyemi @ 2026-04-27 14:05 UTC (permalink / raw)
To: usmanakinyemi202; +Cc: christian.couder, git, gitster, me, phillip.wood123, ps
In-Reply-To: <20260325190906.1153080-1-usmanakinyemi202@gmail.com>
This RFC series adds support for `git push` to accept a remote group
name (as configured via `remotes.<name>` in config) in addition to a
single remote name, mirroring the behaviour that `git fetch` has
supported for some time.
A user with multiple remotes configured as a group can now do:
git push all-remotes
instead of pushing to each remote individually, in the same way that:
git fetch all-remotes
already works.
The series is split into two patches:
- Patch 1 moves `get_remote_group`, `add_remote_or_group`, and the
`remote_group_data` struct out of builtin/fetch.c and into
remote.c/remote.h, making them part of the public remote API.
- Patch 2 extends builtin/push.c to use the newly public
`add_remote_or_group()` to resolve the repository argument as
either a single remote or a group, and pushes to each member of
the group in turn.
Changes in v4:
- Made the multiple push to use child process through `run_command`
thereby making failure to in pushing to one remote not affect the
others no matter the kind of failure it is.
- Update the test and the docs to reflect the above.
Range-diff v3 -> v4:
1: dd370a19e7 = 1: 20ed79546f remote: move remote group resolution to remote.c
2: 6a7957e61c ! 2: 964694e587 push: support pushing to a remote group
@@ Documentation/git-push.adoc: further recursion will occur. In this case, `only`
+behaviour is added or removed — the group is purely a shorthand for
+running the same push command against each member remote individually.
+
-+The behaviour upon failure depends on the kind of error encountered:
-+
-+If a member remote rejects the push, for example due to a
-+non-fast-forward update, force needed but not given, an existing tag,
-+or a server-side hook refusing a ref, Git reports the error and continues
-+pushing to the remaining remotes in the group. The overall exit code is
-+non-zero if any member push fails.
-+
-+If a member remote cannot be contacted at all, for example because the
-+repository does not exist, authentication fails, or the network is
-+unreachable, the push stops at that point and the remaining remotes
-+are not attempted.
++When pushing to a group of more than one remote, Git spawns a separate
++`git push` subprocess for each member remote in sequence. Each subprocess
++receives the same flags and refspecs as the original invocation. This
++means that per-remote push mappings configured via `remote.<name>.push`
++and mirror mode (`remote.<name>.mirror`) are evaluated independently for
++each remote, and a mirror remote in the group cannot affect the push
++behaviour of other non-mirror remotes in the same group.
++
++The `--atomic` option is not supported for group pushes, because atomicity
++can only be guaranteed within a single transport connection to a single
++remote. Git will refuse the invocation with an error if `--atomic` is
++combined with a group name.
++
++If any member remote fails whether due to a push rejection (e.g. a
++non-fast-forward update, a server-side hook refusing a ref) or a connection
++error (e.g. the repository does not exist, authentication fails, or the
++network is unreachable), Git reports the error and continues pushing to
++the remaining remotes in the group. The overall exit code is non-zero if
++any member push fails.
+
+This means the user is responsible for ensuring that the sequence of
+individual pushes makes sense. If `git push r1`` would fail for a given
@@ Documentation/git-push.adoc: further recursion will occur. In this case, `only`
## builtin/push.c ##
+@@
+ #include "config.h"
+ #include "environment.h"
+ #include "gettext.h"
++#include "hex.h"
+ #include "refspec.h"
+ #include "run-command.h"
+ #include "remote.h"
+@@ builtin/push.c: static int git_push_config(const char *k, const char *v,
+ return git_default_config(k, v, ctx, NULL);
+ }
+
++static int push_multiple(struct string_list *list,
++ const struct string_list *push_options,
++ int flags,
++ int tags,
++ const char **refspecs,
++ int refspec_nr)
++{
++ int i, result = 0;
++ struct strvec argv = STRVEC_INIT;
++
++ strvec_push(&argv, "push");
++
++ if (flags & TRANSPORT_PUSH_FORCE)
++ strvec_push(&argv, "--force");
++ if (flags & TRANSPORT_PUSH_DRY_RUN)
++ strvec_push(&argv, "--dry-run");
++ if (flags & TRANSPORT_PUSH_PORCELAIN)
++ strvec_push(&argv, "--porcelain");
++ if (flags & TRANSPORT_PUSH_PRUNE)
++ strvec_push(&argv, "--prune");
++ if (flags & TRANSPORT_PUSH_NO_HOOK)
++ strvec_push(&argv, "--no-verify");
++ if (flags & TRANSPORT_PUSH_FOLLOW_TAGS)
++ strvec_push(&argv, "--follow-tags");
++ if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
++ strvec_push(&argv, "--set-upstream");
++ if (flags & TRANSPORT_PUSH_FORCE_IF_INCLUDES)
++ strvec_push(&argv, "--force-if-includes");
++ if (flags & TRANSPORT_PUSH_ALL)
++ strvec_push(&argv, "--all");
++ if (flags & TRANSPORT_PUSH_MIRROR)
++ strvec_push(&argv, "--mirror");
++
++ if (flags & TRANSPORT_PUSH_CERT_ALWAYS)
++ strvec_push(&argv, "--signed=yes");
++ else if (flags & TRANSPORT_PUSH_CERT_IF_ASKED)
++ strvec_push(&argv, "--signed=if-asked");
++ if (!thin)
++ strvec_push(&argv, "--no-thin");
++
++ if (deleterefs)
++ strvec_push(&argv, "--delete");
++
++ if (receivepack)
++ strvec_pushf(&argv, "--receive-pack=%s", receivepack);
++ if (verbosity >= 2)
++ strvec_push(&argv, "-v");
++ if (verbosity >= 1)
++ strvec_push(&argv, "-v");
++ else if (verbosity < 0)
++ strvec_push(&argv, "-q");
++ if (progress > 0)
++ strvec_push(&argv, "--progress");
++ else if (progress == 0)
++ strvec_push(&argv, "--no-progress");
++
++ if (family == TRANSPORT_FAMILY_IPV4)
++ strvec_push(&argv, "--ipv4");
++ else if (family == TRANSPORT_FAMILY_IPV6)
++ strvec_push(&argv, "--ipv6");
++
++ if (recurse_submodules == RECURSE_SUBMODULES_CHECK)
++ strvec_push(&argv, "--recurse-submodules=check");
++ else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
++ strvec_push(&argv, "--recurse-submodules=on-demand");
++ else if (recurse_submodules == RECURSE_SUBMODULES_ONLY)
++ strvec_push(&argv, "--recurse-submodules=only");
++ else if (recurse_submodules == RECURSE_SUBMODULES_OFF)
++ strvec_push(&argv, "--recurse-submodules=no");
++
++
++ if (tags)
++ strvec_push(&argv, "--tags");
++
++ for (i = 0; i < push_options->nr; i++)
++ strvec_pushf(&argv, "--push-option=%s",
++ push_options->items[i].string);
++
++ for (i = 0; i < cas.nr; i++) {
++ if (cas.entry[i].use_tracking) {
++ strvec_pushf(&argv, "--force-with-lease=%s",
++ cas.entry[i].refname);
++ } else if (!is_null_oid(&cas.entry[i].expect)) {
++ strvec_pushf(&argv, "--force-with-lease=%s:%s",
++ cas.entry[i].refname,
++ oid_to_hex(&cas.entry[i].expect));
++ } else {
++ strvec_push(&argv, "--force-with-lease");
++ }
++ }
++
++ for (i = 0; i < list->nr; i++) {
++ const char *name = list->items[i].string;
++ struct child_process cmd = CHILD_PROCESS_INIT;
++ int j;
++
++ strvec_pushv(&cmd.args, argv.v);
++ strvec_push(&cmd.args, name);
++
++ for (j = 0; j < refspec_nr; j++)
++ strvec_push(&cmd.args, refspecs[j]);
++
++ if (verbosity >= 0)
++ printf(_("Pushing to %s\n"), name);
++
++ cmd.git_cmd = 1;
++ if (run_command(&cmd)) {
++ error(_("could not push to %s"), name);
++ result = 1;
++ }
++ }
++
++ strvec_clear(&argv);
++ return result;
++}
++
+ int cmd_push(int argc,
+ const char **argv,
+ const char *prefix,
@@ builtin/push.c: int cmd_push(int argc,
int flags = 0;
int tags = 0;
@@ builtin/push.c: int cmd_push(int argc,
die(_("push options must not have new line characters"));
- rc = do_push(flags, push_options, remote);
-+ /*
-+ * Push to each remote in remote_group. For a plain "git push <remote>"
-+ * or a default push, remote_group has exactly one entry and the loop
-+ * runs once — there is nothing structurally special about that case.
-+ * For a group, the loop runs once per member remote.
-+ *
-+ * Mirror detection and the --mirror/--all + refspec conflict checks
-+ * are done per remote inside the loop. A remote configured with
-+ * remote.NAME.mirror=true implies mirror mode for that remote only —
-+ * other non-mirror remotes in the same group are unaffected.
-+ *
-+ * rs is rebuilt from scratch for each remote so that per-remote push
-+ * mappings (remote.NAME.push config) are resolved against the correct
-+ * remote. iter_flags is derived from a clean snapshot of flags taken
-+ * before the loop so that a mirror remote cannot bleed
-+ * TRANSPORT_PUSH_FORCE into subsequent non-mirror remotes in the
-+ * same group.
-+ */
-+ base_flags = flags;
-+ for (size_t i = 0; i < remote_group.nr; i++) {
-+ int iter_flags = base_flags;
-+ struct remote *r = pushremote_get(remote_group.items[i].string);
-+ if (!r)
-+ die(_("no such remote or remote group: %s"),
-+ remote_group.items[i].string);
-+
-+ if (r->mirror)
-+ iter_flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
-+
-+ if (iter_flags & TRANSPORT_PUSH_ALL) {
-+ if (argc >= 2)
-+ die(_("--all can't be combined with refspecs"));
-+ }
-+ if (iter_flags & TRANSPORT_PUSH_MIRROR) {
-+ if (argc >= 2)
-+ die(_("--mirror can't be combined with refspecs"));
++ if (remote_group.nr == 1) {
++ /*
++ * Single remote (the common case): run do_push() directly
++ * in this process. The loop runs exactly once.
++ *
++ * Mirror detection and the --mirror/--all + refspec conflict
++ * checks are done here. rs is rebuilt so that per-remote push
++ * mappings (remote.NAME.push config) are resolved against the
++ * correct remote. inner_flags is a snapshot of flags so that a
++ * mirror remote cannot bleed TRANSPORT_PUSH_FORCE into any
++ * subsequent call.
++ */
++ base_flags = flags;
++ {
++ int inner_flags = base_flags;
++ struct remote *r = pushremote_get(remote_group.items[0].string);
++ if (!r)
++ die(_("no such remote or remote group: %s"),
++ remote_group.items[0].string);
++
++ if (r->mirror)
++ inner_flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
++
++ if (inner_flags & TRANSPORT_PUSH_ALL) {
++ if (argc >= 2)
++ die(_("--all can't be combined with refspecs"));
++ }
++ if (inner_flags & TRANSPORT_PUSH_MIRROR) {
++ if (argc >= 2)
++ die(_("--mirror can't be combined with refspecs"));
++ }
++
++ refspec_clear(&rs);
++ rs = (struct refspec) REFSPEC_INIT_PUSH;
++
++ if (tags)
++ refspec_append(&rs, "refs/tags/*");
++ if (argc > 0)
++ set_refspecs(argv + 1, argc - 1, r);
++
++ rc = do_push(inner_flags, push_options, r);
+ }
-+
-+ refspec_clear(&rs);
-+ rs = (struct refspec) REFSPEC_INIT_PUSH;
-+
-+ if (tags)
-+ refspec_append(&rs, "refs/tags/*");
-+ if (argc > 0)
-+ set_refspecs(argv + 1, argc - 1, r);
-+
-+ rc |= do_push(iter_flags, push_options, r);
++ } else {
++ /*
++ * Multiple remotes: spawn one "git push <remote> [<refspecs>]"
++ * subprocess per remote, sequentially.
++ *
++ * Options that only make sense for a single transport connection
++ * are rejected here.
++ */
++ if (flags & TRANSPORT_PUSH_ATOMIC)
++ die(_("--atomic can only be used when pushing to one remote"));
++
++ rc = push_multiple(&remote_group, push_options, flags,
++ tags,
++ argc > 1 ? argv + 1 : NULL,
++ argc > 1 ? argc - 1 : 0);
+ }
+
string_list_clear(&push_options_cmdline, 0);
@@ t/t5566-push-group.sh (new)
+
+test_description='push to remote group'
+
++GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=default
++export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
++
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ for i in 1 2 3
+ do
-+ git init --bare dest-$i.git || return 1
++ git init --bare dest-$i.git &&
++ git -C dest-$i.git symbolic-ref HEAD refs/heads/not-a-branch ||
++ return 1
+ done &&
+ test_tick &&
+ git commit --allow-empty -m "initial" &&
@@ t/t5566-push-group.sh (new)
+ test_grep "mirror" err &&
+ git config unset remote.remote-1.mirror
+'
++
+test_expect_success 'push.default=current works with group push' '
+ git config set push.default current &&
+ test_tick &&
@@ t/t5566-push-group.sh (new)
+ git config unset push.default
+'
+
++test_expect_success '--atomic is rejected for group push' '
++ test_must_fail git push --atomic all-remotes HEAD:refs/heads/main 2>err &&
++ test_grep "atomic" err
++'
++
+test_expect_success 'push continues past rejection to remaining remotes' '
+ for i in c1 c2 c3
+ do
@@ t/t5566-push-group.sh (new)
+ # initial sync
+ git push continue-group HEAD:refs/heads/main &&
+
-+ # advance c2 independently
-+ git clone dest-c2.git tmp-c2 &&
-+ (
-+ cd tmp-c2 &&
-+ git checkout -b main origin/main &&
-+ test_commit c2_independent &&
-+ git push origin HEAD:refs/heads/main
-+ ) &&
-+ rm -rf tmp-c2 &&
++ # advance c2 independently
++ git clone dest-c2.git tmp-c2 &&
++ (
++ cd tmp-c2 &&
++ git checkout -b main origin/main &&
++ test_commit c2_independent &&
++ git push origin HEAD:refs/heads/main
++ ) &&
++ rm -rf tmp-c2 &&
+
+ test_tick &&
+ git commit --allow-empty -m "local diverging commit" &&
@@ t/t5566-push-group.sh (new)
+ ! test_cmp expect actual-c2
+'
+
-+test_expect_success 'fatal connection error stops remaining remotes' '
++test_expect_success 'fatal connection error does not stop remaining remotes' '
+ for i in f1 f2 f3
+ do
+ git init --bare dest-$i.git || return 1
@@ t/t5566-push-group.sh (new)
+ test_tick &&
+ git commit --allow-empty -m "after fatal setup" &&
+
++ # overall exit code is non-zero because f2 failed
+ test_must_fail git push fatal-group HEAD:refs/heads/main &&
+
+ git rev-parse HEAD >expect &&
++
++ # f1 and f3 should both have the new commit — subprocesses are independent
+ git -C dest-f1.git rev-parse refs/heads/main >actual-f1 &&
+ test_cmp expect actual-f1 &&
-+
-+ # f3 should not be updated
+ git -C dest-f3.git rev-parse refs/heads/main >actual-f3 &&
-+ ! test_cmp expect actual-f3 &&
++ test_cmp expect actual-f3 &&
+
+ git config set remote.f2.url "file://$(pwd)/dest-f2.git"
+'
Usman Akinyemi (2):
remote: move remote group resolution to remote.c
push: support pushing to a remote group
Documentation/git-push.adoc | 80 ++++++++++--
builtin/fetch.c | 42 ------
builtin/push.c | 250 +++++++++++++++++++++++++++++++-----
remote.c | 37 ++++++
remote.h | 12 ++
t/meson.build | 1 +
t/t5566-push-group.sh | 160 +++++++++++++++++++++++
7 files changed, 499 insertions(+), 83 deletions(-)
create mode 100755 t/t5566-push-group.sh
--
2.53.0
^ permalink raw reply
* [RFC PATCH v4 1/2] remote: move remote group resolution to remote.c
From: Usman Akinyemi @ 2026-04-27 14:05 UTC (permalink / raw)
To: usmanakinyemi202; +Cc: christian.couder, git, gitster, me, phillip.wood123, ps
In-Reply-To: <20260427140530.856125-1-usmanakinyemi202@gmail.com>
`get_remote_group`, `add_remote_or_group`, and the `remote_group_data`
struct are currently defined as static helpers inside builtin/fetch.c.
They implement generic remote group resolution that is not specific to
fetch — they parse `remotes.<name>` config entries and resolve a name
to either a list of group members or a single configured remote.
Move them to remote.c and declare them in remote.h so that other
builtins can use the same logic without duplication.
Useful for the next patch.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
builtin/fetch.c | 42 ------------------------------------------
remote.c | 37 +++++++++++++++++++++++++++++++++++++
remote.h | 12 ++++++++++++
3 files changed, 49 insertions(+), 42 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a22c319467..cfb26eb284 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2138,48 +2138,6 @@ static int get_one_remote_for_fetch(struct remote *remote, void *priv)
return 0;
}
-struct remote_group_data {
- const char *name;
- struct string_list *list;
-};
-
-static int get_remote_group(const char *key, const char *value,
- const struct config_context *ctx UNUSED,
- void *priv)
-{
- struct remote_group_data *g = priv;
-
- if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
- /* split list by white space */
- while (*value) {
- size_t wordlen = strcspn(value, " \t\n");
-
- if (wordlen >= 1)
- string_list_append_nodup(g->list,
- xstrndup(value, wordlen));
- value += wordlen + (value[wordlen] != '\0');
- }
- }
-
- return 0;
-}
-
-static int add_remote_or_group(const char *name, struct string_list *list)
-{
- int prev_nr = list->nr;
- struct remote_group_data g;
- g.name = name; g.list = list;
-
- repo_config(the_repository, get_remote_group, &g);
- if (list->nr == prev_nr) {
- struct remote *remote = remote_get(name);
- if (!remote_is_configured(remote, 0))
- return 0;
- string_list_append(list, remote->name);
- }
- return 1;
-}
-
static void add_options_to_argv(struct strvec *argv,
const struct fetch_config *config)
{
diff --git a/remote.c b/remote.c
index a664cd166a..7133d29332 100644
--- a/remote.c
+++ b/remote.c
@@ -2114,6 +2114,43 @@ int get_fetch_map(const struct ref *remote_refs,
return 0;
}
+int get_remote_group(const char *key, const char *value,
+ const struct config_context *ctx UNUSED,
+ void *priv)
+{
+ struct remote_group_data *g = priv;
+
+ if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
+ /* split list by white space */
+ while (*value) {
+ size_t wordlen = strcspn(value, " \t\n");
+
+ if (wordlen >= 1)
+ string_list_append_nodup(g->list,
+ xstrndup(value, wordlen));
+ value += wordlen + (value[wordlen] != '\0');
+ }
+ }
+
+ return 0;
+}
+
+int add_remote_or_group(const char *name, struct string_list *list)
+{
+ int prev_nr = list->nr;
+ struct remote_group_data g;
+ g.name = name; g.list = list;
+
+ repo_config(the_repository, get_remote_group, &g);
+ if (list->nr == prev_nr) {
+ struct remote *remote = remote_get(name);
+ if (!remote_is_configured(remote, 0))
+ return 0;
+ string_list_append(list, remote->name);
+ }
+ return 1;
+}
+
int resolve_remote_symref(struct ref *ref, struct ref *list)
{
if (!ref->symref)
diff --git a/remote.h b/remote.h
index fc052945ee..8ff2bd88fa 100644
--- a/remote.h
+++ b/remote.h
@@ -347,6 +347,18 @@ int branch_has_merge_config(struct branch *branch);
int branch_merge_matches(struct branch *, int n, const char *);
+/* list of the remote in a group as configured */
+struct remote_group_data {
+ const char *name;
+ struct string_list *list;
+};
+
+int get_remote_group(const char *key, const char *value,
+ const struct config_context *ctx,
+ void *priv);
+
+int add_remote_or_group(const char *name, struct string_list *list);
+
/**
* Return the fully-qualified refname of the tracking branch for `branch`.
* I.e., what "branch@{upstream}" would give you. Returns NULL if no
--
2.53.0
^ permalink raw reply related
* [RFC PATCH v4 2/2] push: support pushing to a remote group
From: Usman Akinyemi @ 2026-04-27 14:05 UTC (permalink / raw)
To: usmanakinyemi202; +Cc: christian.couder, git, gitster, me, phillip.wood123, ps
In-Reply-To: <20260427140530.856125-1-usmanakinyemi202@gmail.com>
`git fetch` accepts a remote group name (configured via `remotes.<name>`
in config) and fetches from each member remote. `git push` has no
equivalent — it only accepts a single remote name.
Teach `git push` to resolve its repository argument through
`add_remote_or_group()`, which was made public in the previous patch,
so that a user can push to all remotes in a group with:
git push <group>
When the argument resolves to a single remote, the behaviour is
identical to before. When it resolves to a group, each member remote
is pushed in sequence.
The group push path rebuilds the refspec list (`rs`) from scratch for
each member remote so that per-remote push mappings configured via
`remote.<name>.push` are resolved correctly against each specific
remote. Without this, refspec entries would accumulate across iterations
and each subsequent remote would receive a growing list of duplicated
entries.
Mirror detection (`remote->mirror`) is also evaluated per remote using
a copy of the flags, so that a mirror remote in the group cannot set
TRANSPORT_PUSH_FORCE on subsequent non-mirror remotes in the same group.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
Documentation/git-push.adoc | 80 ++++++++++--
builtin/push.c | 250 +++++++++++++++++++++++++++++++-----
t/meson.build | 1 +
t/t5566-push-group.sh | 160 +++++++++++++++++++++++
4 files changed, 450 insertions(+), 41 deletions(-)
create mode 100755 t/t5566-push-group.sh
diff --git a/Documentation/git-push.adoc b/Documentation/git-push.adoc
index e5ba3a6742..aa221c3909 100644
--- a/Documentation/git-push.adoc
+++ b/Documentation/git-push.adoc
@@ -18,17 +18,28 @@ git push [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n
DESCRIPTION
-----------
-
-Updates one or more branches, tags, or other references in a remote
-repository from your local repository, and sends all necessary data
-that isn't already on the remote.
+Updates one or more branches, tags, or other references in one or more
+remote repositories from your local repository, and sends all necessary
+data that isn't already on the remote.
The simplest way to push is `git push <remote> <branch>`.
`git push origin main` will push the local `main` branch to the `main`
branch on the remote named `origin`.
-The `<repository>` argument defaults to the upstream for the current branch,
-or `origin` if there's no configured upstream.
+You can also push to multiple remotes at once by using a remote group.
+A remote group is a named list of remotes configured via `remotes.<name>`
+in your git config:
+
+ $ git config remotes.all-remotes "origin gitlab backup"
+
+Then `git push all-remotes` will push to `origin`, `gitlab`, and
+`backup` in turn, as if you had run `git push` against each one
+individually. Each remote is pushed independently using its own
+push mapping configuration. There is a `remotes.<group>` entry in
+the configuration file. (See linkgit:git-config[1]).
+
+The `<repository>` argument defaults to the upstream for the current
+branch, or `origin` if there's no configured upstream.
To decide which branches, tags, or other refs to push, Git uses
(in order of precedence):
@@ -55,8 +66,10 @@ OPTIONS
_<repository>_::
The "remote" repository that is the destination of a push
operation. This parameter can be either a URL
- (see the section <<URLS,GIT URLS>> below) or the name
- of a remote (see the section <<REMOTES,REMOTES>> below).
+ (see the section <<URLS,GIT URLS>> below), the name
+ of a remote (see the section <<REMOTES,REMOTES>> below),
+ or the name of a remote group
+ (see the section <<REMOTE-GROUPS,REMOTE GROUPS>> below).
`<refspec>...`::
Specify what destination ref to update with what source object.
@@ -430,6 +443,57 @@ further recursion will occur. In this case, `only` is treated as `on-demand`.
include::urls-remotes.adoc[]
+[[REMOTE-GROUPS]]
+REMOTE GROUPS
+-------------
+
+A remote group is a named list of remotes configured via `remotes.<name>`
+in your git config:
+
+ $ git config remotes.all-remotes "r1 r2 r3"
+
+When a group name is given as the `<repository>` argument, the push is
+performed to each member remote in turn. The defining principle is:
+
+ git push <options> all-remotes <args>
+
+is exactly equivalent to:
+
+ git push <options> r1 <args>
+ git push <options> r2 <args>
+ ...
+ git push <options> rN <args>
+
+where r1, r2, ..., rN are the members of `all-remotes`. No special
+behaviour is added or removed — the group is purely a shorthand for
+running the same push command against each member remote individually.
+
+When pushing to a group of more than one remote, Git spawns a separate
+`git push` subprocess for each member remote in sequence. Each subprocess
+receives the same flags and refspecs as the original invocation. This
+means that per-remote push mappings configured via `remote.<name>.push`
+and mirror mode (`remote.<name>.mirror`) are evaluated independently for
+each remote, and a mirror remote in the group cannot affect the push
+behaviour of other non-mirror remotes in the same group.
+
+The `--atomic` option is not supported for group pushes, because atomicity
+can only be guaranteed within a single transport connection to a single
+remote. Git will refuse the invocation with an error if `--atomic` is
+combined with a group name.
+
+If any member remote fails whether due to a push rejection (e.g. a
+non-fast-forward update, a server-side hook refusing a ref) or a connection
+error (e.g. the repository does not exist, authentication fails, or the
+network is unreachable), Git reports the error and continues pushing to
+the remaining remotes in the group. The overall exit code is non-zero if
+any member push fails.
+
+This means the user is responsible for ensuring that the sequence of
+individual pushes makes sense. If `git push r1`` would fail for a given
+set of options and arguments, then `git push all-remotes` will fail in
+the same way when it reaches r1. The group push does not do anything
+special to make a failing individual push succeed.
+
OUTPUT
------
diff --git a/builtin/push.c b/builtin/push.c
index 7100ffba5d..10384f265c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -10,6 +10,7 @@
#include "config.h"
#include "environment.h"
#include "gettext.h"
+#include "hex.h"
#include "refspec.h"
#include "run-command.h"
#include "remote.h"
@@ -544,6 +545,122 @@ static int git_push_config(const char *k, const char *v,
return git_default_config(k, v, ctx, NULL);
}
+static int push_multiple(struct string_list *list,
+ const struct string_list *push_options,
+ int flags,
+ int tags,
+ const char **refspecs,
+ int refspec_nr)
+{
+ int i, result = 0;
+ struct strvec argv = STRVEC_INIT;
+
+ strvec_push(&argv, "push");
+
+ if (flags & TRANSPORT_PUSH_FORCE)
+ strvec_push(&argv, "--force");
+ if (flags & TRANSPORT_PUSH_DRY_RUN)
+ strvec_push(&argv, "--dry-run");
+ if (flags & TRANSPORT_PUSH_PORCELAIN)
+ strvec_push(&argv, "--porcelain");
+ if (flags & TRANSPORT_PUSH_PRUNE)
+ strvec_push(&argv, "--prune");
+ if (flags & TRANSPORT_PUSH_NO_HOOK)
+ strvec_push(&argv, "--no-verify");
+ if (flags & TRANSPORT_PUSH_FOLLOW_TAGS)
+ strvec_push(&argv, "--follow-tags");
+ if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
+ strvec_push(&argv, "--set-upstream");
+ if (flags & TRANSPORT_PUSH_FORCE_IF_INCLUDES)
+ strvec_push(&argv, "--force-if-includes");
+ if (flags & TRANSPORT_PUSH_ALL)
+ strvec_push(&argv, "--all");
+ if (flags & TRANSPORT_PUSH_MIRROR)
+ strvec_push(&argv, "--mirror");
+
+ if (flags & TRANSPORT_PUSH_CERT_ALWAYS)
+ strvec_push(&argv, "--signed=yes");
+ else if (flags & TRANSPORT_PUSH_CERT_IF_ASKED)
+ strvec_push(&argv, "--signed=if-asked");
+ if (!thin)
+ strvec_push(&argv, "--no-thin");
+
+ if (deleterefs)
+ strvec_push(&argv, "--delete");
+
+ if (receivepack)
+ strvec_pushf(&argv, "--receive-pack=%s", receivepack);
+ if (verbosity >= 2)
+ strvec_push(&argv, "-v");
+ if (verbosity >= 1)
+ strvec_push(&argv, "-v");
+ else if (verbosity < 0)
+ strvec_push(&argv, "-q");
+ if (progress > 0)
+ strvec_push(&argv, "--progress");
+ else if (progress == 0)
+ strvec_push(&argv, "--no-progress");
+
+ if (family == TRANSPORT_FAMILY_IPV4)
+ strvec_push(&argv, "--ipv4");
+ else if (family == TRANSPORT_FAMILY_IPV6)
+ strvec_push(&argv, "--ipv6");
+
+ if (recurse_submodules == RECURSE_SUBMODULES_CHECK)
+ strvec_push(&argv, "--recurse-submodules=check");
+ else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
+ strvec_push(&argv, "--recurse-submodules=on-demand");
+ else if (recurse_submodules == RECURSE_SUBMODULES_ONLY)
+ strvec_push(&argv, "--recurse-submodules=only");
+ else if (recurse_submodules == RECURSE_SUBMODULES_OFF)
+ strvec_push(&argv, "--recurse-submodules=no");
+
+
+ if (tags)
+ strvec_push(&argv, "--tags");
+
+ for (i = 0; i < push_options->nr; i++)
+ strvec_pushf(&argv, "--push-option=%s",
+ push_options->items[i].string);
+
+ for (i = 0; i < cas.nr; i++) {
+ if (cas.entry[i].use_tracking) {
+ strvec_pushf(&argv, "--force-with-lease=%s",
+ cas.entry[i].refname);
+ } else if (!is_null_oid(&cas.entry[i].expect)) {
+ strvec_pushf(&argv, "--force-with-lease=%s:%s",
+ cas.entry[i].refname,
+ oid_to_hex(&cas.entry[i].expect));
+ } else {
+ strvec_push(&argv, "--force-with-lease");
+ }
+ }
+
+ for (i = 0; i < list->nr; i++) {
+ const char *name = list->items[i].string;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ int j;
+
+ strvec_pushv(&cmd.args, argv.v);
+ strvec_push(&cmd.args, name);
+
+ for (j = 0; j < refspec_nr; j++)
+ strvec_push(&cmd.args, refspecs[j]);
+
+ if (verbosity >= 0)
+ printf(_("Pushing to %s\n"), name);
+
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
+ error(_("could not push to %s"), name);
+ result = 1;
+ }
+ }
+
+ strvec_clear(&argv);
+ return result;
+}
+
int cmd_push(int argc,
const char **argv,
const char *prefix,
@@ -552,12 +669,13 @@ int cmd_push(int argc,
int flags = 0;
int tags = 0;
int push_cert = -1;
- int rc;
+ int rc = 0;
+ int base_flags;
const char *repo = NULL; /* default repository */
struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
+ struct string_list remote_group = STRING_LIST_INIT_DUP;
struct string_list *push_options;
const struct string_list_item *item;
- struct remote *remote;
struct option options[] = {
OPT__VERBOSITY(&verbosity),
@@ -620,39 +738,45 @@ int cmd_push(int argc,
else if (recurse_submodules == RECURSE_SUBMODULES_ONLY)
flags |= TRANSPORT_RECURSE_SUBMODULES_ONLY;
- if (tags)
- refspec_append(&rs, "refs/tags/*");
-
if (argc > 0)
repo = argv[0];
- remote = pushremote_get(repo);
- if (!remote) {
- if (repo)
- die(_("bad repository '%s'"), repo);
- die(_("No configured push destination.\n"
- "Either specify the URL from the command-line or configure a remote repository using\n"
- "\n"
- " git remote add <name> <url>\n"
- "\n"
- "and then push using the remote name\n"
- "\n"
- " git push <name>\n"));
- }
-
- if (argc > 0)
- set_refspecs(argv + 1, argc - 1, remote);
-
- if (remote->mirror)
- flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
-
- if (flags & TRANSPORT_PUSH_ALL) {
- if (argc >= 2)
- die(_("--all can't be combined with refspecs"));
- }
- if (flags & TRANSPORT_PUSH_MIRROR) {
- if (argc >= 2)
- die(_("--mirror can't be combined with refspecs"));
+ if (repo) {
+ if (!add_remote_or_group(repo, &remote_group)) {
+ /*
+ * Not a configured remote name or group name.
+ * Try treating it as a direct URL or path, e.g.
+ * git push /tmp/foo.git
+ * git push https://github.com/user/repo.git
+ * pushremote_get() creates an anonymous remote
+ * from the URL so the loop below can handle it
+ * identically to a named remote.
+ */
+ struct remote *r = pushremote_get(repo);
+ if (!r)
+ die(_("bad repository '%s'"), repo);
+ string_list_append(&remote_group, r->name);
+ }
+ } else {
+ struct remote *r = pushremote_get(NULL);
+ if (!r)
+ die(_("No configured push destination.\n"
+ "Either specify the URL from the command-line or configure a remote repository using\n"
+ "\n"
+ " git remote add <name> <url>\n"
+ "\n"
+ "and then push using the remote name\n"
+ "\n"
+ " git push <name>\n"
+ "\n"
+ "To push to multiple remotes at once, configure a remote group using\n"
+ "\n"
+ " git config remotes.<groupname> \"<remote1> <remote2>\"\n"
+ "\n"
+ "and then push using the group name\n"
+ "\n"
+ " git push <groupname>\n"));
+ string_list_append(&remote_group, r->name);
}
if (!is_empty_cas(&cas) && (flags & TRANSPORT_PUSH_FORCE_IF_INCLUDES))
@@ -662,10 +786,70 @@ int cmd_push(int argc,
if (strchr(item->string, '\n'))
die(_("push options must not have new line characters"));
- rc = do_push(flags, push_options, remote);
+ if (remote_group.nr == 1) {
+ /*
+ * Single remote (the common case): run do_push() directly
+ * in this process. The loop runs exactly once.
+ *
+ * Mirror detection and the --mirror/--all + refspec conflict
+ * checks are done here. rs is rebuilt so that per-remote push
+ * mappings (remote.NAME.push config) are resolved against the
+ * correct remote. inner_flags is a snapshot of flags so that a
+ * mirror remote cannot bleed TRANSPORT_PUSH_FORCE into any
+ * subsequent call.
+ */
+ base_flags = flags;
+ {
+ int inner_flags = base_flags;
+ struct remote *r = pushremote_get(remote_group.items[0].string);
+ if (!r)
+ die(_("no such remote or remote group: %s"),
+ remote_group.items[0].string);
+
+ if (r->mirror)
+ inner_flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
+
+ if (inner_flags & TRANSPORT_PUSH_ALL) {
+ if (argc >= 2)
+ die(_("--all can't be combined with refspecs"));
+ }
+ if (inner_flags & TRANSPORT_PUSH_MIRROR) {
+ if (argc >= 2)
+ die(_("--mirror can't be combined with refspecs"));
+ }
+
+ refspec_clear(&rs);
+ rs = (struct refspec) REFSPEC_INIT_PUSH;
+
+ if (tags)
+ refspec_append(&rs, "refs/tags/*");
+ if (argc > 0)
+ set_refspecs(argv + 1, argc - 1, r);
+
+ rc = do_push(inner_flags, push_options, r);
+ }
+ } else {
+ /*
+ * Multiple remotes: spawn one "git push <remote> [<refspecs>]"
+ * subprocess per remote, sequentially.
+ *
+ * Options that only make sense for a single transport connection
+ * are rejected here.
+ */
+ if (flags & TRANSPORT_PUSH_ATOMIC)
+ die(_("--atomic can only be used when pushing to one remote"));
+
+ rc = push_multiple(&remote_group, push_options, flags,
+ tags,
+ argc > 1 ? argv + 1 : NULL,
+ argc > 1 ? argc - 1 : 0);
+ }
+
string_list_clear(&push_options_cmdline, 0);
string_list_clear(&push_options_config, 0);
+ string_list_clear(&remote_group, 0);
clear_cas_option(&cas);
+
if (rc == -1)
usage_with_options(push_usage, options);
else
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5..bd090627e9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -704,6 +704,7 @@ integration_tests = [
't5563-simple-http-auth.sh',
't5564-http-proxy.sh',
't5565-push-multiple.sh',
+ 't5566-push-group.sh',
't5570-git-daemon.sh',
't5571-pre-push-hook.sh',
't5572-pull-submodule.sh',
diff --git a/t/t5566-push-group.sh b/t/t5566-push-group.sh
new file mode 100755
index 0000000000..a7d59352b1
--- /dev/null
+++ b/t/t5566-push-group.sh
@@ -0,0 +1,160 @@
+#!/bin/sh
+
+test_description='push to remote group'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=default
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ for i in 1 2 3
+ do
+ git init --bare dest-$i.git &&
+ git -C dest-$i.git symbolic-ref HEAD refs/heads/not-a-branch ||
+ return 1
+ done &&
+ test_tick &&
+ git commit --allow-empty -m "initial" &&
+ git config set remote.remote-1.url "file://$(pwd)/dest-1.git" &&
+ git config set remote.remote-1.fetch "+refs/heads/*:refs/remotes/remote-1/*" &&
+ git config set remote.remote-2.url "file://$(pwd)/dest-2.git" &&
+ git config set remote.remote-2.fetch "+refs/heads/*:refs/remotes/remote-2/*" &&
+ git config set remote.remote-3.url "file://$(pwd)/dest-3.git" &&
+ git config set remote.remote-3.fetch "+refs/heads/*:refs/remotes/remote-3/*" &&
+ git config set remotes.all-remotes "remote-1 remote-2 remote-3"
+'
+
+test_expect_success 'push to remote group updates all members correctly' '
+ git push all-remotes HEAD:refs/heads/main &&
+ git rev-parse HEAD >expect &&
+ for i in 1 2 3
+ do
+ git -C dest-$i.git rev-parse refs/heads/main >actual ||
+ return 1
+ test_cmp expect actual || return 1
+ done
+'
+
+test_expect_success 'push second commit to group updates all members' '
+ test_tick &&
+ git commit --allow-empty -m "second" &&
+ git push all-remotes HEAD:refs/heads/main &&
+ git rev-parse HEAD >expect &&
+ for i in 1 2 3
+ do
+ git -C dest-$i.git rev-parse refs/heads/main >actual ||
+ return 1
+ test_cmp expect actual || return 1
+ done
+'
+
+test_expect_success 'push to single remote in group does not affect others' '
+ test_tick &&
+ git commit --allow-empty -m "third" &&
+ git push remote-1 HEAD:refs/heads/main &&
+ git -C dest-1.git rev-parse refs/heads/main >hash-after-1 &&
+ git -C dest-2.git rev-parse refs/heads/main >hash-after-2 &&
+ ! test_cmp hash-after-1 hash-after-2
+'
+
+test_expect_success 'mirror remote in group with refspec fails' '
+ git config set remote.remote-1.mirror true &&
+ test_must_fail git push all-remotes HEAD:refs/heads/main 2>err &&
+ test_grep "mirror" err &&
+ git config unset remote.remote-1.mirror
+'
+
+test_expect_success 'push.default=current works with group push' '
+ git config set push.default current &&
+ test_tick &&
+ git commit --allow-empty -m "fifth" &&
+ git push all-remotes &&
+ git config unset push.default
+'
+
+test_expect_success '--atomic is rejected for group push' '
+ test_must_fail git push --atomic all-remotes HEAD:refs/heads/main 2>err &&
+ test_grep "atomic" err
+'
+
+test_expect_success 'push continues past rejection to remaining remotes' '
+ for i in c1 c2 c3
+ do
+ git init --bare dest-$i.git || return 1
+ done &&
+ git config set remote.c1.url "file://$(pwd)/dest-c1.git" &&
+ git config set remote.c2.url "file://$(pwd)/dest-c2.git" &&
+ git config set remote.c3.url "file://$(pwd)/dest-c3.git" &&
+ git config set remotes.continue-group "c1 c2 c3" &&
+
+ test_tick &&
+ git commit --allow-empty -m "base for continue test" &&
+
+ # initial sync
+ git push continue-group HEAD:refs/heads/main &&
+
+ # advance c2 independently
+ git clone dest-c2.git tmp-c2 &&
+ (
+ cd tmp-c2 &&
+ git checkout -b main origin/main &&
+ test_commit c2_independent &&
+ git push origin HEAD:refs/heads/main
+ ) &&
+ rm -rf tmp-c2 &&
+
+ test_tick &&
+ git commit --allow-empty -m "local diverging commit" &&
+
+ # push: c2 rejects, others succeed
+ test_must_fail git push continue-group HEAD:refs/heads/main &&
+
+ git rev-parse HEAD >expect &&
+ git -C dest-c1.git rev-parse refs/heads/main >actual-c1 &&
+ git -C dest-c3.git rev-parse refs/heads/main >actual-c3 &&
+ test_cmp expect actual-c1 &&
+ test_cmp expect actual-c3 &&
+
+ # c2 should not have the new commit
+ git -C dest-c2.git rev-parse refs/heads/main >actual-c2 &&
+ ! test_cmp expect actual-c2
+'
+
+test_expect_success 'fatal connection error does not stop remaining remotes' '
+ for i in f1 f2 f3
+ do
+ git init --bare dest-$i.git || return 1
+ done &&
+ git config set remote.f1.url "file://$(pwd)/dest-f1.git" &&
+ git config set remote.f2.url "file://$(pwd)/dest-f2.git" &&
+ git config set remote.f3.url "file://$(pwd)/dest-f3.git" &&
+ git config set remotes.fatal-group "f1 f2 f3" &&
+
+ test_tick &&
+ git commit --allow-empty -m "base for fatal test" &&
+
+ # initial sync
+ git push fatal-group HEAD:refs/heads/main &&
+
+ # break f2
+ git config set remote.f2.url "file:///tmp/does-not-exist-$$" &&
+
+ test_tick &&
+ git commit --allow-empty -m "after fatal setup" &&
+
+ # overall exit code is non-zero because f2 failed
+ test_must_fail git push fatal-group HEAD:refs/heads/main &&
+
+ git rev-parse HEAD >expect &&
+
+ # f1 and f3 should both have the new commit — subprocesses are independent
+ git -C dest-f1.git rev-parse refs/heads/main >actual-f1 &&
+ test_cmp expect actual-f1 &&
+ git -C dest-f3.git rev-parse refs/heads/main >actual-f3 &&
+ test_cmp expect actual-f3 &&
+
+ git config set remote.f2.url "file://$(pwd)/dest-f2.git"
+'
+
+test_done
--
2.53.0
^ permalink raw reply related
* [PATCH] t5564: use a short path for the SOCKS proxy socket
From: Johannes Schindelin via GitGitGadget @ 2026-04-27 14:21 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The SOCKS proxy test introduced in 0ca365c2ed4 (http: do not ignore
proxy path, 2024-08-02) creates a Unix domain socket in
`$TRASH_DIRECTORY`. When the trash directory path is long (e.g.
when running from a deeply nested worktree), the socket path can
exceed the 108-character limit for `struct sockaddr_un.sun_path` on
Linux, causing the test to fail with "Path length ... is longer
than maximum supported length (108)".
Move the socket to `$TMPDIR` (defaulting to `/tmp`) where the path
is short, following the same approach used in t7528 for the SSH
agent socket in b7fb2194b96 (t7528: work around ETOOMANY in OpenSSH
10.1 and newer, 2025-10-23).
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t5564: use a short path for the SOCKS proxy socket
When trying to run the entire test suite in a slightly deeper path than
usual, I was surprised to see that this test failed due to our old
friend, the 108 character limit of Unix sockets.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2100%2Fdscho%2Favoid-too-long-unix-socket-path-in-socks-proxy-test-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2100/dscho/avoid-too-long-unix-socket-path-in-socks-proxy-test-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2100
t/t5564-http-proxy.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/t/t5564-http-proxy.sh b/t/t5564-http-proxy.sh
index 3bcbdef409..cb7ede4ca4 100755
--- a/t/t5564-http-proxy.sh
+++ b/t/t5564-http-proxy.sh
@@ -50,14 +50,19 @@ start_socks() {
# The %30 tests that the correct amount of percent-encoding is applied to the
# proxy string passed to curl.
+# Use a short path for the socket to avoid exceeding the 108-character
+# Unix domain socket limit when the trash directory path is long.
+SOCKS_SOCK="${TMPDIR:-/tmp}/git-test-socks-%30.sock"
+
test_lazy_prereq SOCKS_PROXY '
test_have_prereq PERL &&
- start_socks "$TRASH_DIRECTORY/%30.sock"
+ start_socks "$SOCKS_SOCK"
'
test_atexit '
test ! -e "$TRASH_DIRECTORY/socks.pid" ||
kill "$(cat "$TRASH_DIRECTORY/socks.pid")"
+ rm -f "$SOCKS_SOCK"
'
# The below tests morally ought to be gated on a prerequisite that Git is
@@ -70,7 +75,8 @@ old_libcurl_error() {
test_expect_success SOCKS_PROXY 'clone via Unix socket' '
test_when_finished "rm -rf clone" &&
- test_config_global http.proxy "socks4://localhost$PWD/%2530.sock" && {
+ socks_proxy_url="socks4://localhost$(echo "$SOCKS_SOCK" | sed "s/%/%25/g")" &&
+ test_config_global http.proxy "$socks_proxy_url" && {
{
GIT_TRACE_CURL=$PWD/trace \
GIT_TRACE_CURL_COMPONENTS=socks \
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Scott Bauersfeld via GitGitGadget @ 2026-04-27 16:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Derrick Stolee, Scott Bauersfeld,
Scott Bauersfeld
In-Reply-To: <pull.2282.git.git.1777058098756.gitgitgadget@gmail.com>
From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
index-pack and unpack-objects both read pack data from stdin through
a 4 KiB static buffer. In index-pack, each fill() flushes consumed
bytes to the pack file via write_or_die(), capping every write(2)
at 4 KiB. unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round
trip through the FUSE protocol (userspace -> kernel -> userspace ->
back), so the 4 KiB buffer turns a clone into many unnecessary tiny
writes with noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
DEFAULT_PACKFILE_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the
hashfile layer in csum-file (which already used 128 KiB but
hardcoded the value).
Syscall counts via strace on HTTPS clones of git/git (~296 MB pack,
5 runs per variant, isolated builds from the same v2.54.0 source):
index-pack pack file writes: 72,465 -> 24,943 avg (65% fewer)
total write() syscalls: 310,192 -> 259,530 avg (16% fewer)
writes of exactly 4096 bytes: ~40,077 -> 0
Wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled, 3 runs per variant:
vscode (~1.26 GB pack): 84.5s -> 75.7s avg (10% faster)
git/git (~306 MB pack): 22.6s -> 20.0s avg (11% faster)
Signed-off-by: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
---
index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
index-pack and unpack-objects read pack data from stdin through a 4 KiB
static buffer. In index-pack, each fill() flushes consumed bytes to the
pack file via write_or_die(), capping every write(2) at 4 KiB.
unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round trip
through the FUSE protocol (userspace → kernel → userspace → back), so
the 4 KiB buffer turns a clone into many unnecessary tiny writes with
noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
DEFAULT_PACKFILE_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the hashfile
layer in csum-file (which already used 128 KiB but hardcoded the value).
Syscall reduction
=================
Measured via strace -f on HTTPS clones of git/git (~296 MB pack, 5 runs
per variant, isolated builds from the same v2.54.0 source):
Metric Unpatched (4 KiB) Patched (128 KiB) Change index-pack writes to
pack file 72,465 avg 24,943 avg −65% Total write() syscalls (all
processes) 310,192 avg 259,530 avg −16% Writes of exactly 4096 bytes
~40,077 avg 0 eliminated HEAD / file count / fsck ✓ ✓ identical
Wall-clock time on FUSE
=======================
Measured wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled. 3 runs per variant:
Repo Unpatched avg Patched avg Change microsoft/vscode (~1.26 GB pack)
84.5s 75.7s −10% git/git (~306 MB pack) 22.6s 20.0s −11%
Changes since v1
================
* Introduced shared DEFAULT_PACKFILE_BUFFER_SIZE constant in
git-compat-util.h (next to MAX_IO_SIZE), replacing per-file #define
and the hardcoded value in csum-file.c. Placed here rather than
environment.h since it is an I/O buffer size, not an environment
variable or repo config.
* Added wall-clock timing on a FUSE filesystem.
* Cleaned up the commit description a bit.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2282%2Fsbauersfeld%2Fsb%2Fincrease-index-pack-input-buffer-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2282/sbauersfeld/sb/increase-index-pack-input-buffer-v2
Pull-Request: https://github.com/git/git/pull/2282
Range-diff vs v1:
1: c388e1dc2f ! 1: ac2559ccb5 index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
@@ Metadata
## Commit message ##
index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
- Both index-pack and unpack-objects read pack data from stdin through
- a 4 KiB static buffer (input_buffer[4096]). On each fill(), consumed
- bytes are flushed to the output pack file via write_or_die(), so
- every write(2) moves at most 4 KiB.
+ index-pack and unpack-objects both read pack data from stdin through
+ a 4 KiB static buffer. In index-pack, each fill() flushes consumed
+ bytes to the pack file via write_or_die(), capping every write(2)
+ at 4 KiB. unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round
trip through the FUSE protocol (userspace -> kernel -> userspace ->
back), so the 4 KiB buffer turns a clone into many unnecessary tiny
writes with noticeable latency overhead.
- Increase the buffer from 4 KiB to 128 KiB, matching the default
- already used by the hashfile layer in csum-file.c.
+ Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
+ DEFAULT_PACKFILE_BUFFER_SIZE constant in git-compat-util.h (next to
+ MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the
+ hashfile layer in csum-file (which already used 128 KiB but
+ hardcoded the value).
- Testing with strace on HTTPS clones of git/git (~296 MB pack, 5 runs
- per variant, isolated builds from the same v2.54.0 source) shows:
+ Syscall counts via strace on HTTPS clones of git/git (~296 MB pack,
+ 5 runs per variant, isolated builds from the same v2.54.0 source):
- index-pack pack file writes: 72,465 -> 24,943 avg (66% reduction)
- total write() syscalls: 310,192 -> 259,530 avg (17% reduction)
- writes of exactly 4096 bytes: ~40,077 -> 0 (eliminated)
+ index-pack pack file writes: 72,465 -> 24,943 avg (65% fewer)
+ total write() syscalls: 310,192 -> 259,530 avg (16% fewer)
+ writes of exactly 4096 bytes: ~40,077 -> 0
- All clones produce identical HEAD, file count, and pass fsck.
+ Wall-clock time of git clone over HTTPS onto a FUSE passthrough
+ filesystem with writeback caching disabled, 3 runs per variant:
+
+ vscode (~1.26 GB pack): 84.5s -> 75.7s avg (10% faster)
+ git/git (~306 MB pack): 22.6s -> 20.0s avg (11% faster)
Signed-off-by: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
@@ builtin/index-pack.c: static int check_self_contained_and_connected;
-/* We always read in 4kB chunks. */
-static unsigned char input_buffer[4096];
-+#define INPUT_BUFFER_SIZE (128 * 1024)
-+static unsigned char input_buffer[INPUT_BUFFER_SIZE];
++static unsigned char input_buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
static unsigned int input_offset, input_len;
static off_t consumed_bytes;
static off_t max_input_size;
@@ builtin/unpack-objects.c
-/* We always read in 4kB chunks. */
-static unsigned char buffer[4096];
-+#define INPUT_BUFFER_SIZE (128 * 1024)
-+static unsigned char buffer[INPUT_BUFFER_SIZE];
++static unsigned char buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
static unsigned int offset, len;
static off_t consumed_bytes;
static off_t max_input_size;
+
+ ## csum-file.c ##
+@@ csum-file.c: struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
+ f->algop = unsafe_hash_algo(algop);
+ f->algop->init_fn(&f->ctx);
+
+- f->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;
++ f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_PACKFILE_BUFFER_SIZE;
+ f->buffer = xmalloc(f->buffer_len);
+ f->check_buffer = NULL;
+
+
+ ## git-compat-util.h ##
+@@ git-compat-util.h: static inline uint64_t u64_add(uint64_t a, uint64_t b)
+ # endif
+ #endif
+
++/*
++ * Default buffer size for buffered I/O in pack file operations (index-pack,
++ * unpack-objects) and the hashfile layer in csum-file.
++ */
++#define DEFAULT_PACKFILE_BUFFER_SIZE (128 * 1024)
++
+ #ifdef HAVE_ALLOCA_H
+ # include <alloca.h>
+ # define xalloca(size) (alloca(size))
builtin/index-pack.c | 3 +--
builtin/unpack-objects.c | 3 +--
csum-file.c | 2 +-
git-compat-util.h | 6 ++++++
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c..d86476676f 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -145,8 +145,7 @@ static int check_self_contained_and_connected;
static struct progress *progress;
-/* We always read in 4kB chunks. */
-static unsigned char input_buffer[4096];
+static unsigned char input_buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
static unsigned int input_offset, input_len;
static off_t consumed_bytes;
static off_t max_input_size;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e01cf6e360..da8ec83d9f 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -23,8 +23,7 @@
static int dry_run, quiet, recover, has_errors, strict;
static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict]";
-/* We always read in 4kB chunks. */
-static unsigned char buffer[4096];
+static unsigned char buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
static unsigned int offset, len;
static off_t consumed_bytes;
static off_t max_input_size;
diff --git a/csum-file.c b/csum-file.c
index 9558177a11..c1aeaf587a 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -178,7 +178,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
f->algop = unsafe_hash_algo(algop);
f->algop->init_fn(&f->ctx);
- f->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;
+ f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_PACKFILE_BUFFER_SIZE;
f->buffer = xmalloc(f->buffer_len);
f->check_buffer = NULL;
diff --git a/git-compat-util.h b/git-compat-util.h
index ae1bdc90a4..a2f037811c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -712,6 +712,12 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
# endif
#endif
+/*
+ * Default buffer size for buffered I/O in pack file operations (index-pack,
+ * unpack-objects) and the hashfile layer in csum-file.
+ */
+#define DEFAULT_PACKFILE_BUFFER_SIZE (128 * 1024)
+
#ifdef HAVE_ALLOCA_H
# include <alloca.h>
# define xalloca(size) (alloca(size))
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v4 1/2] revision.c: implement -b-reverse=before for walks
From: Mirko Faina @ 2026-04-27 16:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Mirko Faina, Chris Torek
In-Reply-To: <xmqq8qa852b5.fsf@gitster.g>
On Mon, Apr 27, 2026 at 03:45:34PM +0900, Junio C Hamano wrote:
> > In a revision walk `--reverse` can only be applied after any commit
> > limiting option. This makes getting a limited amount of commits from the
> > tail impossible. E.g.
> >
> > git log --reverse --max-count=3
>
> Can we rephrase "from the tail" somehow to reduce ambiguity?
>
> Normally we generate a list of commits from newer to older, and you
> are saying that it is not possible to take the oldest three commits
> and show them from older to newer (i.e., in reverse). But that, to
> some readers, is showing commits from the beginning end, not from
> the tail end.
>
> Perhaps "... limited number of oldest commits impossible"?
Yes, will rephrase.
> > Teach `get_revision()` to accpet an argument `(after|before)` from the
> > CLI, and apply the reversal before or after the commit limiting options
> > based on this argument.
>
> I think "after" and "before" comes from "Do other things (including
> count limiting) and then apply reverse after all that" and would be
> very much understandable to those who know how the machinery works,
> but should mere mortals need to know the machinery only to use "git
> log"?
No, they shouldn't, but...
> To put it another way, do you tnink experienced Git users who
> haven't seen the actual implementation of revision traversal can
> immediately answer this question:
>
> Now we have --reverse=after and --reverse=before to let you take
> a limited history from both ends when used with --max-count.
> Which between after and before do you think corresponds to the
> traditional --reverse that allowed you to only see the newest
> part of the history?
...while users might not have read the implementation (and they
shouldn't need to to use log) the order in which the class of options is
applied is already documented in the man pages, so having that knowledge
after and before do make sense despite not having seen the
implementation.
But...
> I doubt that the population to answer correctly would not exceed a
> half by large margin (if it is 50% then it means nobody understood
> the difference correctly and they just flipped a coin).
>
> I wonder --reverse=oldest and --reverse=newest is easier to teach
> and explain? I dunno.
...you're right, it is not immediately apparent, but neither are oldest
and newest. Unfortunately I don't think there's any name we can choose
that would make it so without having to read the caveats in the man
pages.
Since many have expressed that this issue is not really about reverse
but about max-count, like you initially assesed, then we should move
towards making changes to the max-count option.
The proposed --max-count-oldest doesn't seem right to me as
max-count-oldest is not about keeping the oldest commits (even tho
that's what we want to achieve when we interact with reverse) but about
when we want to apply max-count. Since [1],
> It might be that the right way to look at this new feature is not that
> "we are changing where reverse is applied", but "count limit is applied
> much later than usual"
maybe --max-count-later as in max count is being applied later than
usual? (either way the users will still need to reach for the man pages
for clarifications).
[1] https://lore.kernel.org/git/xmqqv7dlr4yz.fsf@gitster.g/
^ permalink raw reply
* Re: [PATCH v2] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Derrick Stolee @ 2026-04-27 17:23 UTC (permalink / raw)
To: Scott Bauersfeld via GitGitGadget, git; +Cc: Junio C Hamano, Scott Bauersfeld
In-Reply-To: <pull.2282.v2.git.git.1777306114914.gitgitgadget@gmail.com>
On 4/27/2026 12:08 PM, Scott Bauersfeld via GitGitGadget wrote:
> From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
...
> Wall-clock time of git clone over HTTPS onto a FUSE passthrough
> filesystem with writeback caching disabled, 3 runs per variant:
>
> vscode (~1.26 GB pack): 84.5s -> 75.7s avg (10% faster)
> git/git (~306 MB pack): 22.6s -> 20.0s avg (11% faster)
Wow! This is much higher than I expected. Great find.
I imagine that other platforms or non-FUSE setups will not
have the same benefits. As long as they aren't _regressions_
then this is a great find.
> -/* We always read in 4kB chunks. */
> -static unsigned char input_buffer[4096];
> +static unsigned char input_buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
> -/* We always read in 4kB chunks. */
> -static unsigned char buffer[4096];
> +static unsigned char buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
These changes are what I expected in v2.
> diff --git a/csum-file.c b/csum-file.c
> index 9558177a11..c1aeaf587a 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -178,7 +178,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
> f->algop = unsafe_hash_algo(algop);
> f->algop->init_fn(&f->ctx);
>
> - f->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;
> + f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_PACKFILE_BUFFER_SIZE;
> f->buffer = xmalloc(f->buffer_len);
> f->check_buffer = NULL;
This one surprised me, as this hunk wasn't in your v1 patch.
I think using this replacement makes sense, since it _is_ an
exact value. It did make me think as to how we landed on 128K
for this example.
The previous line is due to a1118c0a446 (csum-file: introduce
`hashfd_ext()`, 2026-03-13), but it only moved the 128K default
from hashfd(). Notably, hashfd_throughput() still uses an 8K
setting in opt->buffer_len.
Hilariously, I went spelunking for the original reason for the
128K and it was 2ca245f8be5 (csum-file.h: increase hashfile
buffer size, 2021-05-18) written by...me. The motivation was
due to using the hashfile logic for the .git/index file which
also used 128K buffers in f279894 (read-cache: make the index
write buffer size 128K, 2021-02-18).
All this is to say that we now have two constants of identical
value, where WRITE_BUFFER_SIZE in read-cache.c could be replaced
with your new DEFAULT_PACKFILE_BUFFER_SIZE.
This does make me think that maybe DEFAULT_PACKFILE_BUFFER_SIZE
is misnamed? Should it be DEFAULT_HASHFILE_BUFFER_SIZE or
DEFAULT_FILESYSTEM_BUFFER_SIZE to better fit this size value
being used in both packfiles and index files?
> diff --git a/git-compat-util.h b/git-compat-util.h
> index ae1bdc90a4..a2f037811c 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -712,6 +712,12 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
> # endif
> #endif
>
> +/*
> + * Default buffer size for buffered I/O in pack file operations (index-pack,
> + * unpack-objects) and the hashfile layer in csum-file.
> + */
> +#define DEFAULT_PACKFILE_BUFFER_SIZE (128 * 1024)
> +
I see. Putting this in git-compat-util.h makes the rest
of the changes good without any need to add a new include.
Thanks,
-Stolee
^ permalink raw reply
* [PATCH 0/2] doc: log: fix --decorate description list
From: kristofferhaugsbakk @ 2026-04-27 19:06 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Jean-Noël Avila
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Topic name: kh/doc-log-decorate-list
Topic summary: Fix formatting of the '--decorate' description list.
[1/2] doc: log: fix --decorate description list
[2/2] doc: log: use the same delimiter in description list
Documentation/git-log.adoc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply
* [PATCH 1/2] doc: log: fix --decorate description list
From: kristofferhaugsbakk @ 2026-04-27 19:06 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <CV_doc_log_--decorate_list.626@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
026f2e3b (doc: convert git-log to new documentation format, 2025-07-07)
transformed the inline description of `--decorate` options to a
description list:
We also transform inline descriptions of possible values of option
--decorate into a list, which is more readable and extensible.
But a source code block was used instead of an open block.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Documentation/git-log.adoc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-log.adoc b/Documentation/git-log.adoc
index e304739c5e8..1c95499060d 100644
--- a/Documentation/git-log.adoc
+++ b/Documentation/git-log.adoc
@@ -36,14 +36,14 @@ OPTIONS
Print out the ref names of any commits that are shown. Possible values
are:
+
-----
+--
`short`;; the ref name prefixes `refs/heads/`, `refs/tags/` and
`refs/remotes/` are not printed.
`full`;; the full ref name (including prefix) is printed.
`auto`:: if the output is going to a terminal, the ref names
are shown as if `short` were given, otherwise no ref names are
shown.
-----
+--
+
The option `--decorate` is short-hand for `--decorate=short`. Default to
configuration value of `log.decorate` if configured, otherwise, `auto`.
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH 2/2] doc: log: use the same delimiter in description list
From: kristofferhaugsbakk @ 2026-04-27 19:06 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <CV_doc_log_--decorate_list.626@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
We must use the same delimiter since this is a meant to be a flat
list. Introducing a new legal delimiter like `::` makes an inner
description list:
...
full
the full ref name ...
auto
if the output ...
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Documentation/git-log.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-log.adoc b/Documentation/git-log.adoc
index 1c95499060d..fb3ac112839 100644
--- a/Documentation/git-log.adoc
+++ b/Documentation/git-log.adoc
@@ -40,7 +40,7 @@ OPTIONS
`short`;; the ref name prefixes `refs/heads/`, `refs/tags/` and
`refs/remotes/` are not printed.
`full`;; the full ref name (including prefix) is printed.
-`auto`:: if the output is going to a terminal, the ref names
+`auto`;; if the output is going to a terminal, the ref names
are shown as if `short` were given, otherwise no ref names are
shown.
--
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v3] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Scott Bauersfeld via GitGitGadget @ 2026-04-27 19:26 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Derrick Stolee, Scott Bauersfeld,
Scott Bauersfeld
In-Reply-To: <pull.2282.v2.git.git.1777306114914.gitgitgadget@gmail.com>
From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
index-pack and unpack-objects both read pack data from stdin through
a 4 KiB static buffer. In index-pack, each fill() flushes consumed
bytes to the pack file via write_or_die(), capping every write(2)
at 4 KiB. unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round
trip through the FUSE protocol (userspace -> kernel -> userspace ->
back), so the 4 KiB buffer turns a clone into many unnecessary tiny
writes with noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
DEFAULT_IO_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the
hashfile layer in csum-file (which already used 128 KiB but
hardcoded the value).
Syscall counts via strace on HTTPS clones of git/git (~296 MB pack,
5 runs per variant, isolated builds from the same v2.54.0 source):
index-pack pack file writes: 72,465 -> 24,943 avg (65% fewer)
total write() syscalls: 310,192 -> 259,530 avg (16% fewer)
writes of exactly 4096 bytes: ~40,077 -> 0
Wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled, 3 runs per variant:
vscode (~1.26 GB pack): 84.5s -> 75.7s avg (10% faster)
git/git (~306 MB pack): 22.6s -> 20.0s avg (11% faster)
Signed-off-by: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
---
index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
index-pack and unpack-objects read pack data from stdin through a 4 KiB
static buffer. In index-pack, each fill() flushes consumed bytes to the
pack file via write_or_die(), capping every write(2) at 4 KiB.
unpack-objects uses the same buffer pattern for reads.
On FUSE-backed filesystems every write(2) is a synchronous round trip
through the FUSE protocol (userspace → kernel → userspace → back), so
the 4 KiB buffer turns a clone into many unnecessary tiny writes with
noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
DEFAULT_IO_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the hashfile
layer in csum-file (which already used 128 KiB but hardcoded the value).
Syscall reduction
=================
Measured via strace -f on HTTPS clones of git/git (~296 MB pack, 5 runs
per variant, isolated builds from the same v2.54.0 source):
Metric Unpatched (4 KiB) Patched (128 KiB) Change index-pack writes to
pack file 72,465 avg 24,943 avg −65% Total write() syscalls (all
processes) 310,192 avg 259,530 avg −16% Writes of exactly 4096 bytes
~40,077 avg 0 eliminated HEAD / file count / fsck ✓ ✓ identical
Wall-clock time on FUSE
=======================
Measured wall-clock time of git clone over HTTPS onto a FUSE passthrough
filesystem with writeback caching disabled. 3 runs per variant:
Repo Unpatched avg Patched avg Change microsoft/vscode (~1.26 GB pack)
84.5s 75.7s −10% git/git (~306 MB pack) 22.6s 20.0s −11%
Changes since v2
================
* Renamed DEFAULT_PACKFILE_BUFFER_SIZE → DEFAULT_IO_BUFFER_SIZE per
Stolee's feedback. The constant is not packfile-specific, since it is
also used by the hashfile layer.
* Stolee noted that WRITE_BUFFER_SIZE in read-cache.c could be
consolidated. That constant was already removed in f6e2cd0625
("read-cache: delete unused hashing methods", 2021-05-18) when
read-cache.c was converted to use the hashfile API, so there is
nothing left to unify. The rename to DEFAULT_IO_BUFFER_SIZE helps
account for the multiple usages of this constant.
Changes since v1
================
* Introduced shared DEFAULT_PACKFILE_BUFFER_SIZE constant in
git-compat-util.h (next to MAX_IO_SIZE), replacing per-file #define
and the hardcoded value in csum-file.c. Placed here rather than
environment.h since it is an I/O buffer size, not an environment
variable or repo config.
* Added wall-clock timing on a FUSE filesystem.
* Cleaned up the commit description a bit.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2282%2Fsbauersfeld%2Fsb%2Fincrease-index-pack-input-buffer-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2282/sbauersfeld/sb/increase-index-pack-input-buffer-v3
Pull-Request: https://github.com/git/git/pull/2282
Range-diff vs v2:
1: ac2559ccb5 ! 1: df754ac879 index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
@@ Commit message
writes with noticeable latency overhead.
Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
- DEFAULT_PACKFILE_BUFFER_SIZE constant in git-compat-util.h (next to
+ DEFAULT_IO_BUFFER_SIZE constant in git-compat-util.h (next to
MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the
hashfile layer in csum-file (which already used 128 KiB but
hardcoded the value).
@@ builtin/index-pack.c: static int check_self_contained_and_connected;
-/* We always read in 4kB chunks. */
-static unsigned char input_buffer[4096];
-+static unsigned char input_buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
++static unsigned char input_buffer[DEFAULT_IO_BUFFER_SIZE];
static unsigned int input_offset, input_len;
static off_t consumed_bytes;
static off_t max_input_size;
@@ builtin/unpack-objects.c
-/* We always read in 4kB chunks. */
-static unsigned char buffer[4096];
-+static unsigned char buffer[DEFAULT_PACKFILE_BUFFER_SIZE];
++static unsigned char buffer[DEFAULT_IO_BUFFER_SIZE];
static unsigned int offset, len;
static off_t consumed_bytes;
static off_t max_input_size;
@@ csum-file.c: struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
f->algop->init_fn(&f->ctx);
- f->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;
-+ f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_PACKFILE_BUFFER_SIZE;
++ f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE;
f->buffer = xmalloc(f->buffer_len);
f->check_buffer = NULL;
@@ git-compat-util.h: static inline uint64_t u64_add(uint64_t a, uint64_t b)
#endif
+/*
-+ * Default buffer size for buffered I/O in pack file operations (index-pack,
-+ * unpack-objects) and the hashfile layer in csum-file.
++ * Default buffer size for buffered I/O in index-pack, unpack-objects,
++ * and the hashfile layer in csum-file.
+ */
-+#define DEFAULT_PACKFILE_BUFFER_SIZE (128 * 1024)
++#define DEFAULT_IO_BUFFER_SIZE (128 * 1024)
+
#ifdef HAVE_ALLOCA_H
# include <alloca.h>
builtin/index-pack.c | 3 +--
builtin/unpack-objects.c | 3 +--
csum-file.c | 2 +-
git-compat-util.h | 6 ++++++
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c..bb3639641c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -145,8 +145,7 @@ static int check_self_contained_and_connected;
static struct progress *progress;
-/* We always read in 4kB chunks. */
-static unsigned char input_buffer[4096];
+static unsigned char input_buffer[DEFAULT_IO_BUFFER_SIZE];
static unsigned int input_offset, input_len;
static off_t consumed_bytes;
static off_t max_input_size;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e01cf6e360..af67d1a1d3 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -23,8 +23,7 @@
static int dry_run, quiet, recover, has_errors, strict;
static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict]";
-/* We always read in 4kB chunks. */
-static unsigned char buffer[4096];
+static unsigned char buffer[DEFAULT_IO_BUFFER_SIZE];
static unsigned int offset, len;
static off_t consumed_bytes;
static off_t max_input_size;
diff --git a/csum-file.c b/csum-file.c
index 9558177a11..d7a682c2b6 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -178,7 +178,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
f->algop = unsafe_hash_algo(algop);
f->algop->init_fn(&f->ctx);
- f->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;
+ f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE;
f->buffer = xmalloc(f->buffer_len);
f->check_buffer = NULL;
diff --git a/git-compat-util.h b/git-compat-util.h
index ae1bdc90a4..5024814bd4 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -712,6 +712,12 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
# endif
#endif
+/*
+ * Default buffer size for buffered I/O in index-pack, unpack-objects,
+ * and the hashfile layer in csum-file.
+ */
+#define DEFAULT_IO_BUFFER_SIZE (128 * 1024)
+
#ifdef HAVE_ALLOCA_H
# include <alloca.h>
# define xalloca(size) (alloca(size))
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Derrick Stolee @ 2026-04-27 20:12 UTC (permalink / raw)
To: Scott Bauersfeld via GitGitGadget, git; +Cc: Junio C Hamano, Scott Bauersfeld
In-Reply-To: <pull.2282.v3.git.git.1777317998098.gitgitgadget@gmail.com>
On 4/27/2026 3:26 PM, Scott Bauersfeld via GitGitGadget wrote:
> From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
> Changes since v2
> ================
>
> * Renamed DEFAULT_PACKFILE_BUFFER_SIZE → DEFAULT_IO_BUFFER_SIZE per
> Stolee's feedback. The constant is not packfile-specific, since it is
> also used by the hashfile layer.
> * Stolee noted that WRITE_BUFFER_SIZE in read-cache.c could be
> consolidated. That constant was already removed in f6e2cd0625
> ("read-cache: delete unused hashing methods", 2021-05-18) when
> read-cache.c was converted to use the hashfile API, so there is
> nothing left to unify. The rename to DEFAULT_IO_BUFFER_SIZE helps
> account for the multiple usages of this constant.
Thank you for discovering this context which made my recommendation
non-actionable. I was looking at the commit that added the 128K limit,
which had that in its context, but not at the latest code. My mistake!
I'm very happy with this version and look forward to the performance
benefits!
Thanks,
-Stolee
^ permalink raw reply
* [PATCH v2 1/2] commit: name UTF-8 function appropriately
From: brian m. carlson @ 2026-04-27 22:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kushal Das, Elijah Newren
In-Reply-To: <aeakf0xcjSteTMZp@fruit.crustytoothpaste.net>
We have a function named verify_utf8, but it does more than verify, it
modifies the buffer if it is not UTF-8. This is different from what
most people would expect, so call the function ensure_utf8, since it
mutates the buffer in some cases.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
commit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/commit.c b/commit.c
index 80d8d07875..790dd2faed 100644
--- a/commit.c
+++ b/commit.c
@@ -1637,12 +1637,12 @@ static int find_invalid_utf8(const char *buf, int len)
}
/*
- * This verifies that the buffer is in proper utf8 format.
+ * This ensures that the buffer is in proper utf8 format.
*
* If it isn't, it assumes any non-utf8 characters are Latin1,
* and does the conversion.
*/
-static int verify_utf8(struct strbuf *buf)
+static int ensure_utf8(struct strbuf *buf)
{
int ok = 1;
long pos = 0;
@@ -1819,7 +1819,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
}
/* And check the encoding. */
- if (encoding_is_utf8 && (!verify_utf8(&buffer) || !verify_utf8(&compat_buffer)))
+ if (encoding_is_utf8 && (!ensure_utf8(&buffer) || !ensure_utf8(&compat_buffer)))
fprintf(stderr, _(commit_utf8_warn));
if (r->compat_hash_algo) {
^ permalink raw reply related
* [PATCH v2 2/2] commit: sign commit after mutating buffer
From: brian m. carlson @ 2026-04-27 22:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kushal Das, Elijah Newren
In-Reply-To: <20260427221834.1824543-1-sandals@crustytoothpaste.net>
The ensure_utf8 function can mutate the buffer to change its encoding,
so we must call it before signing the buffer so that we do not
invalidate the signature, which is made over raw bytes. Fix a bug which
caused the compatibility code to not convert the compatibility buffer if
the main buffer was invalid UTF-8. We expect both buffers to be valid
UTF-8 or both invalid, since the only data that would differ between
them would be hex object IDs, which are always valid UTF-8.
Add a test for this case using 0xfe and 0xff, which are never valid in
UTF-8.
Reported-by: Kushal Das <kushal@sunet.se>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
commit.c | 15 +++++++++++----
t/t7510-signed-commit.sh | 10 ++++++++++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/commit.c b/commit.c
index 790dd2faed..e5d725fe93 100644
--- a/commit.c
+++ b/commit.c
@@ -1726,6 +1726,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
struct repository *r = the_repository;
int result = 0;
int encoding_is_utf8;
+ bool warned = false;
struct strbuf buffer = STRBUF_INIT, compat_buffer = STRBUF_INIT;
struct strbuf sig = STRBUF_INIT, compat_sig = STRBUF_INIT;
struct object_id *parent_buf = NULL, *compat_oid = NULL;
@@ -1747,6 +1748,13 @@ int commit_tree_extended(const char *msg, size_t msg_len,
oidcpy(&parent_buf[i++], &p->item->object.oid);
write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
+
+ /* And check the encoding. */
+ if (encoding_is_utf8 && !ensure_utf8(&buffer)) {
+ fprintf(stderr, _(commit_utf8_warn));
+ warned = true;
+ }
+
if (sign_commit && sign_buffer(&buffer, &sig, sign_commit,
SIGN_BUFFER_USE_DEFAULT_KEY)) {
result = -1;
@@ -1780,6 +1788,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
free_commit_extra_headers(compat_extra);
free(mapped_parents);
+ if (encoding_is_utf8 && !ensure_utf8(&compat_buffer) && !warned)
+ fprintf(stderr, _(commit_utf8_warn));
+
if (sign_commit && sign_buffer(&compat_buffer, &compat_sig,
sign_commit,
SIGN_BUFFER_USE_DEFAULT_KEY)) {
@@ -1818,10 +1829,6 @@ int commit_tree_extended(const char *msg, size_t msg_len,
}
}
- /* And check the encoding. */
- if (encoding_is_utf8 && (!ensure_utf8(&buffer) || !ensure_utf8(&compat_buffer)))
- fprintf(stderr, _(commit_utf8_warn));
-
if (r->compat_hash_algo) {
hash_object_file(r->compat_hash_algo, compat_buffer.buf, compat_buffer.len,
OBJ_COMMIT, &compat_oid_buf);
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 1201c85ba6..aa9108da54 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -462,4 +462,14 @@ test_expect_success 'custom `gpg.program`' '
git commit -S --allow-empty -m signed-commit
'
+test_expect_success GPG 'commit verifies with non-UTF-8 commit message' '
+ printf "I hate\\376\\377UTF-8\\n" >message &&
+ echo unusual-message >file &&
+ git add file &&
+ test_tick && git commit -S -F message 2>err &&
+ git verify-commit HEAD &&
+ grep "commit message did not conform to UTF-8" err >lines &&
+ test_line_count = 1 lines
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH v3 1/2] revision.c: implement --reverse=before for walks
From: Junio C Hamano @ 2026-04-28 1:45 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble
In-Reply-To: <4864ac46dd8ef4b704c29efc96c45f4e1412373b.1776984666.git.mroik@delayed.space>
Mirko Faina <mroik@delayed.space> writes:
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 05cee9e41b..3bfe2c99b8 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
The hardcoded short object names are setting up traps to fail when
$ GIT_TEST_DEFAULT_HASH=sha256 make test
is run. It also may break when the default abbreviation length
and other things change.
> @@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '
> test_cmp_graph --name-status tangle..reach
> '
>
> +cat >expect <<-\EOF
> +c3f451c Merge tag 'reach'
> +046b221 to remove
> +EOF
> +test_expect_success 'log --reverse --oneline --max-count=2' '
> + test_when_finished git reset --hard HEAD~1 &&
> + touch to_remove &&
> + git add to_remove &&
> + git commit -m "to remove" &&
> + git log --reverse --oneline --max-count=2 >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '
> + test_when_finished git reset --hard HEAD~1 &&
> + touch to_remove &&
> + git add to_remove &&
> + git commit -m "to remove" &&
> + git log --reverse --reverse --reverse --oneline --max-count=2 >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'log --reverse=after --oneline --max-count=2' '
> + test_when_finished git reset --hard HEAD~1 &&
> + touch to_remove &&
> + git add to_remove &&
> + git commit -m "to remove" &&
> + git log --reverse=after --oneline --max-count=2 >actual &&
> + test_cmp expect actual
> +'
> +
> +cat >expect <<-\EOF
> +3a2fdcb initial
> +f7dab8e second
> +EOF
> +
> +test_expect_success 'log --reverse=before --oneline --max-count=2' '
> + test_when_finished rm actual &&
> + git log --reverse=before --oneline --max-count=2 >actual &&
> + test_cmp expect actual
> +'
> +
> +cat >expect <<-\EOF
> +046b221 to remove
> +c3f451c Merge tag 'reach'
> +EOF
> +
> +test_expect_success 'log --reverse --reverse --oneline --max-count=2' '
> + test_when_finished git reset --hard HEAD~1 &&
> + touch to_remove &&
> + git add to_remove &&
> + git commit -m "to remove" &&
> + git log --reverse --reverse --oneline --max-count=2 >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '
> + test_when_finished git reset --hard HEAD~1 &&
> + touch to_remove &&
> + git add to_remove &&
> + git commit -m "to remove" &&
> + git log --reverse --no-reverse --oneline --max-count=2 >actual &&
> + test_cmp expect actual
> +'
> +
> cat >expect <<-\EOF
> * reach
> |
^ permalink raw reply
* Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks
From: Junio C Hamano @ 2026-04-28 1:45 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble
In-Reply-To: <4864ac46dd8ef4b704c29efc96c45f4e1412373b.1777249165.git.mroik@delayed.space>
Mirko Faina <mroik@delayed.space> writes:
> + } else if (skip_prefix(arg, "--reverse=", &optarg)) {
> + if (!strcmp(optarg, "after"))
> + revs->reverse = REVERSE_AFTER;
> + else if(!strcmp(optarg, "before"))
Style: missing SP after "else if".
^ permalink raw reply
* Re: [PATCH v4 2/2] revision.c: reduce memory usage on reverse before
From: Junio C Hamano @ 2026-04-28 1:46 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble
In-Reply-To: <7c0bab5d14bb2ce2a10d35d93e3d911ed4c386eb.1777249165.git.mroik@delayed.space>
Mirko Faina <mroik@delayed.space> writes:
> reversed = NULL;
> - while ((c = get_revision_internal(revs)))
> - commit_list_insert(c, &reversed);
> + if (revs->reverse == REVERSE_BEFORE && max_count >= 0) {
> + retrieve_with_window(revs, max_count, &reversed);
> + } else {
> + while ((c = get_revision_internal(revs)))
> + commit_list_insert(c, &reversed);
> + }
Style: needless {} around single-statement body of if/else.
> commit_list_free(revs->commits);
> revs->commits = reversed;
> revs->reverse_output_stage = 1;
^ permalink raw reply
* Re: [PATCH v4 1/2] revision.c: implement -b-reverse=before for walks
From: Junio C Hamano @ 2026-04-28 1:46 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Chris Torek
In-Reply-To: <ae-J4ooz2PJ8bZAq@exploit>
Mirko Faina <mroik@delayed.space> writes:
>> It might be that the right way to look at this new feature is not that
>> "we are changing where reverse is applied", but "count limit is applied
>> much later than usual"
>
> maybe --max-count-later as in max count is being applied later than
> usual? (either way the users will still need to reach for the man pages
> for clarifications).
I very much more prefer what J6t suggested, which (if I am
understanding him correctly) would make
git log --max-count-oldest=3 $options
conceptually run "git log" (without count limit but with other
options like --grep, --author, --since, etc. applied) without
showing anything until the last three commits remains, and then
shows these last three commits.
git log --max-count-oldest=3 --reverse $options
would show these same last three commits, but in the order opposite
to the first one.
^ permalink raw reply
* Re: [PATCH] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Junio C Hamano @ 2026-04-28 1:46 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Scott Bauersfeld via GitGitGadget, git, Scott Bauersfeld
In-Reply-To: <c19a0e29-1218-4239-a362-df514153b5ff@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
>> The difference between that and 66% is coming from where? There are
>> inherently short writes that do not utilize the new larger buffer
>> beyond 4kB? If so, another number of interest might be the number
>> of writes smaller than 4096 bytes, perhaps?
>
> One way to reword what you're asking is to measure "number of writes
> not using the whole buffer" which is basically going to be "the
> number of flush events from the application layer".
I do not think it would differ between the old and the new
implementation.
> Every time the
> application intends to flush, the current buffer is likely to not
> be exactly full. I would expect this number to not change between
> implementations in real experiments.
Yes, I agree.
But what I was trying to get at was a bit different.
The application may have produced only 2kB before it issues a
"flush". Whether the buffer size is 4kB or 128kB, such a flush will
only write out 2kB, and the larger buffer size does not help at all.
But if the application has produced 90kB before it issues a "flush",
the larger buffer size would give us a great improvement. With 4kB
buffer, before such an application level "flush", we would have seen
22 = floor(90/4) calls of write(2) to flush the buffer, plus a 2kB
write(2). With 128kB buffer, we would see a single 90kB write(2).
So the apparently lower improvement than I naively have expected may
be attributable to the fact that many application level "flush" was
not large enough to benefit from 128kB buffer? How much of the
total number of bytes written came in large batches, vs tiny ones?
> The improvement here comes from the reduced number of flushes due
> to buffer limits.
Yes.
> I see that this can be measured in the number of
> system-level events, but what impact does this have on the end-to-
> end time of 'git index-pack' or 'git unpack-objects'? Is there a
> t/perf/ test that can demonstrate this improvement for a variety
> of real repos using GIT_PERF_REPO?
Interesting thought, but the number of system-level events (or the
number of write(2) system calls) is not reduced by 97% because we
apparently are issuing too many of them, and the reason is? I
suspect the reason why we still issue too many write(2) is because
we often do not send enough data between application-level flushes.
^ permalink raw reply
* Re: [PATCH v3] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Junio C Hamano @ 2026-04-28 1:47 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Scott Bauersfeld via GitGitGadget, git, Scott Bauersfeld
In-Reply-To: <469a26e8-4309-4221-abac-e9a09e3f743d@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
> On 4/27/2026 3:26 PM, Scott Bauersfeld via GitGitGadget wrote:
>> From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
>
>> Changes since v2
>> ================
>>
>> * Renamed DEFAULT_PACKFILE_BUFFER_SIZE → DEFAULT_IO_BUFFER_SIZE per
>> Stolee's feedback. The constant is not packfile-specific, since it is
>> also used by the hashfile layer.
>> * Stolee noted that WRITE_BUFFER_SIZE in read-cache.c could be
>> consolidated. That constant was already removed in f6e2cd0625
>> ("read-cache: delete unused hashing methods", 2021-05-18) when
>> read-cache.c was converted to use the hashfile API, so there is
>> nothing left to unify. The rename to DEFAULT_IO_BUFFER_SIZE helps
>> account for the multiple usages of this constant.
>
> Thank you for discovering this context which made my recommendation
> non-actionable. I was looking at the commit that added the 128K limit,
> which had that in its context, but not at the latest code. My mistake!
>
> I'm very happy with this version and look forward to the performance
> benefits!
>
> Thanks,
> -Stolee
Yes, this version was very pleasant to read. Thanks both.
^ permalink raw reply
* Re: [RFC PATCH v3 0/2] push: add support for pushing to remote groups
From: Junio C Hamano @ 2026-04-28 1:47 UTC (permalink / raw)
To: Usman Akinyemi; +Cc: christian.couder, git, me, phillip.wood123, ps
In-Reply-To: <20260427140530.856125-1-usmanakinyemi202@gmail.com>
Something like this is needed to workaround -Werror=sign-compare complaints.
diff --git a/builtin/push.c b/builtin/push.c
index 10384f265c..6021b71d66 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -552,7 +552,8 @@ static int push_multiple(struct string_list *list,
const char **refspecs,
int refspec_nr)
{
- int i, result = 0;
+ int result = 0;
+ size_t i;
struct strvec argv = STRVEC_INIT;
strvec_push(&argv, "push");
diff --git a/remote.h b/remote.h
index 8ff2bd88fa..7915be3111 100644
--- a/remote.h
+++ b/remote.h
@@ -430,8 +430,8 @@ struct push_cas_option {
unsigned use_tracking:1;
char *refname;
} *entry;
- int nr;
- int alloc;
+ size_t nr;
+ size_t alloc;
};
int parseopt_push_cas_option(const struct option *, const char *arg, int unset);
^ permalink raw reply related
* Re: [PATCH v4] checkout: add --fetch to fetch remote before resolving start-point
From: Junio C Hamano @ 2026-04-28 1:47 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk,
Marc Branchaud, Harald Nordgren
In-Reply-To: <pull.2281.v4.git.git.1777228346809.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> A common workflow is:
>
> git fetch origin
> git checkout -b new_branch origin/some-branch
>
> The first command exists purely so the second sees an up-to-date view
> of the remote.
This is only half true, isn't it?
Git is among projects that encourage forking only from a well-known
point in history (like the latest released version), not at a random
"tip of the day" commit from the upstream.
Such projects also tend to discourage people from constantly pulling
updated upstream into their unfinished topic branch, or rebase their
unfinished topic branch on top of updated upstream, only to "catch
up", and instead encourage them to make a trial merge to notice when
the base got too stale to cause eventual merge to conflict too much,
and when it happens, make such a back-merge, but otherwise keep
working on the stable base and avoid such "catching up".
And when working with such a project, what users who do the above is
forgetting is to inspect origin/master between the two steps to see
if it is a good commit to start your topic at.
> If it is forgotten, origin/some-branch points at a stale commit
> and the new local branch is created from the wrong start point.
So this part is not quite true. What makes your topic begin at a
wrong starting point is not that you forget to fetch, but you forget
to verify what you fetched and think if it is a good starting point.
And for that verification to happen, you do not want "checkout" and
"fetch" mixed into one.
On the other hand, if you are allowed to fork at anywhere (as
opposed to a latest release), then not fetching and building on top
of slightly older codebase is not such a huge deal, as you're likely
to be making the "catch up" changes on top of your unfinished work
later anyway.
So as I already said before, I am fairly negative on this topic. It
feels more like a knob to allow and actively encourage people to be
more sloppy than anything else.
I may have already pointed this out (but I do not remember), but
this option would not make any sense when --track is not in effect,
so instead of adding a brand new option, making it an extension to
the existing --track option might make it slightly more palatable.
^ permalink raw reply
* Re: [PATCH] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Jeff King @ 2026-04-28 2:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Derrick Stolee, Scott Bauersfeld via GitGitGadget, git,
Scott Bauersfeld
In-Reply-To: <xmqqecjz26wr.fsf@gitster.g>
On Tue, Apr 28, 2026 at 10:46:44AM +0900, Junio C Hamano wrote:
> The application may have produced only 2kB before it issues a
> "flush". Whether the buffer size is 4kB or 128kB, such a flush will
> only write out 2kB, and the larger buffer size does not help at all.
> But if the application has produced 90kB before it issues a "flush",
> the larger buffer size would give us a great improvement. With 4kB
> buffer, before such an application level "flush", we would have seen
> 22 = floor(90/4) calls of write(2) to flush the buffer, plus a 2kB
> write(2). With 128kB buffer, we would see a single 90kB write(2).
>
> So the apparently lower improvement than I naively have expected may
> be attributable to the fact that many application level "flush" was
> not large enough to benefit from 128kB buffer? How much of the
> total number of bytes written came in large batches, vs tiny ones?
The input to index-pack in a fetch is going to be the demuxing of the
sideband via git-fetch. So it's probably flushing 64k or less each time
(because that's the max size of a packet), and unless index-pack is
going much slower than the input, that maximizes how much it will read.
Depending on the source, though, it may be possible to go faster than
index-pack (which has to at least update the pack checksum for every
byte, and may even zlib inflate and hash the object itself if it's a
non-delta). In which case the sideband demuxer would start filling the
pipe and index-pack may get larger reads.
We could actually reduce the number of syscalls further if index-pack
did the demuxing itself, and we just handed it the descriptor. That
probably doesn't help all that much in this case, though, if the problem
is not raw reads/writes on pipes, but rather ones that go to the slow
FUSE filesystem. And as long as those pipe reads/writes are "wide"
(allowing the eventual filesystem writes to also be wide), then the
exact number may not be as important.
But the demuxing may also explain why the total number of writes did not
decrease as much as you expected, since those ones will probably not be
reduced by the patch in question. So the improvement is a percentage of
only a smaller portion of the total (but not necessarily half, because
they may have been larger writes in the first place).
-Peff
^ 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