* [GSoC Patch 4/7] repo: add path.hooks with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-07-16 1:21 UTC (permalink / raw)
To: git; +Cc: jltobler, lucasseikioshiro, K Jayatheerth
In-Reply-To: <20260716012138.6714-1-jayatheerthkulkarni2005@gmail.com>
External tool integrations and validation systems need a stable way to
identify where the repository hooks are stored. Currently, this involves
relying on `git rev-parse --git-path hooks` or querying `core.hooksPath`
manually.
Introduce `path.hooks.absolute` and `path.hooks.relative` keys to
`git repo info`. This allows tools to discover the active hooks location
natively, ensuring proper resolution regardless of whether Git is using
the standard `.git/hooks` structure or a custom `core.hooksPath` setup.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 8 ++++++++
builtin/repo.c | 22 ++++++++++++++++++++++
t/t1900-repo-info.sh | 6 ++++++
3 files changed, 36 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 8429a44b43..7bc1c51310 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -119,6 +119,14 @@ values that they return:
`path.gitdir.relative`::
The path to the Git repository directory relative to the current working directory.
+`path.hooks.absolute`::
+ The canonical absolute path to the repository's hooks directory.
+ Respects `core.hooksPath` configuration adjustments.
+
+`path.hooks.relative`::
+ The path to the repository's hooks directory relative to the current
+ working directory. Respects `core.hooksPath` configuration adjustments.
+
`path.objects.absolute`::
The canonical absolute path to the repository's object database directory.
Respects the `GIT_OBJECT_DIRECTORY` environment override.
diff --git a/builtin/repo.c b/builtin/repo.c
index d6bdd5bcfa..c921de222d 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -122,6 +122,26 @@ static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_hooks_absolute(struct repository *repo, struct strbuf *buf)
+{
+ struct strbuf hooks_path = STRBUF_INIT;
+
+ repo_git_path_replace(repo, &hooks_path, "hooks");
+ format_path(buf, hooks_path.buf, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ strbuf_release(&hooks_path);
+ return 0;
+}
+
+static int get_path_hooks_relative(struct repository *repo, struct strbuf *buf)
+{
+ struct strbuf hooks_path = STRBUF_INIT;
+
+ repo_git_path_replace(repo, &hooks_path, "hooks");
+ format_path(buf, hooks_path.buf, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ strbuf_release(&hooks_path);
+ return 0;
+}
+
static int get_path_objects_absolute(struct repository *repo, struct strbuf *buf)
{
const char *obj_dir = repo_get_object_directory(repo);
@@ -216,6 +236,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "path.commondir.relative", get_path_commondir_relative },
{ "path.gitdir.absolute", get_path_gitdir_absolute },
{ "path.gitdir.relative", get_path_gitdir_relative },
+ { "path.hooks.absolute", get_path_hooks_absolute },
+ { "path.hooks.relative", get_path_hooks_relative },
{ "path.objects.absolute", get_path_objects_absolute },
{ "path.objects.relative", get_path_objects_relative },
{ "path.superproject-working-tree.absolute", get_path_superproject_absolute },
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 260f4fde43..cd3f856d04 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -213,6 +213,12 @@ test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
'.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_repo_info_path 'hooks standard fallback' 'hooks' '.git/hooks'
+
+test_repo_info_path 'hooks with core.hooksPath override' 'hooks' \
+ 'custom-hooks' \
+ 'git config core.hooksPath "$ROOT/custom-hooks" && mkdir -p "$ROOT/custom-hooks"'
+
test_repo_info_path 'objects standard' 'objects' '.git/objects'
test_repo_info_path 'objects with GIT_OBJECT_DIRECTORY override' 'objects' \
--
2.55.GIT
^ permalink raw reply related
* [GSoC Patch 3/7] repo: add path.objects with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-07-16 1:21 UTC (permalink / raw)
To: git; +Cc: jltobler, lucasseikioshiro, K Jayatheerth
In-Reply-To: <20260716012138.6714-1-jayatheerthkulkarni2005@gmail.com>
Tools and deployment hooks frequently query the location of the object
database directory. Currently, this relies on legacy parsing methods or
manually inspecting `git rev-parse --git-path objects`.
Introduce `path.objects.absolute` and `path.objects.relative` keys to
`git repo info`. This allows tools to discover the object database
location safely while natively adhering to active `GIT_OBJECT_DIRECTORY`
environment variable overrides.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 9 +++++++++
builtin/repo.c | 24 ++++++++++++++++++++++++
t/t1900-repo-info.sh | 7 +++++++
3 files changed, 40 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 03aa57942f..8429a44b43 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -119,6 +119,15 @@ values that they return:
`path.gitdir.relative`::
The path to the Git repository directory relative to the current working directory.
+`path.objects.absolute`::
+ The canonical absolute path to the repository's object database directory.
+ Respects the `GIT_OBJECT_DIRECTORY` environment override.
+
+`path.objects.relative`::
+ The path to the repository's object database directory relative to the
+ current working directory. Respects the `GIT_OBJECT_DIRECTORY`
+ environment override.
+
`path.superproject-working-tree.absolute`::
The canonical absolute path to the working tree root of the superproject
if the current repository is an initialized submodule. Outputs an empty
diff --git a/builtin/repo.c b/builtin/repo.c
index 82359473e9..d6bdd5bcfa 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -122,6 +122,28 @@ static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_objects_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *obj_dir = repo_get_object_directory(repo);
+
+ if (!obj_dir)
+ return error(_("unable to get object directory"));
+
+ format_path(buf, obj_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_objects_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *obj_dir = repo_get_object_directory(repo);
+
+ if (!obj_dir)
+ return error(_("unable to get object directory"));
+
+ format_path(buf, obj_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_path_superproject_absolute(struct repository *repo UNUSED, struct strbuf *buf)
{
struct strbuf superproject = STRBUF_INIT;
@@ -194,6 +216,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "path.commondir.relative", get_path_commondir_relative },
{ "path.gitdir.absolute", get_path_gitdir_absolute },
{ "path.gitdir.relative", get_path_gitdir_relative },
+ { "path.objects.absolute", get_path_objects_absolute },
+ { "path.objects.relative", get_path_objects_relative },
{ "path.superproject-working-tree.absolute", get_path_superproject_absolute },
{ "path.superproject-working-tree.relative", get_path_superproject_relative },
{ "path.toplevel.absolute", get_path_toplevel_absolute },
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 220b3d4d3d..260f4fde43 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -213,6 +213,13 @@ test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
'.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_repo_info_path 'objects standard' 'objects' '.git/objects'
+
+test_repo_info_path 'objects with GIT_OBJECT_DIRECTORY override' 'objects' \
+ 'custom-objects' \
+ 'GIT_OBJECT_DIRECTORY="$ROOT/custom-objects" && export GIT_OBJECT_DIRECTORY &&
+ mkdir -p "$ROOT/custom-objects"'
+
test_expect_success 'path.superproject-working-tree absolute and relative' '
test_when_finished "rm -rf sub super" &&
git init sub &&
--
2.55.GIT
^ permalink raw reply related
* [GSoC Patch 2/7] repo: add path.superproject-working-tree with absolute and relative suffixes
From: K Jayatheerth @ 2026-07-16 1:21 UTC (permalink / raw)
To: git; +Cc: jltobler, lucasseikioshiro, K Jayatheerth
In-Reply-To: <20260716012138.6714-1-jayatheerthkulkarni2005@gmail.com>
Scripts working in multi-repository setups often need to identify the
top-level working tree of a superproject from within a submodule.
Currently, this is only exposed via `git rev-parse
--show-superproject-working-tree`.
Introduce `path.superproject-working-tree.absolute` and
`path.superproject-working-tree.relative` keys to `git repo info`.
This exposes the core submodule context via a scriptable config-like key
using standard format rules.
If requested when not inside a submodule, the command returns an empty
string.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 10 ++++++++++
builtin/repo.c | 33 +++++++++++++++++++++++++++++++++
t/t1900-repo-info.sh | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 77 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index e34abe5fea..03aa57942f 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -119,6 +119,16 @@ values that they return:
`path.gitdir.relative`::
The path to the Git repository directory relative to the current working directory.
+`path.superproject-working-tree.absolute`::
+ The canonical absolute path to the working tree root of the superproject
+ if the current repository is an initialized submodule. Outputs an empty
+ string if not in a submodule.
+
+`path.superproject-working-tree.relative`::
+ The path to the working tree root of the superproject relative to the
+ current working directory if the current repository is an initialized
+ submodule. Outputs an empty string if not in a submodule.
+
`path.toplevel.absolute`::
The canonical absolute path to the top-level directory of the
repository's working tree. Outputs an empty string if the repository
diff --git a/builtin/repo.c b/builtin/repo.c
index 194757eb18..82359473e9 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -18,6 +18,7 @@
#include "strbuf.h"
#include "string-list.h"
#include "shallow.h"
+#include "submodule.h"
#include "tree.h"
#include "tree-walk.h"
#include "utf8.h"
@@ -121,6 +122,36 @@ static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_superproject_absolute(struct repository *repo UNUSED, struct strbuf *buf)
+{
+ struct strbuf superproject = STRBUF_INIT;
+
+ if (!get_superproject_working_tree(&superproject)) {
+ strbuf_release(&superproject);
+ strbuf_addstr(buf, "");
+ return 0;
+ }
+
+ format_path(buf, superproject.buf, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ strbuf_release(&superproject);
+ return 0;
+}
+
+static int get_path_superproject_relative(struct repository *repo UNUSED, struct strbuf *buf)
+{
+ struct strbuf superproject = STRBUF_INIT;
+
+ if (!get_superproject_working_tree(&superproject)) {
+ strbuf_release(&superproject);
+ strbuf_addstr(buf, "");
+ return 0;
+ }
+
+ format_path(buf, superproject.buf, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ strbuf_release(&superproject);
+ return 0;
+}
+
static int get_path_toplevel_absolute(struct repository *repo, struct strbuf *buf)
{
const char *work_tree = repo_get_work_tree(repo);
@@ -163,6 +194,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "path.commondir.relative", get_path_commondir_relative },
{ "path.gitdir.absolute", get_path_gitdir_absolute },
{ "path.gitdir.relative", get_path_gitdir_relative },
+ { "path.superproject-working-tree.absolute", get_path_superproject_absolute },
+ { "path.superproject-working-tree.relative", get_path_superproject_relative },
{ "path.toplevel.absolute", get_path_toplevel_absolute },
{ "path.toplevel.relative", get_path_toplevel_relative },
{ "references.format", get_references_format },
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index fbb9063ee5..220b3d4d3d 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -213,6 +213,40 @@ test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
'.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_expect_success 'path.superproject-working-tree absolute and relative' '
+ test_when_finished "rm -rf sub super" &&
+ git init sub &&
+ test_commit -C sub initial &&
+ git init super &&
+ (
+ cd super &&
+ git -c protocol.file.allow=always submodule add "../sub" sub &&
+ git commit -m "add submodule" &&
+
+ cd sub &&
+ ROOT="$(test-tool path-utils real_path ..)" &&
+
+ echo "path.superproject-working-tree.absolute=$ROOT" >expect.abs &&
+ git repo info path.superproject-working-tree.absolute >actual.abs &&
+ test_cmp expect.abs actual.abs &&
+
+ echo "path.superproject-working-tree.relative=../" >expect.rel &&
+ git repo info path.superproject-working-tree.relative >actual.rel &&
+ test_cmp expect.rel actual.rel
+ )
+'
+
+test_expect_success 'path.superproject-working-tree returns empty when not in a submodule' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ echo "path.superproject-working-tree.absolute=" >expect &&
+ git repo info path.superproject-working-tree.absolute >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'path.toplevel absolute and relative' '
test_when_finished "rm -rf repo" &&
git init repo &&
--
2.55.GIT
^ permalink raw reply related
* [GSoC Patch 1/7] repo: add path.toplevel with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-07-16 1:21 UTC (permalink / raw)
To: git; +Cc: jltobler, lucasseikioshiro, K Jayatheerth
In-Reply-To: <20260716012138.6714-1-jayatheerthkulkarni2005@gmail.com>
Scripts frequently need to find the root directory of a repository's
working tree. Currently, this requires using `git rev-parse --show-toplevel`
or inferring it from other path components.
Introduce `path.toplevel.absolute` and `path.toplevel.relative` keys
to `git repo info`. This allows scripts to retrieve the top-level
working tree path in a predictable, strictly formatted manner without
relying on `rev-parse`.
If requested in a bare repository where no working tree exists, the
command returns an empty string.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 10 ++++++++++
builtin/repo.c | 28 ++++++++++++++++++++++++++++
t/t1900-repo-info.sh | 30 ++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index ed7d80c690..e34abe5fea 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -119,6 +119,16 @@ values that they return:
`path.gitdir.relative`::
The path to the Git repository directory relative to the current working directory.
+`path.toplevel.absolute`::
+ The canonical absolute path to the top-level directory of the
+ repository's working tree. Outputs an empty string if the repository
+ is bare.
+
+`path.toplevel.relative`::
+ The path to the top-level directory of the repository's working
+ tree relative to the current working directory. Outputs an empty
+ string if the repository is bare.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index 042d6de558..194757eb18 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -121,6 +121,32 @@ static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_toplevel_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *work_tree = repo_get_work_tree(repo);
+
+ if (!work_tree) {
+ strbuf_addstr(buf, "");
+ return 0;
+ }
+
+ format_path(buf, work_tree, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_toplevel_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *work_tree = repo_get_work_tree(repo);
+
+ if (!work_tree) {
+ strbuf_addstr(buf, "");
+ return 0;
+ }
+
+ format_path(buf, work_tree, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -137,6 +163,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "path.commondir.relative", get_path_commondir_relative },
{ "path.gitdir.absolute", get_path_gitdir_absolute },
{ "path.gitdir.relative", get_path_gitdir_relative },
+ { "path.toplevel.absolute", get_path_toplevel_absolute },
+ { "path.toplevel.relative", get_path_toplevel_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index ae8c22c817..fbb9063ee5 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -213,4 +213,34 @@ test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
'.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_expect_success 'path.toplevel absolute and relative' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ mkdir -p repo/sub &&
+ cd repo/sub &&
+
+ ROOT="$(test-tool path-utils real_path ..)" &&
+
+ echo "path.toplevel.absolute=$ROOT" >expect.abs &&
+ git repo info path.toplevel.absolute >actual.abs &&
+ test_cmp expect.abs actual.abs &&
+
+ echo "path.toplevel.relative=../" >expect.rel &&
+ git repo info path.toplevel.relative >actual.rel &&
+ test_cmp expect.rel actual.rel
+ )
+'
+
+test_expect_success 'path.toplevel returns empty in a bare repository' '
+ test_when_finished "rm -rf bare.git" &&
+ git init --bare bare.git &&
+ (
+ cd bare.git &&
+ echo "path.toplevel.absolute=" >expect &&
+ git repo info path.toplevel.absolute >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.55.GIT
^ permalink raw reply related
* [GSoC Patch 0/7] repo: add more path keys to git repo info
From: K Jayatheerth @ 2026-07-16 1:21 UTC (permalink / raw)
To: git; +Cc: jltobler, lucasseikioshiro, K Jayatheerth
Series adds keys to git repo info.
Keys output paths of repository components:
* path.toplevel: repository tree.
* path.superproject-working-tree: superproject tree from submodules.
* path.objects: repository objects.
* path.hooks: repository hooks.
* path.index: repository index.
* path.grafts: repository grafts.
* path.git-prefix: prefix offset.
Keys support suffixes for format.
Commits contain documentation and tests.
K Jayatheerth (7):
repo: add path.toplevel with absolute and relative suffix formatting
repo: add path.superproject-working-tree with absolute and relative
suffixes
repo: add path.objects with absolute and relative suffix formatting
repo: add path.hooks with absolute and relative suffix formatting
repo: add path.index with absolute and relative suffix formatting
repo: add path.grafts with absolute and relative suffix formatting
repo: add path.git-prefix path key validation
Documentation/git-repo.adoc | 58 +++++++++++++
builtin/repo.c | 167 ++++++++++++++++++++++++++++++++++++
t/t1900-repo-info.sh | 108 +++++++++++++++++++++++
3 files changed, 333 insertions(+)
--
2.55.GIT
^ permalink raw reply
* Re: [PATCH GSoC v18 00/13] cat-file: add remote-object-info to batch-command
From: Karthik Nayak @ 2026-07-15 22:35 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon
In-Reply-To: <20260715-ps-eric-work-rebase-v18-0-34d7adb051bb@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> This patch series is a continuation of Eric Ju's
> (eric.peijian@gmail.com) and Calvin Wan's (calvinwan@google.com) patch
> series [1] and [2] respectively.
>
> Sometimes it is beneficial to retrieve information about an object
> without having to download it completely. The server logic for
> retrieving size has already been implemented and merged in a2ba162cda
> (object-info: support for retrieving object info, 2021-04-20) [3].
> This patch series implement the client option for it.
>
> Eric's series adds the remote-object-info command to cat-file
> --batch-command. This command allows the client to make an object-info
> command request to a server that supports protocol v2.
>
> If the server uses protocol v2 but does not support the object-info
> capability, cat-file --batch-command will die.
>
> If a user attempts to use remote-object-info with protocol v1, cat-file
> --batch-command will die.
>
> Currently, only the size (%(objectsize)) is supported end to end in this
> implementation. The type (%(objecttype)) is known by the client's
> allow-list and request path but is not supported on the server side
> nor the response parsing. A follow up series will add full end-to-end
> support for %(objecttype).
>
> The default format for remote-object-info is set to "%(objectname)
> %(objectsize)". Once %(objecttype) is supported, the default format will
> be unified accordingly.
>
> If the batch command format includes unsupported fields such as
> %(objecttype), %(objectsize:disk), or %(deltabase), the command will
> return empty strings for each unsupported field.
>
> This series completes Eric's work mainly with the refactor of the
> validation of the placeholder with an allow-list that filters what the
> client asks with what the server is capable of provide following Jeff
> King's idea [4].
>
> GitHub CI: https://github.com/pabloosabaterr/git/actions/runs/29404390713
>
> [1]: https://lore.kernel.org/git/20250221190451.12536-1-eric.peijian@gmail.com/
> [2]: https://lore.kernel.org/git/20220728230210.2952731-1-calvinwan@google.com/#t
> [3]: https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2ba162cda2acc171c3e36acbbc854792b093cb7
> [4]: https://lore.kernel.org/git/20250313060250.GH94015@coredump.intra.peff.net/
>
> Changes since v17:
>
> At 10th commit: transport: add client support for object-info
> - style
> - enforced the server response handling, not allowing bare "<oid>" or
> responses with a different number of attributes different from the
> number asked.
>
Some small comments/nits from my side, mostly looks good otherwise :)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH GSoC v18 11/13] cat-file: add remote-object-info to batch-command
From: Karthik Nayak @ 2026-07-15 22:27 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260715-ps-eric-work-rebase-v18-11-34d7adb051bb@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6749 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
> index 86b9181599..a7fa6674c3 100644
> --- a/Documentation/git-cat-file.adoc
> +++ b/Documentation/git-cat-file.adoc
> @@ -169,6 +169,13 @@ info <object>::
> Print object info for object reference `<object>`. This corresponds to the
> output of `--batch-check`.
>
> +remote-object-info <remote> <object>...::
> + Print object info for object references `<object>` at specified
> + `<remote>` without downloading objects from the remote.
> + Raise an error when the `object-info` capability is not supported by the remote.
> + Raise an error when no object references are provided.
> + This command may be combined with `--buffer`.
> +
> flush::
> Used with `--buffer` to execute all preceding commands that were issued
> since the beginning or since the last flush was issued. When `--buffer`
> @@ -301,7 +308,8 @@ one per line, and print information based on the command given. With
> `--batch-command`, the `info` command followed by an object will print
> information about the object the same way `--batch-check` would, and the
> `contents` command followed by an object prints contents in the same way
> -`--batch` would.
> +`--batch` would. The `remote-object-info` command followed by a remote and
> +objects IDs prints object info from the remote without downloading the objects.
>
> You can specify the information shown for each object by using a custom
> `<format>`. The `<format>` is copied literally to stdout for each
> @@ -324,15 +332,12 @@ newline. The available atoms are:
> reports).
>
> `objectsize:disk`::
> - The size, in bytes, that the object takes up on disk. See the
> - note about on-disk sizes in the `CAVEATS` section below.
> + The size, in bytes, that the object takes up on disk.
>
Why did we remove the second sentence here? Cause the caveat still
exists and holds good.
> `deltabase`::
> If the object is stored as a delta on-disk, this expands to the
> full hex representation of the delta base object name.
> - Otherwise, expands to the null OID (all zeroes). See `CAVEATS`
> - below.
> -
> + Otherwise, expands to the null OID (all zeroes).
We also remove the newline here...
> `rest`::
> If this atom is used in the output string, input lines are split
> at the first whitespace boundary. All characters before that
> @@ -340,8 +345,14 @@ newline. The available atoms are:
> after that first run of whitespace (i.e., the "rest" of the
> line) are output in place of the `%(rest)` atom.
>
> +The command `remote-object-info` only supports the `%(objectname)` and
> +`%(objectsize)` placeholders. See `CAVEATS` below for more information.
> +
> If no format is specified, the default format is `%(objectname)
> -%(objecttype) %(objectsize)`.
> +%(objecttype) %(objectsize)`, except for `remote-object-info` commands which
> +use `%(objectname) %(objectsize)` because "%(objecttype)" is not supported yet.
> +WARNING: When "%(objecttype)" is supported, the default format WILL be unified,
> +so DO NOT RELY on the current default format to stay the same!!!
>
> If `--batch` is specified, or if `--batch-command` is used with the `contents`
> command, the object information is followed by the object contents (consisting
> @@ -438,6 +449,10 @@ scripting purposes.
> CAVEATS
> -------
>
> +Note that since only `%(objectname)` and `%(objectsize)` are currently
> +supported by the `remote-object-info` command. Using any other placeholder in
> +the format string will raise an error.
> +
> Note that the sizes of objects on disk are reported accurately, but care
> should be taken in drawing conclusions about which refs or objects are
> responsible for disk usage. The size of a packed non-delta object may
> be
>
[snip]
> +static void parse_cmd_remote_object_info(struct batch_options *opt,
> + const char *line, struct strbuf *output,
> + struct expand_data *data)
> +{
> + int count;
> + const char **argv;
> + char *line_to_split;
> + struct object_info *remote_object_info = NULL;
> + struct oid_array object_info_oids = OID_ARRAY_INIT;
> +
> + if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
> + die(_("remote-object-info command too long"));
> +
> + line_to_split = xstrdup(line);
> + count = split_cmdline(line_to_split, &argv);
> + if (count < 0)
> + die(_("remote-object-info: %s"), split_cmdline_strerror(count));
Nit: Should we provide some context here? Otherwise no point being
marked for translation?
> + if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
> + die(_("remote-object-info supports at most %d objects"),
> + MAX_ALLOWED_OBJ_LIMIT);
> +
> + if (get_remote_info(opt, count, argv, &remote_object_info,
> + &object_info_oids))
> + goto cleanup;
> +
> + data->skip_object_info = 1;
> + for (size_t i = 0; i < object_info_oids.nr; i++) {
> + data->oid = object_info_oids.oid[i];
> + if (remote_object_info[i].sizep) {
> + /*
> + * When reaching here, it means remote-object-info can retrieve
> + * information from server without downloading them.
> + */
> + data->size = *remote_object_info[i].sizep;
> + opt->batch_mode = BATCH_MODE_INFO;
> + batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
> + } else {
> + report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
> + }
> + }
> + data->skip_object_info = 0;
> +
> +cleanup:
> + for (size_t i = 0; i < object_info_oids.nr; i++)
> + free_object_info_contents(&remote_object_info[i]);
> + free(line_to_split);
> + free(argv);
> + free(remote_object_info);
> + oid_array_clear(&object_info_oids);
> +}
> +
> static void dispatch_calls(struct batch_options *opt,
> struct strbuf *output,
> struct expand_data *data,
> @@ -748,8 +889,9 @@ static const struct parse_cmd {
> } commands[] = {
> { "contents", parse_cmd_contents, 1 },
> { "info", parse_cmd_info, 1 },
> - { "flush", NULL, 0 },
> { "mailmap", parse_cmd_mailmap, 1 },
> + { "remote-object-info", parse_cmd_remote_object_info, 1 },
> + { "flush", NULL, 0 },
Why do we move this? we remove the alphabetical ordering by doing s
> };
>
> static void batch_objects_command(struct batch_options *opt,
> diff --git a/object-file.c b/object-file.c
> index 6453b1d6fa..07f019a0f6 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1694,3 +1694,13 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
>
> return &transaction->base;
> }
> +
> +void free_object_info_contents(struct object_info *object_info)
> +{
> + if (!object_info)
> + return;
> + free(object_info->typep);
> + free(object_info->sizep);
> + free(object_info->disk_sizep);
> + free(object_info->delta_base_oid);
> +}
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH GSoC v18 05/13] fetch-pack: drop static advertise_sid variable
From: Karthik Nayak @ 2026-07-15 21:48 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260715-ps-eric-work-rebase-v18-5-34d7adb051bb@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4307 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> write_fetch_command_and_capabilities() is moved to 'connect.c' in a
> subsequent commit. To prepare for that, drop the static variable usage
> of advertise_sid. Currently advertise_sid is used in two places:
>
> 1. In function do_fetch_pack():
> if (!server_supports("session-id"))
> advertise_sid = 0;
>
> 2. In function fetch_pack_config():
> repo_config_get_bool("transfer.advertisesid", &advertise_sid);
>
Nit: But #2 isn't a usecase, it's where it is set no? Looking at the
usecase, it seems like we have two:
#1 like you stated
#2 within `write_fetch_command_and_capabilities()`. But the flow is
that, the variable is set in `fetch_pack_config()`, right?
> About 1, it is only relevant for v0/v1 protocol, move it into
> find_common().
>
> About 2, call repo_config_get_bool() inside of
> write_fetch_command_and_capabilities() and find_common() replacing the
> static variable.
>
> Because repo_config_get_bool() leaves advertise_sid as is if it is not
> set, initialize it to 0 matching its default.
>
> Helped-by: Jonathan Tan <jonathantanmy@google.com>
> Helped-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> fetch-pack.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index eea72b2500..8e04db8640 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -49,7 +49,6 @@ static int fetch_fsck_objects = -1;
> static int transfer_fsck_objects = -1;
> static int agent_supported;
> static int server_supports_filtering;
> -static int advertise_sid;
> static struct shallow_lock shallow_lock;
> static const char *alternate_shallow_file;
> static struct strbuf fsck_msg_types = STRBUF_INIT;
> @@ -363,6 +362,9 @@ static int find_common(struct fetch_negotiator *negotiator,
> size_t state_len = 0;
> struct packet_reader reader;
> struct oidset negotiation_include_oids = OIDSET_INIT;
> + int advertise_sid = 0;
> +
> + repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
>
> if (args->stateless_rpc && multi_ack == 1)
> die(_("the option '%s' requires '%s'"), "--stateless-rpc", "multi_ack_detailed");
> @@ -414,7 +416,7 @@ static int find_common(struct fetch_negotiator *negotiator,
> if (deepen_not_ok) strbuf_addstr(&c, " deepen-not");
> if (agent_supported) strbuf_addf(&c, " agent=%s",
> git_user_agent_sanitized());
> - if (advertise_sid)
> + if (advertise_sid && server_supports("session-id"))
> strbuf_addf(&c, " session-id=%s", trace2_session_id());
> if (args->filter_options.choice)
> strbuf_addstr(&c, " filter");
> @@ -1160,9 +1162,6 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
> (int)agent_len, agent_feature);
> }
>
> - if (!server_supports("session-id"))
> - advertise_sid = 0;
> -
So earlier we'd set the `advertise_sid` to 0 if the server didn't support
'session-id'. But we could directly check where it is needed, which is
`find_common()`. Is `find_common()` only called in v0/v1 as stated in
the commit message?
> if (server_supports("shallow"))
> print_verbose(args, _("Server supports %s"), "shallow");
> else if (args->depth > 0 || is_repository_shallow(r))
> @@ -1380,6 +1379,9 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> const struct string_list *server_options)
> {
> const char *hash_name;
> + int advertise_sid = 0;
> +
> + repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
>
> ensure_server_supports_v2("fetch");
> packet_buf_write(req_buf, "command=fetch");
> @@ -1998,7 +2000,6 @@ static void fetch_pack_config(void)
> repo_config_get_bool(the_repository, "repack.usedeltabaseoffset", &prefer_ofs_delta);
> repo_config_get_bool(the_repository, "fetch.fsckobjects", &fetch_fsck_objects);
> repo_config_get_bool(the_repository, "transfer.fsckobjects", &transfer_fsck_objects);
> - repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
> if (!uri_protocols.nr) {
> char *str;
>
>
> --
> 2.54.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH GSoC v18 13/13] cat-file: make remote-object-info allow-list dynamic
From: Pablo Sabater @ 2026-07-15 21:44 UTC (permalink / raw)
To: Junio C Hamano, Pablo Sabater
Cc: git, chandrapratap3519, chriscool, eric.peijian, jltobler,
karthik.188, peff, toon
In-Reply-To: <xmqqwluwj8of.fsf@gitster.g>
On Wed Jul 15, 2026 at 10:32 PM CEST, Junio C Hamano wrote:
> "Pablo Sabater" <pabloosabaterr@gmail.com> writes:
>
>>>> 2. Filters the request in fetch_object_info() dropping any option that
>>>> the server does not advertise.
>>>>
>>>> 3. After the fetching, the options that haven't been dropped are the ones
>>>> fetched and supported by the server, these supported options are
>>>> mapped and remote_allowed_atoms is populated with the placeholders.
>>>>
>>>> 4. expand_atom() checks remote_allowed_atoms with the same behaviour as
>>>> the static allow_list had.
>>>
>>> I am not sure I follow the above entirely. Could you add a
>>> concrete example to the commit message?
>>>
>>> For instance, if the client wants "%(objectsize) %(objectcolor)" and
>>> the server only supports 'size' but not 'color', the filtering in
>>> step (2) prevents the client from asking about the color, requesting
>>> only the size instead. When the server says the size is 42, step (3)
>>> uses that to substitute '%(objectsize)'. Would the end result then
>>> be "42 %(objectcolor)"?
>>
>> You've gotten everything right until the last step, because we have only
>> size from the server there is no data to match %(objectcolor) and the
>> end result is an empty string for %(objeccolor):
>>
>> "42 "
>>
>> Note that %(objectcolor) doesn't exists and it would have die(), the
>> empty string is only for known but unsupported placeholders.
>
> It was not clear there is a distinction between "unknown" and
> "known but unsupported". The proposed log message needs to be
> clarified to make this distinction obvious.
Sorry, I'll make sure to write it clear next reroll.
>
>>> And if the request is only for "%(objectname)", an empty
>>> object_info_options is given to get_remote_info().
>>
>> Right now 'name' is not part of the protocol as 'type' or 'size' are,
>> 'objectname' is always allowed but only shown if it's present on the
>> format.
>> If the format is only "%(objectname)" then there's nothing to ask the
>> server for.
>>
>> The current code avoids making the request if there's only objectname or
>> nothing supported, but still goes through the connection work. I will
>> add an early return to just output the oid back without any connection.
>
> I think you are heading in the opposite direction. Rather, when
> only the object name is requested, I was hoping we would pick
> something cheap to retrieve and ask the remote side for it, if
> only to catch a bogus or missing object name.
Looking at send_info() in 'protocol-caps.c', the server only calls
odb_read_object_info() inside the `if (info->size)`. A bare OID without
any requested attribute gets echoed back with no existence check.
We can force "size" when only %(objectname) is requested so the
server validates the OID, and discard the size on the client side.
Because this is a cheap fix, I'll add a NEEDSWORK for the existence
check to be done regardless of the attributes requested.
This can be done on a future series focused on the server side.
>
> Thanks.
Regards,
Pablo
^ permalink raw reply
* Re: Git 2.55.0 breaks revision path filtering with --no-walk
From: Kristofer Karlsson @ 2026-07-15 21:09 UTC (permalink / raw)
To: Peter Colberg; +Cc: git
In-Reply-To: <alfWF9cBPESuLBgS@earendel>
On Wed, 15 Jul 2026 at 20:48, Peter Colberg <pcolberg@redhat.com> wrote:
>
> Is the behaviour in Git 2.55.0 intentional, i.e., was --no-walk never
> intended to support path filtering, or is this indeed a regression?
Ouch, this definitely seems like a regression and it can be
reproduced with a very simple scenario:
git init repo && cd repo
echo a >file-a && git add file-a && git commit -m "add file-a"
echo b >file-b && git add file-b && git commit -m "add file-b"
git rev-list --no-walk HEAD -- file-a
This should produce no output since HEAD only touches file-b,
but with 2.55.0 it incorrectly outputs the commit.
I think you correctly identified the problematic change too,
my refactoring indeed has a bug where process_parents() is skipped.
I think I can fix it, and also add test cases for this,
but I am not sure what the immediate appropriate action is --
revert the entire patch or apply a bugfix?
I think I can also try to check if there are other affected
code paths that need additional test cases.
For what it's worth, I suspect the bugfix would look something
like this, but I have not fully verified it yet.
in revision.c:
case REV_WALK_NO_WALK:
+ try_to_simplify_commit(revs, commit);
+ break;
case REV_WALK_LIMITED:
Thanks for the report, I will try to contribute a fix if possible,
Kristofer
^ permalink raw reply
* Re: [PATCH 0/7] line-log: range-scope stat, check, and -G under -L
From: Michael Montalbo @ 2026-07-15 21:05 UTC (permalink / raw)
To: Michael Montalbo via GitGitGadget; +Cc: git, D. Ben Knoble
In-Reply-To: <pull.2152.git.1781806593.gitgitgadget@gmail.com>
Friendly ping.
^ permalink raw reply
* [PATCH v5 9/9] line-log: consult diff process for range tracking
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:02 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
git log -L tracks line ranges by diffing each commit against its
parent in collect_diff(). This pass used the builtin diff while the
displayed diff (builtin_diff()) consults a configured
diff.<driver>.process, so the two could disagree: a reformat-only
commit selected by builtin tracking was then rendered with an empty
diff because the tool reported the files equivalent.
Consult the process in collect_diff() too, mirroring the blame
integration. When the tool reports the files equivalent, collect no
ranges; the tracked range then maps across unchanged and the commit
drops out of the log, matching what is displayed. Like the summary
formats, the tracking pass diffs raw content, so the tool is consulted
on the raw blobs here.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/gitattributes.adoc | 20 ++++++++++---------
line-log.c | 33 ++++++++++++++++++++++++++++----
t/t4080-diff-process.sh | 33 ++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+), 13 deletions(-)
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index 7cdede6b21..8021dc8e39 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -1037,12 +1037,16 @@ Features that ask "which lines changed" use the tool's hunks in place
of the builtin algorithm:
- `git diff` patch output, together with everything layered on it:
- word diff, function context (`-W`), `--color-moved`, the `@@` hunk
- headers, and the `-L` line-range display. These operate on the
- lines the patch step already emitted, so they reflect the tool's
- hunks without any further negotiation.
+ word diff, function context (`-W`), `--color-moved`, and the `@@`
+ hunk headers. These operate on the lines the patch step already
+ emitted, so they reflect the tool's hunks without any further
+ negotiation.
- `git blame`: a commit whose change the tool reports as equivalent is
skipped, and its lines are attributed to an earlier commit.
+- `git log -L`: both the line-range display and the underlying range
+ tracking consult the tool, so a commit it reports as equivalent is
+ dropped from the log (its tracked range maps across unchanged)
+ rather than selected and then shown with an empty diff.
- `--stat`, `--numstat`, and `--shortstat`: the inserted and deleted
counts come from the tool's hunks, so a file the tool calls
equivalent contributes no stat line, matching the empty patch that
@@ -1079,11 +1083,9 @@ design:
- `--raw`, `--name-only`, and `--name-status` compare object ids at
the tree level and never run a line-level diff at all.
-Two cases ask "which lines changed" but still use the builtin
-algorithm, and may consult the process in a later change: `git log
--L`'s commit selection and parent range propagation (as distinct from
-its display, which is covered above), and combined diffs (`--cc` and
-merge diffs), whose protocol would have to be extended from a single
+Combined diffs (`--cc` and merge diffs) ask "which lines changed" but
+still use the builtin algorithm, and may consult the process in a
+later change; their protocol would have to be extended from a single
old/new pair to one comparison per merge parent.
`--no-ext-diff` and `--diff-algorithm` bypass the process entirely,
diff --git a/line-log.c b/line-log.c
index 5fc75ae275..97b3e0a31d 100644
--- a/line-log.c
+++ b/line-log.c
@@ -7,11 +7,11 @@
#include "tag.h"
#include "tree.h"
#include "diff.h"
+#include "diff-process.h"
#include "commit.h"
#include "decorate.h"
#include "repository.h"
#include "revision.h"
-#include "xdiff-interface.h"
#include "strbuf.h"
#include "line-log.h"
#include "setup.h"
@@ -330,12 +330,15 @@ static int collect_diff_cb(long start_a, long count_a,
return 0;
}
-static int collect_diff(mmfile_t *parent, mmfile_t *target, struct diff_ranges *out)
+static int collect_diff(struct diff_options *diffopt, const char *path,
+ mmfile_t *parent, mmfile_t *target,
+ struct diff_ranges *out)
{
struct collect_diff_cbdata cbdata = {NULL};
xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
+ int ret = 0;
memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
@@ -345,7 +348,23 @@ static int collect_diff(mmfile_t *parent, mmfile_t *target, struct diff_ranges *
xecfg.hunk_func = collect_diff_cb;
memset(&ecb, 0, sizeof(ecb));
ecb.priv = &cbdata;
- return xdi_diff(parent, target, &xpp, &xecfg, &ecb);
+
+ /*
+ * Consult the diff process so range tracking agrees with the
+ * diff that will be shown. When the tool reports the files as
+ * equivalent we collect no ranges, so the tracked range maps
+ * across unchanged and the commit drops out of the log, rather
+ * than being selected here but rendered with an empty diff by
+ * the process-aware builtin_diff(). Blob oids are not threaded to
+ * this path yet, so pass NULL and send no old-oid/new-oid (a later
+ * change can supply the pair, where they would let the tool cache
+ * across the range-tracking and display passes over the same
+ * commit).
+ */
+ if (xdi_diff_process(diffopt, path, parent, target,
+ NULL, NULL, &xpp, &xecfg, &ecb) == DIFF_PROCESS_ERROR)
+ ret = -1;
+ return ret;
}
/*
@@ -927,7 +946,13 @@ static int process_diff_filepair(struct rev_info *rev,
}
diff_ranges_init(&diff);
- if (collect_diff(&file_parent, &file_target, &diff))
+ /*
+ * Select the driver by the old (parent) path, as builtin_diff() does
+ * with name_a, so a renamed file resolves to the same driver for
+ * range tracking as for the diff that is shown.
+ */
+ if (collect_diff(&rev->diffopt, pair->one->path,
+ &file_parent, &file_target, &diff))
die("unable to generate diff for %s", pair->one->path);
/* NEEDSWORK should apply some heuristics to prevent mismatches */
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index e1c7256747..c20ab15ecd 100755
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -914,4 +914,37 @@ test_expect_success 'blame -w bypasses diff process' '
test_path_is_missing backend.log
'
+#
+# Line-log (git log -L) range tracking.
+#
+
+test_expect_success 'diff process drops equivalent commit from log -L' '
+ test_when_finished "rm -f backend.log" &&
+ cat >linelog.c <<-\EOF &&
+ int tracked(void) { return 1; }
+ EOF
+ git add linelog.c &&
+ git commit -m "add linelog.c" &&
+
+ cat >linelog.c <<-\EOF &&
+ int tracked(void) { return 2; }
+ EOF
+ git commit -am "change tracked line" &&
+
+ # Builtin line tracking selects the change commit.
+ git log --no-ext-diff -L1,1:linelog.c --format="%s" >builtin &&
+ test_grep "change tracked line" builtin &&
+
+ # With the tool reporting the change as equivalent, tracking
+ # drops the commit (the range maps across unchanged) instead of
+ # selecting it and rendering an empty diff.
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ log -L1,1:linelog.c --format="%s" >actual &&
+ test_grep ! "change tracked line" actual &&
+ # The creating commit still appears, so the change commit was
+ # selectively dropped rather than the whole log going empty.
+ test_grep "add linelog.c" actual &&
+ test_grep "command=hunks pathname=linelog.c" backend.log
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 8/9] diff: consult diff process for --stat counts
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:02 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
builtin_diff() already consults a configured diff.<driver>.process: a
file the tool reports as equivalent emits no patch, and otherwise the
tool's hunks drive the output. builtin_diffstat() ran its own xdiff
and ignored the process, so "git diff --stat" still counted a
byte-level change for a file that "git diff" showed as unchanged.
Consult diff_process_fill_hunks() before the stat xdiff, as
builtin_diff() does. On DIFF_PROCESS_EQUIVALENT, skip the xdiff so
the file keeps its zero inserted and deleted counts and the existing
"nothing changed" pruning drops it, matching the empty patch.
Otherwise the tool's hunks, or the builtin fallback, feed the counts
through the shared xpparam_t.
Like the builtin summary path, builtin_diffstat() does not apply
textconv, so the process is consulted on the raw blob content here,
unlike builtin_diff() which sends textconv'd content. This keeps
"git diff --stat" counting raw lines as it does today; the asymmetry
between patch output and summary counts under textconv predates this
change. Because the content is the raw blob, the stat path sends the
blob object names to the tool (old-oid/new-oid) for any stored blob,
where the patch path omits the oid under textconv.
Move the summary formats out of the "not yet wired" group of the
"Which features consult the diff process" documentation and into the
list of features that use the tool's hunks, noting the raw,
non-textconv content they receive. Document that the line-counting
--dirstat=lines follows these counts while the default --dirstat does
not, and that summary formats and blame (only under --textconv) differ
from patch output in whether they textconv the content the tool sees.
Add tests covering counts from the tool's hunks (--numstat,
--shortstat), an equivalent file producing no stat line, --stat
--exit-code, the raw non-textconv content the tool receives, a
multi-file mix of equivalent and changed files, and a mode-only
change.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/gitattributes.adoc | 33 +++++---
diff.c | 20 ++++-
t/t4080-diff-process.sh | 130 +++++++++++++++++++++++++++++++
3 files changed, 172 insertions(+), 11 deletions(-)
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index a03fb9deb1..7cdede6b21 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -874,7 +874,10 @@ a flush packet, followed by the old and new file content as packetized
data, each terminated with a flush packet. The pathname is relative
to the repository root. When `diff.<name>.textconv` is also set,
the tool receives the textconv-transformed content rather than the
-raw blob. Git does not send binary files to the diff process.
+raw blob, matching what the consuming feature itself diffs: patch
+output is textconv'd, the summary formats (noted below) are not, and
+`git blame` applies textconv only under `--textconv`. Git does not
+send binary files to the diff process.
-----------------------
packet: git> command=hunks
@@ -960,8 +963,8 @@ still slide or regroup those changes against matching context for
display, exactly as it compacts its own diffs, so the tool controls
which lines are reported as changed, not the precise hunk boundaries.
Patch output features (word diff, function context, color) work
-normally. Summary formats such as `--stat` still compute their counts
-with the builtin diff for now; see "Which features consult the diff
+normally, as do summary formats like `--stat`. Not every feature
+consults the process, though; see "Which features consult the diff
process" below for the full picture and the reasoning behind it.
If no hunk lines precede the flush, followed by "success", Git
@@ -1040,6 +1043,17 @@ of the builtin algorithm:
hunks without any further negotiation.
- `git blame`: a commit whose change the tool reports as equivalent is
skipped, and its lines are attributed to an earlier commit.
+- `--stat`, `--numstat`, and `--shortstat`: the inserted and deleted
+ counts come from the tool's hunks, so a file the tool calls
+ equivalent contributes no stat line, matching the empty patch that
+ `git diff` produces for it. These summary formats do not apply
+ textconv (just as the builtin summary path does not), so the tool
+ is consulted on the raw blob content even when a `textconv` is also
+ configured for patch output; this mirrors how builtin `--stat`
+ already counts raw lines rather than the textconv'd view. The
+ line-counting `--dirstat=lines` uses these same counts; the default
+ `--dirstat`, which weighs byte changes, is computed on its own and
+ does not consult the tool.
Features that ask a different question do not consult the process, by
design:
@@ -1065,13 +1079,12 @@ design:
- `--raw`, `--name-only`, and `--name-status` compare object ids at
the tree level and never run a line-level diff at all.
-Some features ask "which lines changed" but still use the builtin
-algorithm for now, and may consult the process in a later change: the
-summary formats (`--stat`, `--numstat`, `--shortstat`); `git log -L`'s
-commit selection and parent range propagation (as distinct from its
-display, which is covered above); and combined diffs (`--cc` and merge
-diffs), whose protocol would have to be extended from a single old/new
-pair to one comparison per merge parent.
+Two cases ask "which lines changed" but still use the builtin
+algorithm, and may consult the process in a later change: `git log
+-L`'s commit selection and parent range propagation (as distinct from
+its display, which is covered above), and combined diffs (`--cc` and
+merge diffs), whose protocol would have to be extended from a single
+old/new pair to one comparison per merge parent.
`--no-ext-diff` and `--diff-algorithm` bypass the process entirely,
for every feature listed above. The whitespace-ignoring options
diff --git a/diff.c b/diff.c
index a9a732629e..f3c8267a39 100644
--- a/diff.c
+++ b/diff.c
@@ -4269,9 +4269,27 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
xecfg.ctxlen = o->context;
xecfg.interhunkctxlen = o->interhunkcontext;
xecfg.flags = XDL_EMIT_NO_HUNK_HDR;
- if (xdi_diff_outf(&mf1, &mf2, NULL,
+ /*
+ * Consult the diff process so --stat reflects the
+ * tool's view of which lines changed rather than the
+ * builtin line diff. --stat never applies textconv, so
+ * the tool is fed the same raw mmfiles the stat itself
+ * diffs (unlike builtin_diff, which consults the process
+ * on textconv'd content).
+ * When the tool reports the files as equivalent we skip
+ * xdiff entirely, leaving added and deleted at zero so
+ * the file is pruned below, just as builtin_diff() emits
+ * no patch for an equivalent file.
+ */
+ if (diff_process_fill_hunks(o, name_a, &mf1, &mf2,
+ one->oid_valid ? &one->oid : NULL,
+ two->oid_valid ? &two->oid : NULL,
+ &xpp)
+ != DIFF_PROCESS_EQUIVALENT &&
+ xdi_diff_outf(&mf1, &mf2, NULL,
diffstat_consume, diffstat, &xpp, &xecfg))
die("unable to generate diffstat for %s", one->path);
+ free(xpp.external_hunks);
if (DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two)) {
struct diffstat_file *file =
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index 694c94edb2..e1c7256747 100755
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -282,6 +282,21 @@ test_expect_success 'diff process works alongside textconv' '
test_must_be_empty stderr
'
+test_expect_success 'diff process --stat is fed raw, not textconv, content' '
+ # Reuses textconv.c from the previous test (committed "hello
+ # world", modified to "goodbye world"). Unlike patch output,
+ # --stat does not apply textconv, so the tool sees raw lowercase
+ # content here even with a textconv configured.
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.textconv="./uppercase-filter" \
+ -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff --stat -- textconv.c >actual 2>stderr &&
+ test_grep "pathname=textconv.c" backend.log &&
+ test_grep "old=hello world" backend.log &&
+ test_grep "new=goodbye world" backend.log &&
+ test_must_be_empty stderr
+'
+
#
# Downstream features: word diff, log, equivalent files, exit code.
#
@@ -386,6 +401,121 @@ test_expect_success 'diff process with --exit-code and hunks returns failure' '
diff --exit-code newfile.c
'
+test_expect_success 'diff process feeds --numstat counts' '
+ # fixed-hunk reports only lines 5-6 as changed, so the stat
+ # counts come from the tool (2/2), not the builtin diff (4/4).
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk --log=backend.log" \
+ diff --numstat boundary.c >actual 2>stderr &&
+ printf "2\t2\tboundary.c\n" >expect &&
+ test_cmp expect actual &&
+ test_grep "command=hunks pathname=boundary.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process --numstat sums multi-hunk counts' '
+ # multi-hunk reports both 2-line regions (5-6 and 9-10), so the
+ # counts add up across both hunks: 4 inserted, 4 deleted. This
+ # exercises the two-region hunk path through builtin_diffstat.
+ git -c diff.cdiff.process="$BACKEND --mode=multi-hunk" \
+ diff --numstat boundary.c >actual &&
+ printf "4\t4\tboundary.c\n" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'diff process equivalent files produce no --stat line' '
+ # A file the tool calls equivalent contributes no stat line,
+ # matching the empty patch that git diff produces for it.
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ diff --stat worddiff.c >actual 2>stderr &&
+ test_must_be_empty actual &&
+ test_grep "command=hunks pathname=worddiff.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process feeds --shortstat counts' '
+ # fixed-hunk reports lines 5-6 only, so the summary counts come
+ # from the tool (2 insertions, 2 deletions), not builtin (4/4).
+ git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk" \
+ diff --shortstat boundary.c >actual &&
+ test_grep "2 insertions" actual &&
+ test_grep "2 deletions" actual
+'
+
+test_expect_success 'diff process equivalent file makes --stat --exit-code succeed' '
+ # The tool reports worddiff.c equivalent, so --exit-code reports
+ # no change (0); the builtin diff would report a change (1).
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
+ diff --stat --exit-code worddiff.c &&
+ test_expect_code 1 git diff --no-ext-diff --stat --exit-code worddiff.c
+'
+
+test_expect_success 'diff process --numstat with mixed equivalent and changed files' '
+ test_when_finished "rm -f c.log h.log" &&
+ # Self-contained fixtures: *.c uses whole-file (changed); *.mh
+ # uses no-hunks (equivalent).
+ echo "*.mh diff=hdiff" >>.gitattributes &&
+ git add .gitattributes &&
+ printf "int a(void) { return 1; }\n" >mixed.c &&
+ printf "int b(void) { return 1; }\n" >mixed.mh &&
+ git add mixed.c mixed.mh &&
+ git commit -m "add mixed fixtures" &&
+ printf "int a(void) { return 2; }\n" >mixed.c &&
+ printf "int b(void) { return 2; }\n" >mixed.mh &&
+ git -c diff.cdiff.process="$BACKEND --mode=whole-file --log=c.log" \
+ -c diff.hdiff.process="$BACKEND --mode=no-hunks --log=h.log" \
+ diff --numstat mixed.c mixed.mh >actual 2>stderr &&
+ test_grep "mixed.c" actual &&
+ test_grep ! "mixed.mh" actual &&
+ test_grep "pathname=mixed.c" c.log &&
+ test_grep "pathname=mixed.mh" h.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success POSIXPERM 'diff process keeps mode-only change in --stat' '
+ test_when_finished "rm -f backend.log" &&
+ cat >modeonly.c <<-\EOF &&
+ int m(void) { return 1; }
+ EOF
+ git add modeonly.c &&
+ git commit -m "add modeonly.c" &&
+ cat >modeonly.c <<-\EOF &&
+ int m(void) { return 2; }
+ EOF
+ git add modeonly.c &&
+ test_chmod +x modeonly.c &&
+ git commit -m "edit and chmod modeonly.c" &&
+ # Content and mode both changed, but no-hunks reports the content
+ # equivalent. The tool is consulted (counts are zero, not the
+ # builtin 1/1), yet the mode change keeps the file from being
+ # pruned.
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ diff --stat HEAD^ HEAD >actual 2>stderr &&
+ test_grep "modeonly.c" actual &&
+ test_grep "command=hunks pathname=modeonly.c" backend.log &&
+ test_grep ! "1 insertion" actual &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process not consulted for default --dirstat' '
+ # The default (change-based) --dirstat algorithm counts via its
+ # own path and never contacts the tool (here --dirstat=0 just
+ # sets a 0% threshold), so the change is still reported even
+ # though no-hunks would call it equivalent. --dirstat=lines
+ # instead uses the process-aware stat path.
+ test_when_finished "rm -f backend.log" &&
+ mkdir -p dsub &&
+ printf "a\nb\nc\n" >dsub/d.c &&
+ git add dsub/d.c &&
+ git commit -m "add dsub/d.c" &&
+ printf "a\nB\nc\n" >dsub/d.c &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ diff --dirstat=0 dsub/d.c >actual &&
+ test_grep "dsub" actual &&
+ test_path_is_missing backend.log
+'
+
#
# Bypass mechanisms: flags and commands that skip the diff process.
#
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 7/9] blame: consult diff process for no-hunk detection
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:02 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
When a diff process is configured via diff.<driver>.process,
consult it during blame's per-commit diffing. If the process
returns no hunks for a commit's changes to a file, treat the
commit as having no changes, causing blame to attribute lines
to earlier commits.
Introduce xdi_diff_process(), a process-aware xdi_diff() that
consults the process, runs xdiff on the tool's hunks or on the
builtin algorithm when it does not apply, frees the hunks, and
reports DIFF_PROCESS_EQUIVALENT (without running xdiff) so the caller
can drop or skip the change. It is the shared consult-then-diff path
for consumers that work on raw hunks: blame's pass_blame_to_parent()
uses it here, and git log -L reuses it later. builtin_diff() keeps
consulting the process directly, because it tests for equivalence
early, before its funcname-pattern and word-diff setup, so a
reformat-only file short-circuits without that work.
Blame's -w option is not communicated to the process and it could not
honor it, so blame must fall back to the builtin diff there. Because
blame keeps its whitespace flags in sb->xdl_opts rather than diffopt,
the process bypass keys off xpp (the flags the diff actually runs
with), which covers blame without a guard of its own.
The subprocess is long-running (one startup cost amortized across the
blame traversal), but each commit in the file's history incurs a
round-trip to the tool.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
blame.c | 24 +++++++-
diff-process.c | 38 ++++++++++++
diff-process.h | 26 +++++++++
t/t4080-diff-process.sh | 126 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 213 insertions(+), 1 deletion(-)
diff --git a/blame.c b/blame.c
index 126e232416..932a04c4e8 100644
--- a/blame.c
+++ b/blame.c
@@ -19,6 +19,8 @@
#include "tag.h"
#include "trace2.h"
#include "blame.h"
+#include "diff-process.h"
+#include "xdiff-interface.h"
#include "alloc.h"
#include "commit-slab.h"
#include "bloom.h"
@@ -1946,6 +1948,9 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
struct blame_origin *parent, int ignore_diffs)
{
mmfile_t file_p, file_o;
+ xpparam_t xpp = {0};
+ xdemitconf_t xecfg = {0};
+ xdemitcb_t ecb = {NULL};
struct blame_chunk_cb_data d;
struct blame_entry *newdest = NULL;
@@ -1964,7 +1969,24 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
&sb->num_read_blob, ignore_diffs);
sb->num_get_patch++;
- if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
+ xpp.flags = sb->xdl_opts;
+ xecfg.hunk_func = blame_chunk_cb;
+ ecb.priv = &d;
+ /*
+ * Consult the diff process, then attribute the resulting chunks
+ * via blame_chunk_cb. It bypasses the process for the whitespace-
+ * ignoring options it cannot honor (they live in xpp.flags, which
+ * the consultation checks), and when the process reports the blobs
+ * equivalent it runs no diff, so blame passes this commit and looks
+ * past it. Look up the driver by the parent (old) path, as
+ * builtin_diff() does with name_a, so a renamed file resolves to the
+ * same driver across diff, blame, and line-log. Pass no
+ * old-oid/new-oid: blame diffs each blob pair once, so the tool gains
+ * nothing from a per-invocation cache key.
+ */
+ if (xdi_diff_process(&sb->revs->diffopt, parent->path,
+ &file_p, &file_o, NULL, NULL, &xpp, &xecfg, &ecb)
+ == DIFF_PROCESS_ERROR)
die("unable to generate diff (%s -> %s)",
oid_to_hex(&parent->commit->object.oid),
oid_to_hex(&target->commit->object.oid));
diff --git a/diff-process.c b/diff-process.c
index 4c748fdd2a..191b2b67b2 100644
--- a/diff-process.c
+++ b/diff-process.c
@@ -37,6 +37,7 @@
#include "sub-process.h"
#include "pkt-line.h"
#include "strbuf.h"
+#include "xdiff-interface.h"
#include "xdiff/xdiff.h"
#define CAP_HUNKS (1u << 0)
@@ -489,3 +490,40 @@ enum diff_process_result diff_process_fill_hunks(
}
return DIFF_PROCESS_SKIP;
}
+
+enum diff_process_result xdi_diff_process(
+ struct diff_options *diffopt,
+ const char *path,
+ mmfile_t *file_a,
+ mmfile_t *file_b,
+ const struct object_id *oid_a,
+ const struct object_id *oid_b,
+ xpparam_t *xpp,
+ xdemitconf_t *xecfg,
+ xdemitcb_t *ecb)
+{
+ enum diff_process_result res;
+
+ /*
+ * Consult the diff process, then run xdiff either constrained to
+ * the tool's hunks or, when the process does not apply, computing
+ * the diff itself as a fallback. EQUIVALENT short-circuits: the
+ * caller decides what "no change" means for it (drop the commit,
+ * skip the file, ...), so xdiff is not run.
+ *
+ * A SKIP/ERROR from the process just selects the builtin path
+ * (its warning, if any, was already emitted), so the result then
+ * reflects whether xdiff itself succeeded, not the process.
+ */
+ res = diff_process_fill_hunks(diffopt, path, file_a, file_b,
+ oid_a, oid_b, xpp);
+ if (res == DIFF_PROCESS_EQUIVALENT)
+ return res;
+
+ res = xdi_diff(file_a, file_b, xpp, xecfg, ecb) < 0
+ ? DIFF_PROCESS_ERROR : DIFF_PROCESS_OK;
+
+ FREE_AND_NULL(xpp->external_hunks);
+ xpp->external_hunks_nr = 0;
+ return res;
+}
diff --git a/diff-process.h b/diff-process.h
index 8d00dafe1d..5e5b514b77 100644
--- a/diff-process.h
+++ b/diff-process.h
@@ -46,4 +46,30 @@ enum diff_process_result diff_process_fill_hunks(
const struct object_id *oid_b,
xpparam_t *xpp);
+/*
+ * Process-aware xdi_diff(): consult the diff process for 'path', then
+ * run xdiff either constrained to the tool's hunks or computing the
+ * diff itself when the process does not apply or fails. Frees any
+ * hunks it obtained before returning.
+ *
+ * Returns DIFF_PROCESS_EQUIVALENT (without running xdiff) when the tool
+ * reports the blobs equal, so the caller can drop or skip the change;
+ * DIFF_PROCESS_OK when xdiff ran (on tool hunks or builtin); and
+ * DIFF_PROCESS_ERROR if xdiff itself errored.
+ *
+ * The caller fills xpp (flags, ignore_regex, anchors) and xecfg/ecb as
+ * for a direct xdi_diff() call. oid_a/oid_b are forwarded to
+ * diff_process_fill_hunks() (see there).
+ */
+enum diff_process_result xdi_diff_process(
+ struct diff_options *diffopt,
+ const char *path,
+ mmfile_t *file_a,
+ mmfile_t *file_b,
+ const struct object_id *oid_a,
+ const struct object_id *oid_b,
+ xpparam_t *xpp,
+ xdemitconf_t *xecfg,
+ xdemitcb_t *ecb);
+
#endif /* DIFF_PROCESS_H */
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index 7e71b70ab9..694c94edb2 100755
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -658,4 +658,130 @@ test_expect_success 'diff process omits old-oid and new-oid for textconv content
test_must_be_empty stderr
'
+#
+# Blame integration.
+#
+
+test_expect_success 'blame uses tool-provided hunks' '
+ cat >blame-hunk.c <<-\EOF &&
+ line1
+ line2
+ line3
+ line4
+ original5
+ original6
+ line7
+ line8
+ line9
+ line10
+ EOF
+ git add blame-hunk.c &&
+ git commit -m "add blame-hunk.c" &&
+ ORIG=$(git rev-parse --short HEAD) &&
+
+ cat >blame-hunk.c <<-\EOF &&
+ line1
+ line2
+ line3
+ line4
+ changed5
+ changed6
+ line7
+ line8
+ changed9
+ changed10
+ EOF
+ git add blame-hunk.c &&
+ git commit -m "change blame-hunk.c" &&
+ CHANGE=$(git rev-parse --short HEAD) &&
+
+ # With fixed-hunk mode the tool reports only lines 5-6 as changed,
+ # so blame should attribute lines 9-10 to the original commit
+ # even though the builtin diff would show them as changed.
+ git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk" \
+ blame blame-hunk.c >actual &&
+ sed -n "9p" actual >line9 &&
+ sed -n "10p" actual >line10 &&
+ test_grep "$ORIG" line9 &&
+ test_grep "$ORIG" line10 &&
+ sed -n "5p" actual >line5 &&
+ sed -n "6p" actual >line6 &&
+ test_grep "$CHANGE" line5 &&
+ test_grep "$CHANGE" line6
+'
+
+test_expect_success 'blame skips commits with no hunks from diff process' '
+ cat >blame.c <<-\EOF &&
+ int main(void) {
+ return 0;
+ }
+ EOF
+ git add blame.c &&
+ git commit -m "add blame.c" &&
+ ORIG_COMMIT=$(git rev-parse --short HEAD) &&
+
+ cat >blame.c <<-\EOF &&
+ int main(void)
+ {
+ return 0;
+ }
+ EOF
+ git add blame.c &&
+ git commit -m "reformat blame.c" &&
+ BLAME_COMMIT=$(git rev-parse --short HEAD) &&
+
+ # Without no-hunks mode, blame attributes the change.
+ git blame blame.c >without &&
+ test_grep "$BLAME_COMMIT" without &&
+
+ # With no-hunks mode, the process considers the files equivalent
+ # and blame skips the reformat commit, attributing to the original.
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
+ blame blame.c >with &&
+ test_grep ! "$BLAME_COMMIT" with &&
+ test_grep "$ORIG_COMMIT" with
+'
+
+test_expect_success 'blame --no-ext-diff bypasses diff process' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ blame --no-ext-diff blame.c >actual &&
+ # Without the process, blame attributes the reformat commit normally.
+ test_grep "$BLAME_COMMIT" actual &&
+ test_path_is_missing backend.log
+'
+
+test_expect_success 'blame --no-ext-diff uses builtin hunks' '
+ # fixed-hunk mode would narrow blame to lines 5-6, but
+ # --no-ext-diff should bypass it and use the builtin diff.
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk --log=backend.log" \
+ blame --no-ext-diff blame-hunk.c >actual &&
+ # Builtin diff attributes lines 9-10 to the change commit.
+ sed -n "9p" actual >line9 &&
+ test_grep "$CHANGE" line9 &&
+ test_path_is_missing backend.log
+'
+
+test_expect_success 'blame -w bypasses diff process' '
+ test_when_finished "rm -f backend.log" &&
+ printf "alpha\nbeta\ngamma\n" >blamew.c &&
+ git add blamew.c &&
+ git commit -m "add blamew.c" &&
+ orig=$(git rev-parse --short HEAD) &&
+ printf "alpha\n beta \ngamma\n" >blamew.c &&
+ git commit -am "reindent beta" &&
+ reindent=$(git rev-parse --short HEAD) &&
+ # blame -w must ignore the whitespace-only change and attribute
+ # beta to the original commit, not the reindent commit. The tool
+ # is never told about -w, so blame must bypass it (not let tool
+ # hunks override -w).
+ git -c diff.cdiff.process="$BACKEND --mode=whole-file --log=backend.log" \
+ blame -w blamew.c >actual &&
+ sed -n "2p" actual >line2 &&
+ test_grep "$orig" line2 &&
+ test_grep ! "$reindent" line2 &&
+ test_path_is_missing backend.log
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 6/9] diff: bypass diff process with --no-ext-diff and in format-patch
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Make --no-ext-diff disable diff.<driver>.process in addition to
diff.<driver>.command. Although the two mechanisms work differently
(command replaces Git's output, process feeds hunks back into the
pipeline), both invoke external tools and --no-ext-diff means
"no external tools."
Replace the OPT_BOOL for --ext-diff with an OPT_CALLBACK that
sets both allow_external and no_diff_process, so a single option
controls both. Passing --ext-diff explicitly clears
no_diff_process, so a later --ext-diff overrides an earlier
--no-ext-diff.
Disable the diff process unconditionally in format-patch so that
generated patches are always based on the builtin diff algorithm
and can be applied reliably by recipients who do not have the
external tool.
Document that --diff-algorithm also bypasses the diff process,
since it forces the builtin algorithm.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/diff-algorithm-option.adoc | 3 +++
Documentation/diff-options.adoc | 4 +++-
Documentation/gitattributes.adoc | 6 +++---
builtin/log.c | 7 +++++++
diff.c | 16 ++++++++++++++--
diff.h | 5 ++++-
t/t4080-diff-process.sh | 16 ++++++++++++++++
7 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc
index 8e3a0b63d7..4d7e2ec35f 100644
--- a/Documentation/diff-algorithm-option.adoc
+++ b/Documentation/diff-algorithm-option.adoc
@@ -18,3 +18,6 @@
For instance, if you configured the `diff.algorithm` variable to a
non-default value and want to use the default one, then you
have to use `--diff-algorithm=default` option.
++
+If you explicitly choose a diff algorithm, it also bypasses
+`diff.<driver>.process` (see linkgit:gitattributes[5]).
diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc
index c8242e2462..a884445211 100644
--- a/Documentation/diff-options.adoc
+++ b/Documentation/diff-options.adoc
@@ -833,7 +833,9 @@ endif::git-format-patch[]
to use this option with linkgit:git-log[1] and friends.
`--no-ext-diff`::
- Disallow external diff drivers.
+ Disallow external diff helpers, including
+ `diff.<driver>.command` and `diff.<driver>.process`
+ (see linkgit:gitattributes[5]).
`--textconv`::
`--no-textconv`::
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index f4ca4a8c7e..a03fb9deb1 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -1073,9 +1073,9 @@ display, which is covered above); and combined diffs (`--cc` and merge
diffs), whose protocol would have to be extended from a single old/new
pair to one comparison per merge parent.
-`--diff-algorithm` bypasses the process entirely, for every feature
-listed above. The whitespace-ignoring options (`-w`,
-`--ignore-space-change`, `--ignore-blank-lines`, and the like),
+`--no-ext-diff` and `--diff-algorithm` bypass the process entirely,
+for every feature listed above. The whitespace-ignoring options
+(`-w`, `--ignore-space-change`, `--ignore-blank-lines`, and the like),
`-I<regex>`, and `--anchored` also bypass it for the affected files:
the tool is never told about these options, so it could not honor
them, and Git falls back to the builtin diff, which does.
diff --git a/builtin/log.c b/builtin/log.c
index d027ce1e0b..7821a61143 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2217,6 +2217,13 @@ int cmd_format_patch(int argc,
if (argc > 1)
die(_("unrecognized argument: %s"), argv[1]);
+ /*
+ * Disable diff.<driver>.process so that patches generated by
+ * format-patch are always based on the builtin diff algorithm
+ * and can be applied reliably.
+ */
+ rev.diffopt.flags.no_diff_process = 1;
+
if (rev.diffopt.output_format & DIFF_FORMAT_NAME)
die(_("--name-only does not make sense"));
if (rev.diffopt.output_format & DIFF_FORMAT_NAME_STATUS)
diff --git a/diff.c b/diff.c
index af31072858..a9a732629e 100644
--- a/diff.c
+++ b/diff.c
@@ -5948,6 +5948,17 @@ static int diff_opt_submodule(const struct option *opt,
return 0;
}
+static int diff_opt_ext_diff(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct diff_options *options = opt->value;
+
+ BUG_ON_OPT_ARG(arg);
+ options->flags.allow_external = !unset;
+ options->flags.no_diff_process = unset;
+ return 0;
+}
+
static int diff_opt_textconv(const struct option *opt,
const char *arg, int unset)
{
@@ -6278,8 +6289,9 @@ struct option *add_diff_options(const struct option *opts,
N_("exit with 1 if there were differences, 0 otherwise")),
OPT_BOOL(0, "quiet", &options->flags.quick,
N_("disable all output of the program")),
- OPT_BOOL(0, "ext-diff", &options->flags.allow_external,
- N_("allow an external diff helper to be executed")),
+ OPT_CALLBACK_F(0, "ext-diff", options, NULL,
+ N_("allow an external diff helper to be executed"),
+ PARSE_OPT_NOARG, diff_opt_ext_diff),
OPT_CALLBACK_F(0, "textconv", options, NULL,
N_("run external text conversion filters when comparing binary files"),
PARSE_OPT_NOARG, diff_opt_textconv),
diff --git a/diff.h b/diff.h
index 7dc157968d..ee034d240d 100644
--- a/diff.h
+++ b/diff.h
@@ -173,7 +173,10 @@ struct diff_flags {
*/
unsigned allow_external;
- /** Disables diff.<driver>.process. */
+ /**
+ * Disables diff.<driver>.process. Set by --no-ext-diff and by
+ * format-patch.
+ */
unsigned no_diff_process;
/**
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index 3b75df082e..7e71b70ab9 100755
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -398,6 +398,22 @@ test_expect_success 'diff process bypassed by --diff-algorithm' '
test_path_is_missing backend.log
'
+test_expect_success 'diff process bypassed by --no-ext-diff' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff --no-ext-diff worddiff.c >actual &&
+ test_grep "return 999" actual &&
+ test_path_is_missing backend.log
+'
+
+test_expect_success 'diff process not used by format-patch' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ format-patch -1 --stdout -- logtest.c >actual &&
+ test_grep "return 2" actual &&
+ test_path_is_missing backend.log
+'
+
test_expect_success 'diff process bypassed under whitespace-ignoring flags' '
test_when_finished "rm -f backend.log" &&
printf "a\nb\nc\n" >wsbypass.c &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 5/9] diff: add long-running diff process via diff.<driver>.process
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Add support for external diff processes that communicate via the
long-running process protocol (pkt-line over stdin/stdout).
A diff process is configured per userdiff driver:
[diff "cdiff"]
process = /path/to/diff-tool
The tool provides custom line-matching: it receives file pairs
and returns hunks that reference line numbers in the content.
When textconv is also configured, the tool receives the
textconv-transformed content. The tool controls which lines
are marked as changed while the display shows the file content.
Patch output features (word diff, function context, color) work
normally. A new "Which features consult the diff process"
documentation section lays out which features use the tool's hunks,
which compute independently, and why; the summary formats such as
--stat still use the builtin diff for now.
The handshake negotiates version=1 and capability=hunks. Per-file
requests send command=hunks, pathname, the old and new blob object
names as old-oid/new-oid, and both file contents as packetized data.
The tool responds with hunk lines and a status packet (success,
error, or abort). On error, Git warns and falls back to the builtin
diff algorithm for that file. On abort, Git silently falls back for
the current file and stops sending further requests to the tool for
the remainder of the session.
old-oid/new-oid name the two blobs so a tool can cache its analysis
keyed on the pair. A side's oid is sent only when the content the
tool receives is that raw blob: it is omitted under textconv, which
rewrites the bytes, and for a working-tree side with no stored
object, so an oid that is sent always names the bytes the tool
receives. This is where the process protocol diverges from
diff.<driver>.command, which never composes with textconv (the
command replaces the whole diff and always gets the raw blob). Tools
ignore unknown request keys, so old tools skip them.
When the tool returns no hunks followed by status=success, Git
treats the file as having no changes and produces no diff output.
This also means --exit-code reports no changes for that file.
The subprocess is stored on the userdiff_driver struct and
launched on first use. If the process fails to start, the
handshake fails, or a communication error occurs mid-stream,
the failure is cached on the driver to avoid retrying and
re-warning on every subsequent file.
Git falls back to the builtin diff (rather than consulting the
tool) when an option the tool cannot honor is in effect: the
whitespace-ignoring flags, --ignore-blank-lines, -I<regex>, and
--anchored. The bypass keys off the effective diff parameters (xpp)
rather than diffopt, so a later caller whose flags live elsewhere is
covered uniformly. A change that only adds or removes the trailing
newline is likewise not expressible as hunks, so it too uses the
builtin diff. The hunk parser ignores unknown trailing fields on a
hunk line for response forward-compatibility.
Hunk accumulation is bounded by the combined byte count of the two
files, so a misbehaving tool that floods hunk lines cannot grow
memory without bound before validation runs.
diff_process_fill_hunks() is the sole public entry point. It
handles driver lookup, flag checks, subprocess management, and
error reporting, returning an enum that lets callers distinguish
"hunks populated" from "files equivalent" from "not applicable"
from "tool failure."
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/config/diff.adoc | 5 +
Documentation/gitattributes.adoc | 249 +++++++++++
Makefile | 2 +
diff-process.c | 491 ++++++++++++++++++++
diff-process.h | 49 ++
diff.c | 21 +
diff.h | 3 +
meson.build | 1 +
t/helper/meson.build | 1 +
t/helper/test-diff-process-backend.c | 381 ++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/meson.build | 1 +
t/t4080-diff-process.sh | 645 +++++++++++++++++++++++++++
userdiff.h | 3 +
15 files changed, 1854 insertions(+)
create mode 100644 diff-process.c
create mode 100644 diff-process.h
create mode 100644 t/helper/test-diff-process-backend.c
create mode 100755 t/t4080-diff-process.sh
diff --git a/Documentation/config/diff.adoc b/Documentation/config/diff.adoc
index 1135a62a0a..ac0635bb3b 100644
--- a/Documentation/config/diff.adoc
+++ b/Documentation/config/diff.adoc
@@ -218,6 +218,11 @@ endif::git-diff[]
Set this option to `true` to make the diff driver cache the text
conversion outputs. See linkgit:gitattributes[5] for details.
+`diff.<driver>.process`::
+ The command to run as a long-running diff process that
+ provides hunks to Git's diff pipeline.
+ See linkgit:gitattributes[5] for details.
+
`diff.indentHeuristic`::
Set this option to `false` to disable the default heuristics
that shift diff hunk boundaries to make patches easier to read.
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index 2c4fbfd7f1..f4ca4a8c7e 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -831,6 +831,255 @@ NOTE: If `diff.<name>.command` is defined for path with the
(see above), and adding `diff.<name>.algorithm` has no effect, as the
algorithm is not passed to the external diff driver.
+Using an external diff process
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If `diff.<name>.process` is defined, Git sends the old and new file
+content to an external tool and receives back a list of changed
+regions (pairs of line ranges in the old and new file). Git uses
+these instead of its builtin diff algorithm, but still controls
+all output formatting, so features like word diff, function context,
+color, and blame work normally. This is achieved by using the
+long-running process protocol (described in
+Documentation/technical/long-running-process-protocol.adoc).
+Unlike `diff.<name>.command`, which replaces Git's output entirely,
+the diff process feeds results back into the standard pipeline. If
+both are configured for a path, `diff.<name>.command` takes precedence
+for the patch output it replaces; the summary formats, `git blame`,
+and `git log -L` never run the command and still consult the process.
+
+First, in `.gitattributes`, assign the `diff` attribute for paths.
+
+------------------------
+*.c diff=cdiff
+------------------------
+
+Then, define a "diff.<name>.process" configuration to specify
+the diff process command.
+
+----------------------------------------------------------------
+[diff "cdiff"]
+ process = /path/to/diff-process-tool
+----------------------------------------------------------------
+
+When Git encounters the first file that needs to be diffed, it starts
+the process and performs the handshake. In the handshake, the welcome
+message sent by Git is "git-diff-client", only version 1 is supported,
+and the supported capability is "hunks" (the changed regions
+described below). The tool replies with "git-diff-server", the
+version it supports, and the capabilities it supports.
+
+For each file, Git sends a list of "key=value" pairs terminated with
+a flush packet, followed by the old and new file content as packetized
+data, each terminated with a flush packet. The pathname is relative
+to the repository root. When `diff.<name>.textconv` is also set,
+the tool receives the textconv-transformed content rather than the
+raw blob. Git does not send binary files to the diff process.
+
+-----------------------
+packet: git> command=hunks
+packet: git> pathname=path/file.c
+packet: git> old-oid=<hex>
+packet: git> new-oid=<hex>
+packet: git> 0000
+packet: git> OLD_CONTENT
+packet: git> 0000
+packet: git> NEW_CONTENT
+packet: git> 0000
+-----------------------
+
+The optional `old-oid` and `new-oid` keys give the object names of the
+old and new blobs, so a tool can cache its analysis keyed on the pair.
+A side's key is sent only when the content for that side is the raw
+blob it names: it is omitted when the content is textconv-transformed,
+and for a working-tree side that has no stored object. A tool that
+does not recognize these keys ignores them.
+
+The tool is expected to respond with zero or more hunk lines,
+a flush packet, and a status packet terminated with a flush packet.
+Each hunk line has the form:
+
+ `hunk <old_start> <old_count> <new_start> <new_count>`
+
+where `<old_start>` and `<old_count>` identify a range of lines in
+the old file, and `<new_start>` and `<new_count>` identify the
+replacement range in the new file. The four fields are separated by
+single spaces. Start values are 1-based and counts are non-negative.
+For example, `hunk 3 2 3 4` means that 2 lines starting at line 3 in
+the old file were replaced by 4 lines starting at line 3 in the new
+file. An `<old_count>` of 0 means no lines were removed (pure
+insertion); a `<new_count>` of 0 means no lines were added (pure
+deletion). For a side with a count of 0 (a pure insertion or
+deletion) the start is the 1-based line the change sits before,
+ranging from 1 to one past the last line (the line count plus 1, to
+place the change at the end of the file); like every start it must
+keep the unchanged runs aligned on both sides (see below), so for a
+given change it takes one specific value, not an arbitrary one. A
+start of 0 is also accepted and treated as 1, matching the
+empty-file-side form `git diff` emits (e.g. `hunk 0 0 1 5` for a newly
+added file). A nonzero range must not extend beyond the end of the
+file. Git ignores any extra
+whitespace-separated tokens after `<new_count>`, so a future protocol
+version can append fields to a hunk line (for example a "moved"
+marker) without older tools rejecting it.
+
+Lines are delimited by newlines. A file `"foo\nbar\n"` and a
+file `"foo\nbar"` both have 2 lines.
+
+Hunks must be listed in order and must not overlap. Any line not
+covered by a hunk is treated as unchanged and is paired, in order,
+with the unchanged lines on the other side. Each run of unchanged
+lines between two hunks (and the run before the first hunk and
+after the last) must therefore be the same length on both sides,
+not merely equal in total. For the hunks `1 3 1 5` and `10 2 12 2`
+below, lines 4-9 of the old file and lines 6-11 of the new file are
+both the six unchanged lines between the two hunks. A response that
+balances only the total unchanged count but misaligns one of these
+runs is rejected, and Git falls back to the builtin diff.
+
+Git does not check that the lines a hunk leaves unchanged are
+byte-for-byte identical between the two sides; it pairs them by
+position and shows the new side as context. A tool may therefore
+report lines that differ textually (a pure reformatting, say) as
+unchanged, and the diff reflects that judgment. This is
+the point of a semantic backend, but it means a misbehaving tool can
+produce a diff whose context does not match the old blob; as with
+`git diff -w`, such a patch may not apply against the old content.
+
+-----------------------
+packet: git< hunk 1 3 1 5
+packet: git< hunk 10 2 12 2
+packet: git< 0000
+packet: git< status=success
+packet: git< 0000
+-----------------------
+
+If the tool responds with hunks and "success", Git marks those lines
+as changed and feeds them into the standard diff pipeline. Git may
+still slide or regroup those changes against matching context for
+display, exactly as it compacts its own diffs, so the tool controls
+which lines are reported as changed, not the precise hunk boundaries.
+Patch output features (word diff, function context, color) work
+normally. Summary formats such as `--stat` still compute their counts
+with the builtin diff for now; see "Which features consult the diff
+process" below for the full picture and the reasoning behind it.
+
+If no hunk lines precede the flush, followed by "success", Git
+treats the files as having no changes: `git diff` produces no output,
+`git diff --exit-code` and `--quiet` report success even though the
+stored blobs differ, and `git blame` skips the commit, attributing
+lines to earlier commits.
+The one exception is a change that only adds or removes the file's
+trailing newline: it cannot be expressed as line hunks, so when the
+line content otherwise matches Git keeps the builtin diff for that
+file (preserving the `\ No newline at end of file` marker) instead of
+treating the two sides as equal.
+
+-----------------------
+packet: git< 0000
+packet: git< status=success
+packet: git< 0000
+-----------------------
+
+If the tool returns well-formed but invalid hunks (out of bounds,
+overlapping, or with misaligned unchanged runs), Git warns and falls
+back to the builtin diff for that file; the tool stays available for
+subsequent files. A malformed hunk line, by contrast (bad syntax, a
+nonzero count paired with a start of 0, or more hunks than the file
+has lines), is a protocol violation: Git stops the process and does
+not send it further requests, as described below.
+
+In case the tool cannot or does not want to process the content,
+it is expected to respond with an "error" status. Git warns and
+falls back to the builtin diff algorithm for this file, treating any
+status other than "success" or "abort" the same way. The tool
+remains available for subsequent files.
+
+-----------------------
+packet: git< 0000
+packet: git< status=error
+packet: git< 0000
+-----------------------
+
+In case the tool cannot or does not want to process the content as
+well as any future content for the lifetime of the Git process, it
+is expected to respond with an "abort" status. Git silently falls
+back to the builtin diff algorithm for this file and does not send
+further requests to the tool.
+
+-----------------------
+packet: git< 0000
+packet: git< status=abort
+packet: git< 0000
+-----------------------
+
+If the tool dies during the communication or does not adhere to the
+protocol then Git will stop the process and fall back to the builtin
+diff algorithm. Git warns once and does not restart the process for
+subsequent files.
+
+Tools should ignore unknown keys in the per-file request to remain
+forward-compatible. Future versions of Git may send additional
+`command=` values; tools that receive an unrecognized command should
+respond with `status=error` rather than terminating.
+
+Which features consult the diff process
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The diff process answers a single question: given two blobs, which
+line ranges differ? Whether a particular feature consults it follows
+from whether that is the question the feature is really asking.
+
+Features that ask "which lines changed" use the tool's hunks in place
+of the builtin algorithm:
+
+- `git diff` patch output, together with everything layered on it:
+ word diff, function context (`-W`), `--color-moved`, the `@@` hunk
+ headers, and the `-L` line-range display. These operate on the
+ lines the patch step already emitted, so they reflect the tool's
+ hunks without any further negotiation.
+- `git blame`: a commit whose change the tool reports as equivalent is
+ skipped, and its lines are attributed to an earlier commit.
+
+Features that ask a different question do not consult the process, by
+design:
+
+- The pickaxe `-G<regex>` searches the textual diff for a pattern; it
+ asks "does this string appear in the diff," not "did these lines
+ change." (`-S` runs at an earlier stage and is likewise unaffected.)
+- `git patch-id` must produce a stable hash for `git rebase` and
+ cherry-pick detection; deriving it from a configured tool would make
+ equal patches hash differently from machine to machine.
+- The merge machinery (`git merge-tree`, `rerere`) computes merge
+ content and conflict signatures rather than display output, so the
+ tool's hunks must not alter its results.
+- `git range-diff` diffs patch text, not source blobs, so source-file
+ hunks do not apply to it.
+- `--check` reports whitespace errors in added lines using the builtin
+ diff's notion of which lines are added, not the tool's. It can
+ therefore flag (and exit non-zero on) a line the tool treats as
+ unchanged and that `git diff` shows as context. Whitespace breakage
+ is a property of the literal bytes, so `--check` keeps the builtin
+ partition deliberately; a future change could wire it to the tool if
+ matching `git diff` exactly became desirable.
+- `--raw`, `--name-only`, and `--name-status` compare object ids at
+ the tree level and never run a line-level diff at all.
+
+Some features ask "which lines changed" but still use the builtin
+algorithm for now, and may consult the process in a later change: the
+summary formats (`--stat`, `--numstat`, `--shortstat`); `git log -L`'s
+commit selection and parent range propagation (as distinct from its
+display, which is covered above); and combined diffs (`--cc` and merge
+diffs), whose protocol would have to be extended from a single old/new
+pair to one comparison per merge parent.
+
+`--diff-algorithm` bypasses the process entirely, for every feature
+listed above. The whitespace-ignoring options (`-w`,
+`--ignore-space-change`, `--ignore-blank-lines`, and the like),
+`-I<regex>`, and `--anchored` also bypass it for the affected files:
+the tool is never told about these options, so it could not honor
+them, and Git falls back to the builtin diff, which does.
+
Defining a custom hunk-header
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/Makefile b/Makefile
index 1cec251f43..1314c10463 100644
--- a/Makefile
+++ b/Makefile
@@ -811,6 +811,7 @@ TEST_BUILTINS_OBJS += test-csprng.o
TEST_BUILTINS_OBJS += test-date.o
TEST_BUILTINS_OBJS += test-delete-gpgsig.o
TEST_BUILTINS_OBJS += test-delta.o
+TEST_BUILTINS_OBJS += test-diff-process-backend.o
TEST_BUILTINS_OBJS += test-dir-iterator.o
TEST_BUILTINS_OBJS += test-drop-caches.o
TEST_BUILTINS_OBJS += test-dump-cache-tree.o
@@ -1140,6 +1141,7 @@ LIB_OBJS += diff-delta.o
LIB_OBJS += diff-merges.o
LIB_OBJS += diff-lib.o
LIB_OBJS += diff-no-index.o
+LIB_OBJS += diff-process.o
LIB_OBJS += diff.o
LIB_OBJS += diffcore-break.o
LIB_OBJS += diffcore-delta.o
diff --git a/diff-process.c b/diff-process.c
new file mode 100644
index 0000000000..4c748fdd2a
--- /dev/null
+++ b/diff-process.c
@@ -0,0 +1,491 @@
+/*
+ * Diff process backend: communicates with a long-running external
+ * tool via the pkt-line protocol to obtain custom line-matching
+ * results. The tool controls which lines are marked as changed
+ * while the display shows the file content (after any textconv
+ * transformation, if configured).
+ *
+ * Protocol: pkt-line over stdin/stdout, following the pattern of
+ * the long-running filter process protocol (see convert.c).
+ *
+ * Handshake:
+ * git> git-diff-client / version=1 / flush
+ * tool< git-diff-server / version=1 / flush
+ * git> capability=hunks / flush
+ * tool< capability=hunks / flush
+ *
+ * Per-file:
+ * git> command=hunks / pathname=<path> / [old-oid=<hex>] / [new-oid=<hex>] / flush
+ * git> <old content packetized> / flush
+ * git> <new content packetized> / flush
+ * tool< hunk <old_start> <old_count> <new_start> <new_count>
+ * tool< ... / flush
+ * tool< status=success / flush
+ *
+ * When the tool returns no hunks with status=success, it considers
+ * the files equivalent. Git will skip the diff for that file.
+ */
+
+#include "git-compat-util.h"
+#include "diff-process.h"
+#include "diff.h"
+#include "gettext.h"
+#include "hex.h"
+#include "repository.h"
+#include "sigchain.h"
+#include "userdiff.h"
+#include "sub-process.h"
+#include "pkt-line.h"
+#include "strbuf.h"
+#include "xdiff/xdiff.h"
+
+#define CAP_HUNKS (1u << 0)
+
+struct diff_subprocess {
+ struct subprocess_entry subprocess;
+ unsigned int supported_capabilities;
+};
+
+static int start_diff_process_fn(struct subprocess_entry *subprocess)
+{
+ static int versions[] = { 1, 0 };
+ static struct subprocess_capability capabilities[] = {
+ { "hunks", CAP_HUNKS },
+ { NULL, 0 }
+ };
+ struct diff_subprocess *entry =
+ container_of(subprocess, struct diff_subprocess, subprocess);
+
+ return subprocess_handshake(subprocess, "git-diff",
+ versions, NULL,
+ capabilities,
+ &entry->supported_capabilities);
+}
+
+static struct diff_subprocess *get_or_launch_process(
+ struct userdiff_driver *drv)
+{
+ struct diff_subprocess *entry;
+
+ if (drv->diff_subprocess)
+ return drv->diff_subprocess;
+
+ entry = xcalloc(1, sizeof(*entry));
+ if (subprocess_start_command(&entry->subprocess, drv->process,
+ start_diff_process_fn)) {
+ free(entry);
+ drv->diff_process_failed = 1;
+ return NULL;
+ }
+
+ drv->diff_subprocess = entry;
+ return entry;
+}
+
+static int send_file_content(int fd, const char *buf, long size)
+{
+ int ret = 0;
+
+ if (size < 0)
+ return -1;
+ if (size > 0)
+ ret = write_packetized_from_buf_no_flush(buf, size, fd);
+ if (ret)
+ return ret;
+ return packet_flush_gently(fd);
+}
+
+/*
+ * A hunk in the diff process's presentation coordinates: the line
+ * numbering it reports over the protocol. Kept distinct from struct
+ * xdl_hunk (xdiff's coordinates) so that only translated hunks ever
+ * reach the diff algorithm; diff_process_hunk_to_xdl() is the single
+ * crossing point.
+ */
+struct diff_process_hunk {
+ long old_start, old_count;
+ long new_start, new_count;
+};
+
+/*
+ * Parse one non-negative decimal field of a hunk line into *out and
+ * advance *line past it. Fields must be plain decimal with no leading
+ * whitespace or sign (isdigit() takes an unsigned char to stay defined
+ * for high-bit bytes). The first three fields are followed by a single
+ * space; the last (is_last) is followed by end-of-string or a space.
+ * Trailing space-separated tokens after the last field are allowed and
+ * ignored, so a future protocol version can append fields (e.g. a
+ * "moved" marker) without older tools rejecting the line -- mirroring
+ * the request-side rule that tools ignore unknown keys.
+ */
+static int parse_hunk_field(const char **line, long *out, int is_last)
+{
+ const char *p = *line;
+ char *end;
+
+ if (!isdigit((unsigned char)*p))
+ return -1;
+ errno = 0;
+ *out = strtol(p, &end, 10);
+ if (errno || end == p)
+ return -1;
+ if (is_last) {
+ if (*end != '\0' && *end != ' ')
+ return -1;
+ } else {
+ if (*end != ' ')
+ return -1;
+ end++;
+ }
+ *line = end;
+ return 0;
+}
+
+static int parse_hunk_line(const char *line,
+ struct diff_process_hunk *presented)
+{
+ /* Format: "hunk <old_start> <old_count> <new_start> <new_count>" */
+ if (!skip_prefix(line, "hunk ", &line))
+ return -1;
+ if (parse_hunk_field(&line, &presented->old_start, 0) ||
+ parse_hunk_field(&line, &presented->old_count, 0) ||
+ parse_hunk_field(&line, &presented->new_start, 0) ||
+ parse_hunk_field(&line, &presented->new_count, 1))
+ return -1;
+ return 0;
+}
+
+/*
+ * Translate a hunk from the diff process's presentation coordinates
+ * into xdiff's.
+ *
+ * Protocol starts are already 1-based positions (the line a change
+ * sits before), the same numbering xdiff uses, so the only adjustment
+ * is for an empty file side: "git diff" addresses it with a start of 0
+ * and a count of 0 (e.g. "0 0 1 5" adds five lines to an empty old
+ * side), and since xdiff uses start-1 as an array index that 0 becomes
+ * 1 here. This is NOT the full inverse of xdl_emit_hunk_hdr()
+ * (xdiff/xutils.c): that emitter shifts a count-0 range to start-1 for
+ * the displayed "@@" header, but the protocol keeps the unshifted
+ * 1-based position for a mid-file insert or delete. This is the single
+ * point where presentation coordinates become xdiff coordinates, so
+ * xdl_populate_hunks_from_external() may assume 1-based starts.
+ *
+ * Returns -1 for a start of 0 paired with a nonzero count, which names
+ * no line in either coordinate system. (parse_hunk_line() already
+ * guarantees non-negative starts and counts.)
+ */
+static int diff_process_hunk_to_xdl(const struct diff_process_hunk *presented,
+ struct xdl_hunk *xdl)
+{
+ long old_start = presented->old_start;
+ long new_start = presented->new_start;
+
+ if ((!old_start && presented->old_count) ||
+ (!new_start && presented->new_count))
+ return -1;
+ if (!old_start)
+ old_start = 1;
+ if (!new_start)
+ new_start = 1;
+
+ xdl->old_start = old_start;
+ xdl->old_count = presented->old_count;
+ xdl->new_start = new_start;
+ xdl->new_count = presented->new_count;
+ return 0;
+}
+
+static enum diff_process_result get_hunks(
+ struct userdiff_driver *drv,
+ const char *path,
+ const char *old_buf, long old_size,
+ const char *new_buf, long new_size,
+ const struct object_id *oid_a,
+ const struct object_id *oid_b,
+ struct xdl_hunk **hunks_out,
+ size_t *nr_hunks_out)
+{
+ struct diff_subprocess *backend;
+ struct child_process *process;
+ int fd_in, fd_out;
+ struct strbuf status = STRBUF_INIT;
+ struct xdl_hunk *hunks = NULL;
+ struct diff_process_hunk presented;
+ struct xdl_hunk hunk;
+ size_t nr_hunks = 0, alloc_hunks = 0;
+ size_t max_hunks;
+ int len;
+ char *line;
+
+ backend = get_or_launch_process(drv);
+ if (!backend)
+ return DIFF_PROCESS_ERROR;
+
+ if (!(backend->supported_capabilities & CAP_HUNKS))
+ return DIFF_PROCESS_SKIP;
+
+ process = subprocess_get_child_process(&backend->subprocess);
+ fd_in = process->in;
+ fd_out = process->out;
+
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ /* Send request */
+ if (packet_write_fmt_gently(fd_in, "command=hunks\n") ||
+ packet_write_fmt_gently(fd_in, "pathname=%s\n", path))
+ goto comm_error;
+ /*
+ * old-oid/new-oid let the tool key a cache on the blob pair. A
+ * side is sent only when its content is the raw blob (the caller
+ * passes NULL otherwise, e.g. for textconv'd content), so an oid
+ * that is present always names the bytes the tool receives.
+ */
+ if (oid_a &&
+ packet_write_fmt_gently(fd_in, "old-oid=%s\n", oid_to_hex(oid_a)))
+ goto comm_error;
+ if (oid_b &&
+ packet_write_fmt_gently(fd_in, "new-oid=%s\n", oid_to_hex(oid_b)))
+ goto comm_error;
+ if (packet_flush_gently(fd_in))
+ goto comm_error;
+
+ /* Send old file content */
+ if (send_file_content(fd_in, old_buf, old_size))
+ goto comm_error;
+
+ /* Send new file content */
+ if (send_file_content(fd_in, new_buf, new_size))
+ goto comm_error;
+
+ /*
+ * Hunks are non-overlapping and each useful hunk covers at least
+ * one line, so a valid response cannot contain more hunks than the
+ * two files have lines, which is bounded by their byte sizes. Cap
+ * the accumulation accordingly so a misbehaving tool that floods
+ * hunk lines cannot drive unbounded memory growth before validation.
+ */
+ max_hunks = (size_t)old_size + (size_t)new_size + 1;
+
+ /* Read hunks until flush packet */
+ while ((len = packet_read_line_gently(fd_out, NULL, &line)) >= 0 &&
+ line) {
+ if (parse_hunk_line(line, &presented) < 0)
+ goto comm_error;
+ if (diff_process_hunk_to_xdl(&presented, &hunk) < 0)
+ goto comm_error;
+ if (nr_hunks >= max_hunks) {
+ warning(_("diff process '%s' sent too many hunks"
+ " for '%s'"), drv->process, path);
+ goto comm_error;
+ }
+ ALLOC_GROW(hunks, nr_hunks + 1, alloc_hunks);
+ hunks[nr_hunks++] = hunk;
+ }
+ if (len < 0)
+ goto comm_error;
+
+ /* Read status */
+ if (subprocess_read_status(fd_out, &status))
+ goto comm_error;
+
+ if (!strcmp(status.buf, "success")) {
+ *hunks_out = hunks;
+ *nr_hunks_out = nr_hunks;
+ strbuf_release(&status);
+ sigchain_pop(SIGPIPE);
+ return DIFF_PROCESS_OK;
+ }
+
+ if (!strcmp(status.buf, "abort")) {
+ /*
+ * The tool voluntarily withdrew: stop sending requests
+ * but do not warn (this is not a failure).
+ */
+ backend->supported_capabilities &= ~CAP_HUNKS;
+ free(hunks);
+ strbuf_release(&status);
+ sigchain_pop(SIGPIPE);
+ return DIFF_PROCESS_SKIP;
+ }
+
+ /* status=error or unknown status */
+ free(hunks);
+ strbuf_release(&status);
+ sigchain_pop(SIGPIPE);
+ return DIFF_PROCESS_ERROR;
+
+comm_error:
+ /*
+ * Communication failure (broken pipe, malformed response).
+ * Tear down the process and mark as failed so we do not
+ * retry on every subsequent file.
+ */
+ drv->diff_process_failed = 1;
+ drv->diff_subprocess = NULL;
+ subprocess_stop_command(&backend->subprocess);
+ free(backend);
+ free(hunks);
+ strbuf_release(&status);
+ sigchain_pop(SIGPIPE);
+ return DIFF_PROCESS_ERROR;
+}
+
+/*
+ * Whether exactly one of the two blobs ends in a newline. A change
+ * that only adds or removes the trailing newline is not expressible as
+ * line hunks, so a tool comparing lines reports the files as equal.
+ */
+static int eof_newline_differs(const mmfile_t *a, const mmfile_t *b)
+{
+ int a_nl = a->size > 0 && a->ptr[a->size - 1] == '\n';
+ int b_nl = b->size > 0 && b->ptr[b->size - 1] == '\n';
+ return a_nl != b_nl;
+}
+
+/*
+ * Number of lines in a blob, matching xdiff's record count: one per
+ * newline, plus one more if the last line has no trailing newline.
+ */
+static long count_lines(const char *buf, long size)
+{
+ long lines = 0, i;
+
+ for (i = 0; i < size; i++)
+ if (buf[i] == '\n')
+ lines++;
+ if (size > 0 && buf[size - 1] != '\n')
+ lines++;
+ return lines;
+}
+
+/*
+ * Validate the tool's hunks (already in xdiff coordinates) against the
+ * two blobs before they bypass the diff algorithm. Each hunk must fit
+ * within its file, the hunks must be ordered and non-overlapping, and
+ * the unchanged run before each hunk (and after the last) must be the
+ * same length on both sides -- xdl_build_script() walks the two files
+ * in lockstep over unchanged lines, so a mismatched gap desynchronizes
+ * it and yields a corrupt diff even when the totals balance. This is
+ * the git layer's job so xdiff stays diagnostic-free; on a bad response
+ * we warn and the caller falls back to the builtin diff. Returns 0 if
+ * valid, -1 (after warning) otherwise.
+ */
+static int validate_external_hunks(const struct xdl_hunk *hunks, size_t nr,
+ long old_lines, long new_lines,
+ const char *process, const char *path)
+{
+ size_t i;
+ long prev_old_end = 0, prev_new_end = 0;
+
+ for (i = 0; i < nr; i++) {
+ const struct xdl_hunk *h = &hunks[i];
+
+ if (h->old_count > old_lines - h->old_start + 1 ||
+ h->new_count > new_lines - h->new_start + 1) {
+ warning(_("diff process '%s' returned a hunk past the "
+ "end of '%s'; using the builtin diff"),
+ process, path);
+ return -1;
+ }
+ if (h->old_start < prev_old_end || h->new_start < prev_new_end) {
+ warning(_("diff process '%s' returned overlapping hunks "
+ "for '%s'; using the builtin diff"),
+ process, path);
+ return -1;
+ }
+ if (h->old_start - prev_old_end != h->new_start - prev_new_end) {
+ warning(_("diff process '%s' returned hunks that leave "
+ "'%s' misaligned; using the builtin diff"),
+ process, path);
+ return -1;
+ }
+ prev_old_end = h->old_start + h->old_count;
+ prev_new_end = h->new_start + h->new_count;
+ }
+ if (old_lines - prev_old_end != new_lines - prev_new_end) {
+ warning(_("diff process '%s' returned hunks that leave '%s' "
+ "misaligned; using the builtin diff"),
+ process, path);
+ return -1;
+ }
+ return 0;
+}
+
+enum diff_process_result diff_process_fill_hunks(
+ struct diff_options *diffopt,
+ const char *path,
+ const mmfile_t *file_a,
+ const mmfile_t *file_b,
+ const struct object_id *oid_a,
+ const struct object_id *oid_b,
+ xpparam_t *xpp)
+{
+ struct userdiff_driver *drv;
+ struct xdl_hunk *ext_hunks = NULL;
+ size_t nr = 0;
+ enum diff_process_result res;
+
+ if (!diffopt || !path)
+ return DIFF_PROCESS_SKIP;
+ if (diffopt->flags.no_diff_process || diffopt->ignore_driver_algorithm)
+ return DIFF_PROCESS_SKIP;
+ /*
+ * Whitespace-ignoring, regex-ignore (-I) and anchored options
+ * change which lines count as different, but the tool is never
+ * told about them, so its hunks could not honor them. Rather
+ * than silently override the user's request, fall back to the
+ * builtin diff, which does honor these flags. Key this off xpp
+ * (the parameters this diff actually runs with) rather than
+ * diffopt, so a caller like blame that keeps its flags outside
+ * diffopt is covered without a separate guard of its own.
+ */
+ if ((xpp->flags & (XDF_WHITESPACE_FLAGS | XDF_IGNORE_BLANK_LINES)) ||
+ xpp->ignore_regex_nr || xpp->anchors_nr)
+ return DIFF_PROCESS_SKIP;
+
+ drv = userdiff_find_by_path(diffopt->repo->index, path);
+ if (!drv || !drv->process)
+ return DIFF_PROCESS_SKIP;
+ if (drv->diff_process_failed)
+ return DIFF_PROCESS_SKIP;
+
+ res = get_hunks(drv, path,
+ file_a->ptr, file_a->size,
+ file_b->ptr, file_b->size,
+ oid_a, oid_b,
+ &ext_hunks, &nr);
+ if (res == DIFF_PROCESS_OK) {
+ if (!nr) {
+ free(ext_hunks);
+ /*
+ * Zero hunks means the tool considers the line
+ * content identical, but it cannot express a
+ * trailing-newline-only change. When that is the
+ * actual difference, fall back to the builtin diff
+ * so the "\ No newline at end of file" marker is
+ * preserved instead of reporting the files equal.
+ */
+ if (eof_newline_differs(file_a, file_b))
+ return DIFF_PROCESS_SKIP;
+ return DIFF_PROCESS_EQUIVALENT;
+ }
+ if (validate_external_hunks(ext_hunks, nr,
+ count_lines(file_a->ptr, file_a->size),
+ count_lines(file_b->ptr, file_b->size),
+ drv->process, path) < 0) {
+ free(ext_hunks);
+ return DIFF_PROCESS_SKIP;
+ }
+ xpp->external_hunks = ext_hunks;
+ xpp->external_hunks_nr = nr;
+ return DIFF_PROCESS_OK;
+ }
+ if (res == DIFF_PROCESS_ERROR) {
+ warning(_("diff process '%s' failed for '%s',"
+ " falling back to builtin diff"),
+ drv->process, path);
+ return DIFF_PROCESS_ERROR;
+ }
+ return DIFF_PROCESS_SKIP;
+}
diff --git a/diff-process.h b/diff-process.h
new file mode 100644
index 0000000000..8d00dafe1d
--- /dev/null
+++ b/diff-process.h
@@ -0,0 +1,49 @@
+#ifndef DIFF_PROCESS_H
+#define DIFF_PROCESS_H
+
+#include "xdiff/xdiff.h"
+
+struct diff_options;
+struct object_id;
+
+enum diff_process_result {
+ DIFF_PROCESS_ERROR = -1, /* failed; caller falls back to builtin */
+ DIFF_PROCESS_OK = 0, /* hunks populated in xpp */
+ DIFF_PROCESS_SKIP, /* process did not apply: use builtin */
+ DIFF_PROCESS_EQUIVALENT, /* tool says files are equivalent */
+};
+
+/*
+ * Consult the diff process configured for 'path' and populate
+ * xpp->external_hunks with the returned hunks.
+ *
+ * Handles driver lookup, flag checks (--no-ext-diff,
+ * --diff-algorithm), subprocess management, and error reporting.
+ *
+ * Returns DIFF_PROCESS_OK when hunks are populated in xpp.
+ * The caller owns xpp->external_hunks and must free() it.
+ *
+ * Returns DIFF_PROCESS_EQUIVALENT when the tool returns no hunks and
+ * the blobs are not a trailing-newline-only change (files are
+ * considered identical); caller should skip diff/blame.
+ * Returns DIFF_PROCESS_SKIP when no process applies; caller
+ * should use the builtin diff algorithm.
+ * Returns DIFF_PROCESS_ERROR on tool failure (already warned);
+ * caller should fall back to the builtin diff algorithm.
+ *
+ * oid_a/oid_b, when non-NULL, are sent to the tool as old-oid/new-oid
+ * so it can key a cache on the blob pair. Pass NULL for a side whose
+ * content is not the raw blob (e.g. textconv'd) or whose object name is
+ * unknown, so any oid that is sent always names the bytes the tool
+ * receives.
+ */
+enum diff_process_result diff_process_fill_hunks(
+ struct diff_options *diffopt,
+ const char *path,
+ const mmfile_t *file_a,
+ const mmfile_t *file_b,
+ const struct object_id *oid_a,
+ const struct object_id *oid_b,
+ xpparam_t *xpp);
+
+#endif /* DIFF_PROCESS_H */
diff --git a/diff.c b/diff.c
index 2a9d0d8687..af31072858 100644
--- a/diff.c
+++ b/diff.c
@@ -25,6 +25,7 @@
#include "utf8.h"
#include "odb.h"
#include "userdiff.h"
+#include "diff-process.h"
#include "submodule.h"
#include "hashmap.h"
#include "mem-pool.h"
@@ -4055,6 +4056,25 @@ static void builtin_diff(const char *name_a,
xpp.ignore_regex_nr = o->ignore_regex_nr;
xpp.anchors = o->anchors;
xpp.anchors_nr = o->anchors_nr;
+
+ /*
+ * Send the blob oids only for a side whose content is the
+ * raw blob: textconv rewrites the bytes, and a working-tree
+ * side has no stored oid, so pass NULL there rather than an
+ * oid that would not name what the tool receives.
+ */
+ if (diff_process_fill_hunks(o, name_a, &mf1, &mf2,
+ (textconv_one || !one->oid_valid) ? NULL : &one->oid,
+ (textconv_two || !two->oid_valid) ? NULL : &two->oid,
+ &xpp)
+ == DIFF_PROCESS_EQUIVALENT) {
+ if (textconv_one)
+ free(mf1.ptr);
+ if (textconv_two)
+ free(mf2.ptr);
+ goto free_ab_and_return;
+ }
+
xecfg.ctxlen = o->context;
xecfg.interhunkctxlen = o->interhunkcontext;
xecfg.flags = XDL_EMIT_FUNCNAMES;
@@ -4135,6 +4155,7 @@ static void builtin_diff(const char *name_a,
} else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume,
&ecbdata, &xpp, &xecfg))
die("unable to generate diff for %s", one->path);
+ free(xpp.external_hunks);
if (o->word_diff)
free_diff_words_data(&ecbdata);
if (textconv_one)
diff --git a/diff.h b/diff.h
index bb5cddaf34..7dc157968d 100644
--- a/diff.h
+++ b/diff.h
@@ -173,6 +173,9 @@ struct diff_flags {
*/
unsigned allow_external;
+ /** Disables diff.<driver>.process. */
+ unsigned no_diff_process;
+
/**
* For communication between the calling program and the options parser;
* tell the calling program to signal the presence of difference using
diff --git a/meson.build b/meson.build
index 3247697f74..aa532f5200 100644
--- a/meson.build
+++ b/meson.build
@@ -328,6 +328,7 @@ libgit_sources = [
'diff-merges.c',
'diff-lib.c',
'diff-no-index.c',
+ 'diff-process.c',
'diff.c',
'diffcore-break.c',
'diffcore-delta.c',
diff --git a/t/helper/meson.build b/t/helper/meson.build
index 3235f10ab8..6abcda4afb 100644
--- a/t/helper/meson.build
+++ b/t/helper/meson.build
@@ -12,6 +12,7 @@ test_tool_sources = [
'test-date.c',
'test-delete-gpgsig.c',
'test-delta.c',
+ 'test-diff-process-backend.c',
'test-dir-iterator.c',
'test-drop-caches.c',
'test-dump-cache-tree.c',
diff --git a/t/helper/test-diff-process-backend.c b/t/helper/test-diff-process-backend.c
new file mode 100644
index 0000000000..c2ec532c4a
--- /dev/null
+++ b/t/helper/test-diff-process-backend.c
@@ -0,0 +1,381 @@
+/*
+ * Test backend for the long-running diff process protocol
+ * (see diff-process.c and Documentation/gitattributes.adoc).
+ *
+ * Usage: test-tool diff-process-backend --mode=<mode> [--log=<path>]
+ *
+ * Implements the server side of the pkt-line handshake and a per-file
+ * response loop. The --mode= switch selects the response shape
+ * (success, error, abort, crash, malformed hunks).
+ *
+ * Per-file request from Git:
+ *
+ * packet: git> command=hunks
+ * packet: git> pathname=<path>
+ * packet: git> [old-oid=<hex>] (omitted for textconv/worktree)
+ * packet: git> [new-oid=<hex>]
+ * packet: git> 0000
+ * packet: git> OLD_CONTENT
+ * packet: git> 0000
+ * packet: git> NEW_CONTENT
+ * packet: git> 0000
+ *
+ * Response varies by --mode (default: whole-file):
+ *
+ * whole-file packet: git< hunk <1|0> <old_lines> <1|0> <new_lines>
+ * (start is 0 for an empty side, matching git diff)
+ * fixed-hunk packet: git< hunk 5 2 5 2
+ * no-hunks (no hunk packets)
+ * bad-hunk packet: git< hunk 999 1 999 1
+ * bad-parse packet: git< garbage not a hunk
+ * bad-sync packet: git< hunk 1 2 1 1
+ * bad-gap packet: git< hunk 1 1 3 1
+ * bad-start packet: git< hunk 0 1 1 1
+ * multi-hunk packet: git< hunk 5 2 5 2
+ * packet: git< hunk 9 2 9 2
+ * insert packet: git< hunk 3 0 3 2 (mid-file count-0 insertion)
+ * flood packet: git< hunk 1 1 1 1 (x100000)
+ * overlap packet: git< hunk 1 5 1 5
+ * packet: git< hunk 3 2 3 2
+ * no-cap (omits capability=hunks during handshake)
+ * error (status=error instead of status=success)
+ * abort (status=abort instead of status=success)
+ * crash exit(1) before sending any response
+ *
+ * All success modes (not error/abort/crash) end with:
+ *
+ * packet: git< 0000
+ * packet: git< status=success
+ * packet: git< 0000
+ *
+ * Each request is logged to --log as:
+ *
+ * command=<cmd> pathname=<path> old-oid=<hex> new-oid=<hex> old=<first line> new=<first line>
+ */
+
+#include "test-tool.h"
+#include "pkt-line.h"
+#include "parse-options.h"
+#include "strbuf.h"
+
+static FILE *logfile;
+
+enum mode {
+ MODE_WHOLE_FILE,
+ MODE_FIXED_HUNK,
+ MODE_NO_HUNKS,
+ MODE_BAD_HUNK,
+ MODE_BAD_PARSE,
+ MODE_BAD_SYNC,
+ MODE_BAD_GAP,
+ MODE_BAD_START,
+ MODE_MULTI_HUNK,
+ MODE_INSERT,
+ MODE_FLOOD,
+ MODE_OVERLAP,
+ MODE_NO_CAP,
+ MODE_ERROR,
+ MODE_ABORT,
+ MODE_CRASH,
+};
+
+static enum mode parse_mode(const char *s)
+{
+ if (!strcmp(s, "whole-file"))
+ return MODE_WHOLE_FILE;
+ if (!strcmp(s, "fixed-hunk"))
+ return MODE_FIXED_HUNK;
+ if (!strcmp(s, "no-hunks"))
+ return MODE_NO_HUNKS;
+ if (!strcmp(s, "bad-hunk"))
+ return MODE_BAD_HUNK;
+ if (!strcmp(s, "bad-parse"))
+ return MODE_BAD_PARSE;
+ if (!strcmp(s, "bad-sync"))
+ return MODE_BAD_SYNC;
+ if (!strcmp(s, "bad-gap"))
+ return MODE_BAD_GAP;
+ if (!strcmp(s, "bad-start"))
+ return MODE_BAD_START;
+ if (!strcmp(s, "multi-hunk"))
+ return MODE_MULTI_HUNK;
+ if (!strcmp(s, "insert"))
+ return MODE_INSERT;
+ if (!strcmp(s, "flood"))
+ return MODE_FLOOD;
+ if (!strcmp(s, "overlap"))
+ return MODE_OVERLAP;
+ if (!strcmp(s, "no-cap"))
+ return MODE_NO_CAP;
+ if (!strcmp(s, "error"))
+ return MODE_ERROR;
+ if (!strcmp(s, "abort"))
+ return MODE_ABORT;
+ if (!strcmp(s, "crash"))
+ return MODE_CRASH;
+ die("unknown --mode=%s", s);
+}
+
+/*
+ * Read "key=value" packets up to a flush, capturing "command" and
+ * "pathname". Returns 1 if a request was read, 0 on EOF.
+ *
+ * The first packet uses the gentle variant so that a clean shutdown
+ * by Git (EOF) does not produce a spurious "the remote end hung up
+ * unexpectedly" on stderr. Subsequent packets use the non-gentle
+ * variant: once inside a request, truncation is a protocol violation
+ * and dying loudly is the correct response.
+ */
+static int read_request_header(char **command, char **pathname,
+ char **old_oid, char **new_oid)
+{
+ int first = 1;
+ char *line;
+
+ *command = *pathname = *old_oid = *new_oid = NULL;
+ for (;;) {
+ const char *value;
+
+ if (first) {
+ if (packet_read_line_gently(0, NULL, &line) < 0)
+ return 0;
+ first = 0;
+ } else {
+ line = packet_read_line(0, NULL);
+ }
+ if (!line)
+ break;
+ if (skip_prefix(line, "command=", &value))
+ *command = xstrdup(value);
+ else if (skip_prefix(line, "pathname=", &value))
+ *pathname = xstrdup(value);
+ else if (skip_prefix(line, "old-oid=", &value))
+ *old_oid = xstrdup(value);
+ else if (skip_prefix(line, "new-oid=", &value))
+ *new_oid = xstrdup(value);
+ }
+ return 1;
+}
+
+static size_t count_lines(const struct strbuf *buf)
+{
+ size_t lines = 0;
+
+ for (size_t i = 0; i < buf->len; i++)
+ if (buf->buf[i] == '\n')
+ lines++;
+
+ return lines + (buf->len > 0 && buf->buf[buf->len - 1] != '\n');
+}
+
+static void send_status(const char *status)
+{
+ packet_flush(1);
+ packet_write_fmt(1, "%s\n", status);
+ packet_flush(1);
+}
+
+static void respond(enum mode mode,
+ const struct strbuf *old_buf,
+ const struct strbuf *new_buf)
+{
+ switch (mode) {
+ case MODE_ERROR:
+ send_status("status=error");
+ return;
+ case MODE_ABORT:
+ send_status("status=abort");
+ return;
+ case MODE_CRASH:
+ exit(1);
+ case MODE_FIXED_HUNK:
+ packet_write_fmt(1, "hunk 5 2 5 2\n");
+ break;
+ case MODE_BAD_HUNK:
+ packet_write_fmt(1, "hunk 999 1 999 1\n");
+ break;
+ case MODE_BAD_PARSE:
+ packet_write_fmt(1, "garbage not a hunk\n");
+ break;
+ case MODE_BAD_SYNC:
+ packet_write_fmt(1, "hunk 1 2 1 1\n");
+ break;
+ case MODE_BAD_GAP:
+ /*
+ * Globally balanced (1 changed line on each side, so the
+ * total unchanged counts match) but the gap before the
+ * change differs between sides: old line 1 vs new line 3.
+ * Exercises the per-gap lockstep-alignment check.
+ */
+ packet_write_fmt(1, "hunk 1 1 3 1\n");
+ break;
+ case MODE_BAD_START:
+ /*
+ * A start of 0 is valid only for an empty (count 0) range;
+ * pairing it with a nonzero count names no line in either
+ * the protocol's or xdiff's coordinates, so the translation
+ * rejects it and git falls back to the builtin diff.
+ */
+ packet_write_fmt(1, "hunk 0 1 1 1\n");
+ break;
+ case MODE_MULTI_HUNK:
+ /*
+ * Two valid, non-overlapping, gap-aligned hunks. Exercises
+ * the accepting branch of the per-gap lockstep check with a
+ * non-zero previous-hunk end (the realistic two-region case).
+ */
+ packet_write_fmt(1, "hunk 5 2 5 2\n");
+ packet_write_fmt(1, "hunk 9 2 9 2\n");
+ break;
+ case MODE_INSERT:
+ /*
+ * A mid-file pure insertion (count 0 on the old side) in the
+ * protocol's 1-based-position form: 2 lines inserted before
+ * old line 3. Exercises the count-0 path, which uses the
+ * unshifted position (not git diff's "-3,0" display start).
+ */
+ packet_write_fmt(1, "hunk 3 0 3 2\n");
+ break;
+ case MODE_FLOOD: {
+ /*
+ * Emit far more hunks than any small file has lines, so Git
+ * trips its accumulation cap and falls back before reading
+ * them all.
+ */
+ int i;
+ for (i = 0; i < 100000; i++)
+ packet_write_fmt(1, "hunk 1 1 1 1\n");
+ break;
+ }
+ case MODE_OVERLAP:
+ packet_write_fmt(1, "hunk 1 5 1 5\n");
+ packet_write_fmt(1, "hunk 3 2 3 2\n");
+ break;
+ case MODE_NO_HUNKS:
+ break;
+ case MODE_NO_CAP:
+ case MODE_WHOLE_FILE: {
+ size_t old_lines = count_lines(old_buf);
+ size_t new_lines = count_lines(new_buf);
+ /*
+ * Match git diff output: start=0 when count=0
+ * (empty file side), 1 otherwise.
+ */
+ packet_write_fmt(1, "hunk %"PRIuMAX" %"PRIuMAX
+ " %"PRIuMAX" %"PRIuMAX"\n",
+ (uintmax_t)(old_lines ? 1 : 0),
+ (uintmax_t)old_lines,
+ (uintmax_t)(new_lines ? 1 : 0),
+ (uintmax_t)new_lines);
+ break;
+ }
+ }
+ send_status("status=success");
+}
+
+static void command_loop(enum mode mode)
+{
+ for (;;) {
+ char *command = NULL, *pathname = NULL;
+ char *old_oid = NULL, *new_oid = NULL;
+ struct strbuf obuf = STRBUF_INIT;
+ struct strbuf nbuf = STRBUF_INIT;
+
+ if (!read_request_header(&command, &pathname,
+ &old_oid, &new_oid))
+ break; /* EOF: Git closed its end */
+
+ read_packetized_to_strbuf(0, &obuf, 0);
+ read_packetized_to_strbuf(0, &nbuf, 0);
+
+ if (logfile) {
+ fprintf(logfile,
+ "command=%s pathname=%s old-oid=%s new-oid=%s"
+ " old=%.*s new=%.*s\n",
+ command ? command : "(none)",
+ pathname ? pathname : "(none)",
+ old_oid ? old_oid : "(none)",
+ new_oid ? new_oid : "(none)",
+ (int)(strchrnul(obuf.buf, '\n') - obuf.buf),
+ obuf.buf,
+ (int)(strchrnul(nbuf.buf, '\n') - nbuf.buf),
+ nbuf.buf);
+ fflush(logfile);
+ }
+
+ respond(mode, &obuf, &nbuf);
+
+ free(command);
+ free(pathname);
+ free(old_oid);
+ free(new_oid);
+ strbuf_release(&obuf);
+ strbuf_release(&nbuf);
+ }
+}
+
+static void handshake(enum mode mode)
+{
+ char *line;
+
+ line = packet_read_line(0, NULL);
+ if (!line || strcmp(line, "git-diff-client"))
+ die("bad welcome: '%s'", line ? line : "(eof)");
+ line = packet_read_line(0, NULL);
+ if (!line || strcmp(line, "version=1"))
+ die("bad version: '%s'", line ? line : "(eof)");
+ if (packet_read_line(0, NULL))
+ die("expected flush after version");
+
+ packet_write_fmt(1, "git-diff-server\n");
+ packet_write_fmt(1, "version=1\n");
+ packet_flush(1);
+
+ /* Drain capabilities advertised by Git */
+ while ((line = packet_read_line(0, NULL)))
+ ; /* drain */
+
+ /* Respond with our capabilities (or none for no-cap mode) */
+ if (mode != MODE_NO_CAP)
+ packet_write_fmt(1, "capability=hunks\n");
+ packet_flush(1);
+}
+
+static const char *const usage_str[] = {
+ "test-tool diff-process-backend --mode=<mode> [--log=<path>]",
+ NULL
+};
+
+int cmd__diff_process_backend(int argc, const char **argv)
+{
+ const char *mode_str = NULL, *log_path = NULL;
+ enum mode mode = MODE_WHOLE_FILE;
+ struct option options[] = {
+ OPT_STRING(0, "mode", &mode_str, "mode",
+ "response shape (default whole-file);"
+ " see the file header for the full list of modes"),
+ OPT_STRING(0, "log", &log_path, "path",
+ "append per-request summary to this file"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, NULL, options, usage_str, 0);
+ if (argc)
+ usage_with_options(usage_str, options);
+
+ if (mode_str)
+ mode = parse_mode(mode_str);
+
+ if (log_path) {
+ logfile = fopen(log_path, "a");
+ if (!logfile)
+ die_errno("failed to open log '%s'", log_path);
+ }
+
+ handshake(mode);
+ command_loop(mode);
+
+ if (logfile && fclose(logfile))
+ die_errno("error closing log");
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index b71a22b43b..3c3f95269c 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -22,6 +22,7 @@ static struct test_cmd cmds[] = {
{ "date", cmd__date },
{ "delete-gpgsig", cmd__delete_gpgsig },
{ "delta", cmd__delta },
+ { "diff-process-backend", cmd__diff_process_backend },
{ "dir-iterator", cmd__dir_iterator },
{ "drop-caches", cmd__drop_caches },
{ "dump-cache-tree", cmd__dump_cache_tree },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index f2885b33d5..a5bb755516 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -15,6 +15,7 @@ int cmd__csprng(int argc, const char **argv);
int cmd__date(int argc, const char **argv);
int cmd__delta(int argc, const char **argv);
int cmd__delete_gpgsig(int argc, const char **argv);
+int cmd__diff_process_backend(int argc, const char **argv);
int cmd__dir_iterator(int argc, const char **argv);
int cmd__drop_caches(int argc, const char **argv);
int cmd__dump_cache_tree(int argc, const char **argv);
diff --git a/t/meson.build b/t/meson.build
index 3219264fe7..6afbfa6a87 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -512,6 +512,7 @@ integration_tests = [
't4072-diff-max-depth.sh',
't4073-diff-stat-name-width.sh',
't4074-diff-shifted-matched-group.sh',
+ 't4080-diff-process.sh',
't4100-apply-stat.sh',
't4101-apply-nonl.sh',
't4102-apply-rename.sh',
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
new file mode 100755
index 0000000000..3b75df082e
--- /dev/null
+++ b/t/t4080-diff-process.sh
@@ -0,0 +1,645 @@
+#!/bin/sh
+
+test_description='diff process via long-running process'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# See t/helper/test-diff-process-backend.c for the backend implementation
+# and available --mode= options.
+
+BACKEND="test-tool diff-process-backend"
+
+test_expect_success 'setup' '
+ echo "*.c diff=cdiff" >.gitattributes &&
+ git add .gitattributes &&
+
+ # boundary.c: 10 lines, changes at 5-6 and 9-10.
+ # Used by: hunk boundaries, error fallback, crash, bad hunks, overlap.
+ cat >boundary.c <<-\EOF &&
+ line1
+ line2
+ line3
+ line4
+ OLD5
+ OLD6
+ line7
+ line8
+ OLD9
+ OLD10
+ EOF
+ git add boundary.c &&
+
+ # worddiff.c: single-line function, value changes 1 -> 999.
+ # Used by: word-diff, --diff-algorithm, --no-ext-diff, --stat.
+ cat >worddiff.c <<-\EOF &&
+ int value(void) { return 1; }
+ EOF
+ git add worddiff.c &&
+
+ # newfile.c: single-line function, value changes 42 -> 99.
+ # Used by: modified file, --exit-code, multiple drivers.
+ cat >newfile.c <<-\EOF &&
+ int new_func(void) { return 42; }
+ EOF
+ git add newfile.c &&
+
+ # logtest.c: single-line function for log/format-patch tests.
+ # Needs two commits so log -1 has a diff.
+ cat >logtest.c <<-\EOF &&
+ int logfunc(void) { return 1; }
+ EOF
+ git add logtest.c &&
+
+ # one.c/two.c: two-file pair for error/abort/startup-failure tests.
+ cat >one.c <<-\EOF &&
+ int first(void) { return 1; }
+ EOF
+ cat >two.c <<-\EOF &&
+ int second(void) { return 2; }
+ EOF
+ git add one.c two.c &&
+
+ git commit -m "initial" &&
+
+ # Second commit for logtest.c (so log -1 has something to show).
+ cat >logtest.c <<-\EOF &&
+ int logfunc(void) { return 2; }
+ EOF
+ git add logtest.c &&
+ git commit -m "change logtest.c" &&
+
+ # Working tree modifications (not committed).
+ cat >boundary.c <<-\EOF &&
+ line1
+ line2
+ line3
+ line4
+ NEW5
+ NEW6
+ line7
+ line8
+ NEW9
+ NEW10
+ EOF
+
+ cat >worddiff.c <<-\EOF &&
+ int value(void) { return 999; }
+ EOF
+
+ cat >newfile.c <<-\EOF &&
+ int new_func(void) { return 99; }
+ EOF
+
+ cat >one.c <<-\EOF &&
+ int first(void) { return 10; }
+ EOF
+
+ cat >two.c <<-\EOF
+ int second(void) { return 20; }
+ EOF
+'
+
+#
+# Core behavior: the tool controls which lines are marked as changed.
+#
+
+test_expect_success 'diff process hunk boundaries affect output' '
+ # The file has changes at lines 5-6 and 9-10, but fixed-hunk
+ # only reports lines 5-6 as changed. Lines 9-10 should not
+ # appear as changed in the output.
+ git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk" \
+ diff boundary.c >actual &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^-OLD6" actual &&
+ test_grep "^+NEW5" actual &&
+ test_grep "^+NEW6" actual &&
+ test_grep ! "^-OLD9" actual &&
+ test_grep ! "^-OLD10" actual &&
+ test_grep ! "^+NEW9" actual &&
+ test_grep ! "^+NEW10" actual
+'
+
+test_expect_success 'diff process accepts valid multi-hunk output' '
+ # multi-hunk reports both changed regions (5-6 and 9-10) as two
+ # gap-aligned hunks. This exercises the accepting branch of the
+ # per-gap lockstep check (non-zero previous-hunk end) and must
+ # produce a correct two-region diff with the lines between the
+ # hunks kept as context.
+ git -c diff.cdiff.process="$BACKEND --mode=multi-hunk" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual &&
+ test_grep "^-OLD9" actual &&
+ test_grep "^+NEW9" actual &&
+ test_grep "^ line7" actual &&
+ test_grep "^ line8" actual &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process accepts a mid-file count-0 insertion' '
+ # insert mode reports "hunk 3 0 3 2": a pure insertion (count 0 on
+ # the old side) in the protocol 1-based-position form. Exercises
+ # the count-0 hunk path that the other valid-hunk modes (full
+ # replacements, equal-count modifies) never hit. Empty stderr is
+ # the discriminator: a mishandled count-0 start would be rejected
+ # by the lockstep check and warn.
+ cat >insert.c <<-\EOF &&
+ a
+ b
+ c
+ d
+ e
+ EOF
+ git add insert.c &&
+ git commit -m "add insert.c" &&
+ cat >insert.c <<-\EOF &&
+ a
+ b
+ X
+ Y
+ c
+ d
+ e
+ EOF
+ git -c diff.cdiff.process="$BACKEND --mode=insert" \
+ diff insert.c >actual 2>stderr &&
+ test_grep "^+X" actual &&
+ test_grep "^+Y" actual &&
+ test_grep "^ c" actual &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process works with modified file' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff -- newfile.c >actual 2>stderr &&
+ test_grep "return 99" actual &&
+ test_grep "pathname=newfile.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process works with added file (empty old side)' '
+ cat >added.c <<-\EOF &&
+ int added(void) { return 1; }
+ EOF
+ git add added.c &&
+
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff --cached -- added.c >actual 2>stderr &&
+ test_grep "added" actual &&
+ test_grep "pathname=added.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process works with deleted file (empty new side)' '
+ git add added.c &&
+ git commit -m "commit added.c" &&
+ git rm added.c &&
+
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff --cached -- added.c >actual 2>stderr &&
+ test_grep "deleted file" actual &&
+ test_grep "pathname=added.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process skipped for binary files' '
+ printf "\\0binary" >binary.c &&
+ git add binary.c &&
+ git commit -m "add binary" &&
+ printf "\\0changed" >binary.c &&
+
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff -- binary.c >actual &&
+ test_grep "Binary files" actual &&
+ test_path_is_missing backend.log
+'
+
+test_expect_success 'diff process not consulted for unmatched driver' '
+ echo "not tracked by cdiff" >unmatched.txt &&
+ git add unmatched.txt &&
+ git commit -m "add unmatched.txt" &&
+
+ echo "modified" >unmatched.txt &&
+
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff -- unmatched.txt >actual &&
+ test_grep "modified" actual &&
+ test_path_is_missing backend.log
+'
+
+test_expect_success 'multiple drivers use separate processes' '
+ echo "*.h diff=hdiff" >>.gitattributes &&
+ git add .gitattributes &&
+
+ cat >multi.h <<-\EOF &&
+ int header(void) { return 1; }
+ EOF
+ git add multi.h &&
+ git commit -m "add multi.h" &&
+
+ cat >multi.h <<-\EOF &&
+ int header(void) { return 2; }
+ EOF
+
+ test_when_finished "rm -f backend-c.log backend-h.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend-c.log" \
+ -c diff.hdiff.process="$BACKEND --log=backend-h.log" \
+ diff -- newfile.c multi.h >actual 2>stderr &&
+ test_grep "pathname=newfile.c" backend-c.log &&
+ test_grep "pathname=multi.h" backend-h.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process works alongside textconv' '
+ write_script uppercase-filter <<-\EOF &&
+ tr "a-z" "A-Z" <"$1"
+ EOF
+
+ cat >textconv.c <<-\EOF &&
+ hello world
+ EOF
+ git add textconv.c &&
+ git commit -m "add textconv.c" &&
+
+ cat >textconv.c <<-\EOF &&
+ goodbye world
+ EOF
+
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.textconv="./uppercase-filter" \
+ -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff -- textconv.c >actual 2>stderr &&
+ # The diff process receives textconv-transformed (uppercase) content.
+ test_grep "pathname=textconv.c" backend.log &&
+ test_grep "old=HELLO WORLD" backend.log &&
+ test_grep "new=GOODBYE WORLD" backend.log &&
+ test_must_be_empty stderr
+'
+
+#
+# Downstream features: word diff, log, equivalent files, exit code.
+#
+
+test_expect_success 'diff process with --word-diff' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff --word-diff worddiff.c >actual 2>stderr &&
+ test_grep "\[-1;-\]" actual &&
+ test_grep "{+999;+}" actual &&
+ test_grep "pathname=worddiff.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process works with git log -p' '
+ # With no-hunks mode, the tool says the files are equivalent,
+ # so log -p should show the commit but no diff content.
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ log -1 -p -- logtest.c >actual 2>stderr &&
+ test_grep "change logtest.c" actual &&
+ test_grep ! "return 2" actual &&
+ test_grep "command=hunks pathname=logtest.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process no hunks suppresses diff output' '
+ cat >nohunks.c <<-\EOF &&
+ int zero(void) { return 0; }
+ EOF
+ git add nohunks.c &&
+ git commit -m "add nohunks.c" &&
+
+ cat >nohunks.c <<-\EOF &&
+ int zero(void) { return 999; }
+ EOF
+
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
+ diff nohunks.c >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'diff process no hunks with --exit-code returns success' '
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
+ diff --exit-code nohunks.c
+'
+
+test_expect_success 'diff process equivalent commit: --exit-code and --quiet agree' '
+ # A committed blob pair (not a worktree file) whose oids differ but
+ # the tool reports equivalent. --exit-code and --quiet must agree
+ # with the shown diff (empty) and report success, not fall back to
+ # the byte-level "oids differ" answer.
+ cat >ecq.c <<-\EOF &&
+ alpha
+ EOF
+ git add ecq.c &&
+ git commit -m "ecq v1" &&
+ cat >ecq.c <<-\EOF &&
+ beta
+ EOF
+ git add ecq.c &&
+ git commit -m "ecq v2" &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
+ diff --exit-code HEAD^ HEAD -- ecq.c &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
+ diff --quiet HEAD^ HEAD -- ecq.c
+'
+
+test_expect_success 'diff process falls back for trailing-newline-only change' '
+ test_when_finished "rm -f backend.log" &&
+ printf "a\nb\nc\n" >eofnl.c &&
+ git add eofnl.c &&
+ git commit -m "add eofnl.c" &&
+ printf "a\nb\nc" >eofnl.c &&
+ # Same lines, only the final newline removed. The tool reports
+ # no hunks (it sees identical lines), but that change is not
+ # expressible as hunks, so git falls back to the builtin diff
+ # rather than treating the files as equivalent.
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ diff eofnl.c >actual 2>stderr &&
+ test_grep "No newline at end of file" actual &&
+ test_grep "pathname=eofnl.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process falls back for added file (empty old side)' '
+ test_when_finished "rm -f backend.log" &&
+ printf "x\ny\nz\n" >addnl.c &&
+ git add addnl.c &&
+ # The empty old side has no trailing newline while the new side
+ # does, so the newline fallback shows the addition rather than
+ # letting no-hunks suppress the whole new file.
+ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+ diff --cached addnl.c >actual 2>stderr &&
+ test_grep "^+x" actual &&
+ test_grep "pathname=addnl.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process with --exit-code and hunks returns failure' '
+ test_expect_code 1 git -c diff.cdiff.process="$BACKEND" \
+ diff --exit-code newfile.c
+'
+
+#
+# Bypass mechanisms: flags and commands that skip the diff process.
+#
+
+test_expect_success 'diff process bypassed by --diff-algorithm' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff --diff-algorithm=patience worddiff.c >actual &&
+ test_grep "return 999" actual &&
+ test_path_is_missing backend.log
+'
+
+test_expect_success 'diff process bypassed under whitespace-ignoring flags' '
+ test_when_finished "rm -f backend.log" &&
+ printf "a\nb\nc\n" >wsbypass.c &&
+ git add wsbypass.c &&
+ git commit -m "add wsbypass.c" &&
+ printf "a\n b \nc\n" >wsbypass.c &&
+ # The tool is never told about these options and could not honor
+ # them, so git bypasses the process for each (covering the whole
+ # XDF_WHITESPACE_FLAGS | XDF_IGNORE_BLANK_LINES mask, not just -w).
+ for opt in -w -b --ignore-space-at-eol --ignore-blank-lines
+ do
+ rm -f backend.log &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff $opt wsbypass.c >actual 2>stderr &&
+ test_path_is_missing backend.log &&
+ test_must_be_empty stderr ||
+ return 1
+ done &&
+ # -w additionally suppresses the whitespace-only change via the
+ # builtin diff that now runs.
+ git -c diff.cdiff.process="$BACKEND" diff -w wsbypass.c >actual &&
+ test_must_be_empty actual
+'
+
+#
+# Error handling and fallback.
+#
+
+test_expect_success 'diff process fallback on tool error status' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=error --log=backend.log" \
+ diff boundary.c >actual 2>stderr &&
+ # Fallback produces the full builtin diff (both change regions).
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual &&
+ test_grep "^-OLD9" actual &&
+ test_grep "^+NEW9" actual &&
+ # Tool was contacted (it replied with error, not crash).
+ test_grep "command=hunks pathname=boundary.c" backend.log &&
+ test_grep "diff process.*failed" stderr
+'
+
+test_expect_success 'diff process error keeps tool available for next file' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=error --log=backend.log" \
+ diff -- one.c two.c >actual 2>stderr &&
+ # Unlike abort, error keeps the tool available: both files
+ # are sent to the tool (and both fall back).
+ test_grep "pathname=one.c" backend.log &&
+ test_grep "pathname=two.c" backend.log &&
+ test_grep "return 10" actual &&
+ test_grep "return 20" actual &&
+ test_grep "diff process.*failed" stderr
+'
+
+test_expect_success 'diff process abort disables for session' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=abort --log=backend.log" \
+ diff -- one.c two.c >actual 2>stderr &&
+ # Both files should still produce diff output via fallback.
+ test_grep "return 10" actual &&
+ test_grep "return 20" actual &&
+ # The tool aborts on the first file and git clears its
+ # capability. The second file never contacts the tool.
+ test_grep "pathname=one.c" backend.log &&
+ test_grep ! "pathname=two.c" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process fallback on tool crash' '
+ git -c diff.cdiff.process="$BACKEND --mode=crash" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual &&
+ test_grep "^-OLD9" actual &&
+ test_grep "^+NEW9" actual &&
+ # Crash is a communication failure, so a warning is emitted.
+ test_grep "diff process.*failed" stderr
+'
+
+test_expect_success 'diff process startup failure only warns once' '
+ git -c diff.cdiff.process="/nonexistent/tool" \
+ diff -- one.c two.c >actual 2>stderr &&
+ # Both files produce diff output via fallback.
+ test_grep "return 10" actual &&
+ test_grep "return 20" actual &&
+ # Sentinel prevents repeated warnings: only one, not one per file.
+ test_grep "diff process.*failed" stderr >warnings &&
+ test_line_count = 1 warnings
+'
+
+
+test_expect_success 'diff process fallback on bad hunks' '
+ git -c diff.cdiff.process="$BACKEND --mode=bad-hunk" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual &&
+ test_grep "^-OLD9" actual &&
+ test_grep "^+NEW9" actual &&
+ test_grep "hunk past the end" stderr
+'
+
+test_expect_success 'diff process fallback on mismatched unchanged totals' '
+ cat >synctest.c <<-\EOF &&
+ line1
+ line2
+ line3
+ EOF
+ git add synctest.c &&
+ git commit -m "add synctest.c" &&
+
+ cat >synctest.c <<-\EOF &&
+ line1
+ changed
+ line3
+ EOF
+
+ # bad-sync reports hunk 1 2 1 1: marks 2 old lines and 1 new
+ # line as changed, leaving 1 unchanged old vs 2 unchanged new.
+ # The synchronization invariant fails and git falls back.
+ git -c diff.cdiff.process="$BACKEND --mode=bad-sync" \
+ diff synctest.c >actual 2>stderr &&
+ test_grep "changed" actual &&
+ test_grep "misaligned" stderr
+'
+
+test_expect_success 'diff process fallback on misaligned hunk gap' '
+ # bad-gap reports hunk 1 1 3 1 on boundary.c: one changed line
+ # on each side, so the total unchanged counts match, but the
+ # unchanged run before the change differs (old line 1 vs new
+ # line 3). A global count check would accept this and emit a
+ # corrupt diff; the per-gap lockstep check rejects it and git
+ # falls back to the builtin algorithm.
+ git -c diff.cdiff.process="$BACKEND --mode=bad-gap" \
+ diff boundary.c >actual 2>stderr &&
+ # The builtin fallback shows both changed regions as additions
+ # (a corrupt-accepted hunk would show NEW5 only as context).
+ test_grep "^+NEW5" actual &&
+ test_grep "^+NEW9" actual &&
+ test_grep "misaligned" stderr
+'
+
+test_expect_success 'diff process fallback on overlapping hunks' '
+ # boundary.c has 10 lines, so both hunks are in bounds
+ # but they overlap at lines 3-4, triggering the ordering check.
+ git -c diff.cdiff.process="$BACKEND --mode=overlap" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "NEW5" actual &&
+ test_grep "overlapping hunks" stderr
+'
+
+test_expect_success 'diff process fallback on malformed hunk line' '
+ git -c diff.cdiff.process="$BACKEND --mode=bad-parse" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual
+'
+
+test_expect_success 'diff process fallback on start 0 with nonzero count' '
+ # bad-start reports hunk 0 1 1 1. A start of 0 is valid only for
+ # an empty (count 0) range, so the presentation-to-xdiff
+ # translation rejects it and git falls back to the builtin diff
+ # instead of handing xdiff an out-of-range start.
+ git -c diff.cdiff.process="$BACKEND --mode=bad-start" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual &&
+ test_grep "diff process.*failed" stderr
+'
+
+test_expect_success 'diff process caps a flood of hunks and falls back' '
+ # flood emits far more hunks than the file has lines. Git must
+ # stop accumulating and fall back to the builtin diff rather than
+ # grow memory without bound.
+ git -c diff.cdiff.process="$BACKEND --mode=flood" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "too many hunks" stderr
+'
+
+test_expect_success 'diff process skipped when tool omits capability' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --mode=no-cap --log=backend.log" \
+ diff boundary.c >actual 2>stderr &&
+ # Builtin diff runs: all changes appear, including lines 9-10
+ # that a tool-provided hunk would have narrowed away.
+ test_grep "^-OLD5" actual &&
+ test_grep "^-OLD9" actual &&
+ # The process launched (creating the log) but was
+ # never sent a per-file request, so no hunks command is logged.
+ test_path_is_file backend.log &&
+ test_grep ! "command=hunks" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process receives old-oid and new-oid for a blob pair' '
+ test_when_finished "rm -f backend.log" &&
+ cat >oidpair.c <<-\EOF &&
+ int f(void) { return 1; }
+ EOF
+ git add oidpair.c &&
+ git commit -m "oidpair v1" &&
+ old=$(git rev-parse HEAD:oidpair.c) &&
+
+ cat >oidpair.c <<-\EOF &&
+ int f(void) { return 2; }
+ EOF
+ git add oidpair.c &&
+ git commit -m "oidpair v2" &&
+ new=$(git rev-parse HEAD:oidpair.c) &&
+
+ # Both sides are stored blobs, so their object names are sent.
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff HEAD^ HEAD -- oidpair.c >actual 2>stderr &&
+ test_grep "old-oid=$old new-oid=$new" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_expect_success 'diff process omits old-oid and new-oid for textconv content' '
+ test_when_finished "rm -f backend.log" &&
+ write_script oidcat <<-\EOF &&
+ cat "$1"
+ EOF
+ cat >oidtc.c <<-\EOF &&
+ alpha
+ EOF
+ git add oidtc.c &&
+ git commit -m "oidtc v1" &&
+ cat >oidtc.c <<-\EOF &&
+ beta
+ EOF
+ git add oidtc.c &&
+ git commit -m "oidtc v2" &&
+
+ # textconv rewrites the bytes, so the raw-blob object name that
+ # would otherwise identify each side is omitted.
+ git -c diff.cdiff.textconv="./oidcat" \
+ -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ diff HEAD^ HEAD -- oidtc.c >actual 2>stderr &&
+ test_grep "pathname=oidtc.c" backend.log &&
+ test_grep "old-oid=(none) new-oid=(none)" backend.log &&
+ test_must_be_empty stderr
+'
+
+test_done
diff --git a/userdiff.h b/userdiff.h
index 51c26e0d41..a98eabe377 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -3,6 +3,7 @@
#include "notes-cache.h"
+struct diff_subprocess;
struct index_state;
struct repository;
@@ -33,6 +34,8 @@ struct userdiff_driver {
int textconv_want_cache;
const char *process;
char *process_owned;
+ struct diff_subprocess *diff_subprocess;
+ unsigned diff_process_failed : 1;
};
enum userdiff_driver_type {
USERDIFF_DRIVER_TYPE_BUILTIN = 1<<0,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 4/9] sub-process: separate process lifecycle from hashmap management
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
subprocess_start() and subprocess_stop() couple two concerns:
managing a child process (setup, handshake, teardown) and
managing a hashmap that indexes running processes by command
string. The hashmap suits callers like convert.c where many
files may share one filter process looked up by name, but
callers that manage process lifetime through their own data
structures do not need it.
Extract subprocess_start_command() and subprocess_stop_command()
so callers can reuse the child process setup and handshake
machinery without maintaining a hashmap. subprocess_start()
and subprocess_stop() become thin wrappers that add hashmap
operations on top.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
sub-process.c | 28 +++++++++++++++++++++++-----
sub-process.h | 9 ++++++++-
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/sub-process.c b/sub-process.c
index 2d5c965169..3cef42b088 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -49,7 +49,7 @@ int subprocess_read_status(int fd, struct strbuf *status)
return (len < 0) ? len : 0;
}
-void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry)
+void subprocess_stop_command(struct subprocess_entry *entry)
{
if (!entry)
return;
@@ -57,7 +57,14 @@ void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry)
entry->process.clean_on_exit = 0;
kill(entry->process.pid, SIGTERM);
finish_command(&entry->process);
+}
+void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry)
+{
+ if (!entry)
+ return;
+
+ subprocess_stop_command(entry);
hashmap_remove(hashmap, &entry->ent, NULL);
}
@@ -72,7 +79,7 @@ static void subprocess_exit_handler(struct child_process *process)
finish_command(process);
}
-int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
+int subprocess_start_command(struct subprocess_entry *entry, const char *cmd,
subprocess_start_fn startfn)
{
int err;
@@ -96,15 +103,26 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
return err;
}
- hashmap_entry_init(&entry->ent, strhash(cmd));
-
err = startfn(entry);
if (err) {
error("initialization for subprocess '%s' failed", cmd);
- subprocess_stop(hashmap, entry);
+ subprocess_stop_command(entry);
return err;
}
+ return 0;
+}
+
+int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
+ subprocess_start_fn startfn)
+{
+ int err;
+
+ err = subprocess_start_command(entry, cmd, startfn);
+ if (err)
+ return err;
+
+ hashmap_entry_init(&entry->ent, strhash(cmd));
hashmap_add(hashmap, &entry->ent);
return 0;
}
diff --git a/sub-process.h b/sub-process.h
index bfc3959a1b..45f1b8e5e3 100644
--- a/sub-process.h
+++ b/sub-process.h
@@ -52,10 +52,17 @@ int cmd2process_cmp(const void *unused_cmp_data,
*/
typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);
-/* Start a subprocess and add it to the subprocess hashmap. */
+/* Start a subprocess and run the startfn (typically handshake). */
+int subprocess_start_command(struct subprocess_entry *entry, const char *cmd,
+ subprocess_start_fn startfn);
+
+/* Start a subprocess, run startfn, and add it to the subprocess hashmap. */
int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
subprocess_start_fn startfn);
+/* Kill a subprocess. */
+void subprocess_stop_command(struct subprocess_entry *entry);
+
/* Kill a subprocess and remove it from the subprocess hashmap. */
void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 3/9] userdiff: add diff.<driver>.process config
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Add the process field to struct userdiff_driver and teach the
config parser to populate it from diff.<driver>.process.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
userdiff.c | 7 +++++++
userdiff.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/userdiff.c b/userdiff.c
index b5412e6bc3..7547874aa2 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -509,6 +509,13 @@ int userdiff_config(const char *k, const char *v)
drv->algorithm = drv->algorithm_owned;
return ret;
}
+ if (!strcmp(type, "process")) {
+ int ret;
+ FREE_AND_NULL(drv->process_owned);
+ ret = git_config_string(&drv->process_owned, k, v);
+ drv->process = drv->process_owned;
+ return ret;
+ }
return 0;
}
diff --git a/userdiff.h b/userdiff.h
index 827361b0bc..51c26e0d41 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -31,6 +31,8 @@ struct userdiff_driver {
char *textconv_owned;
struct notes_cache *textconv_cache;
int textconv_want_cache;
+ const char *process;
+ char *process_owned;
};
enum userdiff_driver_type {
USERDIFF_DRIVER_TYPE_BUILTIN = 1<<0,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 2/9] xdiff: support external hunks via xpparam_t
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Add two new xpparam_t fields (external_hunks, external_hunks_nr)
that let callers supply pre-computed hunks. When set, xdl_diff()
populates the changed[] arrays from these hunks instead of running
the diff algorithm, then continues through compaction and emission
as usual.
Validate supplied hunks before use. Out-of-bounds line numbers,
overlapping or out-of-order hunks, and misaligned unchanged runs are
treated as a malformed tool response: xdl_populate_hunks_from_external()
warns, returns -1, and xdl_diff() falls back to the builtin diff
algorithm for that file. The run of unchanged lines between two hunks
(and before the first and after the last) must be the same length on
both sides; xdl_build_script() walks the two files in lockstep over
unchanged lines, so a balanced total is not enough. Non-negative
counts and 1-based starts are instead caller preconditions, checked
with BUG(), since the caller normalizes hunks before this point.
On rejection xdl_diff() frees the environment it prepared and falls
through to xdl_do_diff(), which prepares a fresh one for the builtin
pass.
Skip trim_common_tail() in xdi_diff() when external hunks are
present, since external hunks reference line numbers in the
original content.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
xdiff-interface.c | 7 +++-
xdiff/xdiff.h | 16 +++++++++
xdiff/xdiffi.c | 84 +++++++++++++++++++++++++++++++++++++++++++++--
xdiff/xprepare.c | 10 ++++++
xdiff/xprepare.h | 1 +
5 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index db6938689f..1fa16af668 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -124,7 +124,12 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE)
return -1;
- if (!xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
+ /*
+ * External hunks reference line numbers in the original content;
+ * trimming the tail would change line counts and invalidate them.
+ */
+ if (!xpp->external_hunks &&
+ !xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
trim_common_tail(&a, &b);
return xdl_diff(&a, &b, xpp, xecfg, xecb);
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index dc370712e9..4736bcdb07 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -78,6 +78,18 @@ typedef struct s_mmbuffer {
long size;
} mmbuffer_t;
+/*
+ * Hunk descriptor for externally computed diffs, in xdiff's own
+ * coordinates: line numbers are 1-based and a hunk's start is the
+ * first line it covers. A caller translates any external "empty side"
+ * idiom (such as git diff's start-0/count-0) to a 1-based start before
+ * handing hunks over.
+ */
+struct xdl_hunk {
+ long old_start, old_count;
+ long new_start, new_count;
+};
+
typedef struct s_xpparam {
unsigned long flags;
@@ -88,6 +100,10 @@ typedef struct s_xpparam {
/* See Documentation/diff-options.adoc. */
char **anchors;
size_t anchors_nr;
+
+ /* Externally computed hunks: bypass the diff algorithm. Owned by caller. */
+ struct xdl_hunk *external_hunks;
+ size_t external_hunks_nr;
} xpparam_t;
typedef struct s_xdemitcb {
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index c5a892f91e..73a456f5dd 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -1085,16 +1085,96 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,
}
}
+/*
+ * Populate the changed[] arrays from externally supplied hunks,
+ * bypassing the diff algorithm. The caller normalizes and validates
+ * the hunks first (order, overlap, and lockstep alignment), so this
+ * only marks lines changed after asserting the memory-safety
+ * preconditions it depends on: non-negative counts and 1-based starts
+ * (checked with BUG()), and an in-bounds range (a silent -1 so the
+ * caller can fall back to the builtin diff rather than index changed[]
+ * out of range). Keeping this diagnostic-free leaves user-facing
+ * messages to the git layer.
+ *
+ * Returns 0 on success, -1 if a hunk is out of range.
+ */
+static int xdl_populate_hunks_from_external(xdfenv_t *xe,
+ struct xdl_hunk *hunks,
+ size_t nr_hunks)
+{
+ size_t i;
+ long j;
+
+ /*
+ * xdl_prepare_env() may dirty changed[] via xdl_cleanup_records().
+ * Clear them so only the external hunks are marked.
+ */
+ xdl_clear_changed(&xe->xdf1);
+ xdl_clear_changed(&xe->xdf2);
+
+ for (i = 0; i < nr_hunks; i++) {
+ struct xdl_hunk *h = &hunks[i];
+
+ /*
+ * Non-negative counts and 1-based starts are caller
+ * preconditions (it normalizes hunks into xdiff coordinates
+ * before this point), so a violation is a bug, not a bad
+ * tool response.
+ */
+ if (h->old_count < 0 || h->new_count < 0)
+ BUG("external hunk %"PRIuMAX": "
+ "negative count (old=%ld, new=%ld)",
+ (uintmax_t)(i + 1),
+ h->old_count, h->new_count);
+ if (h->old_start < 1 || h->new_start < 1)
+ BUG("external hunk %"PRIuMAX": "
+ "start not 1-based (old=%ld, new=%ld)",
+ (uintmax_t)(i + 1),
+ h->old_start, h->new_start);
+
+ /*
+ * The caller validates ordering, overlap and lockstep
+ * alignment (and diagnoses a bad response). This is only a
+ * silent in-bounds guard so the marking loop cannot index
+ * changed[] out of range: start + count - 1 <= nrec,
+ * rewritten to avoid overflow. A count of 0 (pure
+ * insert/delete) allows start == nrec + 1, the position
+ * after the last line. On a miss, return -1 and let the
+ * caller fall back to the builtin diff.
+ */
+ if (h->old_count > (long)xe->xdf1.nrec - h->old_start + 1 ||
+ h->new_count > (long)xe->xdf2.nrec - h->new_start + 1)
+ return -1;
+
+ for (j = 0; j < h->old_count; j++)
+ xe->xdf1.changed[h->old_start - 1 + j] = true;
+ for (j = 0; j < h->new_count; j++)
+ xe->xdf2.changed[h->new_start - 1 + j] = true;
+ }
+
+ return 0;
+}
+
int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
xdemitconf_t const *xecfg, xdemitcb_t *ecb) {
xdchange_t *xscr;
xdfenv_t xe;
emit_func_t ef = xecfg->hunk_func ? xdl_call_hunk_func : xdl_emit_diff;
- if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) {
+ if (xpp->external_hunks) {
+ if (xdl_prepare_env(mf1, mf2, xpp, &xe) < 0)
+ return -1;
+ if (xdl_populate_hunks_from_external(&xe,
+ xpp->external_hunks,
+ xpp->external_hunks_nr) == 0)
+ goto diff_done;
+ xdl_free_env(&xe);
+ }
+ if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0)
return -1;
- }
+
+diff_done:
if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 ||
xdl_build_script(&xe, &xscr) < 0) {
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 11bada2608..f4ab935332 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -471,3 +471,13 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
return 0;
}
+
+/*
+ * Reset the changed[] array so that no lines are marked as changed.
+ * Also clears the sentinel slots at changed[-1] and changed[nrec]
+ * that xdl_change_compact() relies on during backward scans.
+ */
+void xdl_clear_changed(xdfile_t *xdf)
+{
+ memset(xdf->changed - 1, 0, (xdf->nrec + 2) * sizeof(bool));
+}
diff --git a/xdiff/xprepare.h b/xdiff/xprepare.h
index 947d9fc1bb..0413baf07b 100644
--- a/xdiff/xprepare.h
+++ b/xdiff/xprepare.h
@@ -28,6 +28,7 @@
int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
xdfenv_t *xe);
void xdl_free_env(xdfenv_t *xe);
+void xdl_clear_changed(xdfile_t *xdf);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 0/9] [RFC] diff: add diff.<driver>.process for external hunk providers
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo
In-Reply-To: <pull.2120.v4.git.1781463564.gitgitgadget@gmail.com>
Language-aware diff tools (e.g., Difftastic) and format-specific analyzers
can produce better line matching than Git's builtin diff algorithm, but
diff.<driver>.command replaces Git's diff output with the program's own
output, so display features like word diff, function context, and color
cannot operate on it; and because the program is consulted only for that
patch output, blame, --stat, and git log -L fall back to Git's builtin line
matching and cannot benefit from the tool at all.
This series adds diff.<driver>.process, a long-running subprocess protocol
that lets an external tool control which lines Git considers changed while
Git handles all output formatting. The protocol follows
filter.<driver>.process: pkt-line over stdin/stdout, capability negotiation,
one process per Git invocation.
The tool receives both file versions and returns changed regions (line
ranges in the old and new file). Git validates and feeds them into the xdiff
pipeline in place of the builtin diff algorithm. When the tool returns no
hunks, Git treats the files as having no changes, which propagates through
patch output, the --stat summary, blame, and git log -L. The request also
carries the two blobs' object names (old-oid/new-oid) so a tool can cache
its analysis keyed on the pair.
* Patch 1: document how an external diff driver (diff.<driver>.command)
relates to the rest of Git's diff features, so the contrast with the new
process driver is clear.
* Patch 2: xdiff plumbing for externally supplied hunks.
* Patch 3: diff.<driver>.process config key.
* Patch 4: refactor subprocess API to separate process lifecycle from
hashmap management, since the diff process stores its subprocess on the
userdiff driver rather than in a hashmap.
* Patch 5: the main feature, including the old-oid/new-oid request metadata
for blob-pair caching.
* Patch 6: bypass knobs (--no-ext-diff, format-patch).
* Patch 7: blame integration so the tool can declare commits as having no
changes; introduces the shared xdi_diff_process() consult-then-diff
helper that blame and git log -L both use.
* Patch 8: --stat/--numstat/--shortstat consult the tool, so the summary
agrees with the patch output.
* Patch 9: git log -L range tracking consults the tool, so a reformat-only
commit is dropped from the log rather than shown with an empty diff.
A "Which features consult the diff process" section in gitattributes(5) lays
out, per feature, why each does or does not consult the process (patch
output, blame, summary formats, and the -L line-range view do; pickaxe -G,
patch-id, merge, range-diff, --check, and --raw do not, with reasons).
Combined diffs (--cc) remain on the builtin algorithm and are noted as
future work.
Changes since v4:
* New preparatory doc patch (patch 1): document how an external diff driver
(diff.<driver>.command) relates to the rest of Git's diff features, so
the contrast with the process driver added later in the series is
explicit.
* New: --stat/--numstat/--shortstat now consult the process (patch 8). A
file the tool reports as equivalent contributes no stat line, matching
the empty patch git diff produces for it. Summary formats do not apply
textconv, so the tool is fed raw content there, as the builtin --stat
already counts raw lines.
* New: git log -L range tracking now consults the process (patch 9).
Previously the tracking pass used the builtin diff while the displayed
diff used the tool, so a reformat-only commit could be selected by
tracking and then rendered with an empty diff. Tracking now agrees with
the display and the commit is dropped.
* New: the request carries the old and new blob object names
(old-oid/new-oid), so a tool can cache its line matching keyed on the
blob pair. A side's oid is sent only when the tool receives that raw
blob; it is omitted under textconv (which rewrites the bytes) and for a
working-tree side with no stored object. This is where the process
differs from diff.<name>.command, which never composes with textconv and
so always feeds the raw blob its oid names.
* Refactor: blame and git log -L now consult the process through a single
xdi_diff_process() helper instead of open-coding the consult-then-diff
dance; builtin_diff() keeps its own path so it can short-circuit
equivalent files before its funcname/word-diff setup. The whitespace
bypass keys off the effective diff parameters (xpp), which removes
blame's separate -w guard, and blame's diff_hunks() sheds an intermediate
variant it no longer needs.
* Correctness: external-hunk validation now checks per-gap alignment, not
just the total unchanged-line count. xdl_build_script() walks the two
files in lockstep over unchanged lines, so a hunk set whose totals
balance but whose per-gap line counts do not (e.g. hunk 1 1 3 1)
previously passed validation and produced a corrupt diff, --stat, and
blame attribution. Such responses are now rejected and Git falls back to
the builtin diff.
* Robustness: hunk accumulation is bounded by the two files' combined line
count, so a misbehaving tool that floods hunk lines cannot grow memory
without bound before validation.
* Forward-compat: the hunk-line parser now ignores unknown trailing fields
after the four counts, so a future protocol version can append a field
without older clients rejecting it.
* Feature interactions: the whitespace-ignoring options (-w,
--ignore-space-change, --ignore-blank-lines, ...), -I, and --anchored
bypass the process (the tool is never told about them and could not honor
them), and git blame -w does the same. A change that only adds or removes
the trailing newline cannot be expressed as line hunks, so it also falls
back to the builtin diff (preserving the "\ No newline at end of file"
marker).
* Documentation: added the per-feature "Which features consult the diff
process" section; documented that the process trusts the tool's notion of
"unchanged" (it is not byte-validated, so like git diff -w such a patch
may not apply against the old blob), that --exit-code/--quiet report
success for tool- equivalent files, and that diff.<name>.command takes
precedence when both it and .process are configured.
* Tests: t4080 grew coverage for the per-gap check, the hunk- flood cap,
the whitespace/-w bypasses, the trailing-newline and added/deleted-file
fallbacks, multi-hunk output through patch and --stat, --stat
--exit-code, a mode-only change, and a mixed equivalent/changed
multi-file diffstat.
Michael Montalbo (9):
gitattributes: document how external diff drivers relate to diff
features
xdiff: support external hunks via xpparam_t
userdiff: add diff.<driver>.process config
sub-process: separate process lifecycle from hashmap management
diff: add long-running diff process via diff.<driver>.process
diff: bypass diff process with --no-ext-diff and in format-patch
blame: consult diff process for no-hunk detection
diff: consult diff process for --stat counts
line-log: consult diff process for range tracking
Documentation/config/diff.adoc | 5 +
Documentation/diff-algorithm-option.adoc | 3 +
Documentation/diff-options.adoc | 4 +-
Documentation/gitattributes.adoc | 274 +++++++
Makefile | 2 +
blame.c | 24 +-
builtin/log.c | 7 +
diff-process.c | 529 +++++++++++++
diff-process.h | 75 ++
diff.c | 57 +-
diff.h | 6 +
line-log.c | 33 +-
meson.build | 1 +
sub-process.c | 28 +-
sub-process.h | 9 +-
t/helper/meson.build | 1 +
t/helper/test-diff-process-backend.c | 381 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/meson.build | 1 +
t/t4080-diff-process.sh | 950 +++++++++++++++++++++++
userdiff.c | 7 +
userdiff.h | 5 +
xdiff-interface.c | 7 +-
xdiff/xdiff.h | 16 +
xdiff/xdiffi.c | 84 +-
xdiff/xprepare.c | 10 +
xdiff/xprepare.h | 1 +
28 files changed, 2504 insertions(+), 18 deletions(-)
create mode 100644 diff-process.c
create mode 100644 diff-process.h
create mode 100644 t/helper/test-diff-process-backend.c
create mode 100755 t/t4080-diff-process.sh
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2120%2Fmmontalbo%2Fmm%2Fstructural-diff-backend-clean-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2120/mmontalbo/mm/structural-diff-backend-clean-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/2120
Range-diff vs v4:
-: ---------- > 1: 0fd994a3d3 gitattributes: document how external diff drivers relate to diff features
1: 03f261dfe2 ! 2: 2004502549 xdiff: support external hunks via xpparam_t
@@ Commit message
the diff algorithm, then continues through compaction and emission
as usual.
- Validate supplied hunks before use: reject out-of-bounds line
- numbers, overlapping or out-of-order hunks, negative counts, and
- violations of the synchronization invariant (unchanged line counts
- must match between files). On validation failure, fall back to
- the builtin diff algorithm; this re-runs xdl_prepare_env() since
- the first call may have dirtied the changed[] arrays.
+ Validate supplied hunks before use. Out-of-bounds line numbers,
+ overlapping or out-of-order hunks, and misaligned unchanged runs are
+ treated as a malformed tool response: xdl_populate_hunks_from_external()
+ warns, returns -1, and xdl_diff() falls back to the builtin diff
+ algorithm for that file. The run of unchanged lines between two hunks
+ (and before the first and after the last) must be the same length on
+ both sides; xdl_build_script() walks the two files in lockstep over
+ unchanged lines, so a balanced total is not enough. Non-negative
+ counts and 1-based starts are instead caller preconditions, checked
+ with BUG(), since the caller normalizes hunks before this point.
+
+ On rejection xdl_diff() frees the environment it prepared and falls
+ through to xdl_do_diff(), which prepares a fresh one for the builtin
+ pass.
Skip trim_common_tail() in xdi_diff() when external hunks are
present, since external hunks reference line numbers in the
@@ xdiff/xdiff.h: typedef struct s_mmbuffer {
} mmbuffer_t;
+/*
-+ * Hunk descriptor for externally computed diffs.
-+ * Line numbers are 1-based; a start of 0 is accepted when
-+ * count is 0 (empty file side, matching git diff output).
++ * Hunk descriptor for externally computed diffs, in xdiff's own
++ * coordinates: line numbers are 1-based and a hunk's start is the
++ * first line it covers. A caller translates any external "empty side"
++ * idiom (such as git diff's start-0/count-0) to a 1-based start before
++ * handing hunks over.
+ */
+struct xdl_hunk {
+ long old_start, old_count;
@@ xdiff/xdiffi.c: static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdf
+/*
+ * Populate the changed[] arrays from externally supplied hunks,
-+ * bypassing the diff algorithm. Validates that hunks are in order,
-+ * non-overlapping, and within bounds.
++ * bypassing the diff algorithm. The caller normalizes and validates
++ * the hunks first (order, overlap, and lockstep alignment), so this
++ * only marks lines changed after asserting the memory-safety
++ * preconditions it depends on: non-negative counts and 1-based starts
++ * (checked with BUG()), and an in-bounds range (a silent -1 so the
++ * caller can fall back to the builtin diff rather than index changed[]
++ * out of range). Keeping this diagnostic-free leaves user-facing
++ * messages to the git layer.
+ *
-+ * Returns 0 on success, -1 on validation failure.
++ * Returns 0 on success, -1 if a hunk is out of range.
+ */
+static int xdl_populate_hunks_from_external(xdfenv_t *xe,
+ struct xdl_hunk *hunks,
+ size_t nr_hunks)
+{
+ size_t i;
-+ long j, prev_old_end = 0, prev_new_end = 0;
-+ long changed_old = 0, changed_new = 0;
++ long j;
+
+ /*
+ * xdl_prepare_env() may dirty changed[] via xdl_cleanup_records().
@@ xdiff/xdiffi.c: static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdf
+ for (i = 0; i < nr_hunks; i++) {
+ struct xdl_hunk *h = &hunks[i];
+
-+ if (h->old_count < 0 || h->new_count < 0) {
-+ warning("diff process hunk %"PRIuMAX": "
++ /*
++ * Non-negative counts and 1-based starts are caller
++ * preconditions (it normalizes hunks into xdiff coordinates
++ * before this point), so a violation is a bug, not a bad
++ * tool response.
++ */
++ if (h->old_count < 0 || h->new_count < 0)
++ BUG("external hunk %"PRIuMAX": "
+ "negative count (old=%ld, new=%ld)",
+ (uintmax_t)(i + 1),
+ h->old_count, h->new_count);
-+ return -1;
-+ }
-+ if (h->old_start < 1 || h->new_start < 1) {
-+ warning("diff process hunk %"PRIuMAX": "
-+ "start must be >= 1 (old=%ld, new=%ld)",
++ if (h->old_start < 1 || h->new_start < 1)
++ BUG("external hunk %"PRIuMAX": "
++ "start not 1-based (old=%ld, new=%ld)",
+ (uintmax_t)(i + 1),
+ h->old_start, h->new_start);
-+ return -1;
-+ }
+
+ /*
-+ * Range must fit: start + count - 1 <= nrec,
-+ * rewritten to avoid overflow. Same for both sides.
-+ *
-+ * When count is 0 (pure insert/delete) the check
-+ * reduces to 0 > nrec - start + 1, which rejects
-+ * start > nrec + 1 and allows start == nrec + 1
-+ * (the position after the last line).
++ * The caller validates ordering, overlap and lockstep
++ * alignment (and diagnoses a bad response). This is only a
++ * silent in-bounds guard so the marking loop cannot index
++ * changed[] out of range: start + count - 1 <= nrec,
++ * rewritten to avoid overflow. A count of 0 (pure
++ * insert/delete) allows start == nrec + 1, the position
++ * after the last line. On a miss, return -1 and let the
++ * caller fall back to the builtin diff.
+ */
-+ if (h->old_count > (long)xe->xdf1.nrec - h->old_start + 1) {
-+ warning("diff process hunk %"PRIuMAX": "
-+ "old range %ld+%ld exceeds %lu lines",
-+ (uintmax_t)(i + 1),
-+ h->old_start, h->old_count,
-+ (unsigned long)xe->xdf1.nrec);
-+ return -1;
-+ }
-+ if (h->new_count > (long)xe->xdf2.nrec - h->new_start + 1) {
-+ warning("diff process hunk %"PRIuMAX": "
-+ "new range %ld+%ld exceeds %lu lines",
-+ (uintmax_t)(i + 1),
-+ h->new_start, h->new_count,
-+ (unsigned long)xe->xdf2.nrec);
++ if (h->old_count > (long)xe->xdf1.nrec - h->old_start + 1 ||
++ h->new_count > (long)xe->xdf2.nrec - h->new_start + 1)
+ return -1;
-+ }
-+
-+ /* Ordering: no overlap with previous hunk (adjacent is OK) */
-+ if (h->old_start < prev_old_end ||
-+ h->new_start < prev_new_end) {
-+ warning("diff process hunk %"PRIuMAX": "
-+ "overlaps with previous hunk",
-+ (uintmax_t)(i + 1));
-+ return -1;
-+ }
+
+ for (j = 0; j < h->old_count; j++)
+ xe->xdf1.changed[h->old_start - 1 + j] = true;
+ for (j = 0; j < h->new_count; j++)
+ xe->xdf2.changed[h->new_start - 1 + j] = true;
-+
-+ prev_old_end = h->old_start + h->old_count;
-+ prev_new_end = h->new_start + h->new_count;
-+ }
-+
-+ /*
-+ * Synchronization invariant: unchanged line counts must match.
-+ * Otherwise xdl_build_script() would walk off one array.
-+ *
-+ * Count changed lines from the arrays rather than accumulating
-+ * during the loop to avoid any overflow in the summation.
-+ */
-+ for (j = 0; j < (long)xe->xdf1.nrec; j++)
-+ if (xe->xdf1.changed[j])
-+ changed_old++;
-+ for (j = 0; j < (long)xe->xdf2.nrec; j++)
-+ if (xe->xdf2.changed[j])
-+ changed_new++;
-+ if ((long)xe->xdf1.nrec - changed_old !=
-+ (long)xe->xdf2.nrec - changed_new) {
-+ warning("diff process: unchanged line count mismatch "
-+ "(old: %ld unchanged, new: %ld unchanged)",
-+ (long)xe->xdf1.nrec - changed_old,
-+ (long)xe->xdf2.nrec - changed_new);
-+ return -1;
+ }
+
+ return 0;
2: 30617ee17b = 3: 926cf01af6 userdiff: add diff.<driver>.process config
3: 459e485e6d = 4: 363d459ff6 sub-process: separate process lifecycle from hashmap management
4: 10b3980f59 ! 5: d003bc1f15 diff: add long-running diff process via diff.<driver>.process
@@ Commit message
textconv-transformed content. The tool controls which lines
are marked as changed while the display shows the file content.
Patch output features (word diff, function context, color) work
- normally; --stat uses its own diff codepath and never consults
- the diff process.
+ normally. A new "Which features consult the diff process"
+ documentation section lays out which features use the tool's hunks,
+ which compute independently, and why; the summary formats such as
+ --stat still use the builtin diff for now.
The handshake negotiates version=1 and capability=hunks. Per-file
- requests send command=hunks, pathname, and both file contents as
- packetized data. The tool responds with hunk lines and a status
- packet (success, error, or abort). On error, Git warns and falls
- back to the builtin diff algorithm for that file. On abort, Git
- silently falls back for the current file and stops sending further
- requests to the tool for the remainder of the session.
+ requests send command=hunks, pathname, the old and new blob object
+ names as old-oid/new-oid, and both file contents as packetized data.
+ The tool responds with hunk lines and a status packet (success,
+ error, or abort). On error, Git warns and falls back to the builtin
+ diff algorithm for that file. On abort, Git silently falls back for
+ the current file and stops sending further requests to the tool for
+ the remainder of the session.
+
+ old-oid/new-oid name the two blobs so a tool can cache its analysis
+ keyed on the pair. A side's oid is sent only when the content the
+ tool receives is that raw blob: it is omitted under textconv, which
+ rewrites the bytes, and for a working-tree side with no stored
+ object, so an oid that is sent always names the bytes the tool
+ receives. This is where the process protocol diverges from
+ diff.<driver>.command, which never composes with textconv (the
+ command replaces the whole diff and always gets the raw blob). Tools
+ ignore unknown request keys, so old tools skip them.
When the tool returns no hunks followed by status=success, Git
treats the file as having no changes and produces no diff output.
@@ Commit message
the failure is cached on the driver to avoid retrying and
re-warning on every subsequent file.
+ Git falls back to the builtin diff (rather than consulting the
+ tool) when an option the tool cannot honor is in effect: the
+ whitespace-ignoring flags, --ignore-blank-lines, -I<regex>, and
+ --anchored. The bypass keys off the effective diff parameters (xpp)
+ rather than diffopt, so a later caller whose flags live elsewhere is
+ covered uniformly. A change that only adds or removes the trailing
+ newline is likewise not expressible as hunks, so it too uses the
+ builtin diff. The hunk parser ignores unknown trailing fields on a
+ hunk line for response forward-compatibility.
+
+ Hunk accumulation is bounded by the combined byte count of the two
+ files, so a misbehaving tool that floods hunk lines cannot grow
+ memory without bound before validation runs.
+
diff_process_fill_hunks() is the sole public entry point. It
handles driver lookup, flag checks, subprocess management, and
error reporting, returning an enum that lets callers distinguish
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+long-running process protocol (described in
+Documentation/technical/long-running-process-protocol.adoc).
+Unlike `diff.<name>.command`, which replaces Git's output entirely,
-+the diff process feeds results back into the standard pipeline.
++the diff process feeds results back into the standard pipeline. If
++both are configured for a path, `diff.<name>.command` takes precedence
++for the patch output it replaces; the summary formats, `git blame`,
++and `git log -L` never run the command and still consult the process.
+
+First, in `.gitattributes`, assign the `diff` attribute for paths.
+
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+the process and performs the handshake. In the handshake, the welcome
+message sent by Git is "git-diff-client", only version 1 is supported,
+and the supported capability is "hunks" (the changed regions
-+described below).
++described below). The tool replies with "git-diff-server", the
++version it supports, and the capabilities it supports.
+
+For each file, Git sends a list of "key=value" pairs terminated with
+a flush packet, followed by the old and new file content as packetized
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+-----------------------
+packet: git> command=hunks
+packet: git> pathname=path/file.c
++packet: git> old-oid=<hex>
++packet: git> new-oid=<hex>
+packet: git> 0000
+packet: git> OLD_CONTENT
+packet: git> 0000
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+packet: git> 0000
+-----------------------
+
++The optional `old-oid` and `new-oid` keys give the object names of the
++old and new blobs, so a tool can cache its analysis keyed on the pair.
++A side's key is sent only when the content for that side is the raw
++blob it names: it is omitted when the content is textconv-transformed,
++and for a working-tree side that has no stored object. A tool that
++does not recognize these keys ignores them.
++
+The tool is expected to respond with zero or more hunk lines,
+a flush packet, and a status packet terminated with a flush packet.
+Each hunk line has the form:
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+
+where `<old_start>` and `<old_count>` identify a range of lines in
+the old file, and `<new_start>` and `<new_count>` identify the
-+replacement range in the new file. Start values are 1-based and
-+counts are non-negative. Ranges must not extend beyond the end of
-+the file. For example, `hunk 3 2 3 4` means that 2 lines starting
-+at line 3 in the old file were replaced by 4 lines starting at
-+line 3 in the new file. An `<old_count>` of 0 means no lines were
-+removed (pure insertion); a `<new_count>` of 0 means no lines were
-+added (pure deletion). A start value of 0 is accepted when
-+the corresponding count is 0 (e.g., `hunk 0 0 1 5` for a newly
-+added file), matching what `git diff` itself emits for empty
-+file sides.
++replacement range in the new file. The four fields are separated by
++single spaces. Start values are 1-based and counts are non-negative.
++For example, `hunk 3 2 3 4` means that 2 lines starting at line 3 in
++the old file were replaced by 4 lines starting at line 3 in the new
++file. An `<old_count>` of 0 means no lines were removed (pure
++insertion); a `<new_count>` of 0 means no lines were added (pure
++deletion). For a side with a count of 0 (a pure insertion or
++deletion) the start is the 1-based line the change sits before,
++ranging from 1 to one past the last line (the line count plus 1, to
++place the change at the end of the file); like every start it must
++keep the unchanged runs aligned on both sides (see below), so for a
++given change it takes one specific value, not an arbitrary one. A
++start of 0 is also accepted and treated as 1, matching the
++empty-file-side form `git diff` emits (e.g. `hunk 0 0 1 5` for a newly
++added file). A nonzero range must not extend beyond the end of the
++file. Git ignores any extra
++whitespace-separated tokens after `<new_count>`, so a future protocol
++version can append fields to a hunk line (for example a "moved"
++marker) without older tools rejecting it.
+
+Lines are delimited by newlines. A file `"foo\nbar\n"` and a
+file `"foo\nbar"` both have 2 lines.
+
-+Hunks must be listed in order and must not overlap. Any line
-+not covered by a hunk is treated as unchanged, so the total
-+number of unchanged lines must be the same on both sides.
-+For example, if the old file has 10 lines and the hunks cover
-+4 of them (`old_count` values summing to 4), then 6 old lines
-+are unchanged. The new file must also have exactly 6 lines
-+not covered by hunks, so the `new_count` values must sum to
-+`new_file_lines - 6`.
++Hunks must be listed in order and must not overlap. Any line not
++covered by a hunk is treated as unchanged and is paired, in order,
++with the unchanged lines on the other side. Each run of unchanged
++lines between two hunks (and the run before the first hunk and
++after the last) must therefore be the same length on both sides,
++not merely equal in total. For the hunks `1 3 1 5` and `10 2 12 2`
++below, lines 4-9 of the old file and lines 6-11 of the new file are
++both the six unchanged lines between the two hunks. A response that
++balances only the total unchanged count but misaligns one of these
++runs is rejected, and Git falls back to the builtin diff.
++
++Git does not check that the lines a hunk leaves unchanged are
++byte-for-byte identical between the two sides; it pairs them by
++position and shows the new side as context. A tool may therefore
++report lines that differ textually (a pure reformatting, say) as
++unchanged, and the diff reflects that judgment. This is
++the point of a semantic backend, but it means a misbehaving tool can
++produce a diff whose context does not match the old blob; as with
++`git diff -w`, such a patch may not apply against the old content.
+
+-----------------------
+packet: git< hunk 1 3 1 5
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+-----------------------
+
+If the tool responds with hunks and "success", Git marks those lines
-+as changed and feeds them into the standard diff pipeline. Patch
-+output features (word diff, function context, color) work normally.
-+Note that `--stat` and other summary formats use their own diff path
-+and are not affected by the diff process.
++as changed and feeds them into the standard diff pipeline. Git may
++still slide or regroup those changes against matching context for
++display, exactly as it compacts its own diffs, so the tool controls
++which lines are reported as changed, not the precise hunk boundaries.
++Patch output features (word diff, function context, color) work
++normally. Summary formats such as `--stat` still compute their counts
++with the builtin diff for now; see "Which features consult the diff
++process" below for the full picture and the reasoning behind it.
+
+If no hunk lines precede the flush, followed by "success", Git
-+treats the files as having no changes: `git diff` produces no output
-+and `git blame` skips the commit, attributing lines to earlier commits.
++treats the files as having no changes: `git diff` produces no output,
++`git diff --exit-code` and `--quiet` report success even though the
++stored blobs differ, and `git blame` skips the commit, attributing
++lines to earlier commits.
++The one exception is a change that only adds or removes the file's
++trailing newline: it cannot be expressed as line hunks, so when the
++line content otherwise matches Git keeps the builtin diff for that
++file (preserving the `\ No newline at end of file` marker) instead of
++treating the two sides as equal.
+
+-----------------------
+packet: git< 0000
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+packet: git< 0000
+-----------------------
+
-+If the tool returns invalid hunks (out of bounds, overlapping, or
-+mismatched unchanged line counts), Git warns and falls back to the
-+builtin diff algorithm.
++If the tool returns well-formed but invalid hunks (out of bounds,
++overlapping, or with misaligned unchanged runs), Git warns and falls
++back to the builtin diff for that file; the tool stays available for
++subsequent files. A malformed hunk line, by contrast (bad syntax, a
++nonzero count paired with a start of 0, or more hunks than the file
++has lines), is a protocol violation: Git stops the process and does
++not send it further requests, as described below.
+
+In case the tool cannot or does not want to process the content,
+it is expected to respond with an "error" status. Git warns and
-+falls back to the builtin diff algorithm for this file. The tool
++falls back to the builtin diff algorithm for this file, treating any
++status other than "success" or "abort" the same way. The tool
+remains available for subsequent files.
+
+-----------------------
@@ Documentation/gitattributes.adoc: NOTE: If `diff.<name>.command` is defined for
+forward-compatible. Future versions of Git may send additional
+`command=` values; tools that receive an unrecognized command should
+respond with `status=error` rather than terminating.
++
++Which features consult the diff process
++^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
++
++The diff process answers a single question: given two blobs, which
++line ranges differ? Whether a particular feature consults it follows
++from whether that is the question the feature is really asking.
++
++Features that ask "which lines changed" use the tool's hunks in place
++of the builtin algorithm:
++
++- `git diff` patch output, together with everything layered on it:
++ word diff, function context (`-W`), `--color-moved`, the `@@` hunk
++ headers, and the `-L` line-range display. These operate on the
++ lines the patch step already emitted, so they reflect the tool's
++ hunks without any further negotiation.
++- `git blame`: a commit whose change the tool reports as equivalent is
++ skipped, and its lines are attributed to an earlier commit.
++
++Features that ask a different question do not consult the process, by
++design:
++
++- The pickaxe `-G<regex>` searches the textual diff for a pattern; it
++ asks "does this string appear in the diff," not "did these lines
++ change." (`-S` runs at an earlier stage and is likewise unaffected.)
++- `git patch-id` must produce a stable hash for `git rebase` and
++ cherry-pick detection; deriving it from a configured tool would make
++ equal patches hash differently from machine to machine.
++- The merge machinery (`git merge-tree`, `rerere`) computes merge
++ content and conflict signatures rather than display output, so the
++ tool's hunks must not alter its results.
++- `git range-diff` diffs patch text, not source blobs, so source-file
++ hunks do not apply to it.
++- `--check` reports whitespace errors in added lines using the builtin
++ diff's notion of which lines are added, not the tool's. It can
++ therefore flag (and exit non-zero on) a line the tool treats as
++ unchanged and that `git diff` shows as context. Whitespace breakage
++ is a property of the literal bytes, so `--check` keeps the builtin
++ partition deliberately; a future change could wire it to the tool if
++ matching `git diff` exactly became desirable.
++- `--raw`, `--name-only`, and `--name-status` compare object ids at
++ the tree level and never run a line-level diff at all.
++
++Some features ask "which lines changed" but still use the builtin
++algorithm for now, and may consult the process in a later change: the
++summary formats (`--stat`, `--numstat`, `--shortstat`); `git log -L`'s
++commit selection and parent range propagation (as distinct from its
++display, which is covered above); and combined diffs (`--cc` and merge
++diffs), whose protocol would have to be extended from a single old/new
++pair to one comparison per merge parent.
++
++`--diff-algorithm` bypasses the process entirely, for every feature
++listed above. The whitespace-ignoring options (`-w`,
++`--ignore-space-change`, `--ignore-blank-lines`, and the like),
++`-I<regex>`, and `--anchored` also bypass it for the affected files:
++the tool is never told about these options, so it could not honor
++them, and Git falls back to the builtin diff, which does.
+
Defining a custom hunk-header
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ diff-process.c (new)
+ * tool< capability=hunks / flush
+ *
+ * Per-file:
-+ * git> command=hunks / pathname=<path> / flush
++ * git> command=hunks / pathname=<path> / [old-oid=<hex>] / [new-oid=<hex>] / flush
+ * git> <old content packetized> / flush
+ * git> <new content packetized> / flush
+ * tool< hunk <old_start> <old_count> <new_start> <new_count>
@@ diff-process.c (new)
+#include "diff-process.h"
+#include "diff.h"
+#include "gettext.h"
++#include "hex.h"
+#include "repository.h"
+#include "sigchain.h"
+#include "userdiff.h"
@@ diff-process.c (new)
+ return packet_flush_gently(fd);
+}
+
-+static int parse_hunk_line(const char *line, struct xdl_hunk *hunk)
++/*
++ * A hunk in the diff process's presentation coordinates: the line
++ * numbering it reports over the protocol. Kept distinct from struct
++ * xdl_hunk (xdiff's coordinates) so that only translated hunks ever
++ * reach the diff algorithm; diff_process_hunk_to_xdl() is the single
++ * crossing point.
++ */
++struct diff_process_hunk {
++ long old_start, old_count;
++ long new_start, new_count;
++};
++
++/*
++ * Parse one non-negative decimal field of a hunk line into *out and
++ * advance *line past it. Fields must be plain decimal with no leading
++ * whitespace or sign (isdigit() takes an unsigned char to stay defined
++ * for high-bit bytes). The first three fields are followed by a single
++ * space; the last (is_last) is followed by end-of-string or a space.
++ * Trailing space-separated tokens after the last field are allowed and
++ * ignored, so a future protocol version can append fields (e.g. a
++ * "moved" marker) without older tools rejecting the line -- mirroring
++ * the request-side rule that tools ignore unknown keys.
++ */
++static int parse_hunk_field(const char **line, long *out, int is_last)
+{
++ const char *p = *line;
+ char *end;
+
-+ /*
-+ * Format: "hunk <old_start> <old_count> <new_start> <new_count>"
-+ * All numbers must be non-negative decimal with no leading
-+ * whitespace or sign characters.
-+ */
-+ if (!skip_prefix(line, "hunk ", &line))
-+ return -1;
-+
-+ if (!isdigit(*line))
++ if (!isdigit((unsigned char)*p))
+ return -1;
+ errno = 0;
-+ hunk->old_start = strtol(line, &end, 10);
-+ if (errno || end == line || *end++ != ' ')
++ *out = strtol(p, &end, 10);
++ if (errno || end == p)
+ return -1;
-+ line = end;
++ if (is_last) {
++ if (*end != '\0' && *end != ' ')
++ return -1;
++ } else {
++ if (*end != ' ')
++ return -1;
++ end++;
++ }
++ *line = end;
++ return 0;
++}
+
-+ if (!isdigit(*line))
++static int parse_hunk_line(const char *line,
++ struct diff_process_hunk *presented)
++{
++ /* Format: "hunk <old_start> <old_count> <new_start> <new_count>" */
++ if (!skip_prefix(line, "hunk ", &line))
+ return -1;
-+ errno = 0;
-+ hunk->old_count = strtol(line, &end, 10);
-+ if (errno || end == line || *end++ != ' ')
++ if (parse_hunk_field(&line, &presented->old_start, 0) ||
++ parse_hunk_field(&line, &presented->old_count, 0) ||
++ parse_hunk_field(&line, &presented->new_start, 0) ||
++ parse_hunk_field(&line, &presented->new_count, 1))
+ return -1;
-+ line = end;
++ return 0;
++}
+
-+ if (!isdigit(*line))
-+ return -1;
-+ errno = 0;
-+ hunk->new_start = strtol(line, &end, 10);
-+ if (errno || end == line || *end++ != ' ')
-+ return -1;
-+ line = end;
++/*
++ * Translate a hunk from the diff process's presentation coordinates
++ * into xdiff's.
++ *
++ * Protocol starts are already 1-based positions (the line a change
++ * sits before), the same numbering xdiff uses, so the only adjustment
++ * is for an empty file side: "git diff" addresses it with a start of 0
++ * and a count of 0 (e.g. "0 0 1 5" adds five lines to an empty old
++ * side), and since xdiff uses start-1 as an array index that 0 becomes
++ * 1 here. This is NOT the full inverse of xdl_emit_hunk_hdr()
++ * (xdiff/xutils.c): that emitter shifts a count-0 range to start-1 for
++ * the displayed "@@" header, but the protocol keeps the unshifted
++ * 1-based position for a mid-file insert or delete. This is the single
++ * point where presentation coordinates become xdiff coordinates, so
++ * xdl_populate_hunks_from_external() may assume 1-based starts.
++ *
++ * Returns -1 for a start of 0 paired with a nonzero count, which names
++ * no line in either coordinate system. (parse_hunk_line() already
++ * guarantees non-negative starts and counts.)
++ */
++static int diff_process_hunk_to_xdl(const struct diff_process_hunk *presented,
++ struct xdl_hunk *xdl)
++{
++ long old_start = presented->old_start;
++ long new_start = presented->new_start;
+
-+ if (!isdigit(*line))
++ if ((!old_start && presented->old_count) ||
++ (!new_start && presented->new_count))
+ return -1;
-+ errno = 0;
-+ hunk->new_count = strtol(line, &end, 10);
-+ if (errno || end == line || *end != '\0')
-+ return -1;
-+
-+ /*
-+ * git diff emits start=0 when count=0 (empty file side).
-+ * Normalize to 1-based so downstream validation can assume start >= 1.
-+ */
-+ if (!hunk->old_count && !hunk->old_start)
-+ hunk->old_start = 1;
-+ if (!hunk->new_count && !hunk->new_start)
-+ hunk->new_start = 1;
-+
++ if (!old_start)
++ old_start = 1;
++ if (!new_start)
++ new_start = 1;
++
++ xdl->old_start = old_start;
++ xdl->old_count = presented->old_count;
++ xdl->new_start = new_start;
++ xdl->new_count = presented->new_count;
+ return 0;
+}
+
@@ diff-process.c (new)
+ const char *path,
+ const char *old_buf, long old_size,
+ const char *new_buf, long new_size,
++ const struct object_id *oid_a,
++ const struct object_id *oid_b,
+ struct xdl_hunk **hunks_out,
+ size_t *nr_hunks_out)
+{
@@ diff-process.c (new)
+ int fd_in, fd_out;
+ struct strbuf status = STRBUF_INIT;
+ struct xdl_hunk *hunks = NULL;
++ struct diff_process_hunk presented;
+ struct xdl_hunk hunk;
+ size_t nr_hunks = 0, alloc_hunks = 0;
++ size_t max_hunks;
+ int len;
+ char *line;
+
@@ diff-process.c (new)
+
+ /* Send request */
+ if (packet_write_fmt_gently(fd_in, "command=hunks\n") ||
-+ packet_write_fmt_gently(fd_in, "pathname=%s\n", path) ||
-+ packet_flush_gently(fd_in))
++ packet_write_fmt_gently(fd_in, "pathname=%s\n", path))
++ goto comm_error;
++ /*
++ * old-oid/new-oid let the tool key a cache on the blob pair. A
++ * side is sent only when its content is the raw blob (the caller
++ * passes NULL otherwise, e.g. for textconv'd content), so an oid
++ * that is present always names the bytes the tool receives.
++ */
++ if (oid_a &&
++ packet_write_fmt_gently(fd_in, "old-oid=%s\n", oid_to_hex(oid_a)))
++ goto comm_error;
++ if (oid_b &&
++ packet_write_fmt_gently(fd_in, "new-oid=%s\n", oid_to_hex(oid_b)))
++ goto comm_error;
++ if (packet_flush_gently(fd_in))
+ goto comm_error;
+
+ /* Send old file content */
@@ diff-process.c (new)
+ if (send_file_content(fd_in, new_buf, new_size))
+ goto comm_error;
+
++ /*
++ * Hunks are non-overlapping and each useful hunk covers at least
++ * one line, so a valid response cannot contain more hunks than the
++ * two files have lines, which is bounded by their byte sizes. Cap
++ * the accumulation accordingly so a misbehaving tool that floods
++ * hunk lines cannot drive unbounded memory growth before validation.
++ */
++ max_hunks = (size_t)old_size + (size_t)new_size + 1;
++
+ /* Read hunks until flush packet */
+ while ((len = packet_read_line_gently(fd_out, NULL, &line)) >= 0 &&
+ line) {
-+ if (parse_hunk_line(line, &hunk) < 0)
++ if (parse_hunk_line(line, &presented) < 0)
++ goto comm_error;
++ if (diff_process_hunk_to_xdl(&presented, &hunk) < 0)
++ goto comm_error;
++ if (nr_hunks >= max_hunks) {
++ warning(_("diff process '%s' sent too many hunks"
++ " for '%s'"), drv->process, path);
+ goto comm_error;
++ }
+ ALLOC_GROW(hunks, nr_hunks + 1, alloc_hunks);
+ hunks[nr_hunks++] = hunk;
+ }
@@ diff-process.c (new)
+ return DIFF_PROCESS_ERROR;
+}
+
++/*
++ * Whether exactly one of the two blobs ends in a newline. A change
++ * that only adds or removes the trailing newline is not expressible as
++ * line hunks, so a tool comparing lines reports the files as equal.
++ */
++static int eof_newline_differs(const mmfile_t *a, const mmfile_t *b)
++{
++ int a_nl = a->size > 0 && a->ptr[a->size - 1] == '\n';
++ int b_nl = b->size > 0 && b->ptr[b->size - 1] == '\n';
++ return a_nl != b_nl;
++}
++
++/*
++ * Number of lines in a blob, matching xdiff's record count: one per
++ * newline, plus one more if the last line has no trailing newline.
++ */
++static long count_lines(const char *buf, long size)
++{
++ long lines = 0, i;
++
++ for (i = 0; i < size; i++)
++ if (buf[i] == '\n')
++ lines++;
++ if (size > 0 && buf[size - 1] != '\n')
++ lines++;
++ return lines;
++}
++
++/*
++ * Validate the tool's hunks (already in xdiff coordinates) against the
++ * two blobs before they bypass the diff algorithm. Each hunk must fit
++ * within its file, the hunks must be ordered and non-overlapping, and
++ * the unchanged run before each hunk (and after the last) must be the
++ * same length on both sides -- xdl_build_script() walks the two files
++ * in lockstep over unchanged lines, so a mismatched gap desynchronizes
++ * it and yields a corrupt diff even when the totals balance. This is
++ * the git layer's job so xdiff stays diagnostic-free; on a bad response
++ * we warn and the caller falls back to the builtin diff. Returns 0 if
++ * valid, -1 (after warning) otherwise.
++ */
++static int validate_external_hunks(const struct xdl_hunk *hunks, size_t nr,
++ long old_lines, long new_lines,
++ const char *process, const char *path)
++{
++ size_t i;
++ long prev_old_end = 0, prev_new_end = 0;
++
++ for (i = 0; i < nr; i++) {
++ const struct xdl_hunk *h = &hunks[i];
++
++ if (h->old_count > old_lines - h->old_start + 1 ||
++ h->new_count > new_lines - h->new_start + 1) {
++ warning(_("diff process '%s' returned a hunk past the "
++ "end of '%s'; using the builtin diff"),
++ process, path);
++ return -1;
++ }
++ if (h->old_start < prev_old_end || h->new_start < prev_new_end) {
++ warning(_("diff process '%s' returned overlapping hunks "
++ "for '%s'; using the builtin diff"),
++ process, path);
++ return -1;
++ }
++ if (h->old_start - prev_old_end != h->new_start - prev_new_end) {
++ warning(_("diff process '%s' returned hunks that leave "
++ "'%s' misaligned; using the builtin diff"),
++ process, path);
++ return -1;
++ }
++ prev_old_end = h->old_start + h->old_count;
++ prev_new_end = h->new_start + h->new_count;
++ }
++ if (old_lines - prev_old_end != new_lines - prev_new_end) {
++ warning(_("diff process '%s' returned hunks that leave '%s' "
++ "misaligned; using the builtin diff"),
++ process, path);
++ return -1;
++ }
++ return 0;
++}
++
+enum diff_process_result diff_process_fill_hunks(
+ struct diff_options *diffopt,
+ const char *path,
+ const mmfile_t *file_a,
+ const mmfile_t *file_b,
++ const struct object_id *oid_a,
++ const struct object_id *oid_b,
+ xpparam_t *xpp)
+{
+ struct userdiff_driver *drv;
@@ diff-process.c (new)
+ return DIFF_PROCESS_SKIP;
+ if (diffopt->flags.no_diff_process || diffopt->ignore_driver_algorithm)
+ return DIFF_PROCESS_SKIP;
++ /*
++ * Whitespace-ignoring, regex-ignore (-I) and anchored options
++ * change which lines count as different, but the tool is never
++ * told about them, so its hunks could not honor them. Rather
++ * than silently override the user's request, fall back to the
++ * builtin diff, which does honor these flags. Key this off xpp
++ * (the parameters this diff actually runs with) rather than
++ * diffopt, so a caller like blame that keeps its flags outside
++ * diffopt is covered without a separate guard of its own.
++ */
++ if ((xpp->flags & (XDF_WHITESPACE_FLAGS | XDF_IGNORE_BLANK_LINES)) ||
++ xpp->ignore_regex_nr || xpp->anchors_nr)
++ return DIFF_PROCESS_SKIP;
+
+ drv = userdiff_find_by_path(diffopt->repo->index, path);
+ if (!drv || !drv->process)
@@ diff-process.c (new)
+ res = get_hunks(drv, path,
+ file_a->ptr, file_a->size,
+ file_b->ptr, file_b->size,
++ oid_a, oid_b,
+ &ext_hunks, &nr);
+ if (res == DIFF_PROCESS_OK) {
+ if (!nr) {
+ free(ext_hunks);
++ /*
++ * Zero hunks means the tool considers the line
++ * content identical, but it cannot express a
++ * trailing-newline-only change. When that is the
++ * actual difference, fall back to the builtin diff
++ * so the "\ No newline at end of file" marker is
++ * preserved instead of reporting the files equal.
++ */
++ if (eof_newline_differs(file_a, file_b))
++ return DIFF_PROCESS_SKIP;
+ return DIFF_PROCESS_EQUIVALENT;
+ }
++ if (validate_external_hunks(ext_hunks, nr,
++ count_lines(file_a->ptr, file_a->size),
++ count_lines(file_b->ptr, file_b->size),
++ drv->process, path) < 0) {
++ free(ext_hunks);
++ return DIFF_PROCESS_SKIP;
++ }
+ xpp->external_hunks = ext_hunks;
+ xpp->external_hunks_nr = nr;
+ return DIFF_PROCESS_OK;
@@ diff-process.h (new)
+#include "xdiff/xdiff.h"
+
+struct diff_options;
++struct object_id;
+
+enum diff_process_result {
-+ DIFF_PROCESS_ERROR = -1, /* tool failure: warned, fell back */
++ DIFF_PROCESS_ERROR = -1, /* failed; caller falls back to builtin */
+ DIFF_PROCESS_OK = 0, /* hunks populated in xpp */
-+ DIFF_PROCESS_SKIP, /* no process configured: use builtin */
++ DIFF_PROCESS_SKIP, /* process did not apply: use builtin */
+ DIFF_PROCESS_EQUIVALENT, /* tool says files are equivalent */
+};
+
@@ diff-process.h (new)
+ * Returns DIFF_PROCESS_OK when hunks are populated in xpp.
+ * The caller owns xpp->external_hunks and must free() it.
+ *
-+ * Returns DIFF_PROCESS_EQUIVALENT when the tool returns no hunks
-+ * (files are considered identical); caller should skip diff/blame.
++ * Returns DIFF_PROCESS_EQUIVALENT when the tool returns no hunks and
++ * the blobs are not a trailing-newline-only change (files are
++ * considered identical); caller should skip diff/blame.
+ * Returns DIFF_PROCESS_SKIP when no process applies; caller
+ * should use the builtin diff algorithm.
+ * Returns DIFF_PROCESS_ERROR on tool failure (already warned);
+ * caller should fall back to the builtin diff algorithm.
++ *
++ * oid_a/oid_b, when non-NULL, are sent to the tool as old-oid/new-oid
++ * so it can key a cache on the blob pair. Pass NULL for a side whose
++ * content is not the raw blob (e.g. textconv'd) or whose object name is
++ * unknown, so any oid that is sent always names the bytes the tool
++ * receives.
+ */
+enum diff_process_result diff_process_fill_hunks(
+ struct diff_options *diffopt,
+ const char *path,
+ const mmfile_t *file_a,
+ const mmfile_t *file_b,
++ const struct object_id *oid_a,
++ const struct object_id *oid_b,
+ xpparam_t *xpp);
+
+#endif /* DIFF_PROCESS_H */
@@ diff.c: static void builtin_diff(const char *name_a,
xpp.anchors = o->anchors;
xpp.anchors_nr = o->anchors_nr;
+
-+ if (diff_process_fill_hunks(o, name_a,
-+ &mf1, &mf2, &xpp)
++ /*
++ * Send the blob oids only for a side whose content is the
++ * raw blob: textconv rewrites the bytes, and a working-tree
++ * side has no stored oid, so pass NULL there rather than an
++ * oid that would not name what the tool receives.
++ */
++ if (diff_process_fill_hunks(o, name_a, &mf1, &mf2,
++ (textconv_one || !one->oid_valid) ? NULL : &one->oid,
++ (textconv_two || !two->oid_valid) ? NULL : &two->oid,
++ &xpp)
+ == DIFF_PROCESS_EQUIVALENT) {
+ if (textconv_one)
+ free(mf1.ptr);
@@ t/helper/test-diff-process-backend.c (new)
+ *
+ * packet: git> command=hunks
+ * packet: git> pathname=<path>
++ * packet: git> [old-oid=<hex>] (omitted for textconv/worktree)
++ * packet: git> [new-oid=<hex>]
+ * packet: git> 0000
+ * packet: git> OLD_CONTENT
+ * packet: git> 0000
@@ t/helper/test-diff-process-backend.c (new)
+ *
+ * Response varies by --mode (default: whole-file):
+ *
-+ * whole-file packet: git< hunk 1 <old_lines> 1 <new_lines>
++ * whole-file packet: git< hunk <1|0> <old_lines> <1|0> <new_lines>
++ * (start is 0 for an empty side, matching git diff)
+ * fixed-hunk packet: git< hunk 5 2 5 2
+ * no-hunks (no hunk packets)
+ * bad-hunk packet: git< hunk 999 1 999 1
+ * bad-parse packet: git< garbage not a hunk
+ * bad-sync packet: git< hunk 1 2 1 1
++ * bad-gap packet: git< hunk 1 1 3 1
++ * bad-start packet: git< hunk 0 1 1 1
++ * multi-hunk packet: git< hunk 5 2 5 2
++ * packet: git< hunk 9 2 9 2
++ * insert packet: git< hunk 3 0 3 2 (mid-file count-0 insertion)
++ * flood packet: git< hunk 1 1 1 1 (x100000)
+ * overlap packet: git< hunk 1 5 1 5
+ * packet: git< hunk 3 2 3 2
+ * no-cap (omits capability=hunks during handshake)
@@ t/helper/test-diff-process-backend.c (new)
+ * abort (status=abort instead of status=success)
+ * crash exit(1) before sending any response
+ *
-+ * All non-error/abort modes end with:
++ * All success modes (not error/abort/crash) end with:
+ *
+ * packet: git< 0000
+ * packet: git< status=success
@@ t/helper/test-diff-process-backend.c (new)
+ *
+ * Each request is logged to --log as:
+ *
-+ * command=<cmd> pathname=<path> old=<first line> new=<first line>
++ * command=<cmd> pathname=<path> old-oid=<hex> new-oid=<hex> old=<first line> new=<first line>
+ */
+
+#include "test-tool.h"
@@ t/helper/test-diff-process-backend.c (new)
+ MODE_BAD_HUNK,
+ MODE_BAD_PARSE,
+ MODE_BAD_SYNC,
++ MODE_BAD_GAP,
++ MODE_BAD_START,
++ MODE_MULTI_HUNK,
++ MODE_INSERT,
++ MODE_FLOOD,
+ MODE_OVERLAP,
+ MODE_NO_CAP,
+ MODE_ERROR,
@@ t/helper/test-diff-process-backend.c (new)
+ return MODE_BAD_PARSE;
+ if (!strcmp(s, "bad-sync"))
+ return MODE_BAD_SYNC;
++ if (!strcmp(s, "bad-gap"))
++ return MODE_BAD_GAP;
++ if (!strcmp(s, "bad-start"))
++ return MODE_BAD_START;
++ if (!strcmp(s, "multi-hunk"))
++ return MODE_MULTI_HUNK;
++ if (!strcmp(s, "insert"))
++ return MODE_INSERT;
++ if (!strcmp(s, "flood"))
++ return MODE_FLOOD;
+ if (!strcmp(s, "overlap"))
+ return MODE_OVERLAP;
+ if (!strcmp(s, "no-cap"))
@@ t/helper/test-diff-process-backend.c (new)
+ * variant: once inside a request, truncation is a protocol violation
+ * and dying loudly is the correct response.
+ */
-+static int read_request_header(char **command, char **pathname)
++static int read_request_header(char **command, char **pathname,
++ char **old_oid, char **new_oid)
+{
+ int first = 1;
+ char *line;
+
-+ *command = *pathname = NULL;
++ *command = *pathname = *old_oid = *new_oid = NULL;
+ for (;;) {
+ const char *value;
+
@@ t/helper/test-diff-process-backend.c (new)
+ *command = xstrdup(value);
+ else if (skip_prefix(line, "pathname=", &value))
+ *pathname = xstrdup(value);
++ else if (skip_prefix(line, "old-oid=", &value))
++ *old_oid = xstrdup(value);
++ else if (skip_prefix(line, "new-oid=", &value))
++ *new_oid = xstrdup(value);
+ }
+ return 1;
+}
@@ t/helper/test-diff-process-backend.c (new)
+ case MODE_BAD_SYNC:
+ packet_write_fmt(1, "hunk 1 2 1 1\n");
+ break;
++ case MODE_BAD_GAP:
++ /*
++ * Globally balanced (1 changed line on each side, so the
++ * total unchanged counts match) but the gap before the
++ * change differs between sides: old line 1 vs new line 3.
++ * Exercises the per-gap lockstep-alignment check.
++ */
++ packet_write_fmt(1, "hunk 1 1 3 1\n");
++ break;
++ case MODE_BAD_START:
++ /*
++ * A start of 0 is valid only for an empty (count 0) range;
++ * pairing it with a nonzero count names no line in either
++ * the protocol's or xdiff's coordinates, so the translation
++ * rejects it and git falls back to the builtin diff.
++ */
++ packet_write_fmt(1, "hunk 0 1 1 1\n");
++ break;
++ case MODE_MULTI_HUNK:
++ /*
++ * Two valid, non-overlapping, gap-aligned hunks. Exercises
++ * the accepting branch of the per-gap lockstep check with a
++ * non-zero previous-hunk end (the realistic two-region case).
++ */
++ packet_write_fmt(1, "hunk 5 2 5 2\n");
++ packet_write_fmt(1, "hunk 9 2 9 2\n");
++ break;
++ case MODE_INSERT:
++ /*
++ * A mid-file pure insertion (count 0 on the old side) in the
++ * protocol's 1-based-position form: 2 lines inserted before
++ * old line 3. Exercises the count-0 path, which uses the
++ * unshifted position (not git diff's "-3,0" display start).
++ */
++ packet_write_fmt(1, "hunk 3 0 3 2\n");
++ break;
++ case MODE_FLOOD: {
++ /*
++ * Emit far more hunks than any small file has lines, so Git
++ * trips its accumulation cap and falls back before reading
++ * them all.
++ */
++ int i;
++ for (i = 0; i < 100000; i++)
++ packet_write_fmt(1, "hunk 1 1 1 1\n");
++ break;
++ }
+ case MODE_OVERLAP:
+ packet_write_fmt(1, "hunk 1 5 1 5\n");
+ packet_write_fmt(1, "hunk 3 2 3 2\n");
@@ t/helper/test-diff-process-backend.c (new)
+{
+ for (;;) {
+ char *command = NULL, *pathname = NULL;
++ char *old_oid = NULL, *new_oid = NULL;
+ struct strbuf obuf = STRBUF_INIT;
+ struct strbuf nbuf = STRBUF_INIT;
+
-+ if (!read_request_header(&command, &pathname))
++ if (!read_request_header(&command, &pathname,
++ &old_oid, &new_oid))
+ break; /* EOF: Git closed its end */
+
+ read_packetized_to_strbuf(0, &obuf, 0);
@@ t/helper/test-diff-process-backend.c (new)
+
+ if (logfile) {
+ fprintf(logfile,
-+ "command=%s pathname=%s old=%.*s new=%.*s\n",
++ "command=%s pathname=%s old-oid=%s new-oid=%s"
++ " old=%.*s new=%.*s\n",
+ command ? command : "(none)",
+ pathname ? pathname : "(none)",
++ old_oid ? old_oid : "(none)",
++ new_oid ? new_oid : "(none)",
+ (int)(strchrnul(obuf.buf, '\n') - obuf.buf),
+ obuf.buf,
+ (int)(strchrnul(nbuf.buf, '\n') - nbuf.buf),
@@ t/helper/test-diff-process-backend.c (new)
+
+ free(command);
+ free(pathname);
++ free(old_oid);
++ free(new_oid);
+ strbuf_release(&obuf);
+ strbuf_release(&nbuf);
+ }
@@ t/helper/test-diff-process-backend.c (new)
+ enum mode mode = MODE_WHOLE_FILE;
+ struct option options[] = {
+ OPT_STRING(0, "mode", &mode_str, "mode",
-+ "response shape: whole-file (default), fixed-hunk,"
-+ " no-hunks, bad-hunk, bad-sync, overlap, error,"
-+ " abort, crash"),
++ "response shape (default whole-file);"
++ " see the file header for the full list of modes"),
+ OPT_STRING(0, "log", &log_path, "path",
+ "append per-request summary to this file"),
+ OPT_END()
@@ t/t4080-diff-process.sh (new)
+
+test_description='diff process via long-running process'
+
++TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# See t/helper/test-diff-process-backend.c for the backend implementation
@@ t/t4080-diff-process.sh (new)
+ EOF
+ git add logtest.c &&
+
-+ # two.c/one.c: two-file pair for error/abort/startup-failure tests.
++ # one.c/two.c: two-file pair for error/abort/startup-failure tests.
+ cat >one.c <<-\EOF &&
+ int first(void) { return 1; }
+ EOF
@@ t/t4080-diff-process.sh (new)
+ test_grep ! "^+NEW10" actual
+'
+
++test_expect_success 'diff process accepts valid multi-hunk output' '
++ # multi-hunk reports both changed regions (5-6 and 9-10) as two
++ # gap-aligned hunks. This exercises the accepting branch of the
++ # per-gap lockstep check (non-zero previous-hunk end) and must
++ # produce a correct two-region diff with the lines between the
++ # hunks kept as context.
++ git -c diff.cdiff.process="$BACKEND --mode=multi-hunk" \
++ diff boundary.c >actual 2>stderr &&
++ test_grep "^-OLD5" actual &&
++ test_grep "^+NEW5" actual &&
++ test_grep "^-OLD9" actual &&
++ test_grep "^+NEW9" actual &&
++ test_grep "^ line7" actual &&
++ test_grep "^ line8" actual &&
++ test_must_be_empty stderr
++'
++
++test_expect_success 'diff process accepts a mid-file count-0 insertion' '
++ # insert mode reports "hunk 3 0 3 2": a pure insertion (count 0 on
++ # the old side) in the protocol 1-based-position form. Exercises
++ # the count-0 hunk path that the other valid-hunk modes (full
++ # replacements, equal-count modifies) never hit. Empty stderr is
++ # the discriminator: a mishandled count-0 start would be rejected
++ # by the lockstep check and warn.
++ cat >insert.c <<-\EOF &&
++ a
++ b
++ c
++ d
++ e
++ EOF
++ git add insert.c &&
++ git commit -m "add insert.c" &&
++ cat >insert.c <<-\EOF &&
++ a
++ b
++ X
++ Y
++ c
++ d
++ e
++ EOF
++ git -c diff.cdiff.process="$BACKEND --mode=insert" \
++ diff insert.c >actual 2>stderr &&
++ test_grep "^+X" actual &&
++ test_grep "^+Y" actual &&
++ test_grep "^ c" actual &&
++ test_must_be_empty stderr
++'
++
+test_expect_success 'diff process works with modified file' '
+ test_when_finished "rm -f backend.log" &&
+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
@@ t/t4080-diff-process.sh (new)
+ diff --exit-code nohunks.c
+'
+
++test_expect_success 'diff process equivalent commit: --exit-code and --quiet agree' '
++ # A committed blob pair (not a worktree file) whose oids differ but
++ # the tool reports equivalent. --exit-code and --quiet must agree
++ # with the shown diff (empty) and report success, not fall back to
++ # the byte-level "oids differ" answer.
++ cat >ecq.c <<-\EOF &&
++ alpha
++ EOF
++ git add ecq.c &&
++ git commit -m "ecq v1" &&
++ cat >ecq.c <<-\EOF &&
++ beta
++ EOF
++ git add ecq.c &&
++ git commit -m "ecq v2" &&
++ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
++ diff --exit-code HEAD^ HEAD -- ecq.c &&
++ git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
++ diff --quiet HEAD^ HEAD -- ecq.c
++'
++
++test_expect_success 'diff process falls back for trailing-newline-only change' '
++ test_when_finished "rm -f backend.log" &&
++ printf "a\nb\nc\n" >eofnl.c &&
++ git add eofnl.c &&
++ git commit -m "add eofnl.c" &&
++ printf "a\nb\nc" >eofnl.c &&
++ # Same lines, only the final newline removed. The tool reports
++ # no hunks (it sees identical lines), but that change is not
++ # expressible as hunks, so git falls back to the builtin diff
++ # rather than treating the files as equivalent.
++ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
++ diff eofnl.c >actual 2>stderr &&
++ test_grep "No newline at end of file" actual &&
++ test_grep "pathname=eofnl.c" backend.log &&
++ test_must_be_empty stderr
++'
++
++test_expect_success 'diff process falls back for added file (empty old side)' '
++ test_when_finished "rm -f backend.log" &&
++ printf "x\ny\nz\n" >addnl.c &&
++ git add addnl.c &&
++ # The empty old side has no trailing newline while the new side
++ # does, so the newline fallback shows the addition rather than
++ # letting no-hunks suppress the whole new file.
++ git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
++ diff --cached addnl.c >actual 2>stderr &&
++ test_grep "^+x" actual &&
++ test_grep "pathname=addnl.c" backend.log &&
++ test_must_be_empty stderr
++'
++
+test_expect_success 'diff process with --exit-code and hunks returns failure' '
+ test_expect_code 1 git -c diff.cdiff.process="$BACKEND" \
+ diff --exit-code newfile.c
@@ t/t4080-diff-process.sh (new)
+ test_path_is_missing backend.log
+'
+
-+test_expect_success 'diff process not used by --stat' '
++test_expect_success 'diff process bypassed under whitespace-ignoring flags' '
+ test_when_finished "rm -f backend.log" &&
-+ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
-+ diff --stat worddiff.c >actual &&
-+ test_grep "worddiff.c" actual &&
-+ test_path_is_missing backend.log
++ printf "a\nb\nc\n" >wsbypass.c &&
++ git add wsbypass.c &&
++ git commit -m "add wsbypass.c" &&
++ printf "a\n b \nc\n" >wsbypass.c &&
++ # The tool is never told about these options and could not honor
++ # them, so git bypasses the process for each (covering the whole
++ # XDF_WHITESPACE_FLAGS | XDF_IGNORE_BLANK_LINES mask, not just -w).
++ for opt in -w -b --ignore-space-at-eol --ignore-blank-lines
++ do
++ rm -f backend.log &&
++ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
++ diff $opt wsbypass.c >actual 2>stderr &&
++ test_path_is_missing backend.log &&
++ test_must_be_empty stderr ||
++ return 1
++ done &&
++ # -w additionally suppresses the whitespace-only change via the
++ # builtin diff that now runs.
++ git -c diff.cdiff.process="$BACKEND" diff -w wsbypass.c >actual &&
++ test_must_be_empty actual
+'
+
+#
@@ t/t4080-diff-process.sh (new)
+ test_grep "^+NEW5" actual &&
+ test_grep "^-OLD9" actual &&
+ test_grep "^+NEW9" actual &&
-+ test_grep "exceeds.*lines" stderr
++ test_grep "hunk past the end" stderr
+'
+
+test_expect_success 'diff process fallback on mismatched unchanged totals' '
@@ t/t4080-diff-process.sh (new)
+ git -c diff.cdiff.process="$BACKEND --mode=bad-sync" \
+ diff synctest.c >actual 2>stderr &&
+ test_grep "changed" actual &&
-+ test_grep "unchanged line count mismatch" stderr
++ test_grep "misaligned" stderr
++'
++
++test_expect_success 'diff process fallback on misaligned hunk gap' '
++ # bad-gap reports hunk 1 1 3 1 on boundary.c: one changed line
++ # on each side, so the total unchanged counts match, but the
++ # unchanged run before the change differs (old line 1 vs new
++ # line 3). A global count check would accept this and emit a
++ # corrupt diff; the per-gap lockstep check rejects it and git
++ # falls back to the builtin algorithm.
++ git -c diff.cdiff.process="$BACKEND --mode=bad-gap" \
++ diff boundary.c >actual 2>stderr &&
++ # The builtin fallback shows both changed regions as additions
++ # (a corrupt-accepted hunk would show NEW5 only as context).
++ test_grep "^+NEW5" actual &&
++ test_grep "^+NEW9" actual &&
++ test_grep "misaligned" stderr
+'
+
+test_expect_success 'diff process fallback on overlapping hunks' '
+ # boundary.c has 10 lines, so both hunks are in bounds
-+ # but they overlap at lines 3-5, triggering the ordering check.
++ # but they overlap at lines 3-4, triggering the ordering check.
+ git -c diff.cdiff.process="$BACKEND --mode=overlap" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "NEW5" actual &&
-+ test_grep "overlaps with previous" stderr
++ test_grep "overlapping hunks" stderr
+'
+
+test_expect_success 'diff process fallback on malformed hunk line' '
@@ t/t4080-diff-process.sh (new)
+ test_grep "^+NEW5" actual
+'
+
-+test_expect_success 'diff process skipped when tool omits capability' '
-+ git -c diff.cdiff.process="$BACKEND --mode=no-cap" \
++test_expect_success 'diff process fallback on start 0 with nonzero count' '
++ # bad-start reports hunk 0 1 1 1. A start of 0 is valid only for
++ # an empty (count 0) range, so the presentation-to-xdiff
++ # translation rejects it and git falls back to the builtin diff
++ # instead of handing xdiff an out-of-range start.
++ git -c diff.cdiff.process="$BACKEND --mode=bad-start" \
+ diff boundary.c >actual 2>stderr &&
+ test_grep "^-OLD5" actual &&
+ test_grep "^+NEW5" actual &&
++ test_grep "diff process.*failed" stderr
++'
++
++test_expect_success 'diff process caps a flood of hunks and falls back' '
++ # flood emits far more hunks than the file has lines. Git must
++ # stop accumulating and fall back to the builtin diff rather than
++ # grow memory without bound.
++ git -c diff.cdiff.process="$BACKEND --mode=flood" \
++ diff boundary.c >actual 2>stderr &&
++ test_grep "^-OLD5" actual &&
++ test_grep "too many hunks" stderr
++'
++
++test_expect_success 'diff process skipped when tool omits capability' '
++ test_when_finished "rm -f backend.log" &&
++ git -c diff.cdiff.process="$BACKEND --mode=no-cap --log=backend.log" \
++ diff boundary.c >actual 2>stderr &&
++ # Builtin diff runs: all changes appear, including lines 9-10
++ # that a tool-provided hunk would have narrowed away.
++ test_grep "^-OLD5" actual &&
++ test_grep "^-OLD9" actual &&
++ # The process launched (creating the log) but was
++ # never sent a per-file request, so no hunks command is logged.
++ test_path_is_file backend.log &&
++ test_grep ! "command=hunks" backend.log &&
++ test_must_be_empty stderr
++'
++
++test_expect_success 'diff process receives old-oid and new-oid for a blob pair' '
++ test_when_finished "rm -f backend.log" &&
++ cat >oidpair.c <<-\EOF &&
++ int f(void) { return 1; }
++ EOF
++ git add oidpair.c &&
++ git commit -m "oidpair v1" &&
++ old=$(git rev-parse HEAD:oidpair.c) &&
++
++ cat >oidpair.c <<-\EOF &&
++ int f(void) { return 2; }
++ EOF
++ git add oidpair.c &&
++ git commit -m "oidpair v2" &&
++ new=$(git rev-parse HEAD:oidpair.c) &&
++
++ # Both sides are stored blobs, so their object names are sent.
++ git -c diff.cdiff.process="$BACKEND --log=backend.log" \
++ diff HEAD^ HEAD -- oidpair.c >actual 2>stderr &&
++ test_grep "old-oid=$old new-oid=$new" backend.log &&
++ test_must_be_empty stderr
++'
++
++test_expect_success 'diff process omits old-oid and new-oid for textconv content' '
++ test_when_finished "rm -f backend.log" &&
++ write_script oidcat <<-\EOF &&
++ cat "$1"
++ EOF
++ cat >oidtc.c <<-\EOF &&
++ alpha
++ EOF
++ git add oidtc.c &&
++ git commit -m "oidtc v1" &&
++ cat >oidtc.c <<-\EOF &&
++ beta
++ EOF
++ git add oidtc.c &&
++ git commit -m "oidtc v2" &&
++
++ # textconv rewrites the bytes, so the raw-blob object name that
++ # would otherwise identify each side is omitted.
++ git -c diff.cdiff.textconv="./oidcat" \
++ -c diff.cdiff.process="$BACKEND --log=backend.log" \
++ diff HEAD^ HEAD -- oidtc.c >actual 2>stderr &&
++ test_grep "pathname=oidtc.c" backend.log &&
++ test_grep "old-oid=(none) new-oid=(none)" backend.log &&
+ test_must_be_empty stderr
+'
+
5: 6ec6716ea4 ! 6: b2e80f014e diff: bypass diff process with --no-ext-diff and in format-patch
@@ Documentation/diff-options.adoc: endif::git-format-patch[]
`--textconv`::
`--no-textconv`::
+ ## Documentation/gitattributes.adoc ##
+@@ Documentation/gitattributes.adoc: display, which is covered above); and combined diffs (`--cc` and merge
+ diffs), whose protocol would have to be extended from a single old/new
+ pair to one comparison per merge parent.
+
+-`--diff-algorithm` bypasses the process entirely, for every feature
+-listed above. The whitespace-ignoring options (`-w`,
+-`--ignore-space-change`, `--ignore-blank-lines`, and the like),
++`--no-ext-diff` and `--diff-algorithm` bypass the process entirely,
++for every feature listed above. The whitespace-ignoring options
++(`-w`, `--ignore-space-change`, `--ignore-blank-lines`, and the like),
+ `-I<regex>`, and `--anchored` also bypass it for the affected files:
+ the tool is never told about these options, so it could not honor
+ them, and Git falls back to the builtin diff, which does.
+
## builtin/log.c ##
@@ builtin/log.c: int cmd_format_patch(int argc,
if (argc > 1)
@@ diff.h: struct diff_flags {
- /** Disables diff.<driver>.process. */
+ /**
-+ * Disables diff.<driver>.process. Set by --no-ext-diff.
++ * Disables diff.<driver>.process. Set by --no-ext-diff and by
++ * format-patch.
+ */
unsigned no_diff_process;
@@ t/t4080-diff-process.sh: test_expect_success 'diff process bypassed by --diff-al
+ test_path_is_missing backend.log
+'
+
- test_expect_success 'diff process not used by --stat' '
+ test_expect_success 'diff process bypassed under whitespace-ignoring flags' '
test_when_finished "rm -f backend.log" &&
- git -c diff.cdiff.process="$BACKEND --log=backend.log" \
+ printf "a\nb\nc\n" >wsbypass.c &&
6: 3dadafa1bc ! 7: cf5bb8984a blame: consult diff process for no-hunk detection
@@ Commit message
commit as having no changes, causing blame to attribute lines
to earlier commits.
- The consultation happens at the pass_blame_to_parent() callsite
- using diff_process_fill_hunks(), matching how builtin_diff() in
- diff.c uses the same function. A new diff_hunks_xpp() variant
- accepts a pre-populated xpparam_t so callers can pass external
- hunks, while the existing diff_hunks() retains its original
- signature and behavior. The copy-detection callsite is
- unaffected since it does not use the diff process.
+ Introduce xdi_diff_process(), a process-aware xdi_diff() that
+ consults the process, runs xdiff on the tool's hunks or on the
+ builtin algorithm when it does not apply, frees the hunks, and
+ reports DIFF_PROCESS_EQUIVALENT (without running xdiff) so the caller
+ can drop or skip the change. It is the shared consult-then-diff path
+ for consumers that work on raw hunks: blame's pass_blame_to_parent()
+ uses it here, and git log -L reuses it later. builtin_diff() keeps
+ consulting the process directly, because it tests for equivalence
+ early, before its funcname-pattern and word-diff setup, so a
+ reformat-only file short-circuits without that work.
- The subprocess is long-running (one startup cost amortized
- across the blame traversal), but each commit in the file's
- history incurs a round-trip to the tool.
+ Blame's -w option is not communicated to the process and it could not
+ honor it, so blame must fall back to the builtin diff there. Because
+ blame keeps its whitespace flags in sb->xdl_opts rather than diffopt,
+ the process bypass keys off xpp (the flags the diff actually runs
+ with), which covers blame without a guard of its own.
+
+ The subprocess is long-running (one startup cost amortized across the
+ blame traversal), but each commit in the file's history incurs a
+ round-trip to the tool.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
@@ blame.c
#include "alloc.h"
#include "commit-slab.h"
#include "bloom.h"
-@@ blame.c: static struct commit *fake_working_tree_commit(struct repository *r,
-
-
-
--static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
-- xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts)
-+static int diff_hunks_xpp(mmfile_t *file_a, mmfile_t *file_b,
-+ xdl_emit_hunk_consume_func_t hunk_func,
-+ void *cb_data, xpparam_t *xpp)
- {
-- xpparam_t xpp = {0};
- xdemitconf_t xecfg = {0};
- xdemitcb_t ecb = {NULL};
-
-- xpp.flags = xdl_opts;
- xecfg.hunk_func = hunk_func;
- ecb.priv = cb_data;
-- return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
-+ return xdi_diff(file_a, file_b, xpp, &xecfg, &ecb);
-+}
-+
-+static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
-+ xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts)
-+{
-+ xpparam_t xpp = {0};
-+
-+ xpp.flags = xdl_opts;
-+ return diff_hunks_xpp(file_a, file_b, hunk_func, cb_data, &xpp);
- }
-
- static const char *get_next_line(const char *start, const char *end)
@@ blame.c: static void pass_blame_to_parent(struct blame_scoreboard *sb,
struct blame_origin *parent, int ignore_diffs)
{
mmfile_t file_p, file_o;
+ xpparam_t xpp = {0};
++ xdemitconf_t xecfg = {0};
++ xdemitcb_t ecb = {NULL};
struct blame_chunk_cb_data d;
struct blame_entry *newdest = NULL;
@@ blame.c: static void pass_blame_to_parent(struct blame_scoreboard *sb,
sb->num_get_patch++;
- if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
-- die("unable to generate diff (%s -> %s)",
-- oid_to_hex(&parent->commit->object.oid),
-- oid_to_hex(&target->commit->object.oid));
+ xpp.flags = sb->xdl_opts;
++ xecfg.hunk_func = blame_chunk_cb;
++ ecb.priv = &d;
+ /*
-+ * If the diff process considers the files equivalent,
-+ * skip the diff so blame looks past this commit.
++ * Consult the diff process, then attribute the resulting chunks
++ * via blame_chunk_cb. It bypasses the process for the whitespace-
++ * ignoring options it cannot honor (they live in xpp.flags, which
++ * the consultation checks), and when the process reports the blobs
++ * equivalent it runs no diff, so blame passes this commit and looks
++ * past it. Look up the driver by the parent (old) path, as
++ * builtin_diff() does with name_a, so a renamed file resolves to the
++ * same driver across diff, blame, and line-log. Pass no
++ * old-oid/new-oid: blame diffs each blob pair once, so the tool gains
++ * nothing from a per-invocation cache key.
+ */
-+ if (diff_process_fill_hunks(&sb->revs->diffopt, target->path,
-+ &file_p, &file_o, &xpp)
-+ != DIFF_PROCESS_EQUIVALENT) {
-+ if (diff_hunks_xpp(&file_p, &file_o, blame_chunk_cb,
-+ &d, &xpp))
-+ die("unable to generate diff (%s -> %s)",
-+ oid_to_hex(&parent->commit->object.oid),
-+ oid_to_hex(&target->commit->object.oid));
-+ }
-+ free(xpp.external_hunks);
- /* The rest are the same as the parent */
- blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, 0,
- parent, target, 0);
++ if (xdi_diff_process(&sb->revs->diffopt, parent->path,
++ &file_p, &file_o, NULL, NULL, &xpp, &xecfg, &ecb)
++ == DIFF_PROCESS_ERROR)
+ die("unable to generate diff (%s -> %s)",
+ oid_to_hex(&parent->commit->object.oid),
+ oid_to_hex(&target->commit->object.oid));
+
+ ## diff-process.c ##
+@@
+ #include "sub-process.h"
+ #include "pkt-line.h"
+ #include "strbuf.h"
++#include "xdiff-interface.h"
+ #include "xdiff/xdiff.h"
+
+ #define CAP_HUNKS (1u << 0)
+@@ diff-process.c: enum diff_process_result diff_process_fill_hunks(
+ }
+ return DIFF_PROCESS_SKIP;
+ }
++
++enum diff_process_result xdi_diff_process(
++ struct diff_options *diffopt,
++ const char *path,
++ mmfile_t *file_a,
++ mmfile_t *file_b,
++ const struct object_id *oid_a,
++ const struct object_id *oid_b,
++ xpparam_t *xpp,
++ xdemitconf_t *xecfg,
++ xdemitcb_t *ecb)
++{
++ enum diff_process_result res;
++
++ /*
++ * Consult the diff process, then run xdiff either constrained to
++ * the tool's hunks or, when the process does not apply, computing
++ * the diff itself as a fallback. EQUIVALENT short-circuits: the
++ * caller decides what "no change" means for it (drop the commit,
++ * skip the file, ...), so xdiff is not run.
++ *
++ * A SKIP/ERROR from the process just selects the builtin path
++ * (its warning, if any, was already emitted), so the result then
++ * reflects whether xdiff itself succeeded, not the process.
++ */
++ res = diff_process_fill_hunks(diffopt, path, file_a, file_b,
++ oid_a, oid_b, xpp);
++ if (res == DIFF_PROCESS_EQUIVALENT)
++ return res;
++
++ res = xdi_diff(file_a, file_b, xpp, xecfg, ecb) < 0
++ ? DIFF_PROCESS_ERROR : DIFF_PROCESS_OK;
++
++ FREE_AND_NULL(xpp->external_hunks);
++ xpp->external_hunks_nr = 0;
++ return res;
++}
+
+ ## diff-process.h ##
+@@ diff-process.h: enum diff_process_result diff_process_fill_hunks(
+ const struct object_id *oid_b,
+ xpparam_t *xpp);
+
++/*
++ * Process-aware xdi_diff(): consult the diff process for 'path', then
++ * run xdiff either constrained to the tool's hunks or computing the
++ * diff itself when the process does not apply or fails. Frees any
++ * hunks it obtained before returning.
++ *
++ * Returns DIFF_PROCESS_EQUIVALENT (without running xdiff) when the tool
++ * reports the blobs equal, so the caller can drop or skip the change;
++ * DIFF_PROCESS_OK when xdiff ran (on tool hunks or builtin); and
++ * DIFF_PROCESS_ERROR if xdiff itself errored.
++ *
++ * The caller fills xpp (flags, ignore_regex, anchors) and xecfg/ecb as
++ * for a direct xdi_diff() call. oid_a/oid_b are forwarded to
++ * diff_process_fill_hunks() (see there).
++ */
++enum diff_process_result xdi_diff_process(
++ struct diff_options *diffopt,
++ const char *path,
++ mmfile_t *file_a,
++ mmfile_t *file_b,
++ const struct object_id *oid_a,
++ const struct object_id *oid_b,
++ xpparam_t *xpp,
++ xdemitconf_t *xecfg,
++ xdemitcb_t *ecb);
++
+ #endif /* DIFF_PROCESS_H */
## t/t4080-diff-process.sh ##
-@@ t/t4080-diff-process.sh: test_expect_success 'diff process skipped when tool omits capability' '
+@@ t/t4080-diff-process.sh: test_expect_success 'diff process omits old-oid and new-oid for textconv content
test_must_be_empty stderr
'
@@ t/t4080-diff-process.sh: test_expect_success 'diff process skipped when tool omi
+ test_grep "$CHANGE" line9 &&
+ test_path_is_missing backend.log
+'
++
++test_expect_success 'blame -w bypasses diff process' '
++ test_when_finished "rm -f backend.log" &&
++ printf "alpha\nbeta\ngamma\n" >blamew.c &&
++ git add blamew.c &&
++ git commit -m "add blamew.c" &&
++ orig=$(git rev-parse --short HEAD) &&
++ printf "alpha\n beta \ngamma\n" >blamew.c &&
++ git commit -am "reindent beta" &&
++ reindent=$(git rev-parse --short HEAD) &&
++ # blame -w must ignore the whitespace-only change and attribute
++ # beta to the original commit, not the reindent commit. The tool
++ # is never told about -w, so blame must bypass it (not let tool
++ # hunks override -w).
++ git -c diff.cdiff.process="$BACKEND --mode=whole-file --log=backend.log" \
++ blame -w blamew.c >actual &&
++ sed -n "2p" actual >line2 &&
++ test_grep "$orig" line2 &&
++ test_grep ! "$reindent" line2 &&
++ test_path_is_missing backend.log
++'
+
test_done
-: ---------- > 8: c1d02d0e15 diff: consult diff process for --stat counts
-: ---------- > 9: c3c17ba8fc line-log: consult diff process for range tracking
--
gitgitgadget
^ permalink raw reply
* [PATCH v5 1/9] gitattributes: document how external diff drivers relate to diff features
From: Michael Montalbo via GitGitGadget @ 2026-07-15 21:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
The "Defining an external diff driver" section explains how to
configure diff.<driver>.command but not how the driver relates to the
rest of Git's diff machinery. In particular, the command only
replaces the textual patch: word diff, function context, color, and
the like cannot apply to its output, while the summary formats, blame,
and git log -L do not run it at all and keep using the builtin diff.
Spell this out so the scope of an external diff driver is clear.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/gitattributes.adoc | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index bd76167a45..2c4fbfd7f1 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -784,6 +784,16 @@ with the above configuration, i.e. `j-c-diff`, with 7
parameters, just like `GIT_EXTERNAL_DIFF` program is called.
See linkgit:git[1] for details.
+An external diff driver replaces the patch Git would otherwise
+produce for the path: Git runs the command and shows its output in
+place of its own. Output features that post-process Git's diff do
+not apply to it; word diff, function context (`-W`), `--color-moved`,
+and coloring all act on Git's builtin diff, not the driver's output.
+The driver is consulted only when Git generates a textual patch. The
+summary formats (`--stat`, `--numstat`, `--shortstat`, and
+`--dirstat`), `git blame`, and `git log -L` do not run it and
+continue to use Git's builtin diff.
+
If the program is able to ignore certain changes (similar to
`git diff --ignore-space-change`), then also set the option
`trustExitCode` to true. It is then expected to return exit code 1 if
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH GSoC v18 13/13] cat-file: make remote-object-info allow-list dynamic
From: Junio C Hamano @ 2026-07-15 20:32 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, chandrapratap3519, chriscool, eric.peijian, jltobler,
karthik.188, peff, toon
In-Reply-To: <DJZDEE0G6ZRS.2RT8JTQQ6CUXB@gmail.com>
"Pablo Sabater" <pabloosabaterr@gmail.com> writes:
>>> 2. Filters the request in fetch_object_info() dropping any option that
>>> the server does not advertise.
>>>
>>> 3. After the fetching, the options that haven't been dropped are the ones
>>> fetched and supported by the server, these supported options are
>>> mapped and remote_allowed_atoms is populated with the placeholders.
>>>
>>> 4. expand_atom() checks remote_allowed_atoms with the same behaviour as
>>> the static allow_list had.
>>
>> I am not sure I follow the above entirely. Could you add a
>> concrete example to the commit message?
>>
>> For instance, if the client wants "%(objectsize) %(objectcolor)" and
>> the server only supports 'size' but not 'color', the filtering in
>> step (2) prevents the client from asking about the color, requesting
>> only the size instead. When the server says the size is 42, step (3)
>> uses that to substitute '%(objectsize)'. Would the end result then
>> be "42 %(objectcolor)"?
>
> You've gotten everything right until the last step, because we have only
> size from the server there is no data to match %(objectcolor) and the
> end result is an empty string for %(objeccolor):
>
> "42 "
>
> Note that %(objectcolor) doesn't exists and it would have die(), the
> empty string is only for known but unsupported placeholders.
It was not clear there is a distinction between "unknown" and
"known but unsupported". The proposed log message needs to be
clarified to make this distinction obvious.
>> And if the request is only for "%(objectname)", an empty
>> object_info_options is given to get_remote_info().
>
> Right now 'name' is not part of the protocol as 'type' or 'size' are,
> 'objectname' is always allowed but only shown if it's present on the
> format.
> If the format is only "%(objectname)" then there's nothing to ask the
> server for.
>
> The current code avoids making the request if there's only objectname or
> nothing supported, but still goes through the connection work. I will
> add an early return to just output the oid back without any connection.
I think you are heading in the opposite direction. Rather, when
only the object name is requested, I was hoping we would pick
something cheap to retrieve and ask the remote side for it, if
only to catch a bogus or missing object name.
Thanks.
^ permalink raw reply
* [PATCH] submodule--helper: avoid use of %zu for now
From: Junio C Hamano @ 2026-07-15 20:10 UTC (permalink / raw)
To: git; +Cc: Adrian Ratiu
Since d7d850e2b9 (CodingGuidelines: mention C99 features we can't
use, 2022-10-10), our CodingGuidelines document has explicitly
forbidden the use of '%z' and '%zu' printf() format specifiers,
even though C99 does support them. However, a new instance crept
in via 82c36fa0a9 (submodule: hash the submodule name for the
gitdir path, 2026-01-12).
We could claim that this is an unintentional weather balloon that
nobody has complained about for the past six months since Git 2.54,
proving that it is now safe to use these format specifiers. But
(1) it is probably too early to make that claim, as distributions
often stick to a stale version for several releases, and (2) it is
unlikely that a failure in this code path would manifest as a
major user-visible breakage that would trigger a failure report to
percolate down to us.
Instead, let's stick to the established workaround recommended by
our CodingGuidelines, which is to cast the value to (uintmax_t) and
format it with PRIuMAX, at least for now. Even if we eventually
perform a bulk update using a Coccinelle script to transition to %z
and %zu in the future, adding one more instance to the pile that
will need such a conversion is hardly a tragedy.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git i/builtin/submodule--helper.c w/builtin/submodule--helper.c
index 1cc82a134d..92e38106c1 100644
--- i/builtin/submodule--helper.c
+++ w/builtin/submodule--helper.c
@@ -549,7 +549,8 @@ static void create_default_gitdir_config(const char *submodule_name)
}
/* Case 2.4: If all the above failed, try a hash of the name as a last resort */
- header_len = snprintf(header, sizeof(header), "blob %zu", strlen(submodule_name));
+ header_len = snprintf(header, sizeof(header),
+ "blob %"PRIuMAX, (uintmax_t)strlen(submodule_name));
the_hash_algo->init_fn(&ctx);
the_hash_algo->update_fn(&ctx, header, header_len);
the_hash_algo->update_fn(&ctx, "\0", 1);
^ permalink raw reply related
* Re: [PATCH] t7614: avoid hiding git's exit code in a pipe
From: Junio C Hamano @ 2026-07-15 19:36 UTC (permalink / raw)
To: Shlok Kulshreshtha; +Cc: git
In-Reply-To: <20260715113344.3490-1-diy2903@gmail.com>
Shlok Kulshreshtha <diy2903@gmail.com> writes:
> The exit code of the upstream command in a pipe is ignored, so in
>
> git cat-file commit HEAD | sed -e "1,/^\$/d" >actual
>
> a crash of "git cat-file" would go unnoticed: the exit code of the
> pipeline is that of "sed", which happily succeeds on empty input. The
> test would thus pass even though "git cat-file" failed.
>
> Write the output of "git cat-file" to a file first and run "sed" on
> that file, so that the exit codes of both commands are checked by the
> &&-chain.
>
> Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>
> ---
> This is a microproject ("Avoid suppressing git's exit code in test
> scripts"), applying the same fix as c6f44e1da5 (t9813: avoid using
> pipes) to another script. A search of the list did not turn up anyone
> working on t7614; please let me know if it is already taken.
All look trivially correct.
Two clean-up possibilities that are clearly outside the scope of
this patch are
* There is no need to backslash-quote the dollar sign.
t7604-merge-custom-message.sh next door uses "1,/^$/d" just fine.
* This "cat-file the commit object, and strip away the object
header with sed" pattern appears quite often throughout the test
suite.
$ git grep -B1 -e 'sed -e "1,/^\\*$/d"' t/
shows quite a few hits. It might make sense to give them an easy
to use helper script
commit_body () {
git cat-file commit "$1" >.commit &&
sed -e "1,/^$/d" .commit &&
rm -f .commit
}
or something like that.
But again, these are clearly outside the scope of this patch.
>
> t/t7614-merge-signoff.sh | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
> index fee258d4f0..e58bf07b7a 100755
> --- a/t/t7614-merge-signoff.sh
> +++ b/t/t7614-merge-signoff.sh
> @@ -45,7 +45,8 @@ test_expect_success 'git merge --signoff adds a sign-off line' '
> test_commit main-branch-2 file2 2 &&
> git checkout other-branch &&
> git merge main --signoff --no-edit &&
> - git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + git cat-file commit HEAD >commit &&
> + sed -e "1,/^\$/d" commit >actual &&
> test_cmp expected-signed actual
> '
>
> @@ -55,7 +56,8 @@ test_expect_success 'git merge does not add a sign-off line' '
> test_commit main-branch-3 file3 3 &&
> git checkout other-branch &&
> git merge main --no-edit &&
> - git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + git cat-file commit HEAD >commit &&
> + sed -e "1,/^\$/d" commit >actual &&
> test_cmp expected-unsigned actual
> '
>
> @@ -65,7 +67,8 @@ test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
> test_commit main-branch-4 file4 4 &&
> git checkout other-branch &&
> git merge main --no-edit --signoff --no-signoff &&
> - git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + git cat-file commit HEAD >commit &&
> + sed -e "1,/^\$/d" commit >actual &&
> test_cmp expected-unsigned actual
> '
^ permalink raw reply
* [PATCH] revision: make get_commit_action() a pure predicate
From: Michael Montalbo via GitGitGadget @ 2026-07-15 19:29 UTC (permalink / raw)
To: git; +Cc: SZEDER Gábor, Michael Montalbo, Michael Montalbo
From: Michael Montalbo <mmontalbo@gmail.com>
get_commit_action() reads as a predicate that decides whether a commit
is shown or ignored, but for a line-level log without parent rewriting
it also calls line_log_process_ranges_arbitrary_commit(), which
mutates the tracked line ranges. That hidden side effect makes it unsafe
to evaluate ahead of the walk, the way a lookahead would.
get_commit_action() was split out of simplify_commit() in beb5af43a6
(graph API: fix bug in graph_is_interesting(), 2009-08-18) as the
show/ignore decision minus the parent rewriting, so the graph renderer
could reuse it; line-level log later routed its filtering through it as
well, in 3cb9d2b6 (line-log: more responsive, incremental 'git log -L',
2020-05-11). Besides simplify_commit(), the walk driver,
graph_is_interesting() is its only other caller, and it runs only under
--graph, which sets rewrite_parents and therefore want_ancestry(); the
"-L without ancestry" branch that holds the side effect never fires
there, so it is dormant today.
The line-level processing folds a commit's tracked ranges onto its
parents, which must happen even for a commit that get_commit_action()
filters from the output, or the ranges never reach the parents. Move it
to simplify_commit() and run it before get_commit_action(), gated by
get_commit_action()'s leading checks (already shown, uninteresting, and
the like) so a commit ignored by those is not folded, as before; factor
those checks out as commit_early_ignore(). get_commit_action() is then
side-effect free.
commit_early_ignore() runs twice on the -L path, once for that gate and
once inside get_commit_action(), but it reads only object flags and pack
membership, disjoint from the TREESAME flag the fold sets, so the repeat
is harmless.
Add a "line-log-peek" subcommand to the revision-walking test helper
that evaluates get_commit_action() on a commit the walk has not reached
yet, plus a t4211 check that the call leaves the commit's flags
unchanged. The flags are compared rather than the commit list because
add_line_range() merges ranges by union, which is idempotent, so the
side effect never changed which commits a linear -L history shows.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
revision: make get_commit_action() a pure predicate
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2169%2Fmmontalbo%2Fmm%2Fline-log-tidy-proto-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2169/mmontalbo/mm/line-log-tidy-proto-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2169
revision.c | 70 ++++++++++++++++++++------------
t/helper/test-revision-walking.c | 63 ++++++++++++++++++++++++++++
t/t4211-line-log.sh | 20 +++++++++
3 files changed, 127 insertions(+), 26 deletions(-)
diff --git a/revision.c b/revision.c
index 0c95edef59..5d650affc0 100644
--- a/revision.c
+++ b/revision.c
@@ -4175,37 +4175,39 @@ static timestamp_t comparison_date(const struct rev_info *revs,
commit->date;
}
-enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit)
+/*
+ * Whether the commit is ignored by the cheap checks that read only its
+ * traversal flags and pack membership (e.g. already shown, or marked
+ * uninteresting), before any check that examines the commit's date,
+ * parents, message, or diff.
+ */
+static int commit_early_ignore(struct rev_info *revs, struct commit *commit)
{
if (commit->object.flags & SHOWN)
- return commit_ignore;
+ return 1;
if (revs->maximal_only && (commit->object.flags & CHILD_VISITED))
- return commit_ignore;
+ return 1;
if (revs->unpacked && has_object_pack(revs->repo, &commit->object.oid))
- return commit_ignore;
- if (revs->no_kept_objects) {
- if (has_object_kept_pack(revs->repo, &commit->object.oid,
- revs->keep_pack_cache_flags))
- return commit_ignore;
- }
+ return 1;
+ if (revs->no_kept_objects &&
+ has_object_kept_pack(revs->repo, &commit->object.oid,
+ revs->keep_pack_cache_flags))
+ return 1;
if (commit->object.flags & UNINTERESTING)
+ return 1;
+ return 0;
+}
+
+/*
+ * Decide whether this commit is shown or ignored. Keep it a pure
+ * predicate: callers such as the commit graph depend on it having no
+ * side effects, so per-commit mutations (such as -L range tracking)
+ * belong in the caller, simplify_commit(), not here.
+ */
+enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit)
+{
+ if (commit_early_ignore(revs, commit))
return commit_ignore;
- if (revs->line_level_traverse && !want_ancestry(revs)) {
- /*
- * In case of line-level log with parent rewriting
- * prepare_revision_walk() already took care of all line-level
- * log filtering, and there is nothing left to do here.
- *
- * If parent rewriting was not requested, then this is the
- * place to perform the line-level log filtering. Notably,
- * this check, though expensive, must come before the other,
- * cheaper filtering conditions, because the tracked line
- * ranges must be adjusted even when the commit will end up
- * being ignored based on other conditions.
- */
- if (!line_log_process_ranges_arbitrary_commit(revs, commit))
- return commit_ignore;
- }
if (revs->min_age != -1 &&
comparison_date(revs, commit) > revs->min_age)
return commit_ignore;
@@ -4314,7 +4316,23 @@ struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit
enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
{
- enum commit_action action = get_commit_action(revs, commit);
+ enum commit_action action;
+
+ /*
+ * For a line-level log without parent rewriting, fold each commit's
+ * ranges as the walk reaches it (parent rewriting does this eagerly in
+ * prepare_revision_walk()). Fold before get_commit_action() so the
+ * ranges carry across a commit that a later, cheaper check ignores;
+ * the commit_early_ignore() guard skips a commit get_commit_action()
+ * would ignore outright.
+ */
+ if (revs->line_level_traverse && !want_ancestry(revs) &&
+ !commit_early_ignore(revs, commit)) {
+ if (!line_log_process_ranges_arbitrary_commit(revs, commit))
+ return commit_ignore;
+ }
+
+ action = get_commit_action(revs, commit);
if (action == commit_show &&
revs->prune && revs->dense && want_ancestry(revs)) {
diff --git a/t/helper/test-revision-walking.c b/t/helper/test-revision-walking.c
index 70051eeaf8..24d7f29417 100644
--- a/t/helper/test-revision-walking.c
+++ b/t/helper/test-revision-walking.c
@@ -13,9 +13,12 @@
#include "test-tool.h"
#include "commit.h"
#include "diff.h"
+#include "line-log.h"
+#include "object-name.h"
#include "repository.h"
#include "revision.h"
#include "setup.h"
+#include "string-list.h"
static void print_commit(struct commit *commit)
{
@@ -51,6 +54,60 @@ static int run_revision_walk(void)
return got_revision;
}
+/*
+ * Check that get_commit_action() is a pure predicate by evaluating it on a
+ * commit the walk has not reached yet. No git command makes that out-of-order
+ * call, so this probe does it deliberately, and reports whether the call
+ * mutated the peeked commit: a pure get_commit_action() leaves it untouched.
+ * We compare the commit's flags rather than the emitted commit list because
+ * range merges are idempotent, so a side effect would not change which commits
+ * are shown. Only meaningful for a plain "-L" walk with no parent rewriting.
+ */
+static int line_log_peek(const char **argv)
+{
+ struct repository *repo = the_repository;
+ struct rev_info rev;
+ struct string_list range_args = STRING_LIST_INIT_DUP;
+ struct object_id oid;
+ struct commit *peek;
+ const char *rev_argv[3];
+ unsigned before, after;
+
+ if (repo_get_oid(repo, argv[0], &oid))
+ die("bad peek commit: %s", argv[0]);
+ peek = lookup_commit_reference(repo, &oid);
+ if (!peek || repo_parse_commit(repo, peek))
+ die("cannot parse peek commit: %s", argv[0]);
+
+ repo_init_revisions(repo, &rev, NULL);
+ rev.diffopt.flags.recursive = 1;
+ rev.line_level_traverse = 1;
+ string_list_append(&range_args, argv[1]);
+
+ rev_argv[0] = "line-log-peek";
+ rev_argv[1] = argv[2];
+ rev_argv[2] = NULL;
+ setup_revisions(2, rev_argv, &rev, NULL);
+
+ line_log_init(&rev, NULL, &range_args);
+
+ if (rev.rewrite_parents || rev.children.name)
+ die("line-log-peek requires a non-ancestry (-L, no --graph) walk");
+
+ if (prepare_revision_walk(&rev))
+ die("prepare_revision_walk failed");
+
+ before = peek->object.flags;
+ get_commit_action(&rev, peek);
+ after = peek->object.flags;
+
+ printf("mutated %d\n", before != after);
+
+ release_revisions(&rev);
+ string_list_clear(&range_args, 0);
+ return 0;
+}
+
int cmd__revision_walking(int argc, const char **argv)
{
if (argc < 2)
@@ -69,6 +126,12 @@ int cmd__revision_walking(int argc, const char **argv)
return 0;
}
+ if (!strcmp(argv[1], "line-log-peek")) {
+ if (argc != 5)
+ die("usage: test-tool revision-walking line-log-peek <peek-commit> <start,end:file> <rev>");
+ return line_log_peek(argv + 2);
+ }
+
fprintf(stderr, "check usage\n");
return 1;
}
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index ca4eb7bbc7..f4a7d8ab61 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -781,4 +781,24 @@ test_expect_success '--summary shows new file on root commit' '
test_grep "create mode 100644 file.c" actual
'
+test_expect_success 'get_commit_action() does not mutate a not-yet-walked commit' '
+ git init peek &&
+ (
+ cd peek &&
+ test_write_lines 1 2 3 4 5 >f.c &&
+ git add f.c && test_tick && git commit -m base &&
+ test_write_lines 1 two 3 4 5 >f.c &&
+ test_tick && git commit -am change &&
+
+ # Peek HEAD^, which the walk has not reached (the out-of-order
+ # call a lookahead makes), and confirm get_commit_action() leaves
+ # it untouched. A side effect is invisible in the commit list
+ # (range merges are idempotent), so the helper reports whether the
+ # call mutated the peeked commit at all.
+ echo "mutated 0" >expect &&
+ test-tool revision-walking line-log-peek HEAD^ 1,3:f.c HEAD >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
--
gitgitgadget
^ permalink raw reply related
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