RFC (superseded staging): hunk providers, content-first protocol ordering - #3
Draft
mmontalbo wants to merge 21 commits into
Draft
RFC (superseded staging): hunk providers, content-first protocol ordering#3mmontalbo wants to merge 21 commits into
mmontalbo wants to merge 21 commits into
Conversation
The line-range filter that mm/line-log-cleanup added uses names that
obscure its model. The cursors lno_post/lno_pre and the index lno_0
share an lno_ prefix but conflate the pre/post-image axis with the
0-based/1-based axis, the hunk state is a flat set of rhunk_* fields,
and the filter-state pointer is just s.
The filter bridges two layers of diff.c, and its fields already used
each layer's vocabulary, but in cryptic abbreviations. Spell them out
to the form the rest of the file uses, so that the patches that follow
can simplify and fix it with those clearer names in place:
- lno_post/lno_pre -> lno_in_postimage/lno_in_preimage, the
line-number cursors, matching the counters in struct emit_callback
- lno_0 -> idx_in_postimage, the 0-based range index
- the hunk-header geometry stays old/new (old_begin, new_begin, and
counts) to match the xdiff_emit_hunk_fn callback and the
"@@ -<old> +<new> @@" header it feeds, but moves from flat rhunk_*
fields into a "hunk" sub-struct, so accesses read
filter->hunk.old_begin
- flush_rhunk -> flush_range_hunk
- the filter-state pointer in each callback: s -> filter
Also rename the struct line_range_callback to line_range_filter: it is
a filter over xdiff output, not merely a callback.
No behavior change.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The filter buffered '-' lines in a pending_rm strbuf, deferring their classification until a '+' or ' ' line revealed the post-image position. That buffering is unnecessary: a removal occupies no post-image line, so it does not advance lno_in_postimage, and xdiff emits removals before additions within a change. A '-' therefore arrives while lno_in_postimage already holds the index the following '+'/' ' will occupy, and can be classified against the ranges as it arrives. The buffering also hid a bug: flush_range_hunk() drained pending_rm into the range hunk whenever the hunk was active, even after lno_in_postimage had advanced past the tracked range, so a deletion just after the tracked function leaked into the patch. Classifying each line as it arrives removes the pending_rm buffer, the discard_pending_rm() helper, three struct fields, and makes that bug impossible by construction. With every line classified on arrival, the buffered lines are the hunk's single source of truth, so the old/new counts need not be kept alongside them: flush_range_hunk() derives the counts (and whether the hunk holds any change) from the buffer when it builds the header. Drop the per-line counting and the old_count, new_count, and has_changes fields; there is no longer a second tally that could fall out of sync with the buffer. Add begin_range_hunk() to open the accumulator at the first in-range line, seeding both begins from the live image cursors, as the counterpart to flush_range_hunk(). With the counting gone too, line_range_line_fn() now only appends an in-range line. Document the coordinate model: a block comment on struct line_range_filter states it (the pre/post-image cursors, the 0-based idx_in_postimage, removals classified by the following line) with a worked example. Add tests for the leaked trailing deletion this fixes, the symmetric leading-deletion case, and the filter's range boundaries (a change at the first and last line of a range, and a pure in-range deletion). Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The line-range filter builds its own "@@ -<old> +<new> @@" header for
each range hunk. For a side with no lines (count 0, such as the old
side of a pure insertion), the begin should be the number of the line
before the change, per the convention git diff and xdl_emit_hunk_hdr()
follow. The hand-rolled code's begin was one too high; in t4211 this
produced
@@ -25,0 +18,9 @@
an old begin of 25 in a 24-line file, where git diff would give 24.
Stop hand-rolling the header. flush_range_hunk() now formats it through
xdiff's own emitter: a new xdiff_emit_hunk_header() helper wraps
xdl_emit_hunk_hdr(), the function that produces every other diff's hunk
headers. The count-0 begin is then correct by construction, and as a
side effect -L headers match git diff exactly, including its omission of
a count of 1 ("@@ -22 +22 @@" rather than "@@ -22,1 +22,1 @@").
xdiff's hunk callback already hands line_range_hunk_fn() a count-0 begin
decremented, so undo that when seeding the cursors and let the formatter
re-apply the convention once, at emit time.
The off-by-one predates this series, and the two regenerated fixtures
reach it from different origins: no-assertion-error has carried it since
its test was added in ab60c69 (line-log: fix assertion error,
2025-08-18), while vanishes-early acquired it when 86e986f (line-log:
route -L output through the standard diff pipeline) reshaped its tracked
line into a pure insertion. vanishes-early also drops its count-1
counts.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin_diff() open-codes the line-range filter setup and teardown around its xdi_diff_outf() call: zero the struct, point it at the output callback, inflate ctxlen to the largest range span so each range yields a single xdiff hunk, run the diff, flush the trailing range hunk, and release the buffer. The upcoming -L stat and check formats need the same sequence. Extract line_range_filter_init() for the setup and a line_range_filter_diff() helper that prepares the xdiff config the filter needs, runs an initialized filter through xdi_diff_outf(), flushes the final range hunk, and releases it, returning the latched error. The helper inflates ctxlen to the largest range span so each range yields a single xdiff hunk, and clears XDL_EMIT_NO_HUNK_HDR so the hunk headers the filter seeds its position from are always emitted. Folding both into the helper keeps these invariants, which the filter's position tracking relies on, in a single place for every consumer. builtin_diff() now does init + line_range_filter_diff(); the next two patches reuse them in builtin_diffstat() and builtin_checkdiff() instead of repeating the boilerplate. No behavior change: builtin_diff() leaves XDL_EMIT_NO_HUNK_HDR unset, so clearing it is a no-op until the suppressing consumers arrive. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reuse the line_range_filter in builtin_diffstat() so the stat formats
count only the lines within the tracked range. When a filepair carries
line_ranges, the filter wraps diffstat_consume() as its output callback,
forwarding only the lines inside the range for counting.
flush_range_hunk() replays buffered content through diffstat_consume(),
which ignores synthetic @@ headers since it only counts '+' and '-'
lines.
Expand the output format allowlist in setup_revisions() to accept
--stat, --numstat, and --shortstat with -L.
Leave --dirstat out of the allowlist so it is rejected like any other
unsupported format. Its default mode counts each file's whole-file
byte damage via diffcore_count_changes(), outside the line-based
pipeline that the -L filter scopes, so bare --dirstat cannot honor the
tracked range. The --dirstat=lines mode could: it aggregates the same
per-file line counts as --numstat, which -L already scopes. But
accepting only that sub-mode while bare --dirstat keeps erroring is a
confusing split, so the whole format is deferred to a follow-up;
--numstat already reports the exact per-file counts within the tracked
range.
Also drop "yet" from the generic -L rejection message ("does not
yet support the requested diff format"). Some rejected formats do
not fit a line range at all, so "yet" wrongly implied they are all
just awaiting support.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin_checkdiff() runs its own xdiff pass to detect whitespace errors in newly added lines. When -L is active, the check should be scoped to the tracked line ranges rather than the whole file. Reuse the line_range_filter to wrap checkdiff_consume(), the same pattern already used for patch output and diffstat. The filter forwards only in-range lines for whitespace checking. checkdiff reports the file line number of each error, which it normally learns from the hunk header via checkdiff_consume_hunk(). The filter synthesizes its own hunk headers, so give it an optional hunk callback and route checkdiff_consume_hunk() through it; this sets the post-image position before the in-range lines are replayed. Without it the reported line numbers would count from the start of the range hunk rather than the start of the file. The trailing blank-at-eof check is a second pass that scans the whole file via check_blank_at_eof(), so gate its report on the tracked ranges as well; otherwise a blank line added at end of file is reported even when it lies outside the range. Add DIFF_FORMAT_CHECKDIFF to the -L output format allowlist in setup_revisions() so that -L --check is accepted, and list --check among the supported formats in the documentation. Add tests covering that whitespace errors are reported, scoped to the tracked range, and labeled with the correct file line number, including when two errors in one range are separated by a gap that would otherwise split into multiple xdiff hunks. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git log -L scopes its diff output to the tracked range, but pickaxe (-S, -G) still runs in diffcore over the whole-file change, so -L -G selects a commit whenever the pattern appears in any added or removed line of the file, even outside the tracked range. Teach -G to honor the range. diff_grep() already runs an xdiff pass and greps the +/- lines; route that pass through the line-range filter so only the tracked range's lines are grepped. Expose the filter as diff_emit_line_ranges(), an xdi_diff_outf() that emits only the tracked range's lines, thread the filepair's line_ranges through the pickaxe callback, and pass it from pickaxe_match(). Skip scoping under textconv, whose output is not in the original file's line coordinates. -G needs only a hit/no-hit answer, so the line-number concerns the filter handles for patch and check output do not apply here. -S is left matching the whole file: it counts needle occurrences per blob rather than grepping the diff, so scoping it needs a different approach, left to a follow-up. has_changes() takes the range parameter but ignores it for now. Document the resulting -L pickaxe scoping: -G is scoped to the tracked range, while -S still matches the whole file. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
…ures 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>
blame runs its diff by loading both blobs and then calling xdi_diff() with a hunk consumer. The content load happens before the diff request exists as a value anywhere, so a component that could answer "which line ranges changed between this pair" without computing has no point to plug in, and by the time xdiff runs both blobs have been read. Extract the request into diff_provider_emit_hunks(): the caller states the diff parameters and a hunk consumer, and hands over content loading as a callback the seam invokes when it computes. Every request today is computed, so behavior is unchanged. This is the seam where hunk providers will be consulted. A provider that can answer for a pair before its content is loaded, such as a store of precomputed hunks or a long-running external process, plugs in between the request and the computation; later commits add both, and the request will grow the pair's object ids when the first provider that keys on them lands. Blame is the first consumer because it fits the shape: it diffs blob pairs it can name before reading them, so a provider hit can skip both blob reads. Blame's -C/-M split detection diffs partial buffers with no stable pair identity and stays on xdi_diff() directly. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
mmontalbo
force-pushed
the
mm/hunk-providers
branch
from
July 30, 2026 04:40
2f8045f to
f09378b
Compare
Blame and "git log --stat" recover hunk coordinates by diffing blob pairs, and recompute them on every run because the results are not stored. Add a cache of those coordinates under $GIT_DIR/objects so a later run can look them up instead of decompressing the blobs and running xdiff again. The store is a single chunk-format file (see linkgit:gitformat-chunk[5]): an 8-byte header, a DHIX index of fixed-size entries sorted by key, a DHDT segment of hunk records, and a trailing hash checksum. An entry is keyed by the two blob object IDs and the xdl_opts the pair was diffed under, so a stored result is served only where that exact key recurs, independent of path. The hunks of a pair are not unique: a zero-context diff trims, which can pick a different but equally valid set than an untrimmed diff. A recording caller therefore stores a pair only when its trimmed and untrimmed diffs are identical; such an entry answers any consumer at any context, and the rare divergent pair is always computed. Identical hunk blocks are interned once and shared across keys. The library provides a reader (repo_diff_hunks_store and _replay, gated by core.diffHunks and always miss-tolerant), loaded once and cached on the object database as the commit-graph is, and a writer that accumulates entries and flushes them in one atomic pass. Writing is off by default and enabled per run by the GIT_DIFF_HUNKS_WRITE environment variable or the diffHunks.write config, the environment winning; a writer seeds from the existing store so a flush merges rather than replaces, and verifies the seed's checksum first so a corrupt store is discarded rather than rewritten with a fresh checksum that verify could no longer catch. The writer fsyncs through a new diff-hunks core.fsync component. "git diff-hunks" inspects and manages the file: "verify" checks the checksum, chunk table, sort order, and entry bounds, and "clear" removes it. Later patches wire the readers and the writer into the diff and blame paths. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Teach builtin_diffstat() to consult the hunk provider seam. On a hit it sums the provided hunk counts instead of decompressing the blobs and running xdiff; on a miss it computes the diff as before, and when a writer is attached it records what it computed. It records both the zero-context hunks (what blame reads) and the untrimmed hunks keyed by the configured context (what diffstat sums), so one pass serves both readers. diff_provider_query_hunks() is the seam's consult-only entry: it asks providers for the pair's ranges under the settings that key them, without loading content, and a miss leaves the consumer's own computation untouched. The diff-hunks store is the provider it consults, via diff_hunks_replay(), which validates a recorded sequence before any hunk reaches the consumer's callback; the summing callback here accumulates directly into the diffstat entry on the strength of that ordering. "git diff", "git log", "git show", and "git diff-tree" with the --stat, --numstat, and --shortstat formats consult the seam; when writing is enabled they also attach a writer, then flush it when the walk finishes. This is how the store is populated: a warming run such as GIT_DIFF_HUNKS_WRITE=1 git log --all --stat >/dev/null fills the cache as a side effect of the diff work the command already does. Reading is controlled by core.diffHunks and writing by diffHunks.write and GIT_DIFF_HUNKS_WRITE. A read-path hit is invisible in the output, so it is counted and emitted as a trace2 "read-hits" datum for tests and tuning. diff_hunks_settings_from_diffopt() is the single place that projects a diff_options down to the settings that key the store. Inputs that perturb the hunks outside the key (-B, -I, --anchored, and --ignore-blank-lines) disable both lookup and recording, so output stays identical to a store-less run. A "log -L" range-scoped stat is not the whole-pair diff the key describes, so it neither reads nor records; the line-range filter computes it as before. Add t4218 covering output parity with and without the store for the stat formats at several context lengths, the write gate (off by default, the environment overriding the config), the trim-divergent pair that is never recorded, the settings that must bypass the store, warm merging and corrupt-store discard, and verify and clear. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Before diffing a target blob against a parent, offer the pair's identity to the hunk provider seam. diff_provider_emit_hunks() now takes the repository and the pair's blob object ids and consults providers, keyed by the ids and the request's xdiff flags at zero context, before falling back to its fill-and-compute path. A hit replays the recorded hunks through blame_chunk_cb without loading either blob; a request carrying -I patterns or anchors is outside the key and always computes. Blame withholds the identity where its diff is not the plain blob-pair diff the key describes (reverse blame, ignored revisions, and textconv paths), so those requests always compute. Whitespace and algorithm options such as -w instead change blame's xdl_opts, so the consult keys a different entry and misses a store warmed without them. Blame's default xdl_opts now come from DIFF_HUNKS_DEFAULT_XDL_OPTS, the same macro the diff side uses, so a default blame run and a default "log --stat" warming run share keys by construction. "--show-stats" reports how many pairs were served, for tests and tuning. Extend t4218 with the blame side: parity for plain, --porcelain, and --incremental output, hit accounting across warming runs, the blame inputs that must bypass or miss the store (-w, indent heuristics, --reverse, textconv, -M/-C), rename and merge handling, --contents, and reading a truncated or corrupt store as absent. Add p4218 measuring warm cost and the read speedups. Signed-off-by: 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. The diff-hunks settings projection asserts xpparam_t's layout so that a new field forces an explicit keying decision; extend its reference struct with the two new fields. They carry a caller-supplied answer rather than a setting: the store paths build their xpparam_t locally and never populate them, so a recorded diff is always xdiff's own. Signed-off-by: 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>
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>
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>
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>
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. The consult rides the hunk provider seam blame already diffs through, rather than a spine of its own. diff_provider_emit_hunks() picks the producer before content is loaded, via diff_process_driver(): the entry gates of diff_process_fill_hunks() extracted into a content-free predicate. A path whose driver has a process makes the tool the producer, so the store, which holds xdiff's answer that a semantic tool may deliberately contradict, is not consulted; the seam then loads content, asks the tool, and feeds its hunks to xdiff's emission, or emits no hunks at all for a pair the tool reports equivalent. A new test pins the producer rule: a store warmed with builtin hunks does not override the tool's answer in blame. 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 predicate keys off xpp (the flags the diff actually runs with), which covers blame without a guard of its own; such a request selects no tool, so the store may still serve it. The driver is looked up 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. The seam forwards the pair's blob object ids to the tool under the same gate that keys the store, so old-oid and new-oid always name the bytes the tool receives; a tool that persists a cache across invocations can key on them. 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>
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. A process-capable driver also makes the tool the producer over the diff-hunks store: diffstat_from_hunks() steps aside for such a path, so neither a store read (it holds xdiff's answer) nor its own xdiff-and-record can stand in for the tool. Without this, a warming run with a tool configured would count and record builtin hunks the tool never saw. A new test pins the rule: a store warmed with builtin counts does not override the tool's counts. Under -L, route the surviving hunks through the same line-range filter builtin_diffstat() already uses for a tracked range, so a process-provided diff is scoped to that range: "git log -L<range> --stat" counts the tool's changed lines within the range rather than the builtin line diff's. 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, a mode-only change, a range-scoped --stat under "git log -L" that reflects the tool's hunks, and the warmed-store interplay above. Signed-off-by: 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. Route collect_diff() through the hunk provider seam, as blame is. The caller already holds the content, so its fill callback hands the loaded mmfiles to the seam. When the tool reports the files equivalent, no ranges are collected; 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. Blob oids are not threaded to this path yet, so the store is not consulted and no old-oid/new-oid is sent to the tool; a later change can supply the pair, where they would let both providers serve the range-tracking and display passes over the same commit. The driver is selected 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. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
A diff process receives the full content of both sides with every request, so a tool that already knows the answer for a blob pair (a tool that keys a persistent cache on the old-oid/new-oid it receives) still costs Git the blob loading and the content transfer. Measured on git.git, that content movement is the bulk of a consultation's cost: a consult with content averages roughly 87 microseconds per pair, while a bare round-trip over the same pipe costs roughly 6, so answering from object ids alone keeps most of the benefit of not computing the diff at all. Add a negotiated "hunks-by-oid" capability. When a tool announces it, a consumer that knows both blob object ids asks with a command=hunks-by-oid request carrying only the pathname and the pair; no content sections follow. The tool answers in the usual hunk-line form, or with status=need-content, upon which Git repeats the request as a full command=hunks exchange. Because Git holds no content for the exchange, the answer is used as the tool sent it: the hunks are validated for order, overlap, and lockstep alignment (the checks that need line counts do not apply) and replayed to the consumer without passing through xdiff's compaction, and a zero-hunk success asserts equivalence outright, including the trailing-newline case the content path detects itself; a tool that cannot promise that from its cache answers need-content. diff_process_query_hunks() carries the request. The provider seam consults it in the identity phase, so a blame over a tool-driven path never loads the blobs the tool can answer for, and diffstat_from_hunks() consults it where it steps aside for a tool path, so "git log --stat" gets the same treatment. Patch output always holds content and keeps the content request. The test backend grows oid-fixed and oid-need-content modes, and the tests pin the mechanics via the backend's request log: blame and --numstat answered with no content request sent, the need-content fallback issuing both requests, and a working-tree side (no stored object id) going straight to the content request. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
mmontalbo
force-pushed
the
mm/hunk-providers
branch
from
July 30, 2026 04:56
f09378b to
7b8410a
Compare
mmontalbo
changed the title
mmontalbo
force-pushed
the
mm/hunk-providers-base
branch
from
August 1, 2026 02:46
f67c51d to
43bc512
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Superseded by #4, which is the staging being taken forward. Kept for comparison: same functionality, protocol staged content-first with the oid capability last, before the seam-to-interface terminology sweep.