* [PATCH v2] doc: add caveat about turning off commit-graph
From: kristofferhaugsbakk @ 2026-05-07 18:20 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Derrick Stolee
In-Reply-To: <caveat_commit-graph.671@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
The doc `technical/commit-graph.adoc` says that replace objects and
commit grafts turn off commit-graph:
Commit grafts and replace objects can change the shape of the commit
history. The latter can also be enabled/disabled on the fly using
`--no-replace-objects`. This leads to difficulty storing both possible
interpretations of a commit id, especially when computing generation
numbers. The commit-graph will not be read or written when
replace-objects or grafts are present.
But this isn’t mentioned in the user-facing doc. Let’s mention it on
git-replace(1) and git-commit-graph(1).
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v2: Incorporate “performance issues” suggestion on git-replace(1)
Documentation/git-commit-graph.adoc | 6 ++++++
Documentation/git-replace.adoc | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/Documentation/git-commit-graph.adoc b/Documentation/git-commit-graph.adoc
index 6d19026035f..f2a37e91634 100644
--- a/Documentation/git-commit-graph.adoc
+++ b/Documentation/git-commit-graph.adoc
@@ -146,6 +146,12 @@ $ git show-ref -s | git commit-graph write --stdin-commits
$ git rev-parse HEAD | git commit-graph write --stdin-commits --append
------------------------------------------------
+CAVEATS
+-------
+
+The existence of replace objects or commit grafts turns off reading or
+writing to the commit-graph. See linkgit:git-replace[1].
+
CONFIGURATION
-------------
diff --git a/Documentation/git-replace.adoc b/Documentation/git-replace.adoc
index 0a65460adbd..436a0e58caf 100644
--- a/Documentation/git-replace.adoc
+++ b/Documentation/git-replace.adoc
@@ -145,6 +145,13 @@ commit instead of the replaced commit.
There may be other problems when using 'git rev-list' related to
pending objects.
+CAVEATS
+-------
+
+The existence of replace objects or commit grafts turns off reading or
+writing to the commit-graph, which can cause performance issues. See
+linkgit:git-commit-graph[1].
+
SEE ALSO
--------
linkgit:git-hash-object[1]
Interdiff against v1:
diff --git a/Documentation/git-replace.adoc b/Documentation/git-replace.adoc
index 2c0ea07724d..436a0e58caf 100644
--- a/Documentation/git-replace.adoc
+++ b/Documentation/git-replace.adoc
@@ -149,7 +149,8 @@ CAVEATS
-------
The existence of replace objects or commit grafts turns off reading or
-writing to the commit-graph. See linkgit:git-commit-graph[1].
+writing to the commit-graph, which can cause performance issues. See
+linkgit:git-commit-graph[1].
SEE ALSO
--------
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* Re: [PATCH v2] doc: add caveat about turning off commit-graph
From: Derrick Stolee @ 2026-05-07 18:59 UTC (permalink / raw)
To: kristofferhaugsbakk, git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <V2_caveat_commit-graph.68b@msgid.xyz>
On 5/7/2026 2:20 PM, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> The doc `technical/commit-graph.adoc` says that replace objects and
> commit grafts turn off commit-graph:
>
> Commit grafts and replace objects can change the shape of the commit
> history. The latter can also be enabled/disabled on the fly using
> `--no-replace-objects`. This leads to difficulty storing both possible
> interpretations of a commit id, especially when computing generation
> numbers. The commit-graph will not be read or written when
> replace-objects or grafts are present.
>
> But this isn’t mentioned in the user-facing doc. Let’s mention it on
> git-replace(1) and git-commit-graph(1).
...
> Interdiff against v1:
> diff --git a/Documentation/git-replace.adoc b/Documentation/git-replace.adoc
> index 2c0ea07724d..436a0e58caf 100644
> --- a/Documentation/git-replace.adoc
> +++ b/Documentation/git-replace.adoc
> @@ -149,7 +149,8 @@ CAVEATS
> -------
>
> The existence of replace objects or commit grafts turns off reading or
> -writing to the commit-graph. See linkgit:git-commit-graph[1].
> +writing to the commit-graph, which can cause performance issues. See
> +linkgit:git-commit-graph[1].
Thanks for the update! LGTM.
-Stolee
^ permalink raw reply
* [PATCH v4 0/5] format-rev: introduce builtin for on-demand pretty formatting
From: kristofferhaugsbakk @ 2026-05-07 19:34 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ben.knoble, Phillip Wood, Ramsay Jones
In-Reply-To: <V3_CV_format-rev.66a@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
(Subject from v2: name-rev: learn --format=<pretty>)
Topic name (applied): kh/name-rev-custom-format
Topic summary: Introduce a new builtin for pretty formatting either (1) one
revision expression per line or (2) commit object names found in running
text.
See the last patch for the motivation. In short there isn’t anything that I
have found that lets you format however many commits you want through one
process (so looping over `git show` is excepted).
The other patches prepare for this change.
§ Changes in v4
Main changes are all in the final patch, with details on the Note there:
• Fix `-Werror=maybe-uninitialized` error
• Add NUL terminator options
• Use `maybe_flush_or_die(...)` to avoid potential deadlocks.
Also changes to patch 3/5 (see details on the Note).
There is also a lot of discussion about the input and output format and how
it works. I don’t know if it is valuable or just noise. Like this part:
Regarding input in this mode: using `-z` or `--null-input` makes sure
that _NUL_ characters in the input are passed through correctly.
This is just saying that NUL in the input will cut the processing short
unless `--null-input` is given (because we loop over a NUL-terminated C
string in `name_rev_line(...)`).
§ CI/CB
This time I ran GitHub actions (in my private fork) and let it
finish *before* sending this out.[1]
A lot of failures there but I did not see any errors relating to my
changes. I could just see `git-compat-util.h` errors and C11 `Generic`.
† 1: https://lore.kernel.org/git/374661c1-4676-4538-af24-0564f38469ca@app.fastmail.com/
§ Why experimental command? (unchanged in v4)
This command is marked Experimental three times (like git-replay(1)).[2]
Not so much because the UI design seems difficult. It’s more so that it can
be thrown out if it doesn’t end up being worth having around.
(*Experimental* also implies wholesale trashing. Right? That’s one possible
change in behavior.)
Or the behavior here could be moved somewhere else.
† 2: I vaguely recall reading a good argument for emphasizing this on the
mailing list. I wasn’t able to find back to it right now.
§ Outstanding work
I could look more into how to do the translation strings for “X is
require” (arg), I’m probably missing something that already exists.
§ Link to v3
https://lore.kernel.org/git/V3_format-rev_new_builtin.66f@msgid.xyz/
[1/5] name-rev: wrap both blocks in braces
[2/5] name-rev: run clang-format before factoring code
[3/5] name-rev: factor code for sharing with a new command
[4/5] name-rev: make dedicated --annotate-stdin --name-only test
[5/5] format-rev: introduce builtin for on-demand pretty formatting
.gitignore | 1 +
Documentation/git-format-rev.adoc | 215 +++++++++++++++++++++
Documentation/meson.build | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/name-rev.c | 300 +++++++++++++++++++++++++++---
command-list.txt | 1 +
git.c | 1 +
t/t1517-outside-repo.sh | 3 +-
t/t6120-describe.sh | 208 +++++++++++++++++++++
10 files changed, 706 insertions(+), 26 deletions(-)
create mode 100644 Documentation/git-format-rev.adoc
Interdiff against v3:
diff --git a/.gitignore b/.gitignore
index 24635cf2d6f..e406d3741cd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -71,6 +71,7 @@
/git-for-each-ref
/git-for-each-repo
/git-format-patch
+/git-format-rev
/git-fsck
/git-fsck-objects
/git-fsmonitor--daemon
diff --git a/Documentation/git-format-rev.adoc b/Documentation/git-format-rev.adoc
index d960001d750..436980012bc 100644
--- a/Documentation/git-format-rev.adoc
+++ b/Documentation/git-format-rev.adoc
@@ -9,7 +9,7 @@ git-format-rev - EXPERIMENTAL: Pretty format revisions on demand
SYNOPSIS
--------
[synopsis]
-(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--notes=<ref>]
+(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--[no-]notes=<ref>] [-z] [--[no-]null-output] [--[no-]null-input]
DESCRIPTION
-----------
@@ -25,7 +25,8 @@ OPTIONS
How to interpret standard input data:
+
--
-`revs`:: Each line is interpreted as a commit. Any kind of revision
+`revs`:: Each line or record (see the <<io,INPUT AND OUTPUT FORMATS>>
+ section) is interpreted as a commit. Any kind of revision
expression can be used (see linkgit:gitrevisions[7]). Annotated
tags are peeled (see linkgit:gitglossary[7]).
+
@@ -33,6 +34,9 @@ The argument `rev` is also accepted.
`text`:: Formats all commit object names found in freeform text. These
must the full object names, i.e. abbreviated hexidecimal object
names will not be interpreted.
++
+Anything that is parsed as an object name but that is not found to be a
+commit object name is left alone (echoed).
--
`--format=<pretty>`::
@@ -43,6 +47,63 @@ The argument `rev` is also accepted.
Custom notes ref. Notes are displayed when using the `%N`
atom. See linkgit:git-notes[1].
+`-z`::
+`--null`::
+ Use _NUL_ character to terminate both input and output instead
+ of newline. This option cannot be negated.
++
+This is useful if both the input and output could contain newlines or if
+the input could contain _NUL_ characters; see the <<io,INPUT AND OUTPUT
+FORMATS>> section.
+
+`--null-output`::
+`--no-null-output`::
+ Use _NUL_ character to terminate output instead of newline. The
+ default is `--no-null-output`.
++
+This is useful if the output could contain newlines, for example if the
+`%n` (newline) atom is used.
+
+`--null-input`::
+`--no-null-input`::
+ Use _NUL_ character to terminate input instead of newline. The
+ default is `--no-null-input`.
++
+This is useful if the input revision expressions could contain newlines.
+It is also useful if the input could contain _NUL_ characters; see the
+<<io,INPUT AND OUTPUT FORMATS>> section.
+
+[[io]]
+INPUT AND OUTPUT FORMAT
+-----------------------
+
+The command uses newlines for both input and output termination by
+default. See the `-z`, `--null-output`, and `--null-input` options for
+using _NUL_ character as the terminator.
+
+The mode `--stdin-mode=revs` outputs one formatted commit followed by
+the terminator. This could either be called a _line_ or a _record_ in
+case "line" is too suggestive of newline termination.
+
+Note that this means that the terminator character (newline or _NUL_)
+acts as a _terminator_, not a _separator_. In other words, the final
+line or record is also terminated by the terminator character.
+
+The mode `--stdin-mode=text` replaces each object name with the
+formatted commit, i.e. the format `%s` would transform the object name
+`abcdef012...` to `<subject>` without any termination. Like this:
+
+----
+Did we not fix this in "<subject>"?
+----
+
+Regarding input in this mode: using `-z` or `--null-input` makes sure
+that _NUL_ characters in the input are passed through correctly.
+
+It is safe to interactively read and write from this command since each
+record is immediately flushed.
+
+[[examples]]
EXAMPLES
--------
@@ -110,7 +171,8 @@ DISCUSSION
This command lets you format any number of revisions in any order
through one command invocation. Consider the
-linkgit:git-last-modified[1] case from the "EXAMPLES" section above:
+linkgit:git-last-modified[1] case from the <<examples,EXAMPLES>> section
+above:
1. There might be hundreds of files
2. Commits can be repeated, i.e. two or more files were last modified in
@@ -123,7 +185,7 @@ not a good fit for the above use case.
- The output of linkgit:git-last-modified[1] would have to be processed
in stages since you need to transform the first column separately and
then link the author to the filename. But this is surmountable.
-- You can feed each commit to `git show` or `git show --no-walk -1`. But
+- You can feed each commit to `git show` or `git log --no-walk -1`. But
that means that you need to create a process for each line.
- Let’s say that you want to use one process, not one per line. So you
want to feed all the commits to the command. Now you face the problem
@@ -143,6 +205,11 @@ process per line. It is much more work if you just want to use one
process, but still doable. In contrast, this problem is just another
shell pipeline with this command.
+SEE ALSO
+--------
+linkgit:git-name-rev[1],
+linkgit:git-log[1].
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/Documentation/meson.build b/Documentation/meson.build
index d6365b888bb..58e7c6a0b8a 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -55,6 +55,7 @@ manpages = {
'git-for-each-ref.adoc' : 1,
'git-for-each-repo.adoc' : 1,
'git-format-patch.adoc' : 1,
+ 'git-format-rev.adoc' : 1,
'git-fsck-objects.adoc' : 1,
'git-fsck.adoc' : 1,
'git-fsmonitor--daemon.adoc' : 1,
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index b60cc766279..5494b0424b3 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -21,6 +21,7 @@
#include "pretty.h"
#include "revision.h"
#include "notes.h"
+#include "write-or-die.h"
/*
* One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ -577,7 +578,6 @@ static void name_rev_line(char *p, struct command *cmd)
for (p_start = p; *p; p++) {
#define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))
- start:
if (!ishex(*p)) {
counter = 0;
} else if (++counter == hexsz &&
@@ -593,6 +593,7 @@ static void name_rev_line(char *p, struct command *cmd)
*(p + 1) = 0;
oid_ret = repo_get_oid(the_repository, p - (hexsz - 1), &oid);
+ *(p + 1) = c;
switch (cmd->type) {
case NAME_REV:
@@ -600,9 +601,8 @@ static void name_rev_line(char *p, struct command *cmd)
o = lookup_object(the_repository, &oid);
if (o)
name = get_rev_name(o, &buf);
- *(p + 1) = c;
if (!name)
- goto start;
+ continue;
if (cmd->u.name_only)
printf("%.*s%s", p_len - hexsz, p_start, name);
else
@@ -615,7 +615,6 @@ static void name_rev_line(char *p, struct command *cmd)
name = get_format_rev((const struct commit *)o,
cmd->u.pretty_format,
&buf);
- *(p + 1) = c;
if (name)
printf("%.*s%s", p_len - hexsz, p_start, name);
else
@@ -775,6 +774,23 @@ int cmd_name_rev(int argc,
return 0;
}
+struct format_nul_data {
+ bool nul_input;
+ bool nul_output;
+};
+
+static int format_nul_cb(const struct option *option,
+ const char *arg,
+ int unset)
+{
+ struct format_nul_data *data = option->value;
+ data->nul_input = 1;
+ data->nul_output = 1;
+ BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
+ return 0;
+}
+
static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
{
if (!strcmp(stdin_mode, "text"))
@@ -788,7 +804,9 @@ static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
}
static char const *const format_rev_usage[] = {
- N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--notes=<ref>]"),
+ N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> "
+ "--format=<pretty> [--[no-]notes=<ref>] "
+ "[-z] [--[no-]null-output] [--[no-]null-input]"),
NULL
};
@@ -800,6 +818,9 @@ int cmd_format_rev(int argc,
const char *format = NULL;
enum stdin_mode stdin_mode;
const char *stdin_mode_arg = NULL;
+ struct format_nul_data nul_data = { 0, 0 };
+ char output_terminator;
+ strbuf_getline_fn getline_fn;
struct display_notes_opt format_notes_opt;
struct rev_info format_rev = REV_INFO_INIT;
struct pretty_format format_pp = { 0 };
@@ -813,6 +834,13 @@ int cmd_format_rev(int argc,
N_("how revs are processed")),
OPT_STRING_LIST(0, "notes", ¬es, N_("notes"),
N_("display notes for pretty format")),
+ OPT_CALLBACK_F('z', "null", &nul_data, N_("z"),
+ N_("Use NUL for input and output termination"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, format_nul_cb),
+ OPT_BOOL(0, "null-input", &nul_data.nul_input,
+ N_("Use NUL for input termination")),
+ OPT_BOOL(0, "null-output", &nul_data.nul_output,
+ N_("Use NUL for output termination")),
OPT_END(),
};
@@ -828,6 +856,9 @@ int cmd_format_rev(int argc,
if (!stdin_mode_arg)
die(_("'%s' is required"), "--stdin-mode");
+ getline_fn = nul_data.nul_input ? strbuf_getline_nul : strbuf_getline_lf;
+ output_terminator = nul_data.nul_output ? '\0' : '\n';
+
init_display_notes(&format_notes_opt);
stdin_mode = parse_stdin_mode(stdin_mode_arg);
@@ -856,20 +887,24 @@ int cmd_format_rev(int argc,
switch (stdin_mode) {
case TEXT:
- while (strbuf_getline(&scratch_buf, stdin) != EOF) {
- strbuf_addch(&scratch_buf, '\n');
+ while (getline_fn(&scratch_buf, stdin) != EOF) {
name_rev_line(scratch_buf.buf, &cmd);
+ /*
+ * We do not pass on the terminator to name_rev_line,
+ * unlike name-rev.
+ */
+ printf("%c", output_terminator);
+ maybe_flush_or_die(stdout, "stdout");
}
break;
case REVS:
- while (strbuf_getline(&scratch_buf, stdin) != EOF) {
+ while (getline_fn(&scratch_buf, stdin) != EOF) {
struct object_id oid;
struct object *object;
struct object *peeled;
- struct commit *commit;
if (repo_get_oid(the_repository, scratch_buf.buf, &oid)) {
- fprintf(stderr, "Could not get sha1 for %s. Skipping.\n",
+ fprintf(stderr, "Could not get object name for %s. Skipping.\n",
scratch_buf.buf);
continue;
}
@@ -882,16 +917,17 @@ int cmd_format_rev(int argc,
}
peeled = deref_tag(the_repository, object, scratch_buf.buf, 0);
- if (peeled && peeled->type == OBJ_COMMIT)
- commit = (struct commit *)peeled;
- if (!commit) {
- fprintf(stderr, "Could not get commit for %s. Skipping.\n",
- *argv);
+ if (!peeled || peeled->type != OBJ_COMMIT) {
+ fprintf(stderr,
+ "Could not get commit for %s. Skipping.\n",
+ scratch_buf.buf);
continue;
}
- get_format_rev(commit, &format_pp, &scratch_buf);
- printf("%s\n", scratch_buf.buf);
+ get_format_rev((struct commit *)peeled,
+ &format_pp, &scratch_buf);
+ printf("%s%c", scratch_buf.buf, output_terminator);
+ maybe_flush_or_die(stdout, "stdout");
strbuf_release(&scratch_buf);
}
break;
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 725f7d81b6b..8ee3d2c37d0 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -801,7 +801,7 @@ test_expect_success 'do not be fooled by invalid describe format ' '
test_must_fail git cat-file -t "refs/tags/super-invalid/./../...../ ~^:/?*[////\\\\\\&}/busted.lock-42-g"$(cat out)
'
-test_expect_success 'name-rev --format setup' '
+test_expect_success 'setup: format-rev' '
mkdir repo-format &&
git -C repo-format init &&
test_commit -C repo-format first &&
@@ -839,8 +839,8 @@ test_expect_success 'format-rev --stdin-mode=text from rev-list same as log' '
'
test_expect_success 'format-rev --stdin-mode=text with running text and tree oid' '
- cmit_oid=$(git -C repo-format rev-parse :/fifth) &&
- reference=$(git -C repo-format log -n1 --format=reference :/fifth) &&
+ cmit_oid=$(git -C repo-format rev-parse fifth) &&
+ reference=$(git -C repo-format log -n1 --format=reference fifth) &&
tree=$(git -C repo-format rev-parse HEAD^{tree}) &&
cat >expect <<-EOF &&
We thought we fixed this in ${reference}.
@@ -899,10 +899,100 @@ test_expect_success 'format-rev --stdin-mode=revs on annotated tag peels to comm
EOF
git -C repo-format format-rev --stdin-mode=revs \
--format=%s \
- --notes=* >actual <<-\EOF &&
+ >actual <<-\EOF &&
version
EOF
test_cmp expect actual
'
+test_expect_success 'format-rev --stdin-mode=revs lookup failures' '
+ test_when_finished "git -C repo-format tag -d tag-to-tree" &&
+ invalid_syntax=not-valid &&
+ non_existing_oid=${EMPTY_BLOB} &&
+ tree=$(git -C repo-format rev-parse eighth^{tree}) &&
+ git -C repo-format tag -a -mmessage tag-to-tree "$tree" &&
+ tag_to_tree=$(git -C repo-format rev-parse tag-to-tree) &&
+ cat >expect <<-EOF &&
+ Could not get object name for ${invalid_syntax}. Skipping.
+ Could not get object for ${non_existing_oid}. Skipping.
+ Could not get commit for ${tree}. Skipping.
+ Could not get commit for ${tag_to_tree}. Skipping.
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s \
+ 2>actual >out <<-EOF &&
+ ${invalid_syntax}
+ ${non_existing_oid}
+ ${tree}
+ ${tag_to_tree}
+ EOF
+ test_line_count = 0 out &&
+ test_cmp expect actual
+'
+
+
+test_expect_success 'format-rev -z --stdin-mode=text with object name lookup failures' '
+ printf "%s\0" "$(git -C repo-format rev-parse HEAD)" >input &&
+ printf "%s\0" "$(git -C repo-format rev-parse HEAD^{tree})" >>input &&
+ printf "%s\0" "$EMPTY_BLOB" >>input &&
+ printf "%s\0" "$(git -C repo-format log --format=%s -1)" >expect &&
+ printf "%s\0" "$(git -C repo-format rev-parse HEAD^{tree})" >>expect &&
+ printf "%s\0" "$EMPTY_BLOB" >>expect &&
+ git -C repo-format format-rev --stdin-mode=text \
+ --format=%s -z <input >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'setup: format-rev input and output separators' '
+ git -C repo-format rev-list HEAD >input-lf &&
+ git -C repo-format rev-list -z HEAD >input-nul &&
+ git -C repo-format log --format=%s >output-lf &&
+ git -C repo-format log -z --format=%s >output-nul &&
+ echo revs >stdin-modes &&
+ echo text >>stdin-modes
+'
+
+while read mode
+do
+ test_expect_success "format-rev -z --stdin-mode=$mode" '
+ cat output-nul >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s -z <input-nul >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev -z --no-null-input --no-null-output --stdin-mode=$mode" '
+ cat output-lf >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s -z --no-null-input --no-null-output \
+ <input-lf >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev ---null-input --stdin-mode=$mode" '
+ cat output-lf >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s --null-input \
+ <input-nul >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev --null-output --stdin-mode=$mode" '
+ cat output-nul >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s --null-output \
+ <input-lf >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev -z --stdin-mode=$mode with multi-line output" '
+ format="%s%n%aI" &&
+ git -C repo-format log -z --format="$format" \
+ >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format="$format" -z <input-nul >actual &&
+ test_cmp expect actual
+ '
+done <stdin-modes
+
test_done
Range-diff against v3:
1: 9cb5cfd1ec3 = 1: 9cb5cfd1ec3 name-rev: wrap both blocks in braces
2: 14900271321 = 2: 14900271321 name-rev: run clang-format before factoring code
3: 1f17f0b0090 ! 3: 724ec022894 name-rev: factor code for sharing with a new command
@@ builtin/name-rev.c: static char const * const name_rev_usage[] = {
struct strbuf buf = STRBUF_INIT;
int counter = 0;
@@ builtin/name-rev.c: static void name_rev_line(char *p, struct name_ref_data *data)
-
- for (p_start = p; *p; p++) {
- #define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))
-+ start:
- if (!ishex(*p)) {
- counter = 0;
- } else if (++counter == hexsz &&
-@@ builtin/name-rev.c: static void name_rev_line(char *p, struct name_ref_data *data)
const char *name = NULL;
char c = *(p + 1);
int p_len = p - p_start + 1;
@@ builtin/name-rev.c: static void name_rev_line(char *p, struct name_ref_data *dat
- struct object *o =
- lookup_object(the_repository, &oid);
+ oid_ret = repo_get_oid(the_repository, p - (hexsz - 1), &oid);
++ *(p + 1) = c;
+
+ switch (cmd->type) {
+ case NAME_REV:
@@ builtin/name-rev.c: static void name_rev_line(char *p, struct name_ref_data *dat
+ o = lookup_object(the_repository, &oid);
if (o)
name = get_rev_name(o, &buf);
-+ *(p + 1) = c;
+ if (!name)
-+ goto start;
++ continue;
+ if (cmd->u.name_only)
+ printf("%.*s%s", p_len - hexsz, p_start, name);
+ else
4: 40989b3672a = 4: 382efc3ddb8 name-rev: make dedicated --annotate-stdin --name-only test
5: bb54e8f753e ! 5: 049a45e32bc format-rev: introduce builtin for on-demand pretty formatting
@@ Commit message
`t/t1517-outside-repo.sh` because it uses “EXPERIMENTAL!”
in the usage line.
+ Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
+ Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
+ ## .gitignore ##
+@@
+ /git-for-each-ref
+ /git-for-each-repo
+ /git-format-patch
++/git-format-rev
+ /git-fsck
+ /git-fsck-objects
+ /git-fsmonitor--daemon
+
## Documentation/git-format-rev.adoc (new) ##
@@
+git-format-rev(1)
@@ Documentation/git-format-rev.adoc (new)
+SYNOPSIS
+--------
+[synopsis]
-+(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--notes=<ref>]
++(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--[no-]notes=<ref>] [-z] [--[no-]null-output] [--[no-]null-input]
+
+DESCRIPTION
+-----------
@@ Documentation/git-format-rev.adoc (new)
+ How to interpret standard input data:
++
+--
-+`revs`:: Each line is interpreted as a commit. Any kind of revision
++`revs`:: Each line or record (see the <<io,INPUT AND OUTPUT FORMATS>>
++ section) is interpreted as a commit. Any kind of revision
+ expression can be used (see linkgit:gitrevisions[7]). Annotated
+ tags are peeled (see linkgit:gitglossary[7]).
++
@@ Documentation/git-format-rev.adoc (new)
+`text`:: Formats all commit object names found in freeform text. These
+ must the full object names, i.e. abbreviated hexidecimal object
+ names will not be interpreted.
+++
++Anything that is parsed as an object name but that is not found to be a
++commit object name is left alone (echoed).
+--
+
+`--format=<pretty>`::
@@ Documentation/git-format-rev.adoc (new)
+ Custom notes ref. Notes are displayed when using the `%N`
+ atom. See linkgit:git-notes[1].
+
++`-z`::
++`--null`::
++ Use _NUL_ character to terminate both input and output instead
++ of newline. This option cannot be negated.
+++
++This is useful if both the input and output could contain newlines or if
++the input could contain _NUL_ characters; see the <<io,INPUT AND OUTPUT
++FORMATS>> section.
++
++`--null-output`::
++`--no-null-output`::
++ Use _NUL_ character to terminate output instead of newline. The
++ default is `--no-null-output`.
+++
++This is useful if the output could contain newlines, for example if the
++`%n` (newline) atom is used.
++
++`--null-input`::
++`--no-null-input`::
++ Use _NUL_ character to terminate input instead of newline. The
++ default is `--no-null-input`.
+++
++This is useful if the input revision expressions could contain newlines.
++It is also useful if the input could contain _NUL_ characters; see the
++<<io,INPUT AND OUTPUT FORMATS>> section.
++
++[[io]]
++INPUT AND OUTPUT FORMAT
++-----------------------
++
++The command uses newlines for both input and output termination by
++default. See the `-z`, `--null-output`, and `--null-input` options for
++using _NUL_ character as the terminator.
++
++The mode `--stdin-mode=revs` outputs one formatted commit followed by
++the terminator. This could either be called a _line_ or a _record_ in
++case "line" is too suggestive of newline termination.
++
++Note that this means that the terminator character (newline or _NUL_)
++acts as a _terminator_, not a _separator_. In other words, the final
++line or record is also terminated by the terminator character.
++
++The mode `--stdin-mode=text` replaces each object name with the
++formatted commit, i.e. the format `%s` would transform the object name
++`abcdef012...` to `<subject>` without any termination. Like this:
++
++----
++Did we not fix this in "<subject>"?
++----
++
++Regarding input in this mode: using `-z` or `--null-input` makes sure
++that _NUL_ characters in the input are passed through correctly.
++
++It is safe to interactively read and write from this command since each
++record is immediately flushed.
++
++[[examples]]
+EXAMPLES
+--------
+
@@ Documentation/git-format-rev.adoc (new)
+
+This command lets you format any number of revisions in any order
+through one command invocation. Consider the
-+linkgit:git-last-modified[1] case from the "EXAMPLES" section above:
++linkgit:git-last-modified[1] case from the <<examples,EXAMPLES>> section
++above:
+
+1. There might be hundreds of files
+2. Commits can be repeated, i.e. two or more files were last modified in
@@ Documentation/git-format-rev.adoc (new)
+- The output of linkgit:git-last-modified[1] would have to be processed
+ in stages since you need to transform the first column separately and
+ then link the author to the filename. But this is surmountable.
-+- You can feed each commit to `git show` or `git show --no-walk -1`. But
++- You can feed each commit to `git show` or `git log --no-walk -1`. But
+ that means that you need to create a process for each line.
+- Let’s say that you want to use one process, not one per line. So you
+ want to feed all the commits to the command. Now you face the problem
@@ Documentation/git-format-rev.adoc (new)
+process, but still doable. In contrast, this problem is just another
+shell pipeline with this command.
+
++SEE ALSO
++--------
++linkgit:git-name-rev[1],
++linkgit:git-log[1].
++
+GIT
+---
+Part of the linkgit:git[1] suite
+ ## Documentation/meson.build ##
+@@ Documentation/meson.build: manpages = {
+ 'git-for-each-ref.adoc' : 1,
+ 'git-for-each-repo.adoc' : 1,
+ 'git-format-patch.adoc' : 1,
++ 'git-format-rev.adoc' : 1,
+ 'git-fsck-objects.adoc' : 1,
+ 'git-fsck.adoc' : 1,
+ 'git-fsmonitor--daemon.adoc' : 1,
+
## Makefile ##
@@ Makefile: BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))
BUILT_INS += git-cherry$X
@@ builtin/name-rev.c
+#include "pretty.h"
+#include "revision.h"
+#include "notes.h"
++#include "write-or-die.h"
/*
* One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ builtin/name-rev.c: static void name_rev_line(char *p, struct command *cmd)
+ name = get_format_rev((const struct commit *)o,
+ cmd->u.pretty_format,
+ &buf);
-+ *(p + 1) = c;
+ if (name)
+ printf("%.*s%s", p_len - hexsz, p_start, name);
+ else
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
return 0;
}
+
++struct format_nul_data {
++ bool nul_input;
++ bool nul_output;
++};
++
++static int format_nul_cb(const struct option *option,
++ const char *arg,
++ int unset)
++{
++ struct format_nul_data *data = option->value;
++ data->nul_input = 1;
++ data->nul_output = 1;
++ BUG_ON_OPT_NEG(unset);
++ BUG_ON_OPT_ARG(arg);
++ return 0;
++}
++
+static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
+{
+ if (!strcmp(stdin_mode, "text"))
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
+}
+
+static char const *const format_rev_usage[] = {
-+ N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--notes=<ref>]"),
++ N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> "
++ "--format=<pretty> [--[no-]notes=<ref>] "
++ "[-z] [--[no-]null-output] [--[no-]null-input]"),
+ NULL
+};
+
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
+ const char *format = NULL;
+ enum stdin_mode stdin_mode;
+ const char *stdin_mode_arg = NULL;
++ struct format_nul_data nul_data = { 0, 0 };
++ char output_terminator;
++ strbuf_getline_fn getline_fn;
+ struct display_notes_opt format_notes_opt;
+ struct rev_info format_rev = REV_INFO_INIT;
+ struct pretty_format format_pp = { 0 };
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
+ N_("how revs are processed")),
+ OPT_STRING_LIST(0, "notes", ¬es, N_("notes"),
+ N_("display notes for pretty format")),
++ OPT_CALLBACK_F('z', "null", &nul_data, N_("z"),
++ N_("Use NUL for input and output termination"),
++ PARSE_OPT_NOARG | PARSE_OPT_NONEG, format_nul_cb),
++ OPT_BOOL(0, "null-input", &nul_data.nul_input,
++ N_("Use NUL for input termination")),
++ OPT_BOOL(0, "null-output", &nul_data.nul_output,
++ N_("Use NUL for output termination")),
+ OPT_END(),
+ };
+
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
+ if (!stdin_mode_arg)
+ die(_("'%s' is required"), "--stdin-mode");
+
++ getline_fn = nul_data.nul_input ? strbuf_getline_nul : strbuf_getline_lf;
++ output_terminator = nul_data.nul_output ? '\0' : '\n';
++
+ init_display_notes(&format_notes_opt);
+ stdin_mode = parse_stdin_mode(stdin_mode_arg);
+
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
+
+ switch (stdin_mode) {
+ case TEXT:
-+ while (strbuf_getline(&scratch_buf, stdin) != EOF) {
-+ strbuf_addch(&scratch_buf, '\n');
++ while (getline_fn(&scratch_buf, stdin) != EOF) {
+ name_rev_line(scratch_buf.buf, &cmd);
++ /*
++ * We do not pass on the terminator to name_rev_line,
++ * unlike name-rev.
++ */
++ printf("%c", output_terminator);
++ maybe_flush_or_die(stdout, "stdout");
+ }
+ break;
+ case REVS:
-+ while (strbuf_getline(&scratch_buf, stdin) != EOF) {
++ while (getline_fn(&scratch_buf, stdin) != EOF) {
+ struct object_id oid;
+ struct object *object;
+ struct object *peeled;
-+ struct commit *commit;
+
+ if (repo_get_oid(the_repository, scratch_buf.buf, &oid)) {
-+ fprintf(stderr, "Could not get sha1 for %s. Skipping.\n",
++ fprintf(stderr, "Could not get object name for %s. Skipping.\n",
+ scratch_buf.buf);
+ continue;
+ }
@@ builtin/name-rev.c: int cmd_name_rev(int argc,
+ }
+
+ peeled = deref_tag(the_repository, object, scratch_buf.buf, 0);
-+ if (peeled && peeled->type == OBJ_COMMIT)
-+ commit = (struct commit *)peeled;
-+ if (!commit) {
-+ fprintf(stderr, "Could not get commit for %s. Skipping.\n",
-+ *argv);
++ if (!peeled || peeled->type != OBJ_COMMIT) {
++ fprintf(stderr,
++ "Could not get commit for %s. Skipping.\n",
++ scratch_buf.buf);
+ continue;
+ }
+
-+ get_format_rev(commit, &format_pp, &scratch_buf);
-+ printf("%s\n", scratch_buf.buf);
++ get_format_rev((struct commit *)peeled,
++ &format_pp, &scratch_buf);
++ printf("%s%c", scratch_buf.buf, output_terminator);
++ maybe_flush_or_die(stdout, "stdout");
+ strbuf_release(&scratch_buf);
+ }
+ break;
@@ t/t6120-describe.sh: test_expect_success 'do not be fooled by invalid describe f
test_must_fail git cat-file -t "refs/tags/super-invalid/./../...../ ~^:/?*[////\\\\\\&}/busted.lock-42-g"$(cat out)
'
-+test_expect_success 'name-rev --format setup' '
++test_expect_success 'setup: format-rev' '
+ mkdir repo-format &&
+ git -C repo-format init &&
+ test_commit -C repo-format first &&
@@ t/t6120-describe.sh: test_expect_success 'do not be fooled by invalid describe f
+'
+
+test_expect_success 'format-rev --stdin-mode=text with running text and tree oid' '
-+ cmit_oid=$(git -C repo-format rev-parse :/fifth) &&
-+ reference=$(git -C repo-format log -n1 --format=reference :/fifth) &&
++ cmit_oid=$(git -C repo-format rev-parse fifth) &&
++ reference=$(git -C repo-format log -n1 --format=reference fifth) &&
+ tree=$(git -C repo-format rev-parse HEAD^{tree}) &&
+ cat >expect <<-EOF &&
+ We thought we fixed this in ${reference}.
@@ t/t6120-describe.sh: test_expect_success 'do not be fooled by invalid describe f
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s \
-+ --notes=* >actual <<-\EOF &&
++ >actual <<-\EOF &&
+ version
+ EOF
+ test_cmp expect actual
+'
++
++test_expect_success 'format-rev --stdin-mode=revs lookup failures' '
++ test_when_finished "git -C repo-format tag -d tag-to-tree" &&
++ invalid_syntax=not-valid &&
++ non_existing_oid=${EMPTY_BLOB} &&
++ tree=$(git -C repo-format rev-parse eighth^{tree}) &&
++ git -C repo-format tag -a -mmessage tag-to-tree "$tree" &&
++ tag_to_tree=$(git -C repo-format rev-parse tag-to-tree) &&
++ cat >expect <<-EOF &&
++ Could not get object name for ${invalid_syntax}. Skipping.
++ Could not get object for ${non_existing_oid}. Skipping.
++ Could not get commit for ${tree}. Skipping.
++ Could not get commit for ${tag_to_tree}. Skipping.
++ EOF
++ git -C repo-format format-rev --stdin-mode=revs \
++ --format=%s \
++ 2>actual >out <<-EOF &&
++ ${invalid_syntax}
++ ${non_existing_oid}
++ ${tree}
++ ${tag_to_tree}
++ EOF
++ test_line_count = 0 out &&
++ test_cmp expect actual
++'
++
++
++test_expect_success 'format-rev -z --stdin-mode=text with object name lookup failures' '
++ printf "%s\0" "$(git -C repo-format rev-parse HEAD)" >input &&
++ printf "%s\0" "$(git -C repo-format rev-parse HEAD^{tree})" >>input &&
++ printf "%s\0" "$EMPTY_BLOB" >>input &&
++ printf "%s\0" "$(git -C repo-format log --format=%s -1)" >expect &&
++ printf "%s\0" "$(git -C repo-format rev-parse HEAD^{tree})" >>expect &&
++ printf "%s\0" "$EMPTY_BLOB" >>expect &&
++ git -C repo-format format-rev --stdin-mode=text \
++ --format=%s -z <input >actual &&
++ test_cmp expect actual
++'
++
++test_expect_success 'setup: format-rev input and output separators' '
++ git -C repo-format rev-list HEAD >input-lf &&
++ git -C repo-format rev-list -z HEAD >input-nul &&
++ git -C repo-format log --format=%s >output-lf &&
++ git -C repo-format log -z --format=%s >output-nul &&
++ echo revs >stdin-modes &&
++ echo text >>stdin-modes
++'
++
++while read mode
++do
++ test_expect_success "format-rev -z --stdin-mode=$mode" '
++ cat output-nul >expect &&
++ git -C repo-format format-rev --stdin-mode="$mode" \
++ --format=%s -z <input-nul >actual &&
++ test_cmp expect actual
++ '
++
++ test_expect_success "format-rev -z --no-null-input --no-null-output --stdin-mode=$mode" '
++ cat output-lf >expect &&
++ git -C repo-format format-rev --stdin-mode="$mode" \
++ --format=%s -z --no-null-input --no-null-output \
++ <input-lf >actual &&
++ test_cmp expect actual
++ '
++
++ test_expect_success "format-rev ---null-input --stdin-mode=$mode" '
++ cat output-lf >expect &&
++ git -C repo-format format-rev --stdin-mode="$mode" \
++ --format=%s --null-input \
++ <input-nul >actual &&
++ test_cmp expect actual
++ '
++
++ test_expect_success "format-rev --null-output --stdin-mode=$mode" '
++ cat output-nul >expect &&
++ git -C repo-format format-rev --stdin-mode="$mode" \
++ --format=%s --null-output \
++ <input-lf >actual &&
++ test_cmp expect actual
++ '
++
++ test_expect_success "format-rev -z --stdin-mode=$mode with multi-line output" '
++ format="%s%n%aI" &&
++ git -C repo-format log -z --format="$format" \
++ >expect &&
++ git -C repo-format format-rev --stdin-mode="$mode" \
++ --format="$format" -z <input-nul >actual &&
++ test_cmp expect actual
++ '
++done <stdin-modes
+
test_done
base-commit: 67006b9db8b772423ad0706029286096307d2567
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v4 1/5] name-rev: wrap both blocks in braces
From: kristofferhaugsbakk @ 2026-05-07 19:34 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ben.knoble, Phillip Wood, Ramsay Jones
In-Reply-To: <V4_CV_format-rev.6aa@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
See `CodingGuidelines`:
- When there are multiple arms to a conditional and some of them
require braces, enclose even a single line block in braces for
consistency. [...]
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v2:
Fix stray formatting of `(p+1)`
builtin/name-rev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 6188cf98ce0..171e7bd0e98 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -466,9 +466,9 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf)
if (!n)
return NULL;
- if (!n->generation)
+ if (!n->generation) {
return n->tip_name;
- else {
+ } else {
strbuf_reset(buf);
strbuf_addstr(buf, n->tip_name);
strbuf_strip_suffix(buf, "^0");
@@ -516,9 +516,9 @@ static void name_rev_line(char *p, struct name_ref_data *data)
for (p_start = p; *p; p++) {
#define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))
- if (!ishex(*p))
+ if (!ishex(*p)) {
counter = 0;
- else if (++counter == hexsz &&
+ } else if (++counter == hexsz &&
!ishex(*(p+1))) {
struct object_id oid;
const char *name = NULL;
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v4 2/5] name-rev: run clang-format before factoring code
From: kristofferhaugsbakk @ 2026-05-07 19:34 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ben.knoble, Phillip Wood, Ramsay Jones
In-Reply-To: <V4_CV_format-rev.6aa@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
We are about to move code around to prepare for adding a new
command. Let’s deal with clang-format changes first in the affected
areas.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
builtin/name-rev.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 171e7bd0e98..6357eaa76d0 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -519,22 +519,22 @@ static void name_rev_line(char *p, struct name_ref_data *data)
if (!ishex(*p)) {
counter = 0;
} else if (++counter == hexsz &&
- !ishex(*(p+1))) {
+ !ishex(*(p + 1))) {
struct object_id oid;
const char *name = NULL;
- char c = *(p+1);
+ char c = *(p + 1);
int p_len = p - p_start + 1;
counter = 0;
- *(p+1) = 0;
+ *(p + 1) = 0;
if (!repo_get_oid(the_repository, p - (hexsz - 1), &oid)) {
struct object *o =
lookup_object(the_repository, &oid);
if (o)
name = get_rev_name(o, &buf);
}
- *(p+1) = c;
+ *(p + 1) = c;
if (!name)
continue;
@@ -571,9 +571,9 @@ int cmd_name_rev(int argc,
OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
- N_("only use refs matching <pattern>")),
+ N_("only use refs matching <pattern>")),
OPT_STRING_LIST(0, "exclude", &data.exclude_filters, N_("pattern"),
- N_("ignore refs matching <pattern>")),
+ N_("ignore refs matching <pattern>")),
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
#ifndef WITH_BREAKING_CHANGES
@@ -585,10 +585,10 @@ int cmd_name_rev(int argc,
#endif /* WITH_BREAKING_CHANGES */
OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
- OPT_BOOL(0, "always", &always,
- N_("show abbreviated commit object as fallback")),
+ OPT_BOOL(0, "always", &always,
+ N_("show abbreviated commit object as fallback")),
OPT_HIDDEN_BOOL(0, "peel-tag", &peel_tag,
- N_("dereference tags in the input (internal use)")),
+ N_("dereference tags in the input (internal use)")),
OPT_END(),
};
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v4 3/5] name-rev: factor code for sharing with a new command
From: kristofferhaugsbakk @ 2026-05-07 19:34 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ben.knoble, Phillip Wood, Ramsay Jones
In-Reply-To: <V4_CV_format-rev.6aa@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
We are about to introduce a new command git-format-rev(1) to this
file. Let’s factor some code so that we can share it with the new
command.
We want to be able to format commits found in freeform text, and
git-name-rev(1) already has a function for that but for symbolic
names. Let’s use a tagged union for the command-specific payload.
No functional changes.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v4:
• Pull out `*(p + 1)` instead of doing it in every `case` (Phillip)
• Go back to using `continue` instead of `goto` (Phillip)
builtin/name-rev.c | 53 ++++++++++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 6357eaa76d0..475efb0b82b 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -272,6 +272,24 @@ struct name_ref_data {
struct string_list exclude_filters;
};
+enum command_type {
+ NAME_REV = 1,
+};
+
+struct command {
+ enum command_type type;
+ union {
+ int name_only;
+ } u;
+};
+
+static void init_name_rev_command(struct command *cmd,
+ int name_only)
+{
+ cmd->type = NAME_REV;
+ cmd->u.name_only = name_only;
+}
+
static struct tip_table {
struct tip_table_entry {
struct object_id oid;
@@ -507,7 +525,7 @@ static char const * const name_rev_usage[] = {
NULL
};
-static void name_rev_line(char *p, struct name_ref_data *data)
+static void name_rev_line(char *p, struct command *cmd)
{
struct strbuf buf = STRBUF_INIT;
int counter = 0;
@@ -524,25 +542,32 @@ static void name_rev_line(char *p, struct name_ref_data *data)
const char *name = NULL;
char c = *(p + 1);
int p_len = p - p_start + 1;
+ struct object *o = NULL;
+ int oid_ret = 1;
counter = 0;
*(p + 1) = 0;
- if (!repo_get_oid(the_repository, p - (hexsz - 1), &oid)) {
- struct object *o =
- lookup_object(the_repository, &oid);
+ oid_ret = repo_get_oid(the_repository, p - (hexsz - 1), &oid);
+ *(p + 1) = c;
+
+ switch (cmd->type) {
+ case NAME_REV:
+ if (!oid_ret)
+ o = lookup_object(the_repository, &oid);
if (o)
name = get_rev_name(o, &buf);
+ if (!name)
+ continue;
+ if (cmd->u.name_only)
+ printf("%.*s%s", p_len - hexsz, p_start, name);
+ else
+ printf("%.*s (%s)", p_len, p_start, name);
+ break;
+ default:
+ BUG("uncovered case: %d", cmd->type);
}
- *(p + 1) = c;
-
- if (!name)
- continue;
- if (data->name_only)
- printf("%.*s%s", p_len - hexsz, p_start, name);
- else
- printf("%.*s (%s)", p_len, p_start, name);
p_start = p + 1;
}
}
@@ -567,6 +592,7 @@ int cmd_name_rev(int argc,
#endif
int all = 0, annotate_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
+ struct command cmd;
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
@@ -596,6 +622,7 @@ int cmd_name_rev(int argc,
init_commit_rev_name(&rev_names);
repo_config(the_repository, git_default_config, NULL);
argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
+ init_name_rev_command(&cmd, data.name_only);
#ifndef WITH_BREAKING_CHANGES
if (transform_stdin) {
@@ -663,7 +690,7 @@ int cmd_name_rev(int argc,
while (strbuf_getline(&sb, stdin) != EOF) {
strbuf_addch(&sb, '\n');
- name_rev_line(sb.buf, &data);
+ name_rev_line(sb.buf, &cmd);
}
strbuf_release(&sb);
} else if (all) {
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v4 4/5] name-rev: make dedicated --annotate-stdin --name-only test
From: kristofferhaugsbakk @ 2026-05-07 19:34 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ben.knoble, Phillip Wood, Ramsay Jones
In-Reply-To: <V4_CV_format-rev.6aa@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
The previous commit split the `--name-only` handling:
1. `--annotate-stdin`: uses the new `struct command`
2. The rest: uses `struct name_ref_data`
But there is no dedicated test for the option combination in (1). That
means that the following tests will fail if you neglect to set
`command.u.name_only` properly:
name-rev --annotate-stdin works with commitGraph
name-rev --annotate-stdin works with non-monotonic timestamps
even though it has nothing to do with what these tests are supposed
to test.
Let’s add another regression test now that it is relevant.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
t/t6120-describe.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 2c70cc561ad..62789f76381 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -298,6 +298,20 @@ test_expect_success 'name-rev --annotate-stdin' '
test_cmp expect actual
'
+test_expect_success 'name-rev --annotate-stdin --name-only' '
+ >expect.unsorted &&
+ for rev in $(git rev-list --all)
+ do
+ name=$(git name-rev --name-only $rev) &&
+ echo "$name" >>expect.unsorted || return 1
+ done &&
+ sort <expect.unsorted >expect &&
+ git name-rev --annotate-stdin --name-only \
+ <list >actual.unsorted &&
+ sort <actual.unsorted >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'name-rev --stdin deprecated' '
git rev-list --all >list &&
if ! test_have_prereq WITH_BREAKING_CHANGES
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v4 5/5] format-rev: introduce builtin for on-demand pretty formatting
From: kristofferhaugsbakk @ 2026-05-07 19:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, ben.knoble, Phillip Wood, Ramsay Jones,
Junio C Hamano
In-Reply-To: <V4_CV_format-rev.6aa@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Introduce a new builtin for pretty formatting one revision expression
per line or commit object names found in running text.
Sometimes you want to format commits. Most of the time you’re
walking the graph, e.g. getting a range of commits like
`master..topic`. That’s a job for git-log(1).
But there are times when you want to format commits that you encounter
on demand:
• Full hashes in running text that you might want to pretty-print
• git-last-modified(1) outputs full hashes that you can do the same
with
• git-cherry(1) has `-v` for commit subject, but maybe you want
something else?
But now you can’t use git-log(1), git-show(1), or git-rev-list(1):
• You can’t feed commits piecemeal to these commands, one input
for one output; they block until standard in is closed
• You can’t feed a list of possibly duplicate commits, like the output
of git-last-modified(1); they effectively deduplicate the output
Beyond these two points there’s also the input massage problem: you
cannot feed mixed input (revisions mixed with arbitrary text).
One might hope that git-cat-file(1) can save us. But it doesn’t
support pretty formats.
But there is one command that already both handles revisions as
arguments, revisions on standard input, and even revisions mixed in
with arbitrary text. Namely git-name-rev(1): the command for outputting
symbolic names for commits.
We made some room in `builtin/name-rev.c` two commits ago. Let’s
now add this new git-format-rev(1) command. Taking inspiration from
git-name-rev(1), there are two modes:
• revs: like git-name-rev(1) in argv mode, but one revision per line
on standard in
• text: like git-name-rev(1) with `--annotate-stdin`
***
We need to add this command to the exception list in
`t/t1517-outside-repo.sh` because it uses “EXPERIMENTAL!”
in the usage line.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v4:
• Squash in “SQUASH???” commits from Junio’s branch:
• Fix -Werror=maybe-uninitialized at builtin/name-rev.c:893:25
• Add this new builtin to Meson build file as well
• Flush every output line/record with `maybe_flush_or_die(...)` (Phillip)
• Introduce `-z` and other NUL output options (Phillip)
• Tests:
• More tests: “name lookup failures” (This would have caught my
uninitialized mistake from v3)
• More tests: for NUL/LF termination
• Fix stale setup command which uses “name-rev” in the name
• Use `setup:` prefix for both (now two) setup tests
• Drop some `:/fifth` expressions in favor just `fifth` (the tag from
`test_commit`)
• Flesh out documentation
• --stdin-mode=revs: Replace “Could not get [sha1][object name]” since the
user could be using SHA256
• This error string was originally stolen from name-rev
• Remember to add this command to `.gitignore` this time
• Correct doc to `git [show][log] --no-walk -1` (next to `git show`)
• Plus some minor things!
v3:
• And don’t forget to document --notes this time
https://lore.kernel.org/git/CALnO6CB5WOTp_e7Kv3CrEbQ+3XE-gDxNVHf7qATBEbyKWfxpLg@mail.gmail.com/
.gitignore | 1 +
Documentation/git-format-rev.adoc | 215 ++++++++++++++++++++++++++++
Documentation/meson.build | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/name-rev.c | 223 ++++++++++++++++++++++++++++++
command-list.txt | 1 +
git.c | 1 +
t/t1517-outside-repo.sh | 3 +-
t/t6120-describe.sh | 194 ++++++++++++++++++++++++++
10 files changed, 640 insertions(+), 1 deletion(-)
create mode 100644 Documentation/git-format-rev.adoc
diff --git a/.gitignore b/.gitignore
index 24635cf2d6f..e406d3741cd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -71,6 +71,7 @@
/git-for-each-ref
/git-for-each-repo
/git-format-patch
+/git-format-rev
/git-fsck
/git-fsck-objects
/git-fsmonitor--daemon
diff --git a/Documentation/git-format-rev.adoc b/Documentation/git-format-rev.adoc
new file mode 100644
index 00000000000..436980012bc
--- /dev/null
+++ b/Documentation/git-format-rev.adoc
@@ -0,0 +1,215 @@
+git-format-rev(1)
+=================
+
+NAME
+----
+git-format-rev - EXPERIMENTAL: Pretty format revisions on demand
+
+
+SYNOPSIS
+--------
+[synopsis]
+(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> --format=<pretty> [--[no-]notes=<ref>] [-z] [--[no-]null-output] [--[no-]null-input]
+
+DESCRIPTION
+-----------
+
+Pretty format revisions from standard input.
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+
+OPTIONS
+-------
+
+`--stdin-mode=<mode>`::
+ How to interpret standard input data:
++
+--
+`revs`:: Each line or record (see the <<io,INPUT AND OUTPUT FORMATS>>
+ section) is interpreted as a commit. Any kind of revision
+ expression can be used (see linkgit:gitrevisions[7]). Annotated
+ tags are peeled (see linkgit:gitglossary[7]).
++
+The argument `rev` is also accepted.
+`text`:: Formats all commit object names found in freeform text. These
+ must the full object names, i.e. abbreviated hexidecimal object
+ names will not be interpreted.
++
+Anything that is parsed as an object name but that is not found to be a
+commit object name is left alone (echoed).
+--
+
+`--format=<pretty>`::
+ Pretty format string.
+
+`--notes=<ref>`::
+`--no-notes`::
+ Custom notes ref. Notes are displayed when using the `%N`
+ atom. See linkgit:git-notes[1].
+
+`-z`::
+`--null`::
+ Use _NUL_ character to terminate both input and output instead
+ of newline. This option cannot be negated.
++
+This is useful if both the input and output could contain newlines or if
+the input could contain _NUL_ characters; see the <<io,INPUT AND OUTPUT
+FORMATS>> section.
+
+`--null-output`::
+`--no-null-output`::
+ Use _NUL_ character to terminate output instead of newline. The
+ default is `--no-null-output`.
++
+This is useful if the output could contain newlines, for example if the
+`%n` (newline) atom is used.
+
+`--null-input`::
+`--no-null-input`::
+ Use _NUL_ character to terminate input instead of newline. The
+ default is `--no-null-input`.
++
+This is useful if the input revision expressions could contain newlines.
+It is also useful if the input could contain _NUL_ characters; see the
+<<io,INPUT AND OUTPUT FORMATS>> section.
+
+[[io]]
+INPUT AND OUTPUT FORMAT
+-----------------------
+
+The command uses newlines for both input and output termination by
+default. See the `-z`, `--null-output`, and `--null-input` options for
+using _NUL_ character as the terminator.
+
+The mode `--stdin-mode=revs` outputs one formatted commit followed by
+the terminator. This could either be called a _line_ or a _record_ in
+case "line" is too suggestive of newline termination.
+
+Note that this means that the terminator character (newline or _NUL_)
+acts as a _terminator_, not a _separator_. In other words, the final
+line or record is also terminated by the terminator character.
+
+The mode `--stdin-mode=text` replaces each object name with the
+formatted commit, i.e. the format `%s` would transform the object name
+`abcdef012...` to `<subject>` without any termination. Like this:
+
+----
+Did we not fix this in "<subject>"?
+----
+
+Regarding input in this mode: using `-z` or `--null-input` makes sure
+that _NUL_ characters in the input are passed through correctly.
+
+It is safe to interactively read and write from this command since each
+record is immediately flushed.
+
+[[examples]]
+EXAMPLES
+--------
+
+The command linkgit:git-last-modified[1] shows the commit that each file
+was last modified in.
+
+----
+$ git last-modified -- README.md Makefile
+7798034171030be0909c56377a4e0e10e6d2df93 Makefile
+c50fbb2dd225e7e82abba4380423ae105089f4d7 README.md
+----
+
+We can pipe the result to this command in order to replace the object
+name with the commit author.
+
+----
+$ git last-modified -- README.md Makefile |
+ git format-rev --stdin-mode=text --format=%an
+Junio C Hamano Makefile
+Todd Zullinger README.md
+----
+
+Another example is _formatting commits in commit messages_. Given this commit message:
+
+----
+Fix off-by-one error
+
+Fix off-by-one error introduced in
+e83c5163316f89bfbde7d9ab23ca2e25604af290.
+
+We thought we fixed this in 5569bf9bbedd63a00780fc5c110e0cfab3aa97b9 but
+that only covered 1/3 of the faulty cases.
+----
+
+We can format the commits and use par(1) to reflow the text, say in a
+`commit-msg` hook:
+
+----
+$ git config set hook.reference-commits.event commit-msg
+$ git config set hook.reference-commits.command reference-commits
+$ cat $(which reference-commits)
+#/bin/sh
+
+msg="$1"
+rewritten=$(mktemp)
+git format-rev --stdin-mode=text --format=reference <"$msg" |
+ par >"$rewritten"
+mv "$rewritten" "$msg"
+----
+
+Which will produce something like this:
+
+----
+Fix off-by-one error
+
+Fix off-by-one error introduced in e83c5163316 (Implement better memory
+allocator, 2005-04-07).
+
+We thought we fixed this in 5569bf9bbed (Fix memory allocator,
+2005-06-22) but that only covered 1/3 of the faulty cases.
+----
+
+DISCUSSION
+----------
+
+This command lets you format any number of revisions in any order
+through one command invocation. Consider the
+linkgit:git-last-modified[1] case from the <<examples,EXAMPLES>> section
+above:
+
+1. There might be hundreds of files
+2. Commits can be repeated, i.e. two or more files were last modified in
+ the same commit
+
+Two widely-used commands which pretty formats commits are
+linkgit:git-log[1] and linkgit:git-show[1]. It turns out that they are
+not a good fit for the above use case.
+
+- The output of linkgit:git-last-modified[1] would have to be processed
+ in stages since you need to transform the first column separately and
+ then link the author to the filename. But this is surmountable.
+- You can feed each commit to `git show` or `git log --no-walk -1`. But
+ that means that you need to create a process for each line.
+- Let’s say that you want to use one process, not one per line. So you
+ want to feed all the commits to the command. Now you face the problem
+ that you have to feed all the commits to the commands before you get
+ any output (this is also the case for the `--stdin` modes). In other
+ words, you cannot loop through each line, get the author for the
+ commit, and output the author and the filename. You need to feed all
+ the commits, get back all the output, and match the output with the
+ filename.
+- But the next problem is that commands will deduplicate the input and
+ only output one commit one single time only. Thus you cannot make the
+ output order match the input order, since a commit could have been
+ repeated in the original input.
+
+In short, it is straightforward to use these two commands if you use one
+process per line. It is much more work if you just want to use one
+process, but still doable. In contrast, this problem is just another
+shell pipeline with this command.
+
+SEE ALSO
+--------
+linkgit:git-name-rev[1],
+linkgit:git-log[1].
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/meson.build b/Documentation/meson.build
index d6365b888bb..58e7c6a0b8a 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -55,6 +55,7 @@ manpages = {
'git-for-each-ref.adoc' : 1,
'git-for-each-repo.adoc' : 1,
'git-format-patch.adoc' : 1,
+ 'git-format-rev.adoc' : 1,
'git-fsck-objects.adoc' : 1,
'git-fsck.adoc' : 1,
'git-fsmonitor--daemon.adoc' : 1,
diff --git a/Makefile b/Makefile
index 15b1ded1a0b..cbaf91fd846 100644
--- a/Makefile
+++ b/Makefile
@@ -895,6 +895,7 @@ BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))
BUILT_INS += git-cherry$X
BUILT_INS += git-cherry-pick$X
BUILT_INS += git-format-patch$X
+BUILT_INS += git-format-rev$X
BUILT_INS += git-fsck-objects$X
BUILT_INS += git-init$X
BUILT_INS += git-maintenance$X
diff --git a/builtin.h b/builtin.h
index 235c51f30e5..63813c90125 100644
--- a/builtin.h
+++ b/builtin.h
@@ -189,6 +189,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix, struct re
int cmd_for_each_ref(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_for_each_repo(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_format_patch(int argc, const char **argv, const char *prefix, struct repository *repo);
+int cmd_format_rev(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_fsck(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_gc(int argc, const char **argv, const char *prefix, struct repository *repo);
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 475efb0b82b..5494b0424b3 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -18,6 +18,10 @@
#include "commit-graph.h"
#include "wildmatch.h"
#include "mem-pool.h"
+#include "pretty.h"
+#include "revision.h"
+#include "notes.h"
+#include "write-or-die.h"
/*
* One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ -272,14 +276,26 @@ struct name_ref_data {
struct string_list exclude_filters;
};
+struct pretty_format {
+ struct pretty_print_context ctx;
+ struct userformat_want want;
+};
+
enum command_type {
NAME_REV = 1,
+ FORMAT_REV = 2,
+};
+
+enum stdin_mode {
+ TEXT = 1,
+ REVS = 2,
};
struct command {
enum command_type type;
union {
int name_only;
+ struct pretty_format *pretty_format;
} u;
};
@@ -290,6 +306,13 @@ static void init_name_rev_command(struct command *cmd,
cmd->u.name_only = name_only;
}
+static void init_format_rev_command(struct command *cmd,
+ struct pretty_format *pretty_format)
+{
+ cmd->type = FORMAT_REV;
+ cmd->u.pretty_format = pretty_format;
+}
+
static struct tip_table {
struct tip_table_entry {
struct object_id oid;
@@ -495,6 +518,27 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf)
}
}
+static const char *get_format_rev(const struct commit *c,
+ struct pretty_format *format_ctx,
+ struct strbuf *buf)
+{
+ strbuf_reset(buf);
+
+ if (format_ctx->want.notes) {
+ struct strbuf notebuf = STRBUF_INIT;
+
+ format_display_notes(&c->object.oid, ¬ebuf,
+ get_log_output_encoding(),
+ format_ctx->ctx.fmt == CMIT_FMT_USERFORMAT);
+ format_ctx->ctx.notes_message = strbuf_detach(¬ebuf, NULL);
+ }
+
+ pretty_print_commit(&format_ctx->ctx, c, buf);
+ FREE_AND_NULL(format_ctx->ctx.notes_message);
+
+ return buf->buf;
+}
+
static void show_name(const struct object *obj,
const char *caller_name,
int always, int allow_undefined, int name_only)
@@ -564,6 +608,18 @@ static void name_rev_line(char *p, struct command *cmd)
else
printf("%.*s (%s)", p_len, p_start, name);
break;
+ case FORMAT_REV:
+ if (!oid_ret)
+ o = parse_object(the_repository, &oid);
+ if (o && o->type == OBJ_COMMIT)
+ name = get_format_rev((const struct commit *)o,
+ cmd->u.pretty_format,
+ &buf);
+ if (name)
+ printf("%.*s%s", p_len - hexsz, p_start, name);
+ else
+ printf("%.*s", p_len, p_start);
+ break;
default:
BUG("uncovered case: %d", cmd->type);
}
@@ -717,3 +773,170 @@ int cmd_name_rev(int argc,
object_array_clear(&revs);
return 0;
}
+
+struct format_nul_data {
+ bool nul_input;
+ bool nul_output;
+};
+
+static int format_nul_cb(const struct option *option,
+ const char *arg,
+ int unset)
+{
+ struct format_nul_data *data = option->value;
+ data->nul_input = 1;
+ data->nul_output = 1;
+ BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
+ return 0;
+}
+
+static enum stdin_mode parse_stdin_mode(const char *stdin_mode)
+{
+ if (!strcmp(stdin_mode, "text"))
+ return TEXT;
+ else if (!strcmp(stdin_mode, "revs") ||
+ !strcmp(stdin_mode, "rev"))
+ return REVS;
+ else
+ die(_("'%s' needs to be either text, revs, or rev"),
+ "--stdin-mode");
+}
+
+static char const *const format_rev_usage[] = {
+ N_("(EXPERIMENTAL!) git format-rev --stdin-mode=<mode> "
+ "--format=<pretty> [--[no-]notes=<ref>] "
+ "[-z] [--[no-]null-output] [--[no-]null-input]"),
+ NULL
+};
+
+int cmd_format_rev(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo UNUSED)
+{
+ const char *format = NULL;
+ enum stdin_mode stdin_mode;
+ const char *stdin_mode_arg = NULL;
+ struct format_nul_data nul_data = { 0, 0 };
+ char output_terminator;
+ strbuf_getline_fn getline_fn;
+ struct display_notes_opt format_notes_opt;
+ struct rev_info format_rev = REV_INFO_INIT;
+ struct pretty_format format_pp = { 0 };
+ struct string_list notes = STRING_LIST_INIT_NODUP;
+ struct strbuf scratch_buf = STRBUF_INIT;
+ struct command cmd;
+ struct option opts[] = {
+ OPT_STRING(0, "format", &format, N_("format"),
+ N_("pretty format to use")),
+ OPT_STRING(0, "stdin-mode", &stdin_mode_arg, N_("stdin-mode"),
+ N_("how revs are processed")),
+ OPT_STRING_LIST(0, "notes", ¬es, N_("notes"),
+ N_("display notes for pretty format")),
+ OPT_CALLBACK_F('z', "null", &nul_data, N_("z"),
+ N_("Use NUL for input and output termination"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, format_nul_cb),
+ OPT_BOOL(0, "null-input", &nul_data.nul_input,
+ N_("Use NUL for input termination")),
+ OPT_BOOL(0, "null-output", &nul_data.nul_output,
+ N_("Use NUL for output termination")),
+ OPT_END(),
+ };
+
+ argc = parse_options(argc, argv, prefix, opts, format_rev_usage, 0);
+
+ if (argc > 0) {
+ error(_("too many arguments"));
+ usage_with_options(format_rev_usage, opts);
+ }
+
+ if (!format)
+ die(_("'%s' is required"), "--format");
+ if (!stdin_mode_arg)
+ die(_("'%s' is required"), "--stdin-mode");
+
+ getline_fn = nul_data.nul_input ? strbuf_getline_nul : strbuf_getline_lf;
+ output_terminator = nul_data.nul_output ? '\0' : '\n';
+
+ init_display_notes(&format_notes_opt);
+ stdin_mode = parse_stdin_mode(stdin_mode_arg);
+
+ get_commit_format(format, &format_rev);
+ format_pp.ctx.rev = &format_rev;
+ format_pp.ctx.fmt = format_rev.commit_format;
+ format_pp.ctx.abbrev = format_rev.abbrev;
+ format_pp.ctx.date_mode_explicit = format_rev.date_mode_explicit;
+ format_pp.ctx.date_mode = format_rev.date_mode;
+ format_pp.ctx.color = GIT_COLOR_AUTO;
+
+ userformat_find_requirements(format,
+ &format_pp.want);
+ if (format_pp.want.notes) {
+ int ignore_show_notes = 0;
+ struct string_list_item *n;
+
+ for_each_string_list_item(n, ¬es)
+ enable_ref_display_notes(&format_notes_opt,
+ &ignore_show_notes,
+ n->string);
+ load_display_notes(&format_notes_opt);
+ }
+
+ init_format_rev_command(&cmd, &format_pp);
+
+ switch (stdin_mode) {
+ case TEXT:
+ while (getline_fn(&scratch_buf, stdin) != EOF) {
+ name_rev_line(scratch_buf.buf, &cmd);
+ /*
+ * We do not pass on the terminator to name_rev_line,
+ * unlike name-rev.
+ */
+ printf("%c", output_terminator);
+ maybe_flush_or_die(stdout, "stdout");
+ }
+ break;
+ case REVS:
+ while (getline_fn(&scratch_buf, stdin) != EOF) {
+ struct object_id oid;
+ struct object *object;
+ struct object *peeled;
+
+ if (repo_get_oid(the_repository, scratch_buf.buf, &oid)) {
+ fprintf(stderr, "Could not get object name for %s. Skipping.\n",
+ scratch_buf.buf);
+ continue;
+ }
+
+ object = parse_object(the_repository, &oid);
+ if (!object) {
+ fprintf(stderr, "Could not get object for %s. Skipping.\n",
+ scratch_buf.buf);
+ continue;
+ }
+
+ peeled = deref_tag(the_repository, object, scratch_buf.buf, 0);
+ if (!peeled || peeled->type != OBJ_COMMIT) {
+ fprintf(stderr,
+ "Could not get commit for %s. Skipping.\n",
+ scratch_buf.buf);
+ continue;
+ }
+
+ get_format_rev((struct commit *)peeled,
+ &format_pp, &scratch_buf);
+ printf("%s%c", scratch_buf.buf, output_terminator);
+ maybe_flush_or_die(stdout, "stdout");
+ strbuf_release(&scratch_buf);
+ }
+ break;
+ default:
+ BUG("uncovered case: %d", stdin_mode);
+ }
+
+ strbuf_release(&scratch_buf);
+ string_list_clear(¬es, 0);
+ release_display_notes(&format_notes_opt);
+ return 0;
+}
diff --git a/command-list.txt b/command-list.txt
index f9005cf4597..df729872dca 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -108,6 +108,7 @@ git-fmt-merge-msg purehelpers
git-for-each-ref plumbinginterrogators
git-for-each-repo plumbinginterrogators
git-format-patch mainporcelain
+git-format-rev plumbinginterrogators
git-fsck ancillaryinterrogators complete
git-gc mainporcelain
git-get-tar-commit-id plumbinginterrogators
diff --git a/git.c b/git.c
index 2b212e6675d..af5b0422b00 100644
--- a/git.c
+++ b/git.c
@@ -578,6 +578,7 @@ static struct cmd_struct commands[] = {
{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
{ "for-each-repo", cmd_for_each_repo, RUN_SETUP_GENTLY },
{ "format-patch", cmd_format_patch, RUN_SETUP },
+ { "format-rev", cmd_format_rev, RUN_SETUP },
{ "fsck", cmd_fsck, RUN_SETUP },
{ "fsck-objects", cmd_fsck, RUN_SETUP },
{ "fsmonitor--daemon", cmd_fsmonitor__daemon, RUN_SETUP },
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index c824c1a25cf..360a9323343 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -114,7 +114,8 @@ do
archimport | citool | credential-netrc | credential-libsecret | \
credential-osxkeychain | cvsexportcommit | cvsimport | cvsserver | \
daemon | \
- difftool--helper | filter-branch | fsck-objects | get-tar-commit-id | \
+ difftool--helper | filter-branch | format-rev | fsck-objects | \
+ get-tar-commit-id | \
gui | gui--askpass | \
http-backend | http-fetch | http-push | init-db | \
merge-octopus | merge-one-file | merge-resolve | mergetool | \
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 62789f76381..8ee3d2c37d0 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -801,4 +801,198 @@ test_expect_success 'do not be fooled by invalid describe format ' '
test_must_fail git cat-file -t "refs/tags/super-invalid/./../...../ ~^:/?*[////\\\\\\&}/busted.lock-42-g"$(cat out)
'
+test_expect_success 'setup: format-rev' '
+ mkdir repo-format &&
+ git -C repo-format init &&
+ test_commit -C repo-format first &&
+ test_commit -C repo-format second &&
+ test_commit -C repo-format third &&
+ test_commit -C repo-format fourth &&
+ test_commit -C repo-format fifth &&
+ test_commit -C repo-format sixth &&
+ test_commit -C repo-format seventh &&
+ test_commit -C repo-format eighth
+'
+
+test_expect_success 'format-rev --stdin-mode=revs' '
+ cat >expect <<-\EOF &&
+ eighth
+ seventh
+ fifth
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s >actual <<-\EOF &&
+ HEAD
+ HEAD~
+ HEAD~3
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=text from rev-list same as log' '
+ git -C repo-format log --format=reference >expect &&
+ test_file_not_empty expect &&
+ git -C repo-format rev-list HEAD >list &&
+ git -C repo-format format-rev --stdin-mode=text \
+ --format=reference <list >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=text with running text and tree oid' '
+ cmit_oid=$(git -C repo-format rev-parse fifth) &&
+ reference=$(git -C repo-format log -n1 --format=reference fifth) &&
+ tree=$(git -C repo-format rev-parse HEAD^{tree}) &&
+ cat >expect <<-EOF &&
+ We thought we fixed this in ${reference}.
+ But look at this tree: ${tree}.
+ EOF
+ git -C repo-format format-rev --stdin-mode=text --format=reference \
+ >actual <<-EOF &&
+ We thought we fixed this in ${cmit_oid}.
+ But look at this tree: ${tree}.
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev with %N (note)' '
+ test_when_finished "git -C repo-format notes remove" &&
+ git -C repo-format notes add -m"Make a note" &&
+ printf "Make a note\n\n\n" >expect &&
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format="tformat:%N" \
+ >actual <<-\EOF &&
+ HEAD
+ HEAD~
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --notes<ref> (custom notes ref)' '
+ # One custom notes ref
+ test_when_finished "git -C repo-format notes remove" &&
+ test_when_finished "git -C repo-format notes --ref=word remove" &&
+ git -C repo-format notes add -m"default" &&
+ git -C repo-format notes --ref=word add -m"custom" &&
+ printf "custom\n\n" >expect &&
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format="tformat:%N" \
+ --notes=word \
+ >actual <<-\EOF &&
+ HEAD
+ EOF
+ test_cmp expect actual &&
+ # Glob all
+ printf "default\ncustom\n\n" >expect &&
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format="tformat:%N" \
+ --notes=* >actual <<-\EOF &&
+ HEAD
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=revs on annotated tag peels to commit' '
+ test_when_finished "git -C repo-format tag -d version" &&
+ git -C repo-format tag -a -m"new version" version &&
+ cat >expect <<-\EOF &&
+ eighth
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s \
+ >actual <<-\EOF &&
+ version
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'format-rev --stdin-mode=revs lookup failures' '
+ test_when_finished "git -C repo-format tag -d tag-to-tree" &&
+ invalid_syntax=not-valid &&
+ non_existing_oid=${EMPTY_BLOB} &&
+ tree=$(git -C repo-format rev-parse eighth^{tree}) &&
+ git -C repo-format tag -a -mmessage tag-to-tree "$tree" &&
+ tag_to_tree=$(git -C repo-format rev-parse tag-to-tree) &&
+ cat >expect <<-EOF &&
+ Could not get object name for ${invalid_syntax}. Skipping.
+ Could not get object for ${non_existing_oid}. Skipping.
+ Could not get commit for ${tree}. Skipping.
+ Could not get commit for ${tag_to_tree}. Skipping.
+ EOF
+ git -C repo-format format-rev --stdin-mode=revs \
+ --format=%s \
+ 2>actual >out <<-EOF &&
+ ${invalid_syntax}
+ ${non_existing_oid}
+ ${tree}
+ ${tag_to_tree}
+ EOF
+ test_line_count = 0 out &&
+ test_cmp expect actual
+'
+
+
+test_expect_success 'format-rev -z --stdin-mode=text with object name lookup failures' '
+ printf "%s\0" "$(git -C repo-format rev-parse HEAD)" >input &&
+ printf "%s\0" "$(git -C repo-format rev-parse HEAD^{tree})" >>input &&
+ printf "%s\0" "$EMPTY_BLOB" >>input &&
+ printf "%s\0" "$(git -C repo-format log --format=%s -1)" >expect &&
+ printf "%s\0" "$(git -C repo-format rev-parse HEAD^{tree})" >>expect &&
+ printf "%s\0" "$EMPTY_BLOB" >>expect &&
+ git -C repo-format format-rev --stdin-mode=text \
+ --format=%s -z <input >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'setup: format-rev input and output separators' '
+ git -C repo-format rev-list HEAD >input-lf &&
+ git -C repo-format rev-list -z HEAD >input-nul &&
+ git -C repo-format log --format=%s >output-lf &&
+ git -C repo-format log -z --format=%s >output-nul &&
+ echo revs >stdin-modes &&
+ echo text >>stdin-modes
+'
+
+while read mode
+do
+ test_expect_success "format-rev -z --stdin-mode=$mode" '
+ cat output-nul >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s -z <input-nul >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev -z --no-null-input --no-null-output --stdin-mode=$mode" '
+ cat output-lf >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s -z --no-null-input --no-null-output \
+ <input-lf >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev ---null-input --stdin-mode=$mode" '
+ cat output-lf >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s --null-input \
+ <input-nul >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev --null-output --stdin-mode=$mode" '
+ cat output-nul >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format=%s --null-output \
+ <input-lf >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "format-rev -z --stdin-mode=$mode with multi-line output" '
+ format="%s%n%aI" &&
+ git -C repo-format log -z --format="$format" \
+ >expect &&
+ git -C repo-format format-rev --stdin-mode="$mode" \
+ --format="$format" -z <input-nul >actual &&
+ test_cmp expect actual
+ '
+done <stdin-modes
+
test_done
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* [PATCH v3] doc: add caveat about turning off commit-graph
From: kristofferhaugsbakk @ 2026-05-07 19:42 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Derrick Stolee
In-Reply-To: <V2_caveat_commit-graph.68b@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
The doc `technical/commit-graph.adoc` says that replace objects and
commit grafts turn off commit-graph:
Commit grafts and replace objects can change the shape of the commit
history. The latter can also be enabled/disabled on the fly using
`--no-replace-objects`. This leads to difficulty storing both possible
interpretations of a commit id, especially when computing generation
numbers. The commit-graph will not be read or written when
replace-objects or grafts are present.
But this isn’t mentioned in the user-facing doc. Let’s mention it on
git-replace(1) and git-commit-graph(1).
Acked-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v3: Add Ack
v2: Incorporate “performance issues” suggestion on git-replace(1)
Documentation/git-commit-graph.adoc | 6 ++++++
Documentation/git-replace.adoc | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/Documentation/git-commit-graph.adoc b/Documentation/git-commit-graph.adoc
index 6d19026035f..f2a37e91634 100644
--- a/Documentation/git-commit-graph.adoc
+++ b/Documentation/git-commit-graph.adoc
@@ -146,6 +146,12 @@ $ git show-ref -s | git commit-graph write --stdin-commits
$ git rev-parse HEAD | git commit-graph write --stdin-commits --append
------------------------------------------------
+CAVEATS
+-------
+
+The existence of replace objects or commit grafts turns off reading or
+writing to the commit-graph. See linkgit:git-replace[1].
+
CONFIGURATION
-------------
diff --git a/Documentation/git-replace.adoc b/Documentation/git-replace.adoc
index 0a65460adbd..436a0e58caf 100644
--- a/Documentation/git-replace.adoc
+++ b/Documentation/git-replace.adoc
@@ -145,6 +145,13 @@ commit instead of the replaced commit.
There may be other problems when using 'git rev-list' related to
pending objects.
+CAVEATS
+-------
+
+The existence of replace objects or commit grafts turns off reading or
+writing to the commit-graph, which can cause performance issues. See
+linkgit:git-commit-graph[1].
+
SEE ALSO
--------
linkgit:git-hash-object[1]
Interdiff against v2:
Range-diff against v2:
1: 82faa72f7bf ! 1: fb5ba74ea3e doc: add caveat about turning off commit-graph
@@ Commit message
But this isn’t mentioned in the user-facing doc. Let’s mention it on
git-replace(1) and git-commit-graph(1).
+ Acked-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
## Documentation/git-commit-graph.adoc ##
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
2.54.0.13.g9c7419e39f8
^ permalink raw reply related
* Re: [PATCH v3] doc: add caveat about turning off commit-graph
From: Derrick Stolee @ 2026-05-07 19:56 UTC (permalink / raw)
To: kristofferhaugsbakk, git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <V3_caveat_commit-graph.6b6@msgid.xyz>
On 5/7/2026 3:42 PM, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> Range-diff against v2:
> 1: 82faa72f7bf ! 1: fb5ba74ea3e doc: add caveat about turning off commit-graph
> @@ Commit message
> But this isn’t mentioned in the user-facing doc. Let’s mention it on
> git-replace(1) and git-commit-graph(1).
>
> + Acked-by: Derrick Stolee <stolee@gmail.com>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> ## Documentation/git-commit-graph.adoc ##
In general, you don't need to do this. Junio will add these
during his application of the series, if necessary.
Thanks,
-Stolee
^ permalink raw reply
* [PATCH v16 0/5] checkout: 'autostash' for branch switching
From: Harald Nordgren @ 2026-05-07 20:11 UTC (permalink / raw)
To: gitgitgadget; +Cc: chris.torek, git, haraldnordgren, peff, phillip.wood123
In-Reply-To: <pull.2234.v16.git.git.1777401552.gitgitgadget@gmail.com>
Is this ready to be merged?
Harald
^ permalink raw reply
* [PATCH v6] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren @ 2026-05-07 20:12 UTC (permalink / raw)
To: gitgitgadget
Cc: ben.knoble, git, haraldnordgren, kristofferhaugsbakk, marcnarc,
ramsay
In-Reply-To: <pull.2281.v6.git.git.1777847487823.gitgitgadget@gmail.com>
Is this ready to move to next?
Harald
^ permalink raw reply
* [PATCH v4 0/6] fetch: add fetch.pruneBranches config
From: Harald Nordgren @ 2026-05-07 20:14 UTC (permalink / raw)
To: gitgitgadget; +Cc: git, haraldnordgren, j6t, kristofferhaugsbakk
In-Reply-To: <pull.2285.v4.git.git.1778009038.gitgitgadget@gmail.com>
Can I get some more feedback here? I have used to on all of my repos and
it helped me clean up a lot of branches.
Harald
^ permalink raw reply
* Re: [PATCH v3] doc: add caveat about turning off commit-graph
From: Kristoffer Haugsbakk @ 2026-05-07 21:14 UTC (permalink / raw)
To: Derrick Stolee, git
In-Reply-To: <39f029d7-0c12-4a79-a701-04abf82cfde8@gmail.com>
On Thu, May 7, 2026, at 21:56, Derrick Stolee wrote:
> On 5/7/2026 3:42 PM, kristofferhaugsbakk@fastmail.com wrote:
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
>> Range-diff against v2:
>> 1: 82faa72f7bf ! 1: fb5ba74ea3e doc: add caveat about turning off commit-graph
>> @@ Commit message
>> But this isn’t mentioned in the user-facing doc. Let’s mention it on
>> git-replace(1) and git-commit-graph(1).
>>
>> + Acked-by: Derrick Stolee <stolee@gmail.com>
>> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> ## Documentation/git-commit-graph.adoc ##
> In general, you don't need to do this. Junio will add these
> during his application of the series, if necessary.
It’s certainly not necessary, yeah. :)
I am basing this on a recollection of someone quoting this from
SubmittingPatches:
Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
`Tested-by:` lines as necessary to credit people who helped your
patch, and "cc:" them when sending such a final version for inclusion.
They said that this was outdated since Junio does it himself. But then
Junio replied and said that it’s good/better if the contributor does it.
I’m terrible at finding back to conversations from more than six months
ago, but it might have been this one:[1]
>> +Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
>> +`Tested-by:` lines as necessary to credit people who helped your
>> +patch, and "cc:" them when sending such a final version for inclusion.
>
> Again, not a new problem introduced by this patch, but it seems like
> all of these are actively wrong. In every case, these trailers are
> _given_ by reviewers _after_ a series has been submitted (thus, too
> late for the author to add them), ...
Well, this is another instance that I may be trying to be too
helpful and over extending myself, which does not make the process
scale well (the other one being the "one final resend after the
list reached a consensus").
If the authors collect Acks and Reviewed-by's and resend after the
list reached the concensus, it may take one extra iteration, but I
no longer have to keep track of these trailers myself, which could
be a big win.
So, I dunno.
In conclusion for now: I dunno. :)
† 1: https://lore.kernel.org/git/xmqqo7aiyrxl.fsf@gitster.g/#t
I won’t rush to resubmit over adding a trailer if I know the maintainer
might have already applied the patch. But seeing as how he’s more or
less away-from-inbox right now I figured he won’t beat me to it.
^ permalink raw reply
* Re:git 2.54 fails to build for rhel 6 (2.53 builds fine)
From: Азат Усманов @ 2026-05-07 22:55 UTC (permalink / raw)
To: Ben Knoble; +Cc: brian m. carlson, git
In-Reply-To: <87E428E0-97B3-499F-A352-20A1E617D168@gmail.com>
git --version --build-options on 2.53 on rhel 6 actually revealed the problem. it showed that actually git compiled with system openssl 1.0.1.e
it turns out that ieven though my openssl version(the one shown from the root home is 3.5.6 git tries to compile itself with system openssl which is way of - 1.0.1e) it fails because of this item in the change log of 2.54
""imap-send" used to use functions whose use is going to be removed
with OpenSSL 4.0; rewrite them using public API that has been
available since OpenSSL 1.1 since 2016 or so.
(merge 6392a0b75d bb/imap-send-openssl-4.0-prep later to maint)."
git 2.54 successfully builds on rhel8 because unlike rhel 6 rhel 8 uses OpenSSL 1.1.1 as it system default
specifiying openssl dir where 3.5.6 is located has successfuly solved the issue so for rhel 6 command became
./configure --with-openssl=/usr/local/openssl && make -j $(nproc) CSPRNG_METHOD= && make -j $(nproc) install CSPRNG_METHOD= && make -j $(nproc) clean
and to prevent any possible errors in future I also upgraded my command line for rhel 8 so it became
./configure --with-openssl=/usr/local/openssl && make -j $(nproc) && make -j $(nproc) install && make -j $(nproc) clean
lesson learned check your build options (I was always using regular git version without -- build-options)
thanks for pointing me in the right direction
^ permalink raw reply
* Re:git 2.54 fails to build for rhel 6 (2.53 builds fine)
From: Азат Усманов @ 2026-05-07 22:55 UTC (permalink / raw)
To: Ben Knoble; +Cc: brian m. carlson, git
In-Reply-To: <87E428E0-97B3-499F-A352-20A1E617D168@gmail.com>
git --version --build-options on 2.53 on rhel 6 actually revealed the problem. it showed that actually git compiled with system openssl 1.0.1.e
it turns out that ieven though my openssl version(the one shown from the root home is 3.5.6 git tries to compile itself with system openssl which is way of - 1.0.1e) it fails because of this item in the change log of 2.54
""imap-send" used to use functions whose use is going to be removed
with OpenSSL 4.0; rewrite them using public API that has been
available since OpenSSL 1.1 since 2016 or so.
(merge 6392a0b75d bb/imap-send-openssl-4.0-prep later to maint)."
git 2.54 successfully builds on rhel8 because unlike rhel 6 rhel 8 uses OpenSSL 1.1.1 as it system default
specifiying openssl dir where 3.5.6 is located has successfuly solved the issue so for rhel 6 command became
./configure --with-openssl=/usr/local/openssl && make -j $(nproc) CSPRNG_METHOD= && make -j $(nproc) install CSPRNG_METHOD= && make -j $(nproc) clean
and to prevent any possible errors in future I also upgraded my command line for rhel 8 so it became
./configure --with-openssl=/usr/local/openssl && make -j $(nproc) && make -j $(nproc) install && make -j $(nproc) clean
lesson learned check your build options (I was always using regular git version without -- build-options)
thanks for pointing me in the right direction
^ permalink raw reply
* Re: [PATCH v6] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-08 0:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Chris Torek, git,
Mirko Faina
In-Reply-To: <87o6ireftj.fsf@gitster.g>
On Thu, May 07, 2026 at 06:20:40PM +0900, Junio C Hamano wrote:
> Mirko Faina <mroik@delayed.space> writes:
>
> >> BTW, this makes me think whether this kind of limiting could be
> >> triggered by a negative argument to --max-count.
> >
> > Would be a good idea if it weren't for the fact that --max-count < 0 has
> > for a long time acted like no max count. I'd imagine many could be
> > asssuming this behaviour in their scripts.
>
> Many? I am not sure.
>
> What do these script try to achieve by having "--max-count=-1"? It
> would be to defeat --max-count=<n> coming from elsewhere, but where?
It's not necessarely to defeat a previous use of --max-count. If I know
that --max-count behaves in as certain way, in this case the same as not
having it when below 0, I can always have it in the command that I have
to run and just work with the argument (of course one would do this only
if it is easier to work with just the argument). That way I don't have
to conditionally add the option if it has to be enabled. Something like
N=should_use_max_count_and_if_so_much_to_limit
git rev-list --max-count=$N
Of course these script make use of behaviour that is not documented and
might not even be intended, so really their fault if it breaks.
This has been the behaviour of --max-count for a long time so I'm
assuming that there is a possibility that it will break many scripts.
But like I said, their fault if it breaks, if you think its not that
widespread I'll get rid of --max-count-oldest.
^ permalink raw reply
* [PATCH v2] rebase: ignore non-branch update-refs
From: mail @ 2026-05-08 1:58 UTC (permalink / raw)
To: git, Phillip Wood; +Cc: Abhinav Gupta, Derrick Stolee, Junio C Hamano
In-Reply-To: <20260506023944.90691-1-mail@abhinavg.net>
From: Abhinav Gupta <mail@abhinavg.net>
The following Git configuration breaks git rebase --update-refs:
[rebase]
instructionFormat = %s%d
The '%d' format requests all available decorations for a commit,
filling the global decoration table with all of them,
which --update-refs then uses to populate 'update-ref' instructions
in the rebase todo list.
Specifically, this results in the following instruction:
update-ref HEAD
The todo parser then rejects the instruction:
error: update-ref requires a fully qualified refname e.g. refs/heads/HEAD
error: invalid line 3: update-ref HEAD
To fix, ignore decorations that are not local branches
when scanning through the table.
This matches the documented contract:
it moves branch refs under refs/heads/
and leaves display-only decorations (HEAD, tags, etc.) alone.
Verification:
A regression test that fails without this fix is included.
Signed-off-by: Abhinav Gupta <mail@abhinavg.net>
---
Updates:
v2: incorporate suggestions to simplify the test
sequencer.c | 10 ++++++++++
t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index b7d8dca47f..25bcfc5da0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6428,6 +6428,16 @@ static int add_decorations_to_list(const struct commit *commit,
const char *path;
size_t base_offset = ctx->buf->len;
+ /*
+ * The global decoration table may contain names loaded by
+ * a previous pretty format such as "%d".
+ * This will result in refs such as "HEAD" being present.
+ */
+ if (decoration->type != DECORATION_REF_LOCAL) {
+ decoration = decoration->next;
+ continue;
+ }
+
/*
* If the branch is the current HEAD, then it will be
* updated by the default rebase behavior.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3e44562afa..58b3bb0c27 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1960,6 +1960,24 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '
)
'
+test_expect_success '--update-refs ignores non-branch decorations' '
+ test_when_finished "git branch -D update-refs" &&
+ test_when_finished "git checkout primary" &&
+ git checkout -B update-refs no-conflict-branch &&
+ (
+ set_cat_todo_editor &&
+
+ # rebase.instructionFormat=%d loads normal log decorations before
+ # --update-refs adds its branch placeholders so we must ignore
+ # all non-local decorations.
+ test_must_fail git -c rebase.instructionFormat="%s%d" \
+ rebase -i --update-refs HEAD^ >todo
+ ) &&
+ grep ^update-ref todo >actual &&
+ test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
+ test_cmp expect actual
+'
+
test_expect_success '--update-refs updates refs correctly' '
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
From: Junio C Hamano @ 2026-05-08 2:50 UTC (permalink / raw)
To: Derrick Stolee
Cc: Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Johannes Schindelin
In-Reply-To: <xmqq5x52nhg6.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Derrick Stolee <stolee@gmail.com> writes:
>
>> On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>>
>>> Derrick Stolee suggested [1] that expensive tests should be run at a
>>> regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
>>> on push builds to the integration branches (next, master, main, maint)
>>> so that the EXPENSIVE prereq is satisfied there but not during PR
>>> validation, where the extra minutes of wall-clock time do not justify
>>> themselves.
>> I like that this will be run as part of regular updates to the
>> important branches. The important bit after that is whether or
>> not a human pays attention to the signal of these builds.
>>
>> Junio: Do you pay attention to CI breaks when you push to
>> 'master'?
>
> Well, it is way too late to notice breakage when the faulty update
> hits 'master'. CI failures should be noticed before breakage hits
> 'next'.
>
> I often notice and complain when I see failures on 'seen', and
> sometimes I help original submitter by bisecting, but I do not
> necessarily have enough time and bandwidth to help everybody.
To more directly answer your question, yes I do pay attention, but
not only when I push to 'master', but when I push to any of the
integration branches. I pay most attention to breakage in 'seen',
so that I can notify authors of new topics early. Sometimes you may
see many pushes only to 'seen' at github.com/git/git while 'seen' on
the other hosting sites are not updated with these commits, and if
you notice them, you caught me bisecting the breakage on it. This
is so that I can eject offending topics and notify the author.
But this does not scale, and I shouldn't have to do it myself.
Making sure the topics come in a shape that they pass the tests
before they hit my tree is one way to reduce the need for the
maintainer bottleneck.
> It would be best to find problems early, and make it easier for
> individual contributors to help each other by having a concrete CI
> failure reports in their forks that they can point at when they ask
> for help. And CI run when I push 'seen' or 'master' out would not
> help as much as CI run when they publish their forked branches would.
>
> By the way, please expect slow responses as I am (officially) still
> mostly offline for the rest of the week.
>
> Thanks.
^ permalink raw reply
* Re: [PATCH] Makefile: link osxkeychain helper against Rust
From: Junio C Hamano @ 2026-05-08 2:54 UTC (permalink / raw)
To: Shardul Natu via GitGitGadget
Cc: git, Shnatu, brian m. carlson, Koji Nakamaru
In-Reply-To: <pull.2288.git.git.1778001976709.gitgitgadget@gmail.com>
"Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Shnatu <snatu@google.com>
If your name is "Shardul Natu", we'd prefer (not 'require', but
'prefer') that the patches authored by you also identify with that
name, both on "From:" and "Signed-off-by:"..
> When Rust is enabled, ensure that the git-credential-osxkeychain
> helper is linked with the necessary Rust libraries.
>
> Introduce the RUST_LIBS variable inside ifndef NO_RUST block
> to hold the Rust library dependency, and use it in the helper's
> build target. This cleanly handles cases where Rust is disabled,
> making it a no-op and avoiding any build failures on systems
> without Cargo.
>
> This addresses reviewer feedback from internal CL 910223487
> by simplifying the variables and avoiding confusing "LINK"
> terminology.
>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
> Makefile: link osxkeychain helper against Rust
Thanks. I've added to CC: a few folks who may be more clueful in
the affected area than I am. It somehow feels strange that we have
to have RUST_LIB and RUST_LIBS separately, and apparently with the
new definition the latter is expected to be a superset of the
former, and it is unclear what are the things that should be added
to the latter without getting added to the former.
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2288%2Fkiranani%2Fnext-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2288/kiranani/next-v1
> Pull-Request: https://github.com/git/git/pull/2288
>
> Makefile | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f86173f93a..a17dca22b1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1593,6 +1593,7 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)
> ifndef NO_RUST
> BASIC_CFLAGS += -DWITH_RUST
> GITLIBS += $(RUST_LIB)
> +RUST_LIBS = $(RUST_LIB)
> ifeq ($(uname_S),Windows)
> EXTLIBS += -luserenv
> endif
> @@ -4082,9 +4083,9 @@ $(LIBGIT_HIDDEN_EXPORT): $(LIBGIT_PARTIAL_EXPORT)
> contrib/libgit-sys/libgitpub.a: $(LIBGIT_HIDDEN_EXPORT)
> $(AR) $(ARFLAGS) $@ $^
>
> -contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) GIT-LDFLAGS
> +contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) $(RUST_LIBS) GIT-LDFLAGS
> $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> - $(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework Security -framework CoreFoundation
> + $(filter %.o,$^) $(LIB_FILE) $(RUST_LIBS) $(EXTLIBS) -framework Security -framework CoreFoundation
>
> contrib/credential/osxkeychain/git-credential-osxkeychain.o: contrib/credential/osxkeychain/git-credential-osxkeychain.c GIT-CFLAGS
> $(QUIET_LINK)$(CC) -o $@ -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>
> base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2
^ permalink raw reply
* Re: [PATCH v2 0/6] mingw: stop using nedmalloc
From: Junio C Hamano @ 2026-05-08 2:56 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Patrick Steinhardt, Johannes Schindelin
In-Reply-To: <pull.2104.v2.git.1778169613.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> The patches that remove the vendored sources have a slightly unusual shape:
> the Git mailing list rejects messages over 100kB and
> compat/nedmalloc/malloc.c.h alone is ~196kB of source, so the deletion of
> that file is split at section boundaries into three commits, each
> comfortably under the cap.
The history made strange only by the limitation of the tool (i.e.,
mailing list) we use is like the tail wagging the dog. Could you
give a commit log message that describes droppage of everything done
in the "artificially stepwise only due to mailing list limitation,
but we wish we could do in a single step because the separation is
not logical at all" in the later steps, to the first of such steps
([2/6], I presume), and give each remaining patch a single liner "to
be squashed into [2/6]" log message, or something? Then I can
squash them on my end. Alternatively for this one only after we get
favourable reviews on the early two steps to drop the use of the
library, I can pull a single "discard everything" patch that builds
on these two from your repository.
Thanks.
^ permalink raw reply
* [PATCH] t5551: "GIT_TEST_LONG=Yes make test" is broken
From: Junio C Hamano @ 2026-05-08 5:31 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee
The "test_expect_success 'tag following always works over v0 http'"
test in t5551 fails when it tries to run "git init tags", but this
happens only when EXPENSIVE test is allowed to run.
This is because the step tries to create a repository with "git init
tags" but the EXPENSIVE test that runs way before it creates and
leaves around a temporary file "tags". Have the EXPENSIVE test
clean it up after itself.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* As we should give individual contributors a stable base, I have
identified tests that fail with EXPENSIVE enabled in "master".
With this fixed, we should add GIT_TEST_LONG=YesPlease to the CI
jobs run upon PRs are created by contributors.
t/t5551-http-fetch-smart.sh | 1 +
1 file changed, 1 insertion(+)
diff --git c/t/t5551-http-fetch-smart.sh w/t/t5551-http-fetch-smart.sh
index a26b6c2844..e236e526f0 100755
--- c/t/t5551-http-fetch-smart.sh
+++ w/t/t5551-http-fetch-smart.sh
@@ -481,6 +481,7 @@ test_expect_success 'test allowanysha1inwant with unreachable' '
'
test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
+ test_when_finished "rm -f tags" &&
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
create_tags 2001 50000
^ permalink raw reply related
* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
From: Johannes Schindelin @ 2026-05-08 7:36 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Jeff King
In-Reply-To: <20260505191100.GA12275@tb-raspi4>
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
Hi Torsten,
On Tue, 5 May 2026, Torsten Bögershausen wrote:
> On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > [...]
> > @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
> > struct object_id *oid)
> > {
> > unsigned char *p;
> > - unsigned long size, c;
> > + size_t size;
> > + unsigned long c;
>
> Does this look a little bit strange ?
Good point.
> p points to an unsigned char (better would be *uint8_t)
> then it is dereferenced into an "unsigned long".
> Then it is masked with 0x7f
> In short: should "c" be declared as uint8_t ?
Almost. It should be a `size_t`, so that we don't have to cast it when
shifting it. I'll include a fix in the next iteration.
Thank you!
Johannes
>
> > off_t base_offset;
> > unsigned shift;
> > void *data;
> > @@ -542,7 +543,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
> > p = fill(1);
> > c = *p;
> > use(1);
> > - size += (c & 0x7f) << shift;
> > + size += ((size_t)c & 0x7f) << shift;
> > shift += 7;
> > }
> > obj->size = size;
> > diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> > index e01cf6e360..59a36c2481 100644
> > --- a/builtin/unpack-objects.c
> > +++ b/builtin/unpack-objects.c
> > @@ -533,7 +533,8 @@ static void unpack_one(unsigned nr)
> > {
> > unsigned shift;
> > unsigned char *pack;
> > - unsigned long size, c;
> > + size_t size;
> > + unsigned long c;
> > enum object_type type;
> >
> > obj_list[nr].offset = consumed_bytes;
> > @@ -548,7 +549,7 @@ static void unpack_one(unsigned nr)
> > pack = fill(1);
> > c = *pack;
> > use(1);
> > - size += (c & 0x7f) << shift;
> > + size += ((size_t)c & 0x7f) << shift;
> > shift += 7;
> > }
> >
> > --
> > gitgitgadget
> >
> >
>
^ permalink raw reply
* Re: [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes
From: Johannes Schindelin @ 2026-05-08 7:38 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Jeff King
In-Reply-To: <20260505192722.GB12275@tb-raspi4>
[-- Attachment #1: Type: text/plain, Size: 14133 bytes --]
Hi Torsten,
On Tue, 5 May 2026, Torsten Bögershausen wrote:
> On Mon, May 04, 2026 at 05:08:20PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The odb_read_stream structure uses unsigned long for the size field,
> > which is 32-bit on Windows even in 64-bit builds. When streaming
> > objects larger than 4GB, the size would be truncated to zero or an
> > incorrect value, resulting in empty files being written to disk.
> >
> > Change the size field in odb_read_stream to size_t and introduce
> > unpack_object_header_sz() to return sizes via size_t pointer. Since
> > object_info.sizep remains unsigned long for API compatibility, use
> > temporary variables where the types differ, with comments noting the
> > truncation limitation for code paths that still use unsigned long.
> >
> > This was originally authored by LordKiRon <https://github.com/LordKiRon>,
> > who preferred not to reveal their real name and therefore agreed that I
> > take over authorship.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/pack-objects.c | 23 ++++++++++++++++-------
> > object-file.c | 10 +++++++++-
> > odb/streaming.c | 13 ++++++++++++-
> > odb/streaming.h | 2 +-
> > oss-fuzz/fuzz-pack-headers.c | 2 +-
> > pack-bitmap.c | 2 +-
> > pack-check.c | 6 ++++--
> > packfile.c | 24 +++++++++++++++---------
> > packfile.h | 4 ++--
> > 9 files changed, 61 insertions(+), 25 deletions(-)
> >
>
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index dd2480a73d..aa4b1cb9b8 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
>
> I haven't been able to follow all changes, so this may be false alarm.
> Do we need a cast_size_t_to_ulong() somewhere ?
Oh, that's a really good catch, I forgot to add those. There are a couple
of callers, indeed, and they still have an implicit, _silent_ truncation
to `unsigned long` that I'd rather have explicit, with a check. I'll add
those callers in the next iteration.
Thank you!
Johannes
>
> > @@ -629,14 +629,21 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
> > struct packed_git *p = IN_PACK(entry);
> > struct pack_window *w_curs = NULL;
> > uint32_t pos;
> > - off_t offset;
> > + off_t offset, cur;
> > enum object_type type = oe_type(entry);
> > + enum object_type in_pack_type;
> > off_t datalen;
> > unsigned char header[MAX_PACK_OBJECT_HEADER],
> > dheader[MAX_PACK_OBJECT_HEADER];
> > unsigned hdrlen;
> > const unsigned hashsz = the_hash_algo->rawsz;
> > - unsigned long entry_size = SIZE(entry);
> > + size_t entry_size;
> > +
> > + cur = entry->in_pack_offset;
> > + in_pack_type = unpack_object_header(p, &w_curs, &cur, &entry_size);
> > + if (in_pack_type < 0)
> > + die(_("write_reuse_object: unable to parse object header of %s"),
> > + oid_to_hex(&entry->idx.oid));
> >
> > if (DELTA(entry))
> > type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
> > @@ -1087,7 +1094,7 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile,
> > {
> > off_t offset, next, cur;
> > enum object_type type;
> > - unsigned long size;
> > + size_t size;
> >
> > offset = pack_pos_to_offset(reuse_packfile, pos);
> > next = pack_pos_to_offset(reuse_packfile, pos + 1);
> > @@ -2243,7 +2250,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
> > off_t ofs;
> > unsigned char *buf, c;
> > enum object_type type;
> > - unsigned long in_pack_size;
> > + size_t in_pack_size;
> >
> > buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
> >
> > @@ -2734,16 +2741,18 @@ unsigned long oe_get_size_slow(struct packing_data *pack,
> > struct pack_window *w_curs;
> > unsigned char *buf;
> > enum object_type type;
> > - unsigned long used, avail, size;
> > + unsigned long used, avail;
> > + size_t size;
> >
> > if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
> > + unsigned long sz;
> > packing_data_lock(&to_pack);
> > if (odb_read_object_info(the_repository->objects,
> > - &e->idx.oid, &size) < 0)
> > + &e->idx.oid, &sz) < 0)
> > die(_("unable to get size of %s"),
> > oid_to_hex(&e->idx.oid));
> > packing_data_unlock(&to_pack);
> > - return size;
> > + return sz;
> > }
> >
> > p = oe_in_pack(pack, e);
> > diff --git a/object-file.c b/object-file.c
> > index 086b2b65ff..0be2981c7a 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -2326,6 +2326,7 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> > struct object_info oi = OBJECT_INFO_INIT;
> > struct odb_loose_read_stream *st;
> > unsigned long mapsize;
> > + unsigned long size_ul;
> > void *mapped;
> >
> > mapped = odb_source_loose_map_object(source, oid, &mapsize);
> > @@ -2349,11 +2350,18 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> > goto error;
> > }
> >
> > - oi.sizep = &st->base.size;
> > + /*
> > + * object_info.sizep is unsigned long* (32-bit on Windows), but
> > + * st->base.size is size_t (64-bit). Use temporary variable.
> > + * Note: loose objects >4GB would still truncate here, but such
> > + * large loose objects are uncommon (they'd normally be packed).
> > + */
> > + oi.sizep = &size_ul;
> > oi.typep = &st->base.type;
> >
> > if (parse_loose_header(st->hdr, &oi) < 0 || st->base.type < 0)
> > goto error;
> > + st->base.size = size_ul;
> >
> > st->mapped = mapped;
> > st->mapsize = mapsize;
> > diff --git a/odb/streaming.c b/odb/streaming.c
> > index 5927a12954..af2adf5ce7 100644
> > --- a/odb/streaming.c
> > +++ b/odb/streaming.c
> > @@ -157,15 +157,26 @@ static int open_istream_incore(struct odb_read_stream **out,
> > .base.read = read_istream_incore,
> > };
> > struct odb_incore_read_stream *st;
> > + unsigned long size_ul;
> > int ret;
> >
> > oi.typep = &stream.base.type;
> > - oi.sizep = &stream.base.size;
> > + /*
> > + * object_info.sizep is unsigned long* (32-bit on Windows), but
> > + * stream.base.size is size_t (64-bit). We use a temporary variable
> > + * because the types are incompatible. Note: this path still truncates
> > + * for >4GB objects, but large objects should use pack streaming
> > + * (packfile_store_read_object_stream) which handles size_t properly.
> > + * This incore fallback is only used for small objects or when pack
> > + * streaming is unavailable.
> > + */
> > + oi.sizep = &size_ul;
> > oi.contentp = (void **)&stream.buf;
> > ret = odb_read_object_info_extended(odb, oid, &oi,
> > OBJECT_INFO_DIE_IF_CORRUPT);
> > if (ret)
> > return ret;
> > + stream.base.size = size_ul;
> >
> > CALLOC_ARRAY(st, 1);
> > *st = stream;
> > diff --git a/odb/streaming.h b/odb/streaming.h
> > index c7861f7e13..517e2ea2d3 100644
> > --- a/odb/streaming.h
> > +++ b/odb/streaming.h
> > @@ -21,7 +21,7 @@ struct odb_read_stream {
> > odb_read_stream_close_fn close;
> > odb_read_stream_read_fn read;
> > enum object_type type;
> > - unsigned long size; /* inflated size of full object */
> > + size_t size; /* inflated size of full object */
> > };
> >
> > /*
> > diff --git a/oss-fuzz/fuzz-pack-headers.c b/oss-fuzz/fuzz-pack-headers.c
> > index 150c0f5fa2..ef61ab577c 100644
> > --- a/oss-fuzz/fuzz-pack-headers.c
> > +++ b/oss-fuzz/fuzz-pack-headers.c
> > @@ -6,7 +6,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> > int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> > {
> > enum object_type type;
> > - unsigned long len;
> > + size_t len;
> >
> > unpack_object_header_buffer((const unsigned char *)data,
> > (unsigned long)size, &type, &len);
> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index f6ec18d83a..f9af8a96bd 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -2270,7 +2270,7 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
> > {
> > off_t delta_obj_offset;
> > enum object_type type;
> > - unsigned long size;
> > + size_t size;
> >
> > if (pack_pos >= pack->p->num_objects)
> > return -1; /* not actually in the pack */
> > diff --git a/pack-check.c b/pack-check.c
> > index 79992bb509..2792f34d25 100644
> > --- a/pack-check.c
> > +++ b/pack-check.c
> > @@ -110,7 +110,7 @@ static int verify_packfile(struct repository *r,
> > void *data;
> > struct object_id oid;
> > enum object_type type;
> > - unsigned long size;
> > + size_t size;
> > off_t curpos;
> > int data_valid;
> >
> > @@ -143,7 +143,9 @@ static int verify_packfile(struct repository *r,
> > data = NULL;
> > data_valid = 0;
> > } else {
> > - data = unpack_entry(r, p, entries[i].offset, &type, &size);
> > + unsigned long sz;
> > + data = unpack_entry(r, p, entries[i].offset, &type, &sz);
> > + size = sz;
> > data_valid = 1;
> > }
> >
> > diff --git a/packfile.c b/packfile.c
> > index b012d648ad..fdae91dd11 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -1133,7 +1133,7 @@ out:
> > }
> >
> > unsigned long unpack_object_header_buffer(const unsigned char *buf,
> > - unsigned long len, enum object_type *type, unsigned long *sizep)
> > + unsigned long len, enum object_type *type, size_t *sizep)
> > {
> > unsigned shift;
> > size_t size, c;
> > @@ -1144,7 +1144,11 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
> > size = c & 15;
> > shift = 4;
> > while (c & 0x80) {
> > - if (len <= used || (bitsizeof(long) - 7) < shift) {
> > + /*
> > + * Each continuation byte adds 7 bits. Ensure shift won't
> > + * overflow size_t (use size_t not long for 64-bit on Windows).
> > + */
> > + if (len <= used || (bitsizeof(size_t) - 7) < shift) {
> > error("bad object header");
> > size = used = 0;
> > break;
> > @@ -1153,7 +1157,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
> > size = st_add(size, st_left_shift(c & 0x7f, shift));
> > shift += 7;
> > }
> > - *sizep = cast_size_t_to_ulong(size);
> > + *sizep = size;
> > return used;
> > }
> >
> > @@ -1215,7 +1219,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
> > int unpack_object_header(struct packed_git *p,
> > struct pack_window **w_curs,
> > off_t *curpos,
> > - unsigned long *sizep)
> > + size_t *sizep)
> > {
> > unsigned char *base;
> > unsigned long left;
> > @@ -1367,7 +1371,7 @@ static enum object_type packed_to_object_type(struct repository *r,
> >
> > while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> > off_t base_offset;
> > - unsigned long size;
> > + size_t size;
> > /* Push the object we're going to leave behind */
> > if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
> > poi_stack_alloc = alloc_nr(poi_stack_nr);
> > @@ -1586,7 +1590,7 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
> > uint32_t *maybe_index_pos, struct object_info *oi)
> > {
> > struct pack_window *w_curs = NULL;
> > - unsigned long size;
> > + size_t size;
> > off_t curpos = obj_offset;
> > enum object_type type = OBJ_NONE;
> > uint32_t pack_pos;
> > @@ -1778,7 +1782,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
> > struct pack_window *w_curs = NULL;
> > off_t curpos = obj_offset;
> > void *data = NULL;
> > - unsigned long size;
> > + size_t size;
> > enum object_type type;
> > struct unpack_entry_stack_ent small_delta_stack[UNPACK_ENTRY_STACK_PREALLOC];
> > struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
> > @@ -1943,8 +1947,10 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
> > (uintmax_t)curpos, p->pack_name);
> > data = NULL;
> > } else {
> > + unsigned long sz;
> > data = patch_delta(base, base_size, delta_data,
> > - delta_size, &size);
> > + delta_size, &sz);
> > + size = sz;
> >
> > /*
> > * We could not apply the delta; warn the user, but
> > @@ -2929,7 +2935,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
> > struct odb_packed_read_stream *stream;
> > struct pack_window *window = NULL;
> > enum object_type in_pack_type;
> > - unsigned long size;
> > + size_t size;
> >
> > in_pack_type = unpack_object_header(pack, &window, &offset, &size);
> > unuse_pack(&window);
> > diff --git a/packfile.h b/packfile.h
> > index 9b647da7dd..49d6bdecf6 100644
> > --- a/packfile.h
> > +++ b/packfile.h
> > @@ -456,9 +456,9 @@ off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
> >
> > int is_pack_valid(struct packed_git *);
> > void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
> > -unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
> > +unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, size_t *sizep);
> > unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
> > -int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
> > +int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, size_t *);
> > off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs,
> > off_t *curpos, enum object_type type,
> > off_t delta_obj_offset);
> > --
> > gitgitgadget
> >
> >
>
^ permalink raw reply
* [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin
In-Reply-To: <pull.2102.v2.git.1777914508.gitgitgadget@gmail.com>
On Windows, unsigned long is 32-bit even on 64-bit systems. This causes
multiple problems when Git handles objects larger than 4GB. This patch
series is a very targeted fix for a very early part of the problem: it
addresses the most fundamental truncation points that prevent a >4GB object
from surviving a clone at all.
Specifically, this fixes:
* zlib's uLong wrapping and triggering BUG() assertions in the git_zstream
wrapper
* Object sizes being truncated in pack streaming, delta headers, and
index-pack/unpack-objects
* pack-objects re-encoding reused pack entries with a truncated size,
producing corrupt packs on the wire
Many other code paths still use unsigned long for object sizes (e.g.,
cat-file -s, object_info.sizep, the delta machinery) and will need their own
conversions. This series does not attempt to fix those.
Based on work by @LordKiRon in git-for-windows/git#6076.
For testing, add a test helper that synthesizes a pack with a >4GB blob and
regression tests that clone it via both the unpack-objects and index-pack
code paths using file:// transport. Since these test cases are quite slow
(even after optimizing the pack generation part, the git clone test has no
chance but to hash 2x4GB of data), they are marked as EXPENSIVE. To ensure
that they are passing well in advance of any release, the CI is changed to
run them in the CI builds of relatively infrequent integration branch
updates.
Changes since v2:
* Now uses the proper data type for the varint decoding value (thanks,
Torsten!)
* The callers that now would silently narrow size_t to unsigned long
properly check and error out instead (thanks, Torsten!)
Changes since v1:
* dramatically accelerated the test helper that generates 4GB pack files,
via two separate strategies:
1. using the "unsafe" SHA-1 for the blob OID computation.
2. using pre-computed "Lego blocks" to construct the 4GB packs needed in
the test cases, where the size (and therefore the involved OIDs) are
well-known in advance.
* even with these improvements, the actual git clone is still slow (of
course, because it cannot use any of those shortcuts), therefore the
tests are marked as EXPENSIVE.
* to exercise those tests nevertheless, the last patch lets all EXPENSIVE
test cases be run for the integration branches other than seen.
Johannes Schindelin (11):
index-pack, unpack-objects: use size_t for object size
git-zlib: handle data streams larger than 4GB
odb, packfile: use size_t for streaming object sizes
delta, packfile: use size_t for delta header sizes
test-tool: add a helper to synthesize large packfiles
t5608: add regression test for >4GB object clone
test-tool synthesize: use the unsafe hash for speed
test-tool synthesize: precompute pack for 4 GiB + 1
test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
t5608: mark >4GB tests as EXPENSIVE
ci: run expensive tests on push builds to integration branches
Makefile | 1 +
builtin/index-pack.c | 8 +-
builtin/pack-objects.c | 34 ++-
builtin/unpack-objects.c | 4 +-
ci/lib.sh | 9 +
compat/zlib-compat.h | 2 +
delta.h | 14 +-
git-zlib.c | 25 +-
git-zlib.h | 4 +-
object-file.c | 12 +-
odb/streaming.c | 13 +-
odb/streaming.h | 2 +-
oss-fuzz/fuzz-pack-headers.c | 2 +-
pack-bitmap.c | 2 +-
pack-check.c | 6 +-
packfile.c | 57 ++--
packfile.h | 4 +-
t/helper/meson.build | 1 +
t/helper/test-synthesize.c | 541 +++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t5608-clone-2gb.sh | 37 +++
22 files changed, 724 insertions(+), 56 deletions(-)
create mode 100644 t/helper/test-synthesize.c
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2102%2Fdscho%2Ffix-large-clones-on-windows-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2102/dscho/fix-large-clones-on-windows-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2102
Range-diff vs v2:
1: dc660106ea ! 1: 311cdc601d index-pack, unpack-objects: use size_t for object size
@@ Commit message
64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
space.
+ Declare the per-byte continuation variable `c` as size_t as well,
+ matching the canonical varint decoder unpack_object_header_buffer()
+ in packfile.c. With c as size_t the expression (c & 0x7f) << shift
+ is naturally size_t-typed, so the explicit cast that an earlier
+ iteration carried at the use site is no longer needed.
+
+ While at it, add the same overflow guard that
+ unpack_object_header_buffer() carries: if the cumulative shift would
+ exceed bitsizeof(size_t) - 7, refuse the input rather than invoking
+ undefined behavior. Unlike unpack_object_header_buffer(), which
+ labels this case "bad object header", report it as the platform
+ limit it actually is: a header may be perfectly well-formed and
+ still encode a size we cannot represent locally (notably on a
+ 32-bit build consuming a packfile produced on a 64-bit host).
+
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
+ Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## builtin/index-pack.c ##
@@ builtin/index-pack.c: static void *unpack_raw_entry(struct object_entry *obj,
{
unsigned char *p;
- unsigned long size, c;
-+ size_t size;
-+ unsigned long c;
++ size_t size, c;
off_t base_offset;
unsigned shift;
void *data;
@@ builtin/index-pack.c: static void *unpack_raw_entry(struct object_entry *obj,
+ size = (c & 15);
+ shift = 4;
+ while (c & 0x80) {
++ if ((bitsizeof(size_t) - 7) < shift)
++ die(_("object size too large for this platform"));
p = fill(1);
c = *p;
use(1);
-- size += (c & 0x7f) << shift;
-+ size += ((size_t)c & 0x7f) << shift;
- shift += 7;
- }
- obj->size = size;
## builtin/unpack-objects.c ##
@@ builtin/unpack-objects.c: static void unpack_one(unsigned nr)
@@ builtin/unpack-objects.c: static void unpack_one(unsigned nr)
unsigned shift;
unsigned char *pack;
- unsigned long size, c;
-+ size_t size;
-+ unsigned long c;
++ size_t size, c;
enum object_type type;
obj_list[nr].offset = consumed_bytes;
@@ builtin/unpack-objects.c: static void unpack_one(unsigned nr)
+ size = (c & 15);
+ shift = 4;
+ while (c & 0x80) {
++ if ((bitsizeof(size_t) - 7) < shift)
++ die(_("object size too large for this platform"));
pack = fill(1);
c = *pack;
use(1);
-- size += (c & 0x7f) << shift;
-+ size += ((size_t)c & 0x7f) << shift;
- shift += 7;
- }
-
2: 92f4327b1f = 2: c611913194 git-zlib: handle data streams larger than 4GB
3: 3a539061c5 ! 3: b789f57de9 odb, packfile: use size_t for streaming object sizes
@@ Commit message
temporary variables where the types differ, with comments noting the
truncation limitation for code paths that still use unsigned long.
+ Widening the producers to size_t in this way introduces a handful of
+ silent size_t -> unsigned long narrowings on Windows, all in
+ builtin/pack-objects.c, where the consumers are still typed
+ unsigned long. Make those narrowings explicit with
+ cast_size_t_to_ulong() so they assert loudly the moment an object
+ actually exceeds ULONG_MAX bytes:
+
+ - oe_get_size_slow() returns unsigned long but holds a size_t
+ locally; cast at the return.
+ - write_reuse_object() passes a size_t into check_pack_inflate(),
+ whose expect parameter is unsigned long; cast at the call.
+ - check_object() routes a size_t through SET_SIZE() and
+ SET_DELTA_SIZE(), both of which take unsigned long via
+ oe_set_size() / oe_set_delta_size(); cast at the three call
+ sites in the OBJ_OFS_DELTA / OBJ_REF_DELTA branches and in the
+ non-delta default arm.
+
+ The cast-only treatment is deliberately a stop-gap. Properly
+ widening oe_set_size, oe_get_size_slow's return type,
+ check_pack_inflate's expect parameter, object_info.sizep,
+ patch_delta, and the OE_SIZE_BITS bit-fields cascades into a series
+ that is too large to be reviewable, so the proper widening is
+ deferred to a follow-up topic. Until then,
+ cast_size_t_to_ulong() at least makes the truncation explicit at
+ the source: it documents the boundary, and on a 64-bit non-Windows
+ platform it is a no-op.
+
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
+ Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## builtin/pack-objects.c ##
@@ builtin/pack-objects.c: static off_t write_reuse_object(struct hashfile *f, stru
if (DELTA(entry))
type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
+@@ builtin/pack-objects.c: static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
+ datalen -= entry->in_pack_header_size;
+
+ if (!pack_to_stdout && p->index_version == 1 &&
+- check_pack_inflate(p, &w_curs, offset, datalen, entry_size)) {
++ check_pack_inflate(p, &w_curs, offset, datalen,
++ cast_size_t_to_ulong(entry_size))) {
+ error(_("corrupt packed object for %s"),
+ oid_to_hex(&entry->idx.oid));
+ unuse_pack(&w_curs);
@@ builtin/pack-objects.c: static void write_reused_pack_one(struct packed_git *reuse_packfile,
{
off_t offset, next, cur;
@@ builtin/pack-objects.c: static void check_object(struct object_entry *entry, uin
buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
+@@ builtin/pack-objects.c: static void check_object(struct object_entry *entry, uint32_t object_index)
+ default:
+ /* Not a delta hence we've already got all we need. */
+ oe_set_type(entry, entry->in_pack_type);
+- SET_SIZE(entry, in_pack_size);
++ SET_SIZE(entry, cast_size_t_to_ulong(in_pack_size));
+ entry->in_pack_header_size = used;
+ if (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)
+ goto give_up;
+@@ builtin/pack-objects.c: static void check_object(struct object_entry *entry, uint32_t object_index)
+ if (have_base &&
+ can_reuse_delta(&base_ref, entry, &base_entry)) {
+ oe_set_type(entry, entry->in_pack_type);
+- SET_SIZE(entry, in_pack_size); /* delta size */
+- SET_DELTA_SIZE(entry, in_pack_size);
++ SET_SIZE(entry, cast_size_t_to_ulong(in_pack_size)); /* delta size */
++ SET_DELTA_SIZE(entry, cast_size_t_to_ulong(in_pack_size));
+
+ if (base_entry) {
+ SET_DELTA(entry, base_entry);
@@ builtin/pack-objects.c: unsigned long oe_get_size_slow(struct packing_data *pack,
struct pack_window *w_curs;
unsigned char *buf;
@@ builtin/pack-objects.c: unsigned long oe_get_size_slow(struct packing_data *pack
}
p = oe_in_pack(pack, e);
+@@ builtin/pack-objects.c: unsigned long oe_get_size_slow(struct packing_data *pack,
+
+ unuse_pack(&w_curs);
+ packing_data_unlock(&to_pack);
+- return size;
++ return cast_size_t_to_ulong(size);
+ }
+
+ static int try_delta(struct unpacked *trg, struct unpacked *src,
## object-file.c ##
@@ object-file.c: int odb_source_loose_read_object_stream(struct odb_read_stream **out,
4: 3274cba862 = 4: 8e87a4e71f delta, packfile: use size_t for delta header sizes
5: afa74a3a2b = 5: 34fec4a32d test-tool: add a helper to synthesize large packfiles
6: a3019888d8 = 6: 88f992903f t5608: add regression test for >4GB object clone
7: 859e93e7a9 = 7: 4f207c8a47 test-tool synthesize: use the unsafe hash for speed
8: 29b9a74e91 = 8: 2751c21c6e test-tool synthesize: precompute pack for 4 GiB + 1
9: 8e6e720804 = 9: 3a006d96c3 test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
10: 5b44410b2f = 10: 86c09af4f5 t5608: mark >4GB tests as EXPENSIVE
11: 1eaaa7fad7 = 11: 2159f6a271 ci: run expensive tests on push builds to integration branches
--
gitgitgadget
^ 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