diff --git a/.gitignore b/.gitignore index 4da58c6754..4173111c01 100644 --- a/.gitignore +++ b/.gitignore @@ -56,6 +56,7 @@ /git-diagnose /git-diff /git-diff-files +/git-diff-hunks /git-diff-index /git-diff-pairs /git-diff-tree diff --git a/Documentation/Makefile b/Documentation/Makefile index f8dea4b395..cf9ba445b2 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -33,6 +33,7 @@ MAN5_TXT += gitattributes.adoc MAN5_TXT += gitformat-bundle.adoc MAN5_TXT += gitformat-chunk.adoc MAN5_TXT += gitformat-commit-graph.adoc +MAN5_TXT += gitformat-diff-hunks.adoc MAN5_TXT += gitformat-index.adoc MAN5_TXT += gitformat-loose.adoc MAN5_TXT += gitformat-pack.adoc diff --git a/Documentation/config.adoc b/Documentation/config.adoc index f67dcd2f8e..ceb13ade6e 100644 --- a/Documentation/config.adoc +++ b/Documentation/config.adoc @@ -472,6 +472,8 @@ include::config/credential.adoc[] include::config/diff.adoc[] +include::config/diff-hunks.adoc[] + include::config/difftool.adoc[] include::config/extensions.adoc[] diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc index 340329edc3..e82a09daa0 100644 --- a/Documentation/config/core.adoc +++ b/Documentation/config/core.adoc @@ -678,12 +678,13 @@ but risks losing recent work in the event of an unclean system shutdown. * `pack` hardens objects added to the repo in packfile form. * `pack-metadata` hardens packfile bitmaps and indexes. * `commit-graph` hardens the commit-graph file. +* `diff-hunks` hardens the diff-hunks store. * `index` hardens the index when it is modified. * `objects` is an aggregate option that is equivalent to `loose-object,pack`. * `reference` hardens references modified in the repo. * `derived-metadata` is an aggregate option that is equivalent to - `pack-metadata,commit-graph`. + `pack-metadata,commit-graph,diff-hunks`. * `committed` is an aggregate option that is currently equivalent to `objects`. This mode sacrifices some performance to ensure that work that is committed to the repository with `git commit` or similar commands @@ -758,6 +759,13 @@ core.commitGraph:: to parse the graph structure of commits. Defaults to true. See linkgit:git-commit-graph[1] for more information. +core.diffHunks:: + If true, then Git will consult the diff-hunks store (if it + exists) to skip recomputing diff hunk coordinates in commands + such as `git log --stat` and linkgit:git-blame[1]. This controls + only reading; writing the store is controlled by `diffHunks.write`. + See linkgit:git-diff-hunks[1] for more information. Defaults to true. + core.useReplaceRefs:: If set to `false`, behave as if the `--no-replace-objects` option was given on the command line. See linkgit:git[1] and diff --git a/Documentation/config/diff-hunks.adoc b/Documentation/config/diff-hunks.adoc new file mode 100644 index 0000000000..ad76d1c6a9 --- /dev/null +++ b/Documentation/config/diff-hunks.adoc @@ -0,0 +1,8 @@ +diffHunks.write:: + If true, diff-producing commands (`git diff`, `git log`, + `git show`, or `git diff-tree` with a `--stat`, `--numstat`, or + `--shortstat` format) write the hunks + they compute to the diff-hunks store, filling it as a side effect. + The `GIT_DIFF_HUNKS_WRITE` environment variable overrides this for + a single invocation. Reading the store is controlled separately by + `core.diffHunks`. See linkgit:git-diff-hunks[1]. Defaults to false. diff --git a/Documentation/config/diff.adoc b/Documentation/config/diff.adoc index 1135a62a0a..349bdbe492 100644 --- a/Documentation/config/diff.adoc +++ b/Documentation/config/diff.adoc @@ -218,6 +218,12 @@ 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..process`:: + The command to run as a long-running process that answers + which line ranges changed between two blobs. See + linkgit:gitattributes[5] for the protocol and when it is + consulted. + `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/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc index 8e3a0b63d7..16e6fc7261 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. ++ +Explicitly choosing a diff algorithm on the command line also +bypasses `diff..process` (see linkgit:gitattributes[5]). diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc index e605d28678..c68c53e11c 100644 --- a/Documentation/diff-options.adoc +++ b/Documentation/diff-options.adoc @@ -833,7 +833,20 @@ 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 drivers and processes, including + `diff..command` and `diff..process` + (see linkgit:gitattributes[5]). + +`--diff-process`:: +`--no-diff-process`:: + Allow (or forbid) consulting a diff process configured with + ++diff.++____++.process++ (see linkgit:gitattributes[5]), + leaving external diff drivers unaffected. `git diff`, `git log`, + `git show`, and `git blame` allow consulting by default; the + plumbing diff commands forbid it unless this option or + `--ext-diff` is given. linkgit:git-format-patch[1] accepts the + option but ignores it: a generated patch is always based on the + builtin diff. `--textconv`:: `--no-textconv`:: diff --git a/Documentation/git-diff-hunks.adoc b/Documentation/git-diff-hunks.adoc new file mode 100644 index 0000000000..25cab2ea7d --- /dev/null +++ b/Documentation/git-diff-hunks.adoc @@ -0,0 +1,146 @@ +git-diff-hunks(1) +================= + +NAME +---- +git-diff-hunks - Inspect and manage the diff-hunks store + +SYNOPSIS +-------- +[synopsis] +git diff-hunks verify +git diff-hunks clear + +DESCRIPTION +----------- + +The diff-hunks store is a cache of diff hunk coordinates, the line +ranges that changed between two blobs, so that commands +which need them, such as linkgit:git-blame[1] and `git log` and `git diff` +with the `--stat`, `--numstat`, and `--shortstat` formats, can skip +running the diff algorithm, and blame can skip loading the blob +content. (The summary formats still test each pair for binariness, +which can load the blobs.) + +The store is a single file, `$GIT_DIR/objects/info/diff-hunks`. Reading is +enabled by default; writing is off by default. A `git diff`, `git log`, +`git show`, or `git diff-tree` that produces one of the stat formats +fills the store as a side effect, but only when writing is enabled for +that run (see "WARMING THE STORE" below), so ordinary reads never +modify the repository. When the store does not have the pair, holds a +different object hash, the file is unreadable, or an object replacement +redirects one of the blobs, the consumer falls back to computing the +diff. A store only speeds up these commands; it never changes their +output. + +`git diff-hunks` itself only inspects and manages the file. See +linkgit:gitformat-diff-hunks[5] for the file format. + +WARMING THE STORE +----------------- + +The store is filled by running ordinary commands with writing enabled. +Turn writing on for a single invocation with the `GIT_DIFF_HUNKS_WRITE` +environment variable, or persistently with the `diffHunks.write` +configuration; the environment variable takes precedence. A repository +owner warms the store by running the diff-producing commands they care +about with writing on, for example: + + GIT_DIFF_HUNKS_WRITE=1 git log --all --stat >/dev/null + +A `--stat` walk records one entry per blob pair; +linkgit:git-blame[1] replays the coordinates and the summary formats +sum the counts, so a single warming walk serves both. +A warming run seeds from the existing store and rewrites the file +with the newly computed pairs merged in, so a later run adds to what +earlier runs recorded rather than discarding it. + +A walk records only the pairs it diffs. `git log --all --stat` diffs +each commit against its first parent, so a blame that follows a +merge's second parent computes those pairs itself: blame coverage is +partial on history with merges. Warming with a walk that also diffs +the other parents, for example `git log --all -m --stat`, raises +blame coverage at the cost of a larger store and a longer warming +run. + +COMMANDS +-------- + +`verify`:: + Check the integrity of the store: the trailing hash checksum, the + chunk table of contents, the sort order of the index, and the + bounds of every entry. Exits with non-zero status if the store is + corrupt. An absent store is valid. + +`clear`:: + Remove the store file. + +CORRECTNESS +----------- + +A stored result is interchangeable with a freshly computed one because an +entry is keyed by the inputs that determine the diff: + +* the object IDs of the old and new blob, so a result is used only for + the exact contents it was computed from; and +* the diff algorithm and ignore flags (`xdl_opts`) the hunks were + computed under. A lookup whose `xdl_opts` differ from a stored entry + misses. This is why, for example, `blame -w` and + `--diff-algorithm=` (including a per-path + `diff..algorithm`) do not reuse entries recorded under the + default settings: they change `xdl_opts`. + +The context length is not part of the key because only trim-stable +pairs are recorded: pairs whose zero-context trimmed diff and untrimmed +diff are identical, so one entry answers blame (zero context) and the +summary formats (any context) alike. The rare pair where +the zero-context trimming optimization picks a different but +equally valid set of hunks is +never recorded and is always computed. + +Some options shape the hunks in ways the key does not express, so a +diff that uses them is excluded from the store in both directions: +break detection (`-B`), `--ignore-matching-lines` (`-I`), and +`--anchored`. `--ignore-blank-lines` is different: it is an ignore +flag and therefore part of the key, but the summary formats exclude +it anyway, because it coalesces hunks differently between the code +path that emits text and the one that replays coordinates, so a +served answer would not match a store-less run. +linkgit:git-blame[1] additionally does not +consult the store for reverse blame, ignored revisions, or paths with a +textconv driver. + +The store carries a trailing hash checksum, but readers do not +re-checksum it on every load. As with the commit-graph and +multi-pack-index, the writer fsyncs the file (honoring `core.fsync`) and +commits it atomically, so a committed store is intact; every offset and +count is still bounds-checked as it is read. The checksum is verified by +`git diff-hunks verify`, not on the read path, so structural corruption +that fails a bounds check is read as an absent entry, while a record +that stays within bounds but whose bytes were altered is served until +`verify` detects the mismatch. + +CONFIGURATION +------------- + +`core.diffHunks`:: + Whether commands read the store. Defaults to true. See + linkgit:git-config[1]. + +`diffHunks.write`:: + Whether diff-producing commands write to the store. Defaults to + false. The `GIT_DIFF_HUNKS_WRITE` environment variable overrides it + for a single invocation. See linkgit:git-config[1]. + +Writing the store honors the `core.fsync` configuration through the +`diff-hunks` component; see linkgit:git-config[1]. + +SEE ALSO +-------- +linkgit:git-blame[1], +linkgit:git-log[1], +linkgit:gitformat-diff-hunks[5] + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc index 9fea75f96f..9a50d6338b 100644 --- a/Documentation/gitattributes.adoc +++ b/Documentation/gitattributes.adoc @@ -784,6 +784,17 @@ 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 the driver's output; 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 @@ -821,6 +832,166 @@ NOTE: If `diff..command` is defined for path with the (see above), and adding `diff..algorithm` has no effect, as the algorithm is not passed to the external diff driver. +Answering diffs from a long-running process +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Unlike `diff..command`, which replaces the textual patch, the +process configured in `diff..process` feeds hunks back into +Git's own machinery: +it answers "which line ranges changed between these two blobs", and +Git's output is produced from that answer. A process is started +lazily, once per configured command string and per repository, and +consulted over a pkt-line protocol (following the long-running filter +process protocol; see the "Long Running Filter Process" section above +for the filter analogue). + +The process is asked by object names alone: a request carries the +pathname and the `old-oid`/`new-oid` of the blob pair, and no content. +The pathname is the repository-relative old-side (preimage) path, not +the shortened display path a `--relative` diff shows, so a driver +scoped to a directory matches whatever directory the command runs from. +This suits a process that keeps a persistent cache keyed on the pair, and +a process that fetches the blobs itself (for example via +`git cat-file --batch`) to compute its own notion of the changed +lines. Pairs with a side that is not a stored blob (a working-tree +file, textconv output) are not sent; Git computes those itself. + +The exchange opens with a handshake: Git announces its role and +version, the process replies in kind, and then Git lists the +capabilities it supports and the process replies with the ones it +implements. A process that announces a capability Git did not list +aborts the command. + +----------------------- +packet: git> git-diff-client +packet: git> version=1 +packet: git> 0000 +packet: git< git-diff-server +packet: git< version=1 +packet: git< 0000 +packet: git> capability=hunks-by-oid +packet: git> 0000 +packet: git< capability=hunks-by-oid +packet: git< 0000 +----------------------- + +After the handshake, each request and response looks like: + +----------------------- +packet: git> command=hunks-by-oid +packet: git> pathname=path/file.c +packet: git> old-oid= +packet: git> new-oid= +packet: git> 0000 +packet: git< hunk +packet: git< 0000 +packet: git< status=success +packet: git< 0000 +----------------------- + +Start values are 1-based and counts are non-negative; a count of 0 +describes a pure insertion or deletion at the 1-based line the change +sits before (a start of 0 is accepted for an empty file side). Hunks +must be listed in order, must not overlap, and must keep the unchanged +runs between them the same length on both sides; Git validates this +and, with a warning, falls back to its builtin diff on a response +that violates these rules. + +A `status=success` response with zero hunks asserts that the blobs are +equivalent, including their trailing newlines. A process that cannot +answer a pair from its object names (or cannot rule out a +trailing-newline-only difference) responds `status=need-content`, and +Git produces that pair's diff itself. An asserted equivalence makes +the pair vanish from the summary formats, but the pair still counts +as changed for `--exit-code`, the same way a whitespace-only pair +does under `-w`. + +The status names the disposition of the whole request. Git +understands three: `success` (the hunk lines are the answer), +`need-content` (Git produces this pair's diff itself), and `abort`, which +withdraws the capability the request used: Git stops sending +`hunks-by-oid` requests to that process for the rest of the command, +while the process stays alive for request forms negotiated under +other capabilities. Any other status is a protocol error: Git warns, +stops the process, and uses the builtin diff for the remainder of the +command. + +Every response has the same shape whatever its status: zero or more +hunk lines, a flush packet, and a status packet terminated with a +flush packet. A response that carries no hunks, `need-content` +included, still begins with the empty hunk section's flush packet; a +bare status packet is a protocol error: + +----------------------- +packet: git< 0000 +packet: git< status=need-content +packet: git< 0000 +----------------------- + +The process must read the entire request before it responds; Git +writes the whole request before it reads the response. + +The protocol extends without breaking deployed processes: a process +must ignore request keys it does not recognize, and Git ignores +trailing space-separated tokens after the last field of a hunk line, +so a later protocol version can append request keys and hunk fields. +New request forms arrive as capabilities, which a process may decline +to announce; announcing a capability Git did not request aborts the +command, as it does under the long-running filter process protocol. + +There is no shutdown handshake: Git's side of the pipes closes when +the command exits, and the process should exit when it reads EOF. No +flush point is guaranteed, so a process that maintains persistent +state (such as a cache) should persist as it answers rather than at +exit. Git applies no timeout to a response; a process that hangs +hangs the command, as with the long-running filter processes. + +`git blame` and the `--stat`, `--numstat`, and `--shortstat` formats +consult the process; the textual patch and `git log -L` range +tracking are produced by the builtin machinery, so a process whose +answers deliberately differ from the builtin diff shows that +difference only in blame and those formats. `--dirstat=lines` routes +through the diffstat path and consults; the other `--dirstat` modes do +not. A merge's `--stat` (including under `--cc`) is computed against +the first parent, so it consults like any other stat; the combined +patch itself compares one merge result against all of its parents at +once, which the pairwise request above does not express, so that patch +uses the builtin diff, and extending the protocol to combined diffs is +left for future work. A content-carrying extension of this +protocol would bring patch output and `git log -L` range tracking to +the same answer. + +Consulting is allowed per command, as with textconv: `git diff`, +`git log` (`git whatchanged` included) and `git show`, and +`git blame` consult a configured process; the plumbing diff commands +do not unless `--ext-diff` or `--diff-process` is given explicitly, +and the interactive-patch commands (`git add -p` and friends), which +build the hunks they present from plumbing output, always stage from +the builtin diff. `git range-diff` generates the patches it +compares with `--no-ext-diff`. `--diff-process` and +`--no-diff-process` allow or forbid only the consulting; +`--no-ext-diff` disables all external diff mechanisms, this one +included. Options the process is never told about never select it: +with the whitespace-ignoring options, `--ignore-matching-lines`, and +`--anchored`, the pair is answered as when no process is configured. +`--diff-algorithm` (or a configured `diff.algorithm`) forces a builtin +algorithm and bypasses the process the same way. A per-path +++diff.++____++.algorithm++ does so for `git diff` and the stat +formats, which build their diff parameters from it; `git blame` builds +its parameters from its own diff options, so a per-driver algorithm +does not by itself keep blame from consulting the process. +`git format-patch` never +consults the process, so generated patches are always based on the +builtin diff and apply for recipients without the process. On a +path whose driver has a process, the process is consulted before the +diff-hunks store (see linkgit:git-diff-hunks[1]): a pair the process +answers is never served from the store and never recorded into it. +A pair the process does not answer, for example with +`status=need-content`, gets the builtin diff, so the store may serve +it and a warming run may record it: the store holds builtin results, +and for such a pair the builtin result is what would be computed +anyway. + Defining a custom hunk-header ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/Documentation/gitformat-diff-hunks.adoc b/Documentation/gitformat-diff-hunks.adoc new file mode 100644 index 0000000000..f75ab73dd9 --- /dev/null +++ b/Documentation/gitformat-diff-hunks.adoc @@ -0,0 +1,129 @@ +gitformat-diff-hunks(5) +======================= + +NAME +---- +gitformat-diff-hunks - Precomputed diff hunk store format + +SYNOPSIS +-------- +[verse] +$GIT_DIR/objects/info/diff-hunks + +DESCRIPTION +----------- + +The diff-hunks store memoizes diff hunk coordinates so that commands +that need them, such as `git log --stat` and linkgit:git-blame[1], can +skip running the diff algorithm (and, for blame, loading the blob +content; the summary formats still test each pair for binariness, +which can load the blobs). See +linkgit:git-diff-hunks[1] for how the store is filled and managed and the +configuration that controls it. + +The store is a single file, `$GIT_DIR/objects/info/diff-hunks`, written +in one pass and replaced atomically, so a reader sees either the old +file or the complete new one. + +Entries are keyed by the object IDs of the blob pair that was diffed +and by the diff algorithm and ignore flags (`xdl_opts`) the pair was +diffed under. A blob pair fully determines the diff input, so an entry +is valid regardless of which commits, branches, or index states the +pair was encountered in, and identical diffs performed in different +contexts share one entry. A reader whose `xdl_opts` differ from an +entry does not match it and falls back to computing the diff. + +FILE FORMAT +----------- + +All multi-byte integers are stored in network byte order. The file is an +8-byte header, the chunk table of contents and chunk data described in +linkgit:gitformat-chunk[5], and a trailing checksum. + +HEADER +~~~~~~ + +- 4-byte signature: `DHPF` (diff-hunks precomputed format) +- 1-byte version number: currently 1 +- 1-byte hash version: 1 for SHA-1, 2 for SHA-256. A store whose hash + function differs from the repository's is ignored. +- 1-byte number of chunks +- 1-byte reserved + +CHUNK LOOKUP +~~~~~~~~~~~~ + +A table of contents in the format of linkgit:gitformat-chunk[5], listing +the offset of each chunk. Both chunks below are required; a file missing +either is treated as corrupt. + +CHUNK DATA +~~~~~~~~~~ + +DHIX (index):: + A sorted sequence of fixed-size entries. Each entry is the old + blob object ID, the new blob object ID, a 4-byte `xdl_opts` + value, and a 4-byte offset into the DHDT chunk. Entries are + sorted by old object ID, then new object ID, then `xdl_opts`, + so lookups can use binary search on the full key. + +DHDT (hunk data):: + For each index entry, at its offset: a 4-byte hunk count followed + by that many 16-byte hunk records. A hunk record is four 4-byte + values: old start, old count, new start, new count. + Starts are 0-based line numbers in the old and new blob; counts + are numbers of lines. The hunk count is at least 1: a record with + no hunks would claim the blob pair equivalent, which the store + never records, so readers treat such a record as invalid. + Identical hunk blocks are stored once: + distinct index entries whose recorded hunks are byte-for-byte + equal point at the same offset. + +TRAILER +~~~~~~~ + +A checksum of all preceding bytes, computed with the repository hash +function. + +CORRECTNESS +----------- + +Serving hunks from a valid store produces the same output as recomputing +the diff. The diff of a blob pair is not unique: a zero context length +triggers xdiff's common-tail trimming, which can pick a different but +equally valid set of hunks than an untrimmed diff does. A pair is +therefore recorded only when its trimmed and untrimmed diffs are +identical, which is the common case. Such an entry answers any consumer +at any context: git-blame replays its coordinates directly (it diffs at +zero context), and diffstat sums its per-hunk line counts, which the +context length does not change. The rare pair whose two diffs differ is +never recorded, so every consumer computes it. + +A store that cannot be used is ignored, and the consumer falls back to +computing the diff. Every offset and count read from the file is +bounds-checked, so a store that is missing, truncated, of an unknown +version, or of a different object hash does not change the diff output +and does not produce a diagnostic; `git diff-hunks verify` is what +reports corruption. + +The store is not re-checksummed on the read path. The writer fsyncs the +file (honoring `core.fsync`) and commits it atomically, so a +committed store is intact, the same trust model the commit-graph and +multi-pack-index use. The trailing checksum is recomputed by +`git diff-hunks verify` to detect corruption. + +The checksum detects corruption but does not prove who wrote the file. A +reader trusts the coordinates in a store that passes its checks, so +anything able to write a checksum-valid file at the store path can +influence output, the same as it could by writing objects directly. + +LIMITATIONS +----------- + +- Hunk counts, offsets, and line coordinates are 32-bit, capping the + hunk data at 4 GiB and a single entry at roughly 268 million hunks. + A result whose coordinates cannot be represented is not recorded. + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Documentation/meson.build b/Documentation/meson.build index f4854f802d..85f37da47e 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -41,6 +41,7 @@ manpages = { 'git-describe.adoc' : 1, 'git-diagnose.adoc' : 1, 'git-diff-files.adoc' : 1, + 'git-diff-hunks.adoc' : 1, 'git-diff-index.adoc' : 1, 'git-diff-pairs.adoc' : 1, 'git-difftool.adoc' : 1, @@ -175,6 +176,7 @@ manpages = { 'gitformat-bundle.adoc' : 5, 'gitformat-chunk.adoc' : 5, 'gitformat-commit-graph.adoc' : 5, + 'gitformat-diff-hunks.adoc' : 5, 'gitformat-index.adoc' : 5, 'gitformat-loose.adoc' : 5, 'gitformat-pack.adoc' : 5, diff --git a/Makefile b/Makefile index 0232702520..c21e2ca5d8 100644 --- a/Makefile +++ b/Makefile @@ -822,6 +822,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 @@ -1154,6 +1155,8 @@ 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-provider.o LIB_OBJS += diff.o LIB_OBJS += diffcore-break.o LIB_OBJS += diffcore-delta.o @@ -1161,6 +1164,7 @@ LIB_OBJS += diffcore-order.o LIB_OBJS += diffcore-pickaxe.o LIB_OBJS += diffcore-rename.o LIB_OBJS += diffcore-rotate.o +LIB_OBJS += diff-hunks.o LIB_OBJS += dir-iterator.o LIB_OBJS += dir.o LIB_OBJS += editor.o @@ -1424,6 +1428,7 @@ BUILTIN_OBJS += builtin/credential.o BUILTIN_OBJS += builtin/describe.o BUILTIN_OBJS += builtin/diagnose.o BUILTIN_OBJS += builtin/diff-files.o +BUILTIN_OBJS += builtin/diff-hunks.o BUILTIN_OBJS += builtin/diff-index.o BUILTIN_OBJS += builtin/diff-pairs.o BUILTIN_OBJS += builtin/diff-tree.o diff --git a/blame.c b/blame.c index 126e232416..3189bcdaba 100644 --- a/blame.c +++ b/blame.c @@ -23,6 +23,8 @@ #include "commit-slab.h" #include "bloom.h" #include "commit-graph.h" +#include "diff-provider.h" +#include "userdiff.h" define_commit_slab(blame_suspects, struct blame_origin *); static struct blame_suspects blame_suspects; @@ -1936,6 +1938,46 @@ static int blame_chunk_cb(long start_a, long count_a, return 0; } +/* + * A hunk provider's key names the (old blob, new blob) pair and may only + * serve a diff whose result is determined by that pair and the xdiff + * settings. Textconv rewrites the buffers being diffed away from the + * blob contents the key names, so any origin whose path has a textconv + * driver must withhold the pair's identity. + */ +static int blame_textconv_active(struct blame_scoreboard *sb, + const char *path) +{ + struct userdiff_driver *drv; + + if (!sb->revs->diffopt.flags.allow_textconv) + return 0; + drv = userdiff_find_by_path(sb->repo->index, path); + return drv && drv->textconv; +} + +struct blame_diff_fill_data { + struct blame_scoreboard *sb; + struct blame_origin *parent, *target; + int ignore_diffs; +}; + +/* + * Content load for diff_provider_emit_hunks(): runs when the diff is + * computed. + */ +static int blame_diff_fill(void *data, mmfile_t *old_file, mmfile_t *new_file) +{ + struct blame_diff_fill_data *f = data; + + fill_origin_blob(&f->sb->revs->diffopt, f->parent, old_file, + &f->sb->num_read_blob, f->ignore_diffs); + fill_origin_blob(&f->sb->revs->diffopt, f->target, new_file, + &f->sb->num_read_blob, f->ignore_diffs); + f->sb->num_get_patch++; + return 0; +} + /* * We are looking at the origin 'target' and aiming to pass blame * for the lines it is suspected to its parent. Run diff to find @@ -1945,9 +1987,12 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, struct blame_origin *target, struct blame_origin *parent, int ignore_diffs) { - mmfile_t file_p, file_o; struct blame_chunk_cb_data d; struct blame_entry *newdest = NULL; + struct blame_diff_fill_data fill_data = { sb, parent, target, ignore_diffs }; + xpparam_t xpp = { .flags = sb->xdl_opts }; + struct diff_provider_request req = { .repo = sb->repo, .xpp = &xpp }; + int provider_usable; if (!target->suspects) return; /* nothing remains for this target */ @@ -1958,13 +2003,35 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, d.ignore_diffs = ignore_diffs; d.dstq = &newdest; d.srcq = &target->suspects; - fill_origin_blob(&sb->revs->diffopt, parent, &file_p, - &sb->num_read_blob, ignore_diffs); - fill_origin_blob(&sb->revs->diffopt, target, &file_o, - &sb->num_read_blob, ignore_diffs); - sb->num_get_patch++; + /* + * Offer the pair's identity only where blame's diff is the plain + * blob-pair diff the recording key describes; reverse blame, + * ignored revisions, and textconv paths withhold it and always + * compute. The working-tree/--contents pseudo-commit (marked by + * its null commit id) holds a blob that is not a stored object, + * so its pairs withhold identity too: no id may be sent that + * names bytes a process cannot look up. + */ + provider_usable = !sb->reverse && !ignore_diffs && + !is_null_oid(&target->commit->object.oid) && + !blame_textconv_active(sb, target->path) && + !blame_textconv_active(sb, parent->path); - if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts)) + /* + * 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 and blame. A process that reports a pair + * equivalent emits no hunks, so blame passes the whole commit + * through and looks past it. + */ + if (provider_usable) { + req.old_oid = &parent->blob_oid; + req.new_oid = &target->blob_oid; + } + req.path = parent->path; + req.diffopt = &sb->revs->diffopt; + if (diff_provider_emit_hunks(&req, blame_diff_fill, &fill_data, + blame_chunk_cb, &d) == DIFF_PROVIDER_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/builtin.h b/builtin.h index 4e47a4ebd3..7e64da9f43 100644 --- a/builtin.h +++ b/builtin.h @@ -175,6 +175,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix, struct int cmd_describe(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_diagnose(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_diff_files(int argc, const char **argv, const char *prefix, struct repository *repo); +int cmd_diff_hunks(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_diff_index(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_diff(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_diff_pairs(int argc, const char **argv, const char *prefix, struct repository *repo); diff --git a/builtin/blame.c b/builtin/blame.c index 48d5251c6d..13e3ff9e36 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -15,6 +15,7 @@ #include "hex.h" #include "commit.h" #include "diff.h" +#include "diff-hunks.h" #include "revision.h" #include "quote.h" #include "string-list.h" @@ -1024,6 +1025,7 @@ int cmd_blame(int argc, repo_init_revisions(the_repository, &revs, NULL); revs.date_mode = blame_date_mode; revs.diffopt.flags.allow_textconv = 1; + revs.diffopt.flags.allow_diff_process = 1; revs.diffopt.flags.follow_renames = 1; save_commit_buffer = 0; @@ -1060,7 +1062,7 @@ int cmd_blame(int argc, parse_done: revision_opts_finish(&revs); no_whole_file_rename = !revs.diffopt.flags.follow_renames; - xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC; + xdl_opts |= revs.diffopt.xdl_opts & DIFF_HUNKS_DEFAULT_XDL_OPTS; revs.diffopt.flags.follow_renames = 0; argc = parse_options_end(&ctx); @@ -1315,9 +1317,14 @@ parse_done: output(&sb, output_option); if (show_stats) { + unsigned long hunk_hits, hunk_misses; + + diff_hunks_read_stats(sb.repo, &hunk_hits, &hunk_misses); printf("num read blob: %d\n", sb.num_read_blob); printf("num get patch: %d\n", sb.num_get_patch); printf("num commits: %d\n", sb.num_commits); + printf("num precomputed hits: %lu\n", hunk_hits); + printf("num precomputed misses: %lu\n", hunk_misses); } cleanup: diff --git a/builtin/diff-hunks.c b/builtin/diff-hunks.c new file mode 100644 index 0000000000..3aea2dad56 --- /dev/null +++ b/builtin/diff-hunks.c @@ -0,0 +1,53 @@ +#include "builtin.h" +#include "config.h" +#include "diff-hunks.h" +#include "gettext.h" +#include "parse-options.h" +#include "repository.h" + +static const char * const diff_hunks_usage[] = { + N_("git diff-hunks verify"), + N_("git diff-hunks clear"), + NULL +}; + +static int cmd_diff_hunks_verify(int argc, const char **argv, + const char *prefix UNUSED, + struct repository *r) +{ + struct option options[] = { OPT_END() }; + + argc = parse_options(argc, argv, NULL, options, diff_hunks_usage, 0); + if (argc) + usage_with_options(diff_hunks_usage, options); + return diff_hunks_verify(r) ? 1 : 0; +} + +static int cmd_diff_hunks_clear(int argc, const char **argv, + const char *prefix UNUSED, + struct repository *r) +{ + struct option options[] = { OPT_END() }; + + argc = parse_options(argc, argv, NULL, options, diff_hunks_usage, 0); + if (argc) + usage_with_options(diff_hunks_usage, options); + return diff_hunks_clear(r) ? 1 : 0; +} + +int cmd_diff_hunks(int argc, const char **argv, const char *prefix, + struct repository *repo) +{ + parse_opt_subcommand_fn *fn = NULL; + struct option options[] = { + OPT_SUBCOMMAND("verify", &fn, cmd_diff_hunks_verify), + OPT_SUBCOMMAND("clear", &fn, cmd_diff_hunks_clear), + OPT_END() + }; + + repo_config(repo, git_default_config, NULL); + + argc = parse_options(argc, argv, prefix, options, diff_hunks_usage, 0); + + return fn(argc, argv, prefix, repo); +} diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c index 8b8f8b54e4..296c6a137e 100644 --- a/builtin/diff-tree.c +++ b/builtin/diff-tree.c @@ -170,6 +170,8 @@ int cmd_diff_tree(int argc, opt->diffopt.rotate_to_strict = 1; + diff_hunks_attach(&opt->diffopt); + /* * NOTE! We expect "a..b" to expand to "^a b" but it is * perfectly valid for revision range parser to yield "b ^a", @@ -234,5 +236,6 @@ int cmd_diff_tree(int argc, diff_free(&opt->diffopt); } + diff_hunks_detach(&opt->diffopt); return diff_result_code(opt); } diff --git a/builtin/diff.c b/builtin/diff.c index 18b1083e98..a39ffe69a4 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -510,6 +510,7 @@ int cmd_diff(int argc, init_diffstat_widths(&rev.diffopt); rev.diffopt.flags.allow_external = 1; rev.diffopt.flags.allow_textconv = 1; + rev.diffopt.flags.allow_diff_process = 1; /* If this is a no-index diff, just run it and exit there. */ if (no_index) @@ -568,6 +569,15 @@ int cmd_diff(int argc, } } + /* + * The hunk store is keyed by blob pair, so any diff whose + * file pairs carry known blob object IDs (tree-to-tree, + * index-to-tree) can consult the same entries that + * "git log --stat" and "git blame" use; pairs without known + * blobs bypass it at lookup time. + */ + diff_hunks_attach(&rev.diffopt); + symdiff_prepare(&rev, &sdiff); for (i = 0; i < rev.pending.nr; i++) { struct object_array_entry *entry = &rev.pending.objects[i]; @@ -648,6 +658,7 @@ int cmd_diff(int argc, result = diff_result_code(&rev); if (1 < rev.diffopt.skip_stat_unmatch) refresh_index_quietly(); + diff_hunks_detach(&rev.diffopt); release_revisions(&rev); object_array_clear(&ent); symdiff_release(&sdiff); diff --git a/builtin/log.c b/builtin/log.c index 350b35c556..59ecd34334 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -209,6 +209,7 @@ static void cmd_log_init_defaults(struct rev_info *rev, init_diffstat_widths(&rev->diffopt); rev->diffopt.flags.recursive = 1; rev->diffopt.flags.allow_textconv = 1; + rev->diffopt.flags.allow_diff_process = 1; rev->abbrev_commit = cfg->default_abbrev_commit; rev->show_root_diff = cfg->default_show_root; rev->subject_prefix = cfg->fmt_patch_subject_prefix; @@ -693,8 +694,11 @@ int cmd_show(int argc, opt.tweak = show_setup_revisions_tweak; cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg); + diff_hunks_attach(&rev.diffopt); + if (!rev.no_walk) { ret = cmd_log_walk(&rev); + diff_hunks_detach(&rev.diffopt); release_revisions(&rev); log_config_release(&cfg); return ret; @@ -767,6 +771,7 @@ int cmd_show(int argc, } rev.diffopt.no_free = 0; + diff_hunks_detach(&rev.diffopt); diff_free(&rev.diffopt); release_revisions(&rev); log_config_release(&cfg); @@ -846,8 +851,11 @@ int cmd_log(int argc, opt.tweak = log_setup_revisions_tweak; cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg); + diff_hunks_attach(&rev.diffopt); + ret = cmd_log_walk(&rev); + diff_hunks_detach(&rev.diffopt); release_revisions(&rev); log_config_release(&cfg); return ret; @@ -2218,6 +2226,17 @@ int cmd_format_patch(int argc, if (argc > 1) die(_("unrecognized argument: %s"), argv[1]); + /* + * Patches generated by format-patch must be based on the builtin + * diff so recipients without the store or the process can apply + * them, and so the emitted diffstat does not depend on the sender's + * local cache: the precomputed-hunks store is not consulted for the + * diffstat, and the diff process is not consulted even when + * --ext-diff enables the external diff command. + */ + rev.diffopt.flags.no_precomputed_hunks = 1; + rev.diffopt.flags.allow_diff_process = 0; + 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/chunk-format.c b/chunk-format.c index 51b5a2c959..34ab2750f7 100644 --- a/chunk-format.c +++ b/chunk-format.c @@ -101,12 +101,14 @@ cleanup: return result; } -int read_table_of_contents(struct chunkfile *cf, - const unsigned char *mfile, - size_t mfile_size, - uint64_t toc_offset, - int toc_length, - unsigned expected_alignment) +static int read_table_of_contents_1(struct chunkfile *cf, + const unsigned char *mfile, + size_t mfile_size, + uint64_t toc_offset, + int toc_length, + unsigned expected_alignment, + const struct git_hash_algo *algo, + int quiet) { int i; uint32_t chunk_id; @@ -121,12 +123,14 @@ int read_table_of_contents(struct chunkfile *cf, chunk_offset = get_be64(table_of_contents + 4); if (!chunk_id) { - error(_("terminating chunk id appears earlier than expected")); + if (!quiet) + error(_("terminating chunk id appears earlier than expected")); return 1; } if (chunk_offset % expected_alignment != 0) { - error(_("chunk id %"PRIx32" not %d-byte aligned"), - chunk_id, expected_alignment); + if (!quiet) + error(_("chunk id %"PRIx32" not %d-byte aligned"), + chunk_id, expected_alignment); return 1; } @@ -134,16 +138,18 @@ int read_table_of_contents(struct chunkfile *cf, next_chunk_offset = get_be64(table_of_contents + 4); if (next_chunk_offset < chunk_offset || - next_chunk_offset > mfile_size - the_hash_algo->rawsz) { - error(_("improper chunk offset(s) %"PRIx64" and %"PRIx64""), - chunk_offset, next_chunk_offset); + next_chunk_offset > mfile_size - algo->rawsz) { + if (!quiet) + error(_("improper chunk offset(s) %"PRIx64" and %"PRIx64""), + chunk_offset, next_chunk_offset); return -1; } for (i = 0; i < cf->chunks_nr; i++) { if (cf->chunks[i].id == chunk_id) { - error(_("duplicate chunk ID %"PRIx32" found"), - chunk_id); + if (!quiet) + error(_("duplicate chunk ID %"PRIx32" found"), + chunk_id); return -1; } } @@ -156,13 +162,39 @@ int read_table_of_contents(struct chunkfile *cf, chunk_id = get_be32(table_of_contents); if (chunk_id) { - error(_("final chunk has non-zero id %"PRIx32""), chunk_id); + if (!quiet) + error(_("final chunk has non-zero id %"PRIx32""), chunk_id); return -1; } return 0; } +int read_table_of_contents(struct chunkfile *cf, + const unsigned char *mfile, + size_t mfile_size, + uint64_t toc_offset, + int toc_length, + unsigned expected_alignment) +{ + return read_table_of_contents_1(cf, mfile, mfile_size, toc_offset, + toc_length, expected_alignment, + the_hash_algo, 0); +} + +int read_table_of_contents_quiet(struct chunkfile *cf, + const unsigned char *mfile, + size_t mfile_size, + uint64_t toc_offset, + int toc_length, + unsigned expected_alignment, + const struct git_hash_algo *algo) +{ + return read_table_of_contents_1(cf, mfile, mfile_size, toc_offset, + toc_length, expected_alignment, + algo, 1); +} + struct pair_chunk_data { const unsigned char **p; size_t *size; diff --git a/chunk-format.h b/chunk-format.h index 212a0a6af1..bc31302ed0 100644 --- a/chunk-format.h +++ b/chunk-format.h @@ -39,6 +39,20 @@ int read_table_of_contents(struct chunkfile *cf, int toc_length, unsigned expected_alignment); +/* + * Like read_table_of_contents(), for a reader that treats a malformed + * table as an absent file rather than reporting it: nothing is printed + * on failure, and the trailing-checksum bound is computed with the + * given hash algorithm instead of the_hash_algo. + */ +int read_table_of_contents_quiet(struct chunkfile *cf, + const unsigned char *mfile, + size_t mfile_size, + uint64_t toc_offset, + int toc_length, + unsigned expected_alignment, + const struct git_hash_algo *algo); + #define CHUNK_NOT_FOUND (-2) /* diff --git a/command-list.txt b/command-list.txt index 21b802c420..e7b241e6ad 100644 --- a/command-list.txt +++ b/command-list.txt @@ -95,6 +95,7 @@ git-describe mainporcelain git-diagnose ancillaryinterrogators git-diff mainporcelain info git-diff-files plumbinginterrogators +git-diff-hunks plumbingmanipulators git-diff-index plumbinginterrogators git-diff-pairs plumbinginterrogators git-diff-tree plumbinginterrogators @@ -223,6 +224,7 @@ gitfaq guide gitformat-bundle developerinterfaces gitformat-chunk developerinterfaces gitformat-commit-graph developerinterfaces +gitformat-diff-hunks developerinterfaces gitformat-index developerinterfaces gitformat-pack developerinterfaces gitformat-signature developerinterfaces diff --git a/diff-hunks.c b/diff-hunks.c new file mode 100644 index 0000000000..9830df07fa --- /dev/null +++ b/diff-hunks.c @@ -0,0 +1,1034 @@ +/* + * Precomputed diff hunks, keyed by diff input. + * + * A single store at .git/objects/info/diff-hunks maps an (old blob, + * new blob, xdl_opts) key to the hunk coordinates of diffing the pair. + * The key determines the diff result (only trim-stable pairs are + * recorded; see diff-hunks.h), so an entry is valid in any context it + * recurs in, independent of path. Reading is on by default + * (core.diffHunks); writing is off by default and enabled per run or + * by configuration (see diff_hunks_write_enabled), so an ordinary + * command populates the store only during a warming run the + * repository owner opts into. + * + * File layout: + * Header: "DHPF"(4) + version(1) + hash_version(1) + * + num_chunks(1) + reserved(1) + * Table of contents (chunk-format) + * DHIX chunk: sorted entries, each + * old_blob_oid, new_blob_oid, xdl_opts(4), hdat_offset(4) + * DHDT chunk: per entry, num_hunks(4) followed by that many 16-byte hunks + * Trailing hash checksum + */ +#include "git-compat-util.h" +#include "chunk-format.h" +#include "config.h" +#include "csum-file.h" +#include "diff-hunks.h" +#include "diff-provider-internal.h" +#include "diff.h" +#include "gettext.h" +#include "hash.h" +#include "hashmap.h" +#include "lockfile.h" +#include "odb.h" +#include "path.h" +#include "repo-settings.h" +#include "repository.h" +#include "strbuf.h" +#include "wrapper.h" + +#define DIFF_HUNKS_SIGNATURE 0x44485046 /* "DHPF" */ +/* + * Bump when the on-disk format changes, or when xdiff's emitted hunk + * coordinates change for a fixed (blobs, xdl_opts) key: an old store + * would otherwise serve stale hunks and change command output. + */ +#define DIFF_HUNKS_VERSION 1 +#define DIFF_HUNKS_HEADER_SIZE 8 + +#define DIFF_HUNKS_CHUNKID_INDEX 0x44484958 /* "DHIX" */ +#define DIFF_HUNKS_CHUNKID_DATA 0x44484454 /* "DHDT" */ + +/* + * Each hunk is 16 bytes on disk: + * old_start(4) old_count(4) new_start(4) new_count(4) + */ +#define DIFF_HUNKS_HUNK_SIZE (4 * sizeof(uint32_t)) + +/* + * Result of a store lookup: num_hunks records encoded in the store's mmap, + * valid until the store is freed. Read them with nth_precomputed_hunk(). + */ +struct precomputed_entry { + uint32_t num_hunks; + const unsigned char *hunk_data; +}; + +/* Decode a single hunk from the raw on-disk format. */ +static inline void decode_precomputed_hunk(const unsigned char *data, + struct precomputed_hunk *h) +{ + h->old_start = get_be32(data); + h->old_count = get_be32(data + 4); + h->new_start = get_be32(data + 8); + h->new_count = get_be32(data + 12); +} + +/* Decode the nth hunk of a lookup result into *h. */ +static inline void nth_precomputed_hunk(const struct precomputed_entry *e, + uint32_t n, struct precomputed_hunk *h) +{ + decode_precomputed_hunk(e->hunk_data + (size_t)n * DIFF_HUNKS_HUNK_SIZE, h); +} + +/* Byte length of the (old_oid, new_oid, xdl_opts) lookup key. */ +static size_t store_index_key_size(const struct git_hash_algo *algo) +{ + return 2 * algo->rawsz + sizeof(uint32_t); +} + +/* Index entry: the lookup key followed by the 4-byte offset into DHDT. */ +static size_t store_index_entry_size(const struct git_hash_algo *algo) +{ + return store_index_key_size(algo) + sizeof(uint32_t); +} + +/* + * The smallest a valid store file can be: the header, a table of contents + * with one entry per chunk plus a terminating entry, and the trailing + * checksum. + */ +static size_t store_min_size(const struct git_hash_algo *algo, + uint8_t num_chunks) +{ + size_t toc_size = (num_chunks + 1) * CHUNK_TOC_ENTRY_SIZE; + + return DIFF_HUNKS_HEADER_SIZE + toc_size + algo->rawsz; +} + +/* + * Decode an index entry's key into pointers to the two oids and the + * xdl_opts value (on-disk: old_oid, new_oid, then xdl_opts as a + * big-endian uint32). + */ +static void decode_store_index_key(const unsigned char *entry, unsigned int rawsz, + const unsigned char **old_hash, + const unsigned char **new_hash, + uint32_t *xdl_opts) +{ + *old_hash = entry; + *new_hash = entry + rawsz; + *xdl_opts = get_be32(entry + 2 * rawsz); +} + +/* The DHDT offset stored in an index entry, in the field after its key. */ +static uint32_t index_entry_hdat_offset(const unsigned char *entry, size_t keysz) +{ + return get_be32(entry + keysz); +} + +static char *diff_hunks_store_path(struct repository *r) +{ + return xstrfmt("%s/info/diff-hunks", repo_get_object_directory(r)); +} + +struct diff_hunks_store { + const unsigned char *data; + size_t data_len; + const struct git_hash_algo *hash_algo; + const unsigned char *index; + uint32_t num_entries; + const unsigned char *hdat; + size_t hdat_size; + + /* Consultation counters; see diff_hunks_read_stats(). */ + unsigned long read_hits; + unsigned long read_misses; +}; + +static void free_store(struct diff_hunks_store *s) +{ + if (!s) + return; + if (s->data) + munmap((void *)s->data, s->data_len); + free(s); +} + +/* + * Open, mmap, and parse the store at fname. Returns the parsed store + * or NULL on any error. The diff output is unaffected either way; + * corruption is reported by verify, not treated as fatal here. + */ +static struct diff_hunks_store *load_store_at( + const struct git_hash_algo *repo_algo, const char *fname) +{ + struct diff_hunks_store *s; + struct chunkfile *cf; + int fd; + struct stat st; + void *data; + const unsigned char *p; + uint8_t num_chunks; + size_t index_size, entry_size, data_len; + + fd = git_open(fname); + if (fd < 0) + return NULL; + if (fstat(fd, &st) || st.st_size < DIFF_HUNKS_HEADER_SIZE) { + close(fd); + return NULL; + } + data_len = xsize_t(st.st_size); + data = xmmap(NULL, data_len, PROT_READ, MAP_PRIVATE, fd, 0); + close(fd); + p = data; + + num_chunks = p[6]; + + /* + * Reject a file that is not a readable store: wrong signature, + * version, or object hash, or too small to hold the table of + * contents that read_table_of_contents() walks (it dereferences + * each entry before range-checking its offset). + */ + if (get_be32(p) != DIFF_HUNKS_SIGNATURE || + p[4] != DIFF_HUNKS_VERSION || + p[5] != oid_version(repo_algo) || + data_len < store_min_size(repo_algo, num_chunks)) { + munmap(data, data_len); + return NULL; + } + + /* + * The trailing checksum is not verified here: the writer fsyncs + * and commits atomically, so a committed file is intact, and + * every record is bounds-checked at read (see precomputed_entry_at). + * The checksum is checked separately, by diff_hunks_verify(). + */ + + CALLOC_ARRAY(s, 1); + s->data = data; + s->data_len = data_len; + s->hash_algo = repo_algo; + + cf = init_chunkfile(NULL); + if (read_table_of_contents_quiet(cf, p, data_len, + DIFF_HUNKS_HEADER_SIZE, num_chunks, 1, + repo_algo) || + pair_chunk(cf, DIFF_HUNKS_CHUNKID_INDEX, &s->index, &index_size) || + pair_chunk(cf, DIFF_HUNKS_CHUNKID_DATA, &s->hdat, &s->hdat_size)) { + free_chunkfile(cf); + goto corrupt; + } + free_chunkfile(cf); + + entry_size = store_index_entry_size(s->hash_algo); + if (index_size % entry_size) + goto corrupt; + s->num_entries = index_size / entry_size; + return s; + +corrupt: + free_store(s); + return NULL; +} + +static struct diff_hunks_store *diff_hunks_store_load(struct repository *r) +{ + struct diff_hunks_store *s; + char *fname; + + prepare_repo_settings(r); + if (!r->settings.core_diff_hunks) + return NULL; + + fname = diff_hunks_store_path(r); + s = load_store_at(r->hash_algo, fname); + free(fname); + return s; +} + +struct diff_hunks_store *repo_diff_hunks_store(struct repository *r) +{ + if (!r->objects) + return NULL; + if (r->objects->diff_hunks_store_attempted) + return r->objects->diff_hunks_store; + r->objects->diff_hunks_store_attempted = 1; + r->objects->diff_hunks_store = diff_hunks_store_load(r); + return r->objects->diff_hunks_store; +} + +void diff_hunks_read_stats(struct repository *r, + unsigned long *hits, unsigned long *misses) +{ + struct diff_hunks_store *s = repo_diff_hunks_store(r); + + *hits = s ? s->read_hits : 0; + *misses = s ? s->read_misses : 0; +} + +void close_diff_hunks_store(struct object_database *o) +{ + if (!o->diff_hunks_store) + return; + free_store(o->diff_hunks_store); + o->diff_hunks_store = NULL; +} + +/* + * Fill *out with the hunk record at offset in the data chunk, and return + * 1 if the record is in bounds, 0 otherwise. The read path does not + * re-verify the checksum, and a valid checksum would not bound the count + * anyway, so a read must call this and use *out only when it returns + * non-zero. + * + * A record is a be32 hunk count followed by that many DIFF_HUNKS_HUNK_SIZE + * hunks. "remaining" tracks the bytes from offset to the end of the data + * chunk: it must hold the count, and after the count is consumed it must + * hold every hunk. The bounds are written as subtraction and division + * (never addition or multiplication) so a crafted offset or count cannot + * overflow them. + */ +static int precomputed_entry_at(const struct diff_hunks_store *s, + uint32_t offset, struct precomputed_entry *out) +{ + size_t remaining; + uint32_t num_hunks; + + if (offset >= s->hdat_size) + return 0; + remaining = s->hdat_size - offset; + if (remaining < sizeof(uint32_t)) + return 0; + + num_hunks = get_be32(s->hdat + offset); + remaining -= sizeof(uint32_t); + if (num_hunks > remaining / DIFF_HUNKS_HUNK_SIZE) + return 0; + + out->num_hunks = num_hunks; + out->hunk_data = s->hdat + offset + sizeof(uint32_t); + return 1; +} + +struct lookup_key { + const struct object_id *old_oid; + const struct object_id *new_oid; + int xdl_opts; + unsigned int rawsz; +}; + +/* + * The store's total order over (old_oid, new_oid, xdl_opts), defined + * once so the write-side sort (writer_entry_cmp) and the read-side + * search (store_bsearch_cmp) order the keys identically. + */ +static int cmp_store_index_key(const unsigned char *old_a, const unsigned char *new_a, + uint32_t opts_a, + const unsigned char *old_b, const unsigned char *new_b, + uint32_t opts_b, unsigned int rawsz) +{ + int cmp = memcmp(old_a, old_b, rawsz); + if (!cmp) + cmp = memcmp(new_a, new_b, rawsz); + if (!cmp) + cmp = (opts_a > opts_b) - (opts_a < opts_b); + return cmp; +} + +static int store_bsearch_cmp(const void *key, const void *entry_ptr) +{ + const struct lookup_key *k = key; + const unsigned char *old_hash, *new_hash; + uint32_t xdl_opts; + + decode_store_index_key(entry_ptr, k->rawsz, &old_hash, &new_hash, + &xdl_opts); + return cmp_store_index_key(k->old_oid->hash, k->new_oid->hash, + (uint32_t)k->xdl_opts, + old_hash, new_hash, xdl_opts, k->rawsz); +} + +static int store_get_one(struct diff_hunks_store *s, const struct lookup_key *key, + struct precomputed_entry *out) +{ + size_t entry_size = store_index_entry_size(s->hash_algo); + const unsigned char *found; + + found = bsearch(key, s->index, s->num_entries, entry_size, + store_bsearch_cmp); + if (!found) + return 0; + return precomputed_entry_at(s, + index_entry_hdat_offset(found, store_index_key_size(s->hash_algo)), + out); +} + +static int diff_hunks_store_get(struct diff_hunks_store *s, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + struct precomputed_entry *out) +{ + struct lookup_key key; + + if (!s) + return 0; + /* The null OID names no blob and cannot key an entry. */ + if (is_null_oid(old_oid) || is_null_oid(new_oid)) + return 0; + + key.old_oid = old_oid; + key.new_oid = new_oid; + key.xdl_opts = xdl_opts; + key.rawsz = s->hash_algo->rawsz; + + return store_get_one(s, &key, out); +} + +/* + * A recorded hunk sequence must satisfy the provider interface's + * shared check (diff_provider_check_hunk()) before it may be replayed: + * coordinates decode from be32 into long, which is 32-bit on some + * platforms, so a crafted value can decode negative or out of order. + * An entry that fails reads as a miss, so the caller recomputes. + */ +static int replayable_hunks(const struct precomputed_entry *e) +{ + struct diff_provider_hunks_check c = { 0 }; + uint32_t i; + + /* + * Replaying a record with no hunks would assert the blob pair + * equivalent, a claim the store must never make (the writer + * refuses to record one), so such a record is invalid. + */ + if (!e->num_hunks) + return 0; + for (i = 0; i < e->num_hunks; i++) { + struct precomputed_hunk h; + nth_precomputed_hunk(e, i, &h); + if (diff_provider_check_hunk(&c, h.old_start, h.old_count, + h.new_start, h.new_count)) + return 0; + } + return 1; +} + +int diff_hunks_replay(struct diff_hunks_store *s, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + xdl_emit_hunk_consume_func_t hunk_func, void *cb_data) +{ + struct precomputed_entry e; + uint32_t i; + + if (!s) + return 0; + if (!diff_hunks_store_get(s, old_oid, new_oid, xdl_opts, &e) || + !replayable_hunks(&e)) { + s->read_misses++; + return 0; + } + for (i = 0; i < e.num_hunks; i++) { + struct precomputed_hunk h; + nth_precomputed_hunk(&e, i, &h); + hunk_func(h.old_start, h.old_count, + h.new_start, h.new_count, cb_data); + } + s->read_hits++; + return 1; +} + +/* + * The store's consult implementation. The store is not + * authoritative, so it serves a recorded pair or passes; what the + * recording key cannot express, it excludes here with the + * stop-no-record disposition. None of those legs reaches + * diff_hunks_replay(), so none of them counts as a miss. + */ +static enum diff_provider_disposition +diff_hunks_store_consult(struct diff_provider *provider UNUSED, + const struct diff_provider_request *req, + diff_provider_fill_fn fill UNUSED, + void *fill_data UNUSED, + xdl_emit_hunk_consume_func_t hunk_cb, void *cb_data) +{ + /* + * xpparam_t is the consult's parameter input. Its flags are + * the store key's xdl_opts; ignore_regex (-I) and anchors + * (--anchored) shape the diff outside the key, so such a + * request is neither served nor recorded. + * + * Adding an xpparam_t field fires this assert (its size no + * longer matches the reference struct). To clear it: (1) add + * the field to the reference struct below; then (2) decide how + * it affects the key: make it part of the key, or exclude + * diffs that use it here with the disposition below. The + * assert only tracks size: a same-size reorder or a changed + * field meaning slips past, so re-read the fields when it + * fires. + */ + (void)BUILD_ASSERT_OR_ZERO(sizeof(xpparam_t) == sizeof(struct { + unsigned long flags; + regex_t **ignore_regex; + size_t ignore_regex_nr; + char **anchors; + size_t anchors_nr; + })); + if (req->xpp->ignore_regex_nr || req->xpp->anchors_nr) + return DIFF_PROVIDER_DISP_STOP_NO_RECORD; + /* + * Break detection (-B) rescores the pair outside xpparam_t, so + * it is outside the key for the same reason. + */ + if (req->diffopt && req->diffopt->break_opt != -1) + return DIFF_PROVIDER_DISP_STOP_NO_RECORD; + + if (!req->old_oid || !req->new_oid) + return DIFF_PROVIDER_DISP_PASS; + if (diff_hunks_replay(repo_diff_hunks_store(req->repo), + req->old_oid, req->new_oid, + req->xpp->flags, hunk_cb, cb_data)) + return DIFF_PROVIDER_DISP_ANSWERED; + return DIFF_PROVIDER_DISP_PASS; +} + +/* + * The provider borrows the repository's store through + * repo_diff_hunks_store() per request; the object database owns the + * file and tears it down, so there is nothing to release here. + */ +struct diff_provider *diff_hunks_store_provider_new(void) +{ + struct diff_provider *p = xcalloc(1, sizeof(*p)); + + p->consult = diff_hunks_store_consult; + return p; +} + +/* Validate one store file. Returns 0 if valid or absent, -1 on any error. */ +static int verify_store_at(struct repository *r, const char *fname) +{ + struct diff_hunks_store *s; + size_t entry_size; + uint32_t i; + int fd; + int ret = 0; + + /* + * A file that cannot be opened is not evidence of corruption: + * report the open error, and reserve the corruption diagnostics + * below for a file that was read and failed to parse. + */ + fd = git_open(fname); + if (fd < 0) { + if (errno == ENOENT) + return 0; /* absent is valid */ + return error_errno(_("unable to open diff-hunks store %s"), + fname); + } + close(fd); + s = load_store_at(r->hash_algo, fname); + if (!s) + return error(_("diff-hunks store failed to load (corrupt " + "header or hash mismatch): %s"), fname); + if (!hashfile_checksum_valid(r->hash_algo, s->data, s->data_len)) { + error(_("diff-hunks store has incorrect checksum and is " + "likely corrupt: %s"), fname); + free_store(s); + return -1; + } + + entry_size = store_index_entry_size(s->hash_algo); + for (i = 0; i < s->num_entries; i++) { + const unsigned char *ep = s->index + st_mult(entry_size, i); + size_t keysz = store_index_key_size(s->hash_algo); + uint32_t offset = index_entry_hdat_offset(ep, keysz); + struct precomputed_entry pe; + + /* + * Keyed by (old_oid, new_oid, xdl_opts), increasing. memcmp + * matches cmp_store_index_key's integer comparison of + * xdl_opts because it is non-negative, so its big-endian + * bytes order the same as its value. + */ + if (i > 0 && memcmp(ep - entry_size, ep, keysz) >= 0) { + error(_("diff-hunks entry %u not in sorted order"), i); + ret = -1; + } + if (!precomputed_entry_at(s, offset, &pe)) { + error(_("diff-hunks entry %u has out-of-bounds hunk " + "data"), i); + ret = -1; + } else if (!replayable_hunks(&pe)) { + error(_("diff-hunks entry %u holds an invalid hunk " + "sequence"), i); + ret = -1; + } + } + + free_store(s); + return ret; +} + +int diff_hunks_verify(struct repository *r) +{ + char *fname = diff_hunks_store_path(r); + int ret = 0; + + if (verify_store_at(r, fname)) + ret = -1; + free(fname); + return ret; +} + +int diff_hunks_clear(struct repository *r) +{ + char *fname = diff_hunks_store_path(r); + int ret = 0; + + if (unlink(fname) && errno != ENOENT) + ret = error_errno(_("unable to remove %s"), fname); + free(fname); + return ret; +} + +struct writer_entry { + struct object_id old_oid; + struct object_id new_oid; + int xdl_opts; + uint32_t hdat_offset; +}; + +struct diff_hunks_writer { + struct repository *r; + struct writer_entry *entries; + size_t nr, alloc; + size_t seed_nr; /* nr after seeding; finish skips a no-op flush */ + unsigned force_flush : 1; /* seed pruned: rewrite even a no-op warm */ + struct strbuf hdat; + struct hashmap dedup; /* hunk block content -> offset in hdat */ +}; + +/* A record of one distinct hunk block already present in hdat. */ +struct dedup_entry { + struct hashmap_entry ent; + uint32_t offset; + uint32_t len; +}; + +static int dedup_cmp(const void *cmp_data, + const struct hashmap_entry *a, + const struct hashmap_entry *b, + const void *keydata UNUSED) +{ + const struct diff_hunks_writer *writer = cmp_data; + const struct dedup_entry *ea = container_of(a, const struct dedup_entry, ent); + const struct dedup_entry *eb = container_of(b, const struct dedup_entry, ent); + + if (ea->len != eb->len) + return 1; + return memcmp(writer->hdat.buf + ea->offset, + writer->hdat.buf + eb->offset, ea->len); +} + +static struct diff_hunks_writer *diff_hunks_writer_new(struct repository *r) +{ + struct diff_hunks_writer *w; + + CALLOC_ARRAY(w, 1); + w->r = r; + strbuf_init(&w->hdat, 0); + hashmap_init(&w->dedup, dedup_cmp, w, 0); + return w; +} + +static void strbuf_put_be32(struct strbuf *sb, uint32_t val) +{ + unsigned char buf[4]; + put_be32(buf, val); + strbuf_add(sb, buf, 4); +} + +/* + * The hunk block just appended at `start` is deduplicated: if an + * identical block is already in hdat, this copy is dropped and the + * earlier offset returned; otherwise it is kept and remembered. + * Distinct keys that diff to the same hunks then share one block. + */ +static uint32_t intern_block(struct diff_hunks_writer *w, size_t start) +{ + size_t len = w->hdat.len - start; + struct dedup_entry key, *found, *added; + + hashmap_entry_init(&key.ent, memhash(w->hdat.buf + start, len)); + key.offset = (uint32_t)start; + key.len = (uint32_t)len; + + found = hashmap_get_entry(&w->dedup, &key, ent, NULL); + if (found) { + strbuf_setlen(&w->hdat, start); + return found->offset; + } + + added = xmalloc(sizeof(*added)); + hashmap_entry_init(&added->ent, key.ent.hash); + added->offset = key.offset; + added->len = key.len; + hashmap_add(&w->dedup, &added->ent); + return key.offset; +} + +int diff_hunks_writer_add(struct diff_hunks_writer *w, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + const struct precomputed_hunk *hunks, + size_t nr_hunks) +{ + struct writer_entry *e; + size_t i, block_start; + + if (!w) + return 0; + /* + * The block appended for this entry is sizeof(uint32_t) + + * nr_hunks * DIFF_HUNKS_HUNK_SIZE bytes. Bound nr_hunks so that + * length fits the uint32_t the dedup index records (and so the + * count itself fits the uint32_t written to the store). + */ + if (!nr_hunks || + nr_hunks > (UINT32_MAX - sizeof(uint32_t)) / DIFF_HUNKS_HUNK_SIZE || + is_null_oid(old_oid) || is_null_oid(new_oid)) + return 0; + if (w->hdat.len > UINT32_MAX) + return 0; + /* + * Coordinates are stored as 32-bit values; a result that cannot + * round-trip is dropped rather than silently truncated. + */ + for (i = 0; i < nr_hunks; i++) + if ((uintmax_t)hunks[i].old_start > (uintmax_t)INT32_MAX || + (uintmax_t)hunks[i].old_count > (uintmax_t)INT32_MAX || + (uintmax_t)hunks[i].new_start > (uintmax_t)INT32_MAX || + (uintmax_t)hunks[i].new_count > (uintmax_t)INT32_MAX) + return 0; + + ALLOC_GROW(w->entries, w->nr + 1, w->alloc); + e = &w->entries[w->nr++]; + oidcpy(&e->old_oid, old_oid); + oidcpy(&e->new_oid, new_oid); + e->xdl_opts = xdl_opts; + + block_start = w->hdat.len; + strbuf_put_be32(&w->hdat, (uint32_t)nr_hunks); + for (i = 0; i < nr_hunks; i++) { + strbuf_put_be32(&w->hdat, hunks[i].old_start); + strbuf_put_be32(&w->hdat, hunks[i].old_count); + strbuf_put_be32(&w->hdat, hunks[i].new_start); + strbuf_put_be32(&w->hdat, hunks[i].new_count); + } + e->hdat_offset = intern_block(w, block_start); + return 1; +} + +void diff_hunks_writer_record_stable(struct diff_hunks_writer *w, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + const struct precomputed_hunk *trimmed, + size_t nr_trimmed, + const struct precomputed_hunk *full, + size_t nr_full) +{ + size_t i; + + if (!w) + return; + /* + * Record only a trim-stable pair, one whose trimmed and + * untrimmed diffs are identical, so the single entry answers + * any consumer at any context (see the top of this file). A + * pair where the two diffs differ is never recorded and every + * consumer computes it. + */ + if (nr_trimmed != nr_full) + return; + for (i = 0; i < nr_trimmed; i++) + if (trimmed[i].old_start != full[i].old_start || + trimmed[i].old_count != full[i].old_count || + trimmed[i].new_start != full[i].new_start || + trimmed[i].new_count != full[i].new_count) + return; + diff_hunks_writer_add(w, old_oid, new_oid, xdl_opts, + trimmed, nr_trimmed); +} + +/* + * Seed the writer with fname's entries so a rewrite preserves them, + * setting *pruned when the rewrite will not carry the whole file + * forward: the file failed its checksum and was discarded outright, or + * individual entries were dropped because they failed the replayable + * check or the writer refused them (a key naming no blob). A + * rewrite re-checksums, so corruption must not be carried forward: + * that would launder it into a checksum-valid file that verify can no + * longer catch. This path already reads the whole file, so verify the + * checksum here (the reader keeps trusting committed files, without + * re-checksumming); an invalid + * entry reads as a miss anyway, so dropping it heals the store rather + * than losing anything a reader could use. + */ +static void diff_hunks_writer_seed(struct diff_hunks_writer *w, + const char *fname, int *pruned) +{ + struct diff_hunks_store *s = load_store_at(w->r->hash_algo, fname); + unsigned int rawsz; + size_t entry_size, keysz; + struct precomputed_hunk *hunks = NULL; + size_t hunks_alloc = 0; + uint32_t i, dropped = 0; + + if (!s) + return; + if (!hashfile_checksum_valid(w->r->hash_algo, s->data, s->data_len)) { + warning(_("diff-hunks store %s failed its checksum; " + "discarding it"), fname); + free_store(s); + *pruned = 1; + return; + } + rawsz = s->hash_algo->rawsz; + entry_size = store_index_entry_size(s->hash_algo); + keysz = store_index_key_size(s->hash_algo); + + for (i = 0; i < s->num_entries; i++) { + const unsigned char *ep = s->index + st_mult(entry_size, i); + const unsigned char *old_hash, *new_hash; + struct object_id old_oid, new_oid; + uint32_t xdl_opts, j; + struct precomputed_entry pe; + + decode_store_index_key(ep, rawsz, &old_hash, &new_hash, + &xdl_opts); + oidread(&old_oid, old_hash, s->hash_algo); + oidread(&new_oid, new_hash, s->hash_algo); + if (!precomputed_entry_at(s, index_entry_hdat_offset(ep, keysz), &pe) || + !replayable_hunks(&pe)) { + dropped++; + continue; + } + ALLOC_GROW(hunks, pe.num_hunks, hunks_alloc); + for (j = 0; j < pe.num_hunks; j++) + nth_precomputed_hunk(&pe, j, &hunks[j]); + if (!diff_hunks_writer_add(w, &old_oid, &new_oid, + (int)xdl_opts, hunks, pe.num_hunks)) + dropped++; + } + if (dropped) { + warning(Q_("diff-hunks store %s: dropping %u invalid entry", + "diff-hunks store %s: dropping %u invalid entries", + dropped), fname, dropped); + *pruned = 1; + } + free(hunks); + free_store(s); +} + +/* + * Writing is off by default. It is enabled per invocation by the + * GIT_DIFF_HUNKS_WRITE environment variable, or persistently by the + * diffHunks.write config, with the environment variable winning when + * set. Only a warming run (a diff or log the repository owner chooses + * to run with writing on) enables it, so ordinary reads never mutate + * the store. + */ +static int diff_hunks_write_enabled(struct repository *r) +{ + const char *env = getenv("GIT_DIFF_HUNKS_WRITE"); + int val; + + if (env) { + /* + * This is a warming opt-in, so an unparseable value must not + * abort an ordinary read command: treat it as disabled. + */ + val = git_parse_maybe_bool(env); + return val < 0 ? 0 : val; + } + if (!repo_config_get_bool(r, "diffhunks.write", &val)) + return val; + return 0; +} + +struct diff_hunks_writer *diff_hunks_writer_maybe_new(struct repository *r) +{ + struct diff_hunks_writer *w; + char *fname; + int pruned; + + if (!diff_hunks_write_enabled(r)) + return NULL; + /* + * Seed from the existing store so a flush merges with it rather + * than replacing it: a later warm adds newly computed pairs + * without discarding what earlier warms recorded. + */ + w = diff_hunks_writer_new(r); + fname = diff_hunks_store_path(r); + pruned = 0; + diff_hunks_writer_seed(w, fname, &pruned); + free(fname); + w->seed_nr = w->nr; + /* + * A pruning seed means the file on disk holds material the + * rewrite must not preserve; flush even if this warm computes + * nothing new, so the store on disk is repaired rather than + * left serving what the seed refused. + */ + w->force_flush = !!pruned; + return w; +} + +static int writer_entry_cmp(const void *va, const void *vb, void *ctx) +{ + const struct writer_entry *a = va, *b = vb; + unsigned int rawsz = *(const unsigned int *)ctx; + return cmp_store_index_key(a->old_oid.hash, a->new_oid.hash, + (uint32_t)a->xdl_opts, + b->old_oid.hash, b->new_oid.hash, + (uint32_t)b->xdl_opts, + rawsz); +} + +struct write_ctx { + struct diff_hunks_writer *w; + unsigned int rawsz; +}; + +static int write_index_chunk(struct hashfile *f, void *data) +{ + struct write_ctx *ctx = data; + size_t i; + + for (i = 0; i < ctx->w->nr; i++) { + hashwrite(f, ctx->w->entries[i].old_oid.hash, ctx->rawsz); + hashwrite(f, ctx->w->entries[i].new_oid.hash, ctx->rawsz); + hashwrite_be32(f, ctx->w->entries[i].xdl_opts); + hashwrite_be32(f, ctx->w->entries[i].hdat_offset); + } + return 0; +} + +static int write_data_chunk(struct hashfile *f, void *data) +{ + struct write_ctx *ctx = data; + hashwrite(f, ctx->w->hdat.buf, ctx->w->hdat.len); + return 0; +} + +/* Sort, dedup, and write the accumulated entries to the file at fname. */ +static int diff_hunks_writer_flush(struct diff_hunks_writer *w, char *fname) +{ + struct lock_file lk = LOCK_INIT; + struct hashfile *f; + struct chunkfile *cf; + unsigned int rawsz = w->r->hash_algo->rawsz; + struct write_ctx ctx = { w, rawsz }; + size_t entry_size; + + QSORT_S(w->entries, w->nr, writer_entry_cmp, &rawsz); + + /* + * The same blob pair recurs across history (reverts, cherry- + * picks); identical keys carry identical hunks, so keep one of + * each. The index must stay duplicate-free for binary search. + */ + if (w->nr > 1) { + size_t kept = 1, i; + for (i = 1; i < w->nr; i++) + if (writer_entry_cmp(&w->entries[kept - 1], + &w->entries[i], &rawsz)) + w->entries[kept++] = w->entries[i]; + w->nr = kept; + } + + if (safe_create_leading_directories(w->r, fname)) { + error(_("unable to create directory for %s"), fname); + return -1; + } + if (hold_lock_file_for_update(&lk, fname, 0) < 0) { + error_errno(_("unable to lock %s"), fname); + return -1; + } + adjust_shared_perm(w->r, get_lock_file_path(&lk)); + f = hashfd(w->r->hash_algo, get_lock_file_fd(&lk), + get_lock_file_path(&lk)); + + entry_size = store_index_entry_size(w->r->hash_algo); + cf = init_chunkfile(f); + add_chunk(cf, DIFF_HUNKS_CHUNKID_INDEX, w->nr * entry_size, + write_index_chunk); + add_chunk(cf, DIFF_HUNKS_CHUNKID_DATA, w->hdat.len, write_data_chunk); + + hashwrite_be32(f, DIFF_HUNKS_SIGNATURE); + hashwrite_u8(f, DIFF_HUNKS_VERSION); + hashwrite_u8(f, oid_version(w->r->hash_algo)); + hashwrite_u8(f, get_num_chunks(cf)); + hashwrite_u8(f, 0); /* reserved */ + + write_chunkfile(cf, &ctx); + free_chunkfile(cf); + + /* + * fsync per the user's configuration (like commit-graph and the + * multi-pack-index), then commit atomically. Readers trust the + * committed file rather than re-checksumming it; diff_hunks_verify() + * checks the checksum separately. + */ + finalize_hashfile(f, NULL, FSYNC_COMPONENT_DIFF_HUNKS, + CSUM_HASH_IN_STREAM | CSUM_FSYNC); + /* + * This same process may hold the current store mmapped (a warm + * that also reads); the commit below renames over it, which must + * never land on a live mapping (Windows refuses it). Close the + * store and clear the load-attempted flag first, so the next + * read loads the committed file. + */ + if (w->r->objects) { + close_diff_hunks_store(w->r->objects); + w->r->objects->diff_hunks_store_attempted = 0; + } + if (commit_lock_file(&lk)) { + error_errno(_("unable to write %s"), fname); + return -1; + } + return 0; +} + +static void diff_hunks_writer_free(struct diff_hunks_writer *w) +{ + if (!w) + return; + hashmap_clear_and_free(&w->dedup, struct dedup_entry, ent); + free(w->entries); + strbuf_release(&w->hdat); + free(w); +} + +void diff_hunks_writer_finish(struct diff_hunks_writer *w) +{ + if (!w) + return; + /* Skip the flush when the warm recorded nothing beyond its seed. */ + if (w->nr != w->seed_nr || w->force_flush) { + char *fname = diff_hunks_store_path(w->r); + diff_hunks_writer_flush(w, fname); + free(fname); + } + diff_hunks_writer_free(w); +} diff --git a/diff-hunks.h b/diff-hunks.h new file mode 100644 index 0000000000..c58500d05c --- /dev/null +++ b/diff-hunks.h @@ -0,0 +1,141 @@ +#ifndef DIFF_HUNKS_H +#define DIFF_HUNKS_H + +#include "hash.h" +#include "xdiff-interface.h" /* xdl_emit_hunk_consume_func_t */ + +struct object_id; +struct repository; +struct object_database; + +/* + * A persistent store of precomputed diff hunk coordinates, at + * .git/objects/info/diff-hunks. Entries are keyed by the two blobs diffed + * and the xdl_opts they were diffed under, so a cached result is valid + * in any context that key recurs in, independent of path. The xdl_opts + * key component mirrors the (always non-negative) diff_options field it + * projects from, and is serialized and compared as a 4-byte big-endian + * integer. + * + * The hunks a pair produces are not unique. They vary with the xdiff + * algorithm and ignore flags (xdl_opts, part of the key), and with + * whether the diff was trimmed: a zero-context diff runs + * trim_common_tail, which can pick a different but equally valid set of + * hunks than an untrimmed diff. The store holds one entry per key, so a + * pair is recorded only when its trimmed and untrimmed diffs are + * identical (the recording caller checks); such an entry serves a + * consumer at any context. The rare pair where the two diffs differ is + * never recorded and is always computed. + * + * The store is a cache: ordinary commands read it and fall back to + * computing the diff when it is absent, stale, or corrupt. It is filled + * as a side effect of diff and log runs, but only when writing is + * enabled (such a write-enabled run is a warming run); writing is off + * by default, so an ordinary command reads the store without recording + * into it. + */ + +/* + * A hunk's coordinates. The type is long to match the xdiff emit + * callback; the values are a diff's line numbers and counts, always + * within the int32 range the on-disk format stores (see + * diff_hunks_writer_add()). + */ +struct precomputed_hunk { + long old_start; + long old_count; + long new_start; + long new_count; +}; + +/* + * The repository's store, loaded once on first use and cached on the + * object database. Returns NULL when reading is disabled + * (core.diffHunks=false), the store is absent, or it fails to parse + * (wrong signature, version, or object hash, or a corrupt structure). + * The lookup functions below accept a NULL store and treat it as + * empty (every lookup misses), so callers need not check for NULL. + * The object database owns the store; callers must not free it. + */ +struct diff_hunks_store *repo_diff_hunks_store(struct repository *r); + +/* Free the repository's cached store, at object-database teardown. */ +void close_diff_hunks_store(struct object_database *o); + +/* + * Consultation counters for the repository's store: pairs the store + * served (hits) and pairs it was consulted for but could not serve + * (misses). Both zero when reading is disabled or no store exists. + */ +void diff_hunks_read_stats(struct repository *r, + unsigned long *hits, unsigned long *misses); + +/* + * Replay the recorded hunks of an (old blob, new blob) pair diffed + * under xdl_opts through hunk_func. The sequence is validated before + * any callback runs: on a hit (return 1) every hunk is emitted, on a + * miss (return 0: absent pair, xdl_opts mismatch, or an entry that + * fails validation) nothing is emitted, so a caller may accumulate + * directly into its result. + */ +int diff_hunks_replay(struct diff_hunks_store *s, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + xdl_emit_hunk_consume_func_t hunk_func, void *cb_data); + +/* + * A warming run's writer: it accumulates the hunks it computes in memory + * and flushes them to the store in one pass at finish. + */ +struct diff_hunks_writer; + +/* + * Return a writer for a warming run, or NULL when writing is disabled + * (the default). diff_hunks_writer_add() tolerates a NULL writer, so a + * caller may attach the result unconditionally. Pair with + * diff_hunks_writer_finish(). + */ +struct diff_hunks_writer *diff_hunks_writer_maybe_new(struct repository *r); + +/* + * Record a blob pair's hunks as computed under xdl_opts; a later lookup + * with a matching key is served these hunks. The caller must have + * checked that the pair's trimmed and untrimmed diffs are identical + * (see the top of this file), so the entry answers at any context; + * diff_hunks_writer_record_stable() below performs that check. + * NULL-safe. Returns 1 when the entry was recorded, 0 when the writer + * refused it (no hunks, a null object id, or values the on-disk + * 32-bit fields cannot hold). + */ +int diff_hunks_writer_add(struct diff_hunks_writer *w, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + const struct precomputed_hunk *hunks, + size_t nr_hunks); + +/* + * Record the pair only if it is trim-stable: the recording caller + * hands over both the trimmed (xdi_diff) and untrimmed (xdl_diff) + * zero-context hunk sequences it computed, and the entry is added + * only when the two are identical. NULL-safe. + */ +void diff_hunks_writer_record_stable(struct diff_hunks_writer *w, + const struct object_id *old_oid, + const struct object_id *new_oid, + int xdl_opts, + const struct precomputed_hunk *trimmed, + size_t nr_trimmed, + const struct precomputed_hunk *full, + size_t nr_full); + +/* Flush the accumulated entries to the store and free the writer. NULL-safe. */ +void diff_hunks_writer_finish(struct diff_hunks_writer *w); + +/* Remove the store file. Returns 0 (incl. absent) or -1. */ +int diff_hunks_clear(struct repository *r); +/* Validate the store. Returns 0 if valid/absent, -1 if corrupt. */ +int diff_hunks_verify(struct repository *r); + +#endif /* DIFF_HUNKS_H */ diff --git a/diff-process.c b/diff-process.c new file mode 100644 index 0000000000..121903a6c8 --- /dev/null +++ b/diff-process.c @@ -0,0 +1,669 @@ +/* + * The process provider of the hunk provider interface: consult a + * long-running external process via the pkt-line protocol for the + * hunks of a blob pair. The process answers from the pair's object + * names alone: it can serve a persistent cache keyed on the pair, or + * fetch the blobs from the repository itself (e.g. via "git cat-file + * --batch") and compute its own notion of which lines changed. The + * provider sits at the head of its repository's chain and gates + * itself per request; its state is the repository's pool of running + * processes, one per configured command, stopped when the provider + * is released. + * + * 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 + * process< git-diff-server / version=1 / flush + * git> capability=hunks-by-oid / flush + * process< capability=hunks-by-oid / flush + * + * Per-pair, when both sides are stored blobs: + * git> command=hunks-by-oid / pathname= + * git> old-oid= / new-oid= / flush + * process< hunk + * process< ... / flush + * process< status=success / flush + * + * No content is sent. Because Git holds no content for the exchange, + * the answer is used as the process sent it: the hunks are not re-run + * through xdiff's compaction, and a status=success response with zero + * hunks asserts that the blobs are equivalent, including their + * trailing newlines. A process that cannot answer from the object names + * (or cannot rule out a trailing-newline-only difference) responds + * status=need-content; the pair then gets the builtin answer, served + * from the diff-hunks store or computed. A later + * protocol extension can define a content-carrying request for such + * processes and for sides that are not stored blobs. + */ + +#include "git-compat-util.h" +#include "diff.h" +#include "diff-provider-internal.h" +#include "gettext.h" +#include "hex.h" +#include "odb.h" +#include "repository.h" +#include "sigchain.h" +#include "userdiff.h" +#include "sub-process.h" +#include "pkt-line.h" +#include "strbuf.h" + +#define CAP_OID_HUNKS (1u << 0) + +/* + * The provider's state: the repository's diff processes, keyed by + * their command string, so drivers that configure the same command + * share one process. An entry whose process failed stays in the + * pool with the failed bit set, so the command is not retried while + * the entry lives; the pool and its entries last until the provider + * is released. + */ +struct diff_process_state { + struct hashmap subprocesses; +}; + +struct diff_subprocess { + struct subprocess_entry subprocess; + /* + * Owns the string subprocess.cmd and the hashmap key borrow: the + * entry outlives the userdiff config a re-read may replace. + */ + char *cmd; + unsigned int supported_capabilities; + unsigned failed : 1; +}; + +static int start_diff_process_fn(struct subprocess_entry *subprocess) +{ + static int versions[] = { 1, 0 }; + static struct subprocess_capability capabilities[] = { + { "hunks-by-oid", CAP_OID_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); +} + +/* + * The pool entry for a command, or NULL when its process fails to + * start here: the failure leaves a failed entry in the pool, so only + * the request that observed it maps it to an error and later + * requests pass the provider by. + */ +static struct diff_subprocess *get_or_launch_process( + struct diff_process_state *state, + struct userdiff_driver *drv) +{ + struct subprocess_entry *running; + struct diff_subprocess *entry; + + running = subprocess_find_entry(&state->subprocesses, drv->process); + if (running) { + entry = container_of(running, struct diff_subprocess, + subprocess); + return entry->failed ? NULL : entry; + } + + entry = xcalloc(1, sizeof(*entry)); + entry->cmd = xstrdup(drv->process); + if (subprocess_start_command(&entry->subprocess, entry->cmd, + start_diff_process_fn)) + entry->failed = 1; + hashmap_entry_init(&entry->subprocess.ent, strhash(entry->cmd)); + hashmap_add(&state->subprocesses, &entry->subprocess.ent); + if (entry->failed) { + warning(_("diff process '%s' failed to start;" + " using the builtin diff"), drv->process); + return NULL; + } + return entry; +} + +/* + * 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 a consumer; 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 an older Git rejecting the line, mirroring + * the request-side rule that processes ignore unknown keys. + * + * A value that overflows strtol() is not a parse failure: the line is + * well-formed, so the stream stays in protocol sync. It is reported + * through *out_of_range, and the caller skips the pair the same way + * it skips any other out-of-range coordinate. + */ +static int parse_hunk_field(const char **line, long *out, int is_last, + int *out_of_range) +{ + const char *p = *line; + char *end; + + if (!isdigit((unsigned char)*p)) + return -1; + errno = 0; + *out = strtol(p, &end, 10); + if (end == p) + return -1; + if (errno == ERANGE) + *out_of_range = 1; + else if (errno) + 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, + int *out_of_range) +{ + *out_of_range = 0; + /* Format: "hunk " */ + if (!skip_prefix(line, "hunk ", &line)) + return -1; + if (parse_hunk_field(&line, &presented->old_start, 0, out_of_range) || + parse_hunk_field(&line, &presented->old_count, 0, out_of_range) || + parse_hunk_field(&line, &presented->new_start, 0, out_of_range) || + parse_hunk_field(&line, &presented->new_count, 1, out_of_range)) + 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 + * any consumer of these coordinates 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; +} + +/* + * Validate the process's hunks (already in xdiff coordinates) before they + * bypass the diff algorithm. The content-independent rules (in-order, + * non-overlapping, lockstep-aligned, int32-bounded coordinates) are the + * provider interface's shared rule, diff_provider_check_hunk(); this + * function adds the two checks that need the blobs' line counts (a hunk + * past the end of a file, the run after the last hunk) and the + * per-rule diagnostics naming the process. On a bad response we warn + * and the caller falls back to the builtin diff. Returns 0 if valid, + * -1 (after warning) otherwise. + * + * old_lines/new_lines bound the line count of each side, or are + * negative when no bound is known. An oid-only answer arrives without + * content, so its caller passes upper bounds derived from the blobs' + * byte sizes, which caps coordinate magnitude but cannot support the + * run-after-the-last-hunk check: that one compares exact line counts, + * so it runs only when lines_exact is set, which no caller does today. + * It is kept for a content-carrying request, whose loaded buffers + * would provide exact counts. + */ +static int validate_external_hunks(const struct xdl_hunk *hunks, size_t nr, + long old_lines, long new_lines, + int lines_exact, + const char *process, const char *path) +{ + struct diff_provider_hunks_check c = { 0 }; + size_t i; + + for (i = 0; i < nr; i++) { + const struct xdl_hunk *h = &hunks[i]; + + if (old_lines >= 0 && + (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; + } + switch (diff_provider_check_hunk(&c, h->old_start, + h->old_count, h->new_start, + h->new_count)) { + case DIFF_PROVIDER_HUNKS_OK: + break; + case DIFF_PROVIDER_HUNKS_RANGE: + warning(_("diff process '%s' returned out-of-range " + "coordinates for '%s'; using the builtin diff"), + process, path); + return -1; + case DIFF_PROVIDER_HUNKS_OVERLAP: + warning(_("diff process '%s' returned overlapping hunks " + "for '%s'; using the builtin diff"), + process, path); + return -1; + case DIFF_PROVIDER_HUNKS_MISALIGNED: + warning(_("diff process '%s' returned hunks that leave " + "'%s' misaligned; using the builtin diff"), + process, path); + return -1; + } + } + if (lines_exact && + old_lines - c.prev_old_end != new_lines - c.prev_new_end) { + warning(_("diff process '%s' returned hunks that leave '%s' " + "misaligned; using the builtin diff"), + process, path); + return -1; + } + return 0; +} + +/* + * The most lines a blob can hold, from its size alone: every line, + * even an empty one, costs at least one byte, so a blob of N bytes + * holds at most N lines. Returns -1 when the size is unavailable, + * leaving the response bounded only by the shared int32 rule. A size + * beyond INT32_MAX clamps to it, which loses nothing: a coordinate + * that large fails the shared rule anyway. In a partial clone the + * size lookup must not fetch the blob from the promisor remote: + * validating an answer that exists to avoid loading content must not + * itself download that content, so a missing blob reads as size + * unavailable instead. + */ +static long blob_line_cap(struct repository *r, const struct object_id *oid) +{ + unsigned long size; + struct object_info oi = OBJECT_INFO_INIT; + + oi.sizep = &size; + if (odb_read_object_info_extended(r->objects, oid, &oi, + OBJECT_INFO_SKIP_FETCH_OBJECT) < 0) + return -1; + if (size > INT32_MAX) + return INT32_MAX; + return (long)size; +} + +/* + * The driver whose process a consultation for path would ask, or NULL + * when none applies (no driver, process not allowed, or xpp carries + * options the process is never told about). Needs no content, so + * the driver is picked before any blob is loaded. + */ +static struct userdiff_driver *diff_process_driver(struct diff_options *diffopt, + const char *path, + const xpparam_t *xpp) +{ + struct userdiff_driver *drv; + + if (!diffopt || !path) + return NULL; + if (!diffopt->flags.allow_diff_process || diffopt->ignore_driver_algorithm) + return NULL; + /* + * Whitespace-ignoring, regex-ignore (-I) and anchored options + * change which lines count as different, but the process is never + * told about them, so its hunks could not honor them. A forced + * diff algorithm (an option or configured algorithm setting) + * requests a specific builtin computation, which an + * authoritative answer would override. 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, which keeps its algorithm and + * whitespace flags outside diffopt, is covered without a + * separate guard of its own. + */ + if ((xpp->flags & (XDF_WHITESPACE_FLAGS | XDF_IGNORE_BLANK_LINES | + XDF_DIFF_ALGORITHM_MASK)) || + xpp->ignore_regex_nr || xpp->anchors_nr) + return NULL; + + /* + * A path the protocol cannot carry never selects a process: an + * embedded newline would let the rest of the path forge further + * request keys, and the pathname must fit one packet. Passing + * here keeps the cost local to the path; a failed write would + * instead cost the whole command its process. + */ + if (strchr(path, '\n') || + strlen(path) > LARGE_PACKET_DATA_MAX - strlen("pathname=\n")) + return NULL; + + drv = userdiff_find_by_path(diffopt->repo->index, path); + if (!drv || !drv->process) + return NULL; + return drv; +} + +/* + * Without content there is no size-derived bound on a response, so cap + * accumulation at a constant instead. A response that exceeds the + * cap is a protocol error: the process is disabled for the rest of + * the command and the caller falls back to the builtin diff. + */ +#define OID_HUNKS_MAX (1 << 20) + +enum diff_process_result { + DIFF_PROCESS_ERROR = -1, /* failed; caller falls back to builtin */ + DIFF_PROCESS_OK = 0, /* the process supplied hunks */ + DIFF_PROCESS_SKIP, /* process did not apply: use builtin */ + DIFF_PROCESS_EQUIVALENT, /* process says files are equivalent */ +}; + +/* + * Ask drv's diff process to answer the request from the blob pair's + * object ids alone (the "hunks-by-oid" capability): no content is + * loaded or sent. On DIFF_PROCESS_OK the process's hunks are emitted + * through hunk_cb in 0-based emission coordinates, validated for order, + * overlap, and lockstep alignment first; because Git holds no content, + * the answer is used as the process sent it, without xdiff's compaction. + * DIFF_PROCESS_EQUIVALENT means the process asserts the pair equal. + * DIFF_PROCESS_SKIP covers everything that should fall through to the + * builtin computation: a missing capability, a missing object id, a + * status=need-content answer, or an invalid response. + */ +static enum diff_process_result diff_process_query_hunks( + struct diff_process_state *state, + struct userdiff_driver *drv, + const struct diff_provider_request *req, + xdl_emit_hunk_consume_func_t hunk_cb, + void *cb_data) +{ + const char *path = req->path; + struct diff_subprocess *entry; + struct child_process *process; + int fd_in, fd_out; + struct packet_reader reader; + 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, i; + int bad_coords = 0; + long old_cap, new_cap; + enum diff_process_result res; + + if (!req->old_oid || !req->new_oid) + return DIFF_PROCESS_SKIP; + + entry = get_or_launch_process(state, drv); + if (!entry) + return DIFF_PROCESS_ERROR; + if (!(entry->supported_capabilities & CAP_OID_HUNKS)) + return DIFF_PROCESS_SKIP; + + process = subprocess_get_child_process(&entry->subprocess); + fd_in = process->in; + fd_out = process->out; + + sigchain_push(SIGPIPE, SIG_IGN); + + if (packet_write_fmt_gently(fd_in, "command=hunks-by-oid\n") || + packet_write_fmt_gently(fd_in, "pathname=%s\n", path) || + packet_write_fmt_gently(fd_in, "old-oid=%s\n", + oid_to_hex(req->old_oid)) || + packet_write_fmt_gently(fd_in, "new-oid=%s\n", + oid_to_hex(req->new_oid)) || + packet_flush_gently(fd_in)) + goto comm_error; + + packet_reader_init(&reader, fd_out, NULL, 0, + PACKET_READ_CHOMP_NEWLINE | + PACKET_READ_GENTLE_ON_EOF | + PACKET_READ_GENTLE_ON_READ_ERROR); + for (;;) { + enum packet_read_status rs = packet_reader_read(&reader); + int out_of_range; + + if (rs == PACKET_READ_FLUSH) + break; + /* + * Only a hunk line may precede the flush. EOF and a + * malformed frame end the session; an empty packet, which + * a length-only read cannot tell from a flush, would + * truncate the hunk section here and leave the status + * section to poison the next request, so it is a protocol + * error too. + */ + if (rs != PACKET_READ_NORMAL || !reader.pktlen) + goto comm_error; + if (parse_hunk_line(reader.line, &presented, + &out_of_range) < 0) + goto comm_error; + if (bad_coords) + continue; + if (out_of_range || + diff_process_hunk_to_xdl(&presented, &hunk) < 0) { + /* + * Semantically invalid coordinates in a well-formed + * response: the stream stays in protocol sync, so + * drain the rest and fall back for this file while + * keeping the process alive, the same treatment + * validate_external_hunks() failures receive. + */ + bad_coords = 1; + continue; + } + if (nr_hunks >= OID_HUNKS_MAX) { + warning(_("diff process '%s' sent too many hunks" + " for '%s'; disabling it for the" + " remainder of this command"), + drv->process, path); + goto disable; + } + ALLOC_GROW(hunks, nr_hunks + 1, alloc_hunks); + hunks[nr_hunks++] = hunk; + } + + if (subprocess_read_status_gently(fd_out, &status)) + goto comm_error; + + if (!strcmp(status.buf, "success")) { + if (bad_coords) { + warning(_("diff process '%s' returned out-of-range " + "coordinates for '%s'; using the builtin diff"), + drv->process, path); + res = DIFF_PROCESS_SKIP; + goto out; + } + if (!nr_hunks) { + res = DIFF_PROCESS_EQUIVALENT; + goto out; + } + /* + * Bound the coordinates by the blobs' sizes, read from the + * object database without loading content. Either both + * bounds hold or neither is applied: a partial bound would + * misclassify a response that the other side's size would + * have caught. + */ + old_cap = blob_line_cap(req->repo, req->old_oid); + new_cap = blob_line_cap(req->repo, req->new_oid); + if (old_cap < 0 || new_cap < 0) + old_cap = new_cap = -1; + if (validate_external_hunks(hunks, nr_hunks, old_cap, new_cap, + 0, drv->process, path) < 0) { + res = DIFF_PROCESS_SKIP; + goto out; + } + /* + * Replay in the coordinates a hunk consumer receives from + * xdiff's emission: 0-based starts. The answer is used as + * the process sent it; with no content in hand it cannot be + * re-run through xdiff's compaction. + */ + for (i = 0; i < nr_hunks; i++) + hunk_cb(hunks[i].old_start - 1, hunks[i].old_count, + hunks[i].new_start - 1, hunks[i].new_count, + cb_data); + res = DIFF_PROCESS_OK; + goto out; + } + if (!strcmp(status.buf, "need-content")) { + /* + * The process cannot answer this pair from its object names; + * the caller computes the diff itself. + */ + res = DIFF_PROCESS_SKIP; + goto out; + } + if (!strcmp(status.buf, "abort")) { + /* The process withdrew: stop asking it for this session. */ + entry->supported_capabilities &= ~CAP_OID_HUNKS; + res = DIFF_PROCESS_SKIP; + goto out; + } + /* + * An unrecognized status is a protocol error, not a per-pair + * failure: this Git did not request anything it does not know, + * so the process is answering some other protocol, and asking + * it again would warn on every pair of the traversal. + */ + warning(_("diff process '%s' sent unrecognized status '%s' for " + "'%s'; disabling it for the remainder of this command"), + drv->process, status.buf, path); + goto disable; +out: + free(hunks); + strbuf_release(&status); + sigchain_pop(SIGPIPE); + return res; + +comm_error: + warning(_("diff process '%s' failed for '%s'; disabling it" + " for the remainder of this command"), + drv->process, path); +disable: + subprocess_stop_command(&entry->subprocess); + entry->failed = 1; + free(hunks); + strbuf_release(&status); + sigchain_pop(SIGPIPE); + return DIFF_PROCESS_ERROR; +} + +/* + * The process outranks every later provider through its chain + * position: when it answers, the walk ends, so no later provider + * serves the pair, and an answered pair is never recorded. When it + * does not answer (it defers with need-content, lacks the + * capability, or failed), the caller computes the builtin diff for + * that pair. The store holds builtin results and nothing else, so + * an identity answer for such a pair equals what the caller would + * compute. Every non-answer is therefore a pass: a refusal would + * suppress that equal answer, and would keep a warming run from + * recording the builtin result the caller computes anyway. + */ +static enum diff_provider_disposition +diff_process_consult(struct diff_provider *provider, + const struct diff_provider_request *req, + diff_provider_fill_fn fill UNUSED, void *fill_data UNUSED, + xdl_emit_hunk_consume_func_t hunk_cb, void *cb_data) +{ + struct diff_process_state *state = provider->state; + struct userdiff_driver *drv; + struct subprocess_entry *running; + + drv = diff_process_driver(req->diffopt, req->path, req->xpp); + if (!drv) + return DIFF_PROVIDER_DISP_PASS; + running = subprocess_find_entry(&state->subprocesses, drv->process); + if (running && container_of(running, struct diff_subprocess, + subprocess)->failed) + return DIFF_PROVIDER_DISP_PASS; + + switch (diff_process_query_hunks(state, drv, req, + hunk_cb, cb_data)) { + case DIFF_PROCESS_OK: + case DIFF_PROCESS_EQUIVALENT: + return DIFF_PROVIDER_DISP_ANSWERED; + case DIFF_PROCESS_SKIP: + case DIFF_PROCESS_ERROR: + break; + } + return DIFF_PROVIDER_DISP_PASS; +} + +static void diff_process_release(struct diff_provider *provider) +{ + struct diff_process_state *state = provider->state; + struct hashmap_iter iter; + struct diff_subprocess *entry; + + /* A failed entry's process is already stopped or never ran. */ + hashmap_for_each_entry(&state->subprocesses, &iter, entry, + subprocess.ent) { + if (!entry->failed) + subprocess_stop_command(&entry->subprocess); + free(entry->cmd); + } + hashmap_clear_and_free(&state->subprocesses, + struct diff_subprocess, subprocess.ent); + free(state); +} + +struct diff_provider *diff_process_provider_new(void) +{ + struct diff_process_state *state = xcalloc(1, sizeof(*state)); + struct diff_provider *p = xcalloc(1, sizeof(*p)); + + hashmap_init(&state->subprocesses, cmd2process_cmp, NULL, 0); + p->consult = diff_process_consult; + p->release = diff_process_release; + p->state = state; + return p; +} diff --git a/diff-provider-internal.h b/diff-provider-internal.h new file mode 100644 index 0000000000..cba1fa271a --- /dev/null +++ b/diff-provider-internal.h @@ -0,0 +1,130 @@ +#ifndef DIFF_PROVIDER_INTERNAL_H +#define DIFF_PROVIDER_INTERNAL_H + +#include "diff-provider.h" + +/* + * The implementor-facing half of the hunk provider interface: the + * provider chain a repository owns, and the rules a provider applies + * to its own answer before any consumer sees it. Provider + * implementations include this header; consumers of the interface + * use only diff-provider.h. + */ + +/* + * A provider's verdict on one request. Only the chain walk + * (diff-provider.c) sees these; it maps the dispositions of a whole + * walk onto the public outcome set. + */ +enum diff_provider_disposition { + /* + * The provider failed to produce the answer it owns. Only + * the computing provider returns this: its compute leg is + * the one part of a consultation that can fail, and the walk + * ends with the public error outcome. + */ + DIFF_PROVIDER_DISP_ERROR = -1, + + /* + * Answered: every hunk of the pair has been emitted through + * the consumer's callback. + */ + DIFF_PROVIDER_DISP_ANSWERED = 0, + + /* Not this provider's request: the walk consults the next one. */ + DIFF_PROVIDER_DISP_PASS, + + /* + * The pair must not be answered from identity nor recorded: + * the request is shaped by parameters the provider's + * recording key cannot express, so a recorded answer would + * not match this request, and this request's result must not + * be recorded under that key. The walk goes on, but consults + * only the computing provider, and its fall-through outcome + * tells the consumer not to record. + */ + DIFF_PROVIDER_DISP_STOP_NO_RECORD, +}; + +/* + * One provider in a repository's chain (repository.h). The chain is + * assembled in diff-provider.c with a fixed composition; whether a + * provider applies to a request is decided by nobody but the + * provider, whose consult gates itself and passes. Chain position + * carries the authority resolution: an earlier provider's answer or + * refusal outranks every provider after it. + */ +struct diff_provider { + /* + * Consult this provider for one request. fill is NULL on a + * consult-only walk; only the computing provider reads it, + * and it must pass when fill is NULL. + */ + enum diff_provider_disposition + (*consult)(struct diff_provider *provider, + const struct diff_provider_request *req, + diff_provider_fill_fn fill, void *fill_data, + xdl_emit_hunk_consume_func_t hunk_cb, + void *cb_data); + + /* + * Tear down the provider's state, or NULL when it owns none. + * Runs when the owning repository is cleared; the chain frees + * the provider itself afterwards. + */ + void (*release)(struct diff_provider *provider); + + void *state; + + /* + * Set on the provider that loads content and computes rather + * than answering from the request's identity. It alone is + * still consulted after a stop-no-record: an identity answer + * may no longer be served, but the computation must still + * run. + */ + unsigned computes:1; + + struct diff_provider *next; +}; + +/* + * The providers Git ships, besides the builtin computation that + * diff-provider.c holds itself. Each call returns a fresh provider + * for one repository's chain. + */ +struct diff_provider *diff_process_provider_new(void); +struct diff_provider *diff_hunks_store_provider_new(void); + +/* + * Incremental well-formedness check for a provider-supplied hunk + * sequence, shared by every provider. Each coordinate, and each + * hunk's end (its start plus count), must fit int32 (a consumer may + * truncate to int, and a provider may serialize as such); hunks must + * be in order and must not overlap; and the unchanged run between + * hunks must be the same length on both sides, or a consumer that + * walks the two files in lockstep desynchronizes. Every rule + * constrains differences between coordinates, so the check applies + * to 0-based and 1-based sequences alike. + * + * Feed the hunks in order to a zero-initialized struct; the first + * nonzero return names the violated rule, and the whole sequence must + * then be discarded unemitted. + */ +struct diff_provider_hunks_check { + int64_t prev_old_end, prev_new_end; +}; + +enum diff_provider_hunks_error { + DIFF_PROVIDER_HUNKS_OK = 0, + DIFF_PROVIDER_HUNKS_RANGE, /* negative or beyond int32 */ + DIFF_PROVIDER_HUNKS_OVERLAP, /* out of order or overlapping */ + DIFF_PROVIDER_HUNKS_MISALIGNED, /* unchanged runs differ in length */ +}; + +enum diff_provider_hunks_error +diff_provider_check_hunk(struct diff_provider_hunks_check *c, + long old_start, long old_count, + long new_start, long new_count); + +#endif /* DIFF_PROVIDER_INTERNAL_H */ diff --git a/diff-provider.c b/diff-provider.c new file mode 100644 index 0000000000..c8aaf8e857 --- /dev/null +++ b/diff-provider.c @@ -0,0 +1,190 @@ +#include "git-compat-util.h" +#include "diff.h" +#include "diff-provider-internal.h" +#include "replace-object.h" +#include "repository.h" + +/* + * The terminal provider: the builtin computation. A request that + * carries a fill callback is answered by loading the pair's content + * and running xdiff, so a walk that reaches it never falls through + * to the consumer. On a consult-only walk it passes, and the walk's + * fall-through outcome tells the consumer to compute. + */ +static enum diff_provider_disposition +builtin_consult(struct diff_provider *provider UNUSED, + const struct diff_provider_request *req, + diff_provider_fill_fn fill, void *fill_data, + xdl_emit_hunk_consume_func_t hunk_cb, void *cb_data) +{ + xdemitconf_t xecfg = { .hunk_func = hunk_cb }; + xdemitcb_t ecb = { .priv = cb_data }; + mmfile_t old_file, new_file; + + if (!fill) + return DIFF_PROVIDER_DISP_PASS; + if (fill(fill_data, &old_file, &new_file) < 0) + return DIFF_PROVIDER_DISP_ERROR; + if (xdi_diff(&old_file, &new_file, req->xpp, &xecfg, &ecb) < 0) + return DIFF_PROVIDER_DISP_ERROR; + return DIFF_PROVIDER_DISP_ANSWERED; +} + +static struct diff_provider *builtin_provider_new(void) +{ + struct diff_provider *p = xcalloc(1, sizeof(*p)); + + p->consult = builtin_consult; + p->computes = 1; + return p; +} + +/* + * The repository's chain, assembled on first walk. The composition + * is fixed, and the order is the authority resolution: the process + * outranks the store, and the builtin computation is the terminal + * provider, so the chain always ends in an implementor that can + * answer. Nothing is decided per repository here; each provider + * gates itself per request. + */ +static struct diff_provider *provider_chain(struct repository *r) +{ + struct diff_provider **tail = &r->diff_providers; + + if (*tail) + return *tail; + *tail = diff_process_provider_new(); + tail = &(*tail)->next; + *tail = diff_hunks_store_provider_new(); + tail = &(*tail)->next; + *tail = builtin_provider_new(); + return r->diff_providers; +} + +void diff_providers_clear(struct repository *r) +{ + struct diff_provider *p = r->diff_providers; + + while (p) { + struct diff_provider *next = p->next; + + if (p->release) + p->release(p); + free(p); + p = next; + } + r->diff_providers = NULL; +} + +/* + * The walk shared by diff_provider_consult() and + * diff_provider_emit_hunks(): consult the chain in order and map its + * dispositions onto the outcome set. The first answer ends the + * walk. A stop-no-record disposition (diff-provider-internal.h) + * is a refusal, not a pass: the provider does not answer, but rules + * the pair out of identity service and out of recording, so from + * then on the walk consults only the computing provider, and a walk + * that ends unanswered carries the no-record verdict. With a fill + * callback the terminal provider computes instead of passing, so an + * emit walk returns only answered or error. + */ +static enum diff_provider_outcome +walk_providers(const struct diff_provider_request *req, + diff_provider_fill_fn fill, void *fill_data, + xdl_emit_hunk_consume_func_t hunk_cb, void *cb_data) +{ + struct diff_provider *p; + int no_record = 0; + + if (req->diffopt && req->diffopt->repo != req->repo) + BUG("diff provider request walks one repository's chain " + "with another repository's diff options"); + + /* + * An object replacement redirects a blob's content + * (OBJECT_INFO_LOOKUP_REPLACE) while leaving the id that names it + * unchanged, so an answer keyed on the raw id would be the + * pre-replacement diff. A replacement is therefore a parameter + * outside the recording key: no provider may serve a replaced pair + * from its identity, and a result computed for it must not be + * recorded under the raw id. Mark the walk no-record so the + * identity providers step aside and the builtin computes from the + * replaced content. The check is a no-op when the repository has + * no replace refs. + */ + if ((req->old_oid && + lookup_replace_object(req->repo, req->old_oid) != req->old_oid) || + (req->new_oid && + lookup_replace_object(req->repo, req->new_oid) != req->new_oid)) + no_record = 1; + + for (p = provider_chain(req->repo); p; p = p->next) { + enum diff_provider_disposition disp; + + if (no_record && !p->computes) + continue; + disp = p->consult(p, req, fill, fill_data, + hunk_cb, cb_data); + if (disp == DIFF_PROVIDER_DISP_ERROR && !p->computes) + BUG("only the computing provider may return the " + "error disposition"); + if (p->computes && !fill && disp != DIFF_PROVIDER_DISP_PASS) + BUG("the computing provider must pass on a " + "fill-less walk"); + switch (disp) { + case DIFF_PROVIDER_DISP_ANSWERED: + return DIFF_PROVIDER_ANSWERED; + case DIFF_PROVIDER_DISP_PASS: + continue; + case DIFF_PROVIDER_DISP_STOP_NO_RECORD: + no_record = 1; + continue; + case DIFF_PROVIDER_DISP_ERROR: + return DIFF_PROVIDER_ERROR; + } + } + return no_record ? DIFF_PROVIDER_UNANSWERED_NO_RECORD : + DIFF_PROVIDER_UNANSWERED; +} + +enum diff_provider_outcome +diff_provider_consult(const struct diff_provider_request *req, + xdl_emit_hunk_consume_func_t hunk_cb, void *cb_data) +{ + return walk_providers(req, NULL, NULL, hunk_cb, cb_data); +} + +enum diff_provider_hunks_error +diff_provider_check_hunk(struct diff_provider_hunks_check *c, + long old_start, long old_count, + long new_start, long new_count) +{ + if (old_start < 0 || old_count < 0 || + new_start < 0 || new_count < 0 || + old_start > INT32_MAX || old_count > INT32_MAX || + new_start > INT32_MAX || new_count > INT32_MAX || + (int64_t)old_start + old_count > INT32_MAX || + (int64_t)new_start + new_count > INT32_MAX) + return DIFF_PROVIDER_HUNKS_RANGE; + if (old_start < c->prev_old_end || new_start < c->prev_new_end) + return DIFF_PROVIDER_HUNKS_OVERLAP; + if (old_start - c->prev_old_end != new_start - c->prev_new_end) + return DIFF_PROVIDER_HUNKS_MISALIGNED; + /* + * With each field bounded to int32 above, the int64 sums cannot + * overflow even where long is 32-bit, and the range rule has + * already capped them at INT32_MAX. + */ + c->prev_old_end = (int64_t)old_start + old_count; + c->prev_new_end = (int64_t)new_start + new_count; + return DIFF_PROVIDER_HUNKS_OK; +} + +enum diff_provider_outcome +diff_provider_emit_hunks(const struct diff_provider_request *req, + diff_provider_fill_fn fill, void *fill_data, + xdl_emit_hunk_consume_func_t hunk_cb, + void *cb_data) +{ + return walk_providers(req, fill, fill_data, hunk_cb, cb_data); +} diff --git a/diff-provider.h b/diff-provider.h new file mode 100644 index 0000000000..061e1c2f5c --- /dev/null +++ b/diff-provider.h @@ -0,0 +1,159 @@ +#ifndef DIFF_PROVIDER_H +#define DIFF_PROVIDER_H + +#include "xdiff-interface.h" + +/* + * The hunk provider interface sits between naming a pair of file + * versions to diff and computing their changed line ranges. + * Consumers that operate on hunk coordinates route their diff + * through here, so that a provider can answer for the pair before + * its content is loaded. + * + * A hunk provider answers a consumer's request from the pair's + * identity, its blob object ids and the settings that determine the + * diff, before any content is loaded; a request no provider answers + * falls through to the consumer's own computation. Two providers implement this + * interface with different authority. The diff-hunks store + * (diff-hunks.h) is in-process and not authoritative: it may only + * reproduce the builtin result, so it never asserts a pair + * equivalent, and it stands aside wherever a process outranks it. A + * process configured in diff..process (diff-process.c) is + * authoritative for its paths: its answer may deliberately differ + * from the builtin diff, including asserting a pair equivalent. The + * interface resolves that authority through a provider chain owned + * by the repository, built on first consultation and released by + * repo_clear(): chain order is the resolution, and the builtin + * computation itself is the chain's terminal provider. A consumer + * never names a provider; it reads the outcome below. Every answer a + * provider serves from identity passes the shared coordinate check + * (diff-provider-internal.h) before any consumer sees it. + */ + +struct diff_options; +struct object_id; +struct repository; + +/* + * The result of a consultation: two dependent axes flattened into + * their four valid points. The first axis is the state of the + * response: the pair was answered, no provider answered, or (from + * diff_provider_emit_hunks() alone) the attempt failed. The second + * axis exists only in the unanswered state: whether what the caller + * computes for this request may be recorded, the one rule the + * interface imposes on an otherwise free caller. The rule travels + * in the outcome because the knowledge is a provider's while the + * recording is the caller's, and it shares the enum with the state, + * rather than riding a separate flag, so that no meaningless + * combination is representable and -Wswitch forces every consumer + * that switches to place the no-record arm. + * + * These values describe consultations, not providers: the set does + * not grow when a provider is added; a new provider maps onto these + * values inside the interface, so consumer code is written once. + * Each entry point returns a subrange of the set (stated at its + * declaration); a switch over this enum should list every value and + * omit "default:" so -Wswitch keeps it exhaustive, and a caller for + * whom only one value is actionable may compare against that value + * alone. + */ +enum diff_provider_outcome { + /* + * Loading or diffing the pair failed. Returned only by + * diff_provider_emit_hunks(), whose compute leg is the only + * part of a consultation that can fail. + */ + DIFF_PROVIDER_ERROR = -1, + + /* + * The request is answered: every hunk of the pair has been + * emitted through the callback. An authoritative provider + * that finds the pair equivalent answers with no hunks at + * all, so a callback that never fired is an answer, not an + * accident. + */ + DIFF_PROVIDER_ANSWERED = 0, + + /* + * No provider answered. What happens next is the caller's + * business, typically computing the diff itself; a result it + * computes for this request may be recorded. + */ + DIFF_PROVIDER_UNANSWERED, + + /* + * No provider answered, and what the caller computes for + * this request must not be recorded: either an authoritative + * provider owns the pair and declined this request, or the + * request is shaped by parameters outside the recording key, + * the key a recorded result is later served by. + */ + DIFF_PROVIDER_UNANSWERED_NO_RECORD, +}; + +/* + * A consultation request. The interface consults providers from + * these fields alone; no content is loaded before an answer. + * + * repo owns the provider chain the request walks. old_oid/new_oid + * name the blobs whose bytes are diffed; pass NULL for a side whose + * bytes are not a stored blob (a working-tree file, textconv output, + * a gitlink), so no provider answers from an id it cannot look up. + * path names the file the pair is diffed as; a provider selected by + * path applies only where it is set. diffopt carries the diff + * settings that live outside xpp; xpp carries the parameters the + * diff runs with. Each provider gates itself on the fields that + * concern it. + */ +struct diff_provider_request { + struct repository *repo; + const struct object_id *old_oid; + const struct object_id *new_oid; + const char *path; + struct diff_options *diffopt; + const xpparam_t *xpp; +}; + +/* + * Consult the providers for the request's pair without computing. + * On DIFF_PROVIDER_ANSWERED the hunks were emitted through hunk_cb + * (0-based emission coordinates, context 0) and were validated + * before the first callback ran, so a consumer may accumulate + * directly into its result. Never returns DIFF_PROVIDER_ERROR. + * The callback's return value is not consulted: emission of a + * validated answer has no error leg, so the callback must return 0. + */ +enum diff_provider_outcome +diff_provider_consult(const struct diff_provider_request *req, + xdl_emit_hunk_consume_func_t hunk_cb, void *cb_data); + +/* + * Load the pair's content. Called at most once per request, only + * when the ranges are computed rather than provided. The buffers + * borrow storage owned by the callback's owner. + */ +typedef int (*diff_provider_fill_fn)(void *data, mmfile_t *old_file, + mmfile_t *new_file); + +/* + * Consult the providers and, when no identity answer serves the + * request, load the pair's content through fill and compute its + * exact changed ranges (context 0). Emits to hunk_cb either way and + * returns DIFF_PROVIDER_ANSWERED, or DIFF_PROVIDER_ERROR when fill + * or the diff fails. The unanswered outcomes are never returned: a + * pair no provider answers is computed here instead of in the caller. + */ +enum diff_provider_outcome +diff_provider_emit_hunks(const struct diff_provider_request *req, + diff_provider_fill_fn fill, void *fill_data, + xdl_emit_hunk_consume_func_t hunk_cb, + void *cb_data); + +/* + * Release the repository's provider chain: stop any provider-owned + * processes and free the providers. Called by repo_clear(); the + * chain builds again on the next consultation. + */ +void diff_providers_clear(struct repository *r); + +#endif /* DIFF_PROVIDER_H */ diff --git a/diff.c b/diff.c index 17325be45f..5b3fa51a29 100644 --- a/diff.c +++ b/diff.c @@ -16,6 +16,8 @@ #include "revision.h" #include "quote.h" #include "diff.h" +#include "diff-hunks.h" +#include "diff-provider.h" #include "diffcore.h" #include "delta.h" #include "hex.h" @@ -34,6 +36,7 @@ #include "tmp-objdir.h" #include "graph.h" #include "oid-array.h" +#include "trace2.h" #include "packfile.h" #include "pager.h" #include "parse-options.h" @@ -2929,6 +2932,77 @@ static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat, return x; } +struct diffstat_hunk_cb_data { + struct precomputed_hunk **h; + size_t *nr, *alloc; +}; + +/* + * Hunk callback that appends each hunk's coordinates to a growable + * array, so one xdiff pass can both sum a diffstat and record hunks for + * the store. + */ +static int diffstat_hunk_cb(long start_a, long count_a, + long start_b, long count_b, + void *cb_data) +{ + struct diffstat_hunk_cb_data *d = cb_data; + + ALLOC_GROW(*d->h, *d->nr + 1, *d->alloc); + (*d->h)[*d->nr].old_start = start_a; + (*d->h)[*d->nr].old_count = count_a; + (*d->h)[*d->nr].new_start = start_b; + (*d->h)[*d->nr].new_count = count_b; + (*d->nr)++; + return 0; +} + +/* + * Collect the hunks of the two files at zero context. diff_fn chooses + * whether trimming runs: xdi_diff applies trim_common_tail, yielding the + * zero-context hunks blame reads; xdl_diff does not, yielding the + * untrimmed hunks. Both run at zero context, so the untrimmed hunks are + * not grouped the way a nonzero context would group them; diffstat only + * sums their counts, which grouping does not change. Sets *ph (caller + * frees) and *ph_nr. + */ +typedef int (*xdiff_fn)(mmfile_t *, mmfile_t *, xpparam_t const *, + xdemitconf_t const *, xdemitcb_t *); +static int collect_hunks(xdiff_fn diff_fn, mmfile_t *mf1, mmfile_t *mf2, + xpparam_t *xpp, struct precomputed_hunk **ph, + size_t *ph_nr) +{ + size_t ph_alloc = 0; + xdemitcb_t ecb = { 0 }; + xdemitconf_t xecfg = { 0 }; + struct diffstat_hunk_cb_data cd = { ph, ph_nr, &ph_alloc }; + + *ph = NULL; + *ph_nr = 0; + xecfg.hunk_func = diffstat_hunk_cb; + ecb.priv = &cd; + return diff_fn(mf1, mf2, xpp, &xecfg, &ecb); +} + +void diff_hunks_attach(struct diff_options *o) +{ + if (!(o->output_format & + (DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_NUMSTAT))) + return; + o->hunks_writer = diff_hunks_writer_maybe_new(o->repo); +} + +void diff_hunks_detach(struct diff_options *o) +{ + unsigned long hits, misses; + + diff_hunks_read_stats(o->repo, &hits, &misses); + if (hits) + trace2_data_intmax("diff-hunks", o->repo, "read-hits", hits); + diff_hunks_writer_finish(o->hunks_writer); + o->hunks_writer = NULL; +} + static int diffstat_consume(void *priv, char *line, unsigned long len) { struct diffstat_t *diffstat = priv; @@ -4232,6 +4306,125 @@ static const char *get_compact_summary(const struct diff_filepair *p, int is_ren return NULL; } +/* + * Hunk callback for the provider interface: sum counts into a + * diffstat entry. + */ +static int diffstat_sum_hunk_cb(long start_a UNUSED, long count_a, + long start_b UNUSED, long count_b, + void *cb_data) +{ + struct diffstat_file *data = cb_data; + + data->added += count_b; + data->deleted += count_a; + return 0; +} + +/* + * Fill data->added/deleted for a modified pair through the hunk provider + * interface: on an answer, sum the provided counts; on a warming run, + * compute and record them. Returns 1 when it produced the counts, 0 when + * the caller must compute the diffstat itself. + * + * The providers own the exclusions the request can express (-B, -I, + * and --anchored are outside the store key). This consumer additionally + * excludes --ignore-blank-lines before consulting: that flag is part of + * the key, but it coalesces hunks differently between the emit and + * hunk-callback paths, so a served answer would not match a store-less + * run's --stat output. (--inter-hunk-context is not excluded: it only + * groups hunks, and diffstat sums their counts, which grouping does not + * change.) Recording requires both sides to be valid regular files whose + * blobs the key can name. + */ +static int diffstat_from_hunks(struct diff_options *o, + struct diff_filespec *one, + struct diff_filespec *two, + struct diffstat_file *data) +{ + struct precomputed_hunk *ph_trim, *ph_full, *counts; + size_t n_trim, n_full, n_counts, k; + mmfile_t mf1, mf2; + xpparam_t xpp = { .flags = o->xdl_opts, + .ignore_regex = o->ignore_regex, + .ignore_regex_nr = o->ignore_regex_nr, + .anchors = o->anchors, + .anchors_nr = o->anchors_nr }; + struct diff_provider_request req = { + .repo = o->repo, + .old_oid = (one->oid_valid && !S_ISGITLINK(one->mode)) ? + &one->oid : NULL, + .new_oid = (two->oid_valid && !S_ISGITLINK(two->mode)) ? + &two->oid : NULL, + /* + * Attribute lookup and the process protocol need the + * repo-relative path; the display name a caller passes + * around may be stripped of o->prefix and would miss a + * driver scoped to a directory. + */ + .path = one->path, + .diffopt = o, + .xpp = &xpp, + }; + + if (o->xdl_opts & XDF_IGNORE_BLANK_LINES) + return 0; + /* format-patch keeps its diffstat off the store (see the flag). */ + if (o->flags.no_precomputed_hunks) + return 0; + + switch (diff_provider_consult(&req, diffstat_sum_hunk_cb, data)) { + case DIFF_PROVIDER_ANSWERED: + return 1; + case DIFF_PROVIDER_UNANSWERED: + break; + case DIFF_PROVIDER_ERROR: /* not returned by a consult */ + case DIFF_PROVIDER_UNANSWERED_NO_RECORD: + return 0; + } + + /* A miss on a read-only run: let the caller compute the diffstat. */ + if (!o->hunks_writer) + return 0; + /* Recording needs blobs the key can name, on both sides. */ + if (!req.old_oid || !req.new_oid || + !DIFF_FILE_VALID(one) || !DIFF_FILE_VALID(two) || + !S_ISREG(one->mode) || !S_ISREG(two->mode)) + return 0; + + if (fill_mmfile(o->repo, &mf1, one) < 0 || + fill_mmfile(o->repo, &mf2, two) < 0) + die("unable to read files to diff"); + + /* + * Compute the zero-context trimmed diff (what blame reads) and the + * untrimmed diff (whose counts a nonzero-context stat matches). + * xdi_diff runs first: it enforces the size limit, so the xdl_diff + * call is already bounded. + */ + if (collect_hunks(xdi_diff, &mf1, &mf2, &xpp, &ph_trim, &n_trim) || + collect_hunks(xdl_diff, &mf1, &mf2, &xpp, &ph_full, &n_full)) + die("unable to generate diffstat for %s", one->path); + + /* + * Match a store-less run: at zero context xdi_diff trims, so sum the + * trimmed diff; otherwise sum the untrimmed one. + */ + counts = o->context ? ph_full : ph_trim; + n_counts = o->context ? n_full : n_trim; + for (k = 0; k < n_counts; k++) { + data->added += counts[k].new_count; + data->deleted += counts[k].old_count; + } + + diff_hunks_writer_record_stable(o->hunks_writer, &one->oid, &two->oid, + o->xdl_opts, ph_trim, n_trim, + ph_full, n_full); + free(ph_trim); + free(ph_full); + return 1; +} + static void builtin_diffstat(const char *name_a, const char *name_b, struct diff_filespec *one, struct diff_filespec *two, @@ -4283,38 +4476,52 @@ static void builtin_diffstat(const char *name_a, const char *name_b, } else if (may_differ) { - /* Crazy xdl interfaces.. */ - xpparam_t xpp; - xdemitconf_t xecfg; + /* + * Serve from a hunk provider (the process, then the store), + * or record into the store on a warming run. A "log -L" + * range-scoped stat is not the whole-pair diff the store + * keys, so it neither reads nor records. Otherwise diff + * normally. + */ + if (p->line_ranges || + !diffstat_from_hunks(o, one, two, data)) { + /* Crazy xdl interfaces.. */ + xpparam_t xpp; + xdemitconf_t xecfg; - if (fill_mmfile(o->repo, &mf1, one) < 0 || - fill_mmfile(o->repo, &mf2, two) < 0) - die("unable to read files to diff"); + if (fill_mmfile(o->repo, &mf1, one) < 0 || + fill_mmfile(o->repo, &mf2, two) < 0) + die("unable to read files to diff"); - memset(&xpp, 0, sizeof(xpp)); - memset(&xecfg, 0, sizeof(xecfg)); - xpp.flags = o->xdl_opts; - xpp.ignore_regex = o->ignore_regex; - xpp.ignore_regex_nr = o->ignore_regex_nr; - xpp.anchors = o->anchors; - xpp.anchors_nr = o->anchors_nr; - xecfg.ctxlen = o->context; - xecfg.interhunkctxlen = o->interhunkcontext; - xecfg.flags = XDL_EMIT_NO_HUNK_HDR; + memset(&xpp, 0, sizeof(xpp)); + memset(&xecfg, 0, sizeof(xecfg)); + xpp.flags = o->xdl_opts; + xpp.ignore_regex = o->ignore_regex; + xpp.ignore_regex_nr = o->ignore_regex_nr; + xpp.anchors = o->anchors; + xpp.anchors_nr = o->anchors_nr; + xecfg.ctxlen = o->context; + xecfg.interhunkctxlen = o->interhunkcontext; + xecfg.flags = XDL_EMIT_NO_HUNK_HDR; - if (p->line_ranges) { - struct line_range_filter lr_filter; + if (p->line_ranges) { + struct line_range_filter lr_filter; - line_range_filter_init(&lr_filter, p->line_ranges, - diffstat_consume, diffstat); + line_range_filter_init(&lr_filter, + p->line_ranges, + diffstat_consume, + diffstat); - if (line_range_filter_diff(&lr_filter, &mf1, &mf2, - &xpp, &xecfg)) + if (line_range_filter_diff(&lr_filter, &mf1, + &mf2, &xpp, &xecfg)) + die("unable to generate diffstat for %s", + one->path); + } else if (xdi_diff_outf(&mf1, &mf2, NULL, + diffstat_consume, diffstat, + &xpp, &xecfg)) die("unable to generate diffstat for %s", one->path); - } else if (xdi_diff_outf(&mf1, &mf2, NULL, - diffstat_consume, diffstat, &xpp, &xecfg)) - die("unable to generate diffstat for %s", one->path); + } if (DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two)) { struct diffstat_file *file = @@ -6033,6 +6240,27 @@ 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.allow_diff_process = !unset; + return 0; +} + +static int diff_opt_diff_process(const struct option *opt, + const char *arg, int unset) +{ + struct diff_options *options = opt->value; + + BUG_ON_OPT_ARG(arg); + options->flags.allow_diff_process = !unset; + return 0; +} + static int diff_opt_textconv(const struct option *opt, const char *arg, int unset) { @@ -6363,8 +6591,12 @@ 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, "diff-process", options, NULL, + N_("allow a configured diff process to be consulted"), + PARSE_OPT_NOARG, diff_opt_diff_process), 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 bb5cddaf34..e0b58a8105 100644 --- a/diff.h +++ b/diff.h @@ -173,6 +173,15 @@ struct diff_flags { */ unsigned allow_external; + /** + * Allows diff..process to be consulted. Set by the + * porcelain commands whose output may reflect a diff process + * (diff, log, show, blame) and by --ext-diff or --diff-process; + * plumbing does not set it by default, so its output stays + * builtin. Cleared by --no-ext-diff or --no-diff-process. + */ + unsigned allow_diff_process; + /** * For communication between the calling program and the options parser; * tell the calling program to signal the presence of difference using @@ -206,6 +215,13 @@ struct diff_flags { unsigned suppress_diff_headers; unsigned dual_color_diffed_diffs; unsigned suppress_hunk_header_line_count; + + /* + * Do not serve the diffstat from the precomputed-hunks store. + * Set by format-patch so a generated patch carries the builtin + * counts and does not depend on the sender's local store state. + */ + unsigned no_precomputed_hunks; }; static inline void diff_flags_or(struct diff_flags *a, @@ -224,6 +240,17 @@ static inline void diff_flags_or(struct diff_flags *a, #define DIFF_WITH_ALG(opts, flag) (((opts)->xdl_opts & ~XDF_DIFF_ALGORITHM_MASK) | XDF_##flag) +/* + * The xdl_opts bits git turns on by default that a from-scratch xdl_opts + * (git blame's own option parsing) does not set, and so must OR in to match + * a store warmed at the default diff settings; a diff_options-based consumer + * (diffstat) already has them in o->xdl_opts. Today this is only the indent + * heuristic. It does NOT cover a non-default diff.algorithm: a repo that + * configures one records under that algorithm, and a consumer keying without + * it misses (a lost hit, not wrong output). + */ +#define DIFF_HUNKS_DEFAULT_XDL_OPTS XDF_INDENT_HEURISTIC + enum diff_words_type { DIFF_WORDS_NONE = 0, DIFF_WORDS_PORCELAIN, @@ -420,6 +447,15 @@ struct diff_options { */ int max_depth; int max_depth_valid; + + /* + * Precomputed diff hunks (see diff-hunks.h). diffstat consults the + * hunk provider interface before running xdiff, keyed by each file + * pair's blob object IDs. When hunks_writer is set (a warming run), + * diffstat also records the hunks it computes; the writer is + * attached only for the stat output formats. + */ + struct diff_hunks_writer *hunks_writer; }; unsigned diff_filter_bit(char status); @@ -668,6 +704,17 @@ void diffcore_fix_diff_index(void); int diff_queue_is_empty(struct diff_options *o); void diff_flush(struct diff_options*); void diff_free(struct diff_options*); + +/* + * Attach a diff-hunks writer to a diff producing a stat format, so a + * warming run records the hunks it computes; a no-op when writing is off + * or for other formats. (Reading is separate: consumers consult the + * providers through diff_provider_consult(); see diff-provider.h.) Pair + * with diff_hunks_detach() once the diff is done. + */ +void diff_hunks_attach(struct diff_options *o); +void diff_hunks_detach(struct diff_options *o); + void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc); /* diff-raw status letters */ diff --git a/environment.c b/environment.c index 76ee65e62b..c756d88053 100644 --- a/environment.c +++ b/environment.c @@ -253,6 +253,7 @@ static const struct fsync_component_name { { "pack", FSYNC_COMPONENT_PACK }, { "pack-metadata", FSYNC_COMPONENT_PACK_METADATA }, { "commit-graph", FSYNC_COMPONENT_COMMIT_GRAPH }, + { "diff-hunks", FSYNC_COMPONENT_DIFF_HUNKS }, { "index", FSYNC_COMPONENT_INDEX }, { "objects", FSYNC_COMPONENTS_OBJECTS }, { "reference", FSYNC_COMPONENT_REFERENCE }, diff --git a/git.c b/git.c index e5f1811b6b..cb149e4b4e 100644 --- a/git.c +++ b/git.c @@ -566,6 +566,7 @@ static struct cmd_struct commands[] = { { "diagnose", cmd_diagnose, RUN_SETUP_GENTLY }, { "diff", cmd_diff, NO_PARSEOPT }, { "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "diff-hunks", cmd_diff_hunks, RUN_SETUP }, { "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT }, { "diff-pairs", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT }, { "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT }, diff --git a/meson.build b/meson.build index 6eed709264..596367f939 100644 --- a/meson.build +++ b/meson.build @@ -356,6 +356,8 @@ libgit_sources = [ 'diff-merges.c', 'diff-lib.c', 'diff-no-index.c', + 'diff-process.c', + 'diff-provider.c', 'diff.c', 'diffcore-break.c', 'diffcore-delta.c', @@ -363,6 +365,7 @@ libgit_sources = [ 'diffcore-pickaxe.c', 'diffcore-rename.c', 'diffcore-rotate.c', + 'diff-hunks.c', 'dir-iterator.c', 'dir.c', 'editor.c', @@ -633,6 +636,7 @@ builtin_sources = [ 'builtin/describe.c', 'builtin/diagnose.c', 'builtin/diff-files.c', + 'builtin/diff-hunks.c', 'builtin/diff-index.c', 'builtin/diff-pairs.c', 'builtin/diff-tree.c', diff --git a/odb.c b/odb.c index bd02d8ad54..7c81ba20c8 100644 --- a/odb.c +++ b/odb.c @@ -2,6 +2,7 @@ #include "abspath.h" #include "commit-graph.h" #include "config.h" +#include "diff-hunks.h" #include "dir.h" #include "environment.h" #include "gettext.h" @@ -1079,6 +1080,7 @@ void odb_close(struct object_database *o) for (source = o->sources; source; source = source->next) odb_source_close(source); close_commit_graph(o); + close_diff_hunks_store(o); } static void odb_free_sources(struct object_database *o) diff --git a/odb.h b/odb.h index 7fc29bdbc3..edd415c431 100644 --- a/odb.h +++ b/odb.h @@ -8,6 +8,7 @@ #include "thread-utils.h" struct cached_object_entry; +struct diff_hunks_store; struct list_objects_filter_options; struct odb_source_inmemory; struct packed_git; @@ -76,6 +77,9 @@ struct object_database { struct commit_graph *commit_graph; unsigned commit_graph_attempted : 1; /* if loading has been attempted */ + struct diff_hunks_store *diff_hunks_store; + unsigned diff_hunks_store_attempted : 1; /* if loading has been attempted */ + /* * This is meant to hold a *small* number of objects that you would * want odb_read_object() to be able to return, but yet you do not want diff --git a/range-diff.c b/range-diff.c index 8e2dd2eb19..3cadbcd7e1 100644 --- a/range-diff.c +++ b/range-diff.c @@ -52,6 +52,12 @@ static int read_patches(const char *range, struct string_list *list, int ret = -1; strvec_pushl(&cp.args, "log", "--no-color", "-p", + /* + * The patches being compared must be the builtin + * diff's: an external diff command or diff process + * could change either side of the comparison. + */ + "--no-ext-diff", "--reverse", "--date-order", "--decorate=no", "--no-prefix", "--submodule=short", /* diff --git a/repo-settings.c b/repo-settings.c index f3be3b8c5a..c3015356ba 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -77,6 +77,7 @@ void prepare_repo_settings(struct repository *r) repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1); repo_cfg_bool(r, "pack.usepathwalk", &r->settings.pack_use_path_walk, 0); repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1); + repo_cfg_bool(r, "core.diffhunks", &r->settings.core_diff_hunks, 1); repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0); repo_cfg_bool(r, "index.skiphash", &r->settings.index_skip_hash, r->settings.index_skip_hash); repo_cfg_bool(r, "pack.readreverseindex", &r->settings.pack_read_reverse_index, 1); diff --git a/repo-settings.h b/repo-settings.h index e5253ead02..615a55cac4 100644 --- a/repo-settings.h +++ b/repo-settings.h @@ -22,6 +22,7 @@ struct repo_settings { int core_commit_graph; int commit_graph_generation_version; int commit_graph_changed_paths_version; + int core_diff_hunks; int gc_write_commit_graph; int fetch_write_commit_graph; int command_requires_full_index; diff --git a/repository.c b/repository.c index db4f9d006e..f4522155d0 100644 --- a/repository.c +++ b/repository.c @@ -5,6 +5,7 @@ #include "odb.h" #include "odb/source.h" #include "config.h" +#include "diff-provider.h" #include "gettext.h" #include "object.h" #include "lockfile.h" @@ -382,6 +383,8 @@ void repo_clear(struct repository *repo) FREE_AND_NULL(repo->submodule_prefix); FREE_AND_NULL(repo->ref_storage_payload); + diff_providers_clear(repo); + odb_free(repo->objects); repo->objects = NULL; diff --git a/repository.h b/repository.h index 3b467a2513..710ddb7938 100644 --- a/repository.h +++ b/repository.h @@ -7,6 +7,7 @@ #include "environment.h" struct config_set; +struct diff_provider; struct git_hash_algo; struct index_state; struct lock_file; @@ -161,6 +162,13 @@ struct repository { /* Repository's remotes and associated structures. */ struct remote_state *remote_state; + /* + * The repository's diff hunk provider chain, NULL until the + * first consultation builds it (diff-provider.c); repo_clear() + * releases it. + */ + struct diff_provider *diff_providers; + /* Repository's current hash algorithm, as serialized on disk. */ const struct git_hash_algo *hash_algo; diff --git a/sub-process.c b/sub-process.c index 2d5c965169..33bd789618 100644 --- a/sub-process.c +++ b/sub-process.c @@ -49,7 +49,31 @@ int subprocess_read_status(int fd, struct strbuf *status) return (len < 0) ? len : 0; } -void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry) +int subprocess_read_status_gently(int fd, struct strbuf *status) +{ + for (;;) { + int pktlen = -1; + enum packet_read_status rs; + const char *value; + + rs = packet_read_with_status(fd, NULL, NULL, packet_buffer, + sizeof(packet_buffer), &pktlen, + PACKET_READ_CHOMP_NEWLINE | + PACKET_READ_GENTLE_ON_EOF | + PACKET_READ_GENTLE_ON_READ_ERROR); + if (rs == PACKET_READ_FLUSH) + return 0; + if (rs != PACKET_READ_NORMAL || !pktlen) + return -1; + if (skip_prefix(packet_buffer, "status=", &value)) { + /* the last "status=" line wins */ + strbuf_reset(status); + strbuf_addstr(status, value); + } + } +} + +void subprocess_stop_command(struct subprocess_entry *entry) { if (!entry) return; @@ -57,7 +81,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 +103,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 +127,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..8655b38897 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); @@ -94,4 +101,14 @@ int subprocess_handshake(struct subprocess_entry *entry, int subprocess_read_status(int fd, struct strbuf *status); +/* + * Like subprocess_read_status(), but a malformed status section fails + * instead of dying: a truncated or malformed packet, and an empty + * packet where a status line or the terminating flush belongs, return + * -1 and leave the stream unusable. subprocess_read_status() cannot + * tell an empty packet from the flush that ends the section, and dies + * on a framing error inside packet_read_line_gently(). + */ +int subprocess_read_status_gently(int fd, struct strbuf *status); + #endif 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..b0bb78d9f3 --- /dev/null +++ b/t/helper/test-diff-process-backend.c @@ -0,0 +1,349 @@ +/* + * Test process implementing the diff process protocol (diff..process). + * + * Speaks the long-running process protocol over stdin/stdout and + * answers command=hunks-by-oid requests from the blob object names + * alone; no content is exchanged. The --mode= switch selects the + * response shape: + * + * oid-fixed packet: git< hunk 5 2 5 2 + * oid-equal packet: git< status=success (zero hunks: equivalent) + * oid-need-content packet: git< status=need-content + * oid-empty packet: git< hunk 0 0 1 2 (empty old side) + * + * and the adversarial shapes the protocol error paths are tested + * with: + * + * oid-trailing a hunk line with a trailing token to ignore + * oid-malformed a hunk line that does not parse + * oid-huge coordinates far past the end of any test blob + * oid-erange a count too large for any long + * oid-overlap two hunks out of order + * oid-misaligned two hunks whose unchanged runs differ in length + * oid-badstart a start of 0 paired with a nonzero count + * oid-unknown-status status=frobnicate + * oid-abort status=abort + * oid-bare-status a status packet without the hunk-section flush + * oid-empty-packet an empty packet (0004) inside the hunk section + * oid-crash one hunk line, then exit with no flush or status + * oid-garbage raw non-pkt-line bytes, then exit + * cap-none handshake announcing no capability at all + * + * Success responses end with: + * + * packet: git< 0000 + * packet: git< status=success + * packet: git< 0000 + * + * Each request is logged to --log as: + * + * command= pathname= old-oid= new-oid= + */ + +#include "test-tool.h" +#include "pkt-line.h" +#include "parse-options.h" +#include "strbuf.h" + +static FILE *logfile; + +enum mode { + MODE_OID_FIXED, + MODE_OID_EQUAL, + MODE_OID_NEED_CONTENT, + MODE_OID_EMPTY, + MODE_OID_TRAILING, + MODE_OID_MALFORMED, + MODE_OID_HUGE, + MODE_OID_ERANGE, + MODE_OID_OVERLAP, + MODE_OID_MISALIGNED, + MODE_OID_BADSTART, + MODE_OID_UNKNOWN_STATUS, + MODE_OID_ABORT, + MODE_OID_BARE_STATUS, + MODE_OID_EMPTY_PACKET, + MODE_OID_CRASH, + MODE_OID_GARBAGE, + MODE_CAP_NONE, +}; + +static enum mode parse_mode(const char *s) +{ + if (!strcmp(s, "oid-fixed")) + return MODE_OID_FIXED; + if (!strcmp(s, "oid-equal")) + return MODE_OID_EQUAL; + if (!strcmp(s, "oid-need-content")) + return MODE_OID_NEED_CONTENT; + if (!strcmp(s, "oid-empty")) + return MODE_OID_EMPTY; + if (!strcmp(s, "oid-trailing")) + return MODE_OID_TRAILING; + if (!strcmp(s, "oid-malformed")) + return MODE_OID_MALFORMED; + if (!strcmp(s, "oid-huge")) + return MODE_OID_HUGE; + if (!strcmp(s, "oid-erange")) + return MODE_OID_ERANGE; + if (!strcmp(s, "oid-overlap")) + return MODE_OID_OVERLAP; + if (!strcmp(s, "oid-misaligned")) + return MODE_OID_MISALIGNED; + if (!strcmp(s, "oid-badstart")) + return MODE_OID_BADSTART; + if (!strcmp(s, "oid-unknown-status")) + return MODE_OID_UNKNOWN_STATUS; + if (!strcmp(s, "oid-abort")) + return MODE_OID_ABORT; + if (!strcmp(s, "oid-bare-status")) + return MODE_OID_BARE_STATUS; + if (!strcmp(s, "oid-empty-packet")) + return MODE_OID_EMPTY_PACKET; + if (!strcmp(s, "oid-crash")) + return MODE_OID_CRASH; + if (!strcmp(s, "oid-garbage")) + return MODE_OID_GARBAGE; + if (!strcmp(s, "cap-none")) + return MODE_CAP_NONE; + 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 void send_status(const char *status) +{ + packet_flush(1); + packet_write_fmt(1, "%s\n", status); + packet_flush(1); +} + +static void command_loop(enum mode mode) +{ + for (;;) { + char *command = NULL, *pathname = NULL; + char *old_oid = NULL, *new_oid = NULL; + + if (!read_request_header(&command, &pathname, + &old_oid, &new_oid)) + break; /* EOF: Git closed its end */ + + if (!command || strcmp(command, "hunks-by-oid")) + die("unexpected command: '%s'", + command ? command : "(none)"); + + if (logfile) { + fprintf(logfile, + "command=%s pathname=%s old-oid=%s new-oid=%s\n", + command, + pathname ? pathname : "(none)", + old_oid ? old_oid : "(none)", + new_oid ? new_oid : "(none)"); + fflush(logfile); + } + + switch (mode) { + case MODE_OID_FIXED: + packet_write_fmt(1, "hunk 5 2 5 2\n"); + send_status("status=success"); + break; + case MODE_OID_EQUAL: + send_status("status=success"); + break; + case MODE_OID_EMPTY: + /* + * An empty old side: the "git diff" convention + * addresses it with a start of 0 and a count of 0. + * Claims two lines added, fewer than the builtin + * would show, so the answer is observable. + */ + packet_write_fmt(1, "hunk 0 0 1 2\n"); + send_status("status=success"); + break; + case MODE_OID_TRAILING: + /* + * Git must ignore trailing space-separated tokens + * on a hunk line (the appendability rule), so this + * must behave exactly like oid-fixed. + */ + packet_write_fmt(1, "hunk 5 2 5 2 moved=yes\n"); + send_status("status=success"); + break; + case MODE_OID_MALFORMED: + packet_write_fmt(1, "hunk five two 5 2\n"); + send_status("status=success"); + break; + case MODE_OID_HUGE: + /* + * In-range for int32 (and for a 32-bit long), so + * only the blob-size bound can reject it. + */ + packet_write_fmt(1, "hunk 1 1000000000 1 1000000000\n"); + send_status("status=success"); + break; + case MODE_OID_ERANGE: + /* Overflows strtol() even where long is 64-bit. */ + packet_write_fmt(1, "hunk 1 99999999999999999999 1 1\n"); + send_status("status=success"); + break; + case MODE_OID_OVERLAP: + packet_write_fmt(1, "hunk 3 2 3 2\n"); + packet_write_fmt(1, "hunk 2 2 2 2\n"); + send_status("status=success"); + break; + case MODE_OID_MISALIGNED: + packet_write_fmt(1, "hunk 2 1 2 1\n"); + packet_write_fmt(1, "hunk 5 1 6 1\n"); + send_status("status=success"); + break; + case MODE_OID_BADSTART: + /* + * A start of 0 names an empty side, so a nonzero + * count beside it names no line; the coordinate is + * rejected per pair while the process stays alive. + */ + packet_write_fmt(1, "hunk 0 2 1 2\n"); + send_status("status=success"); + break; + case MODE_OID_UNKNOWN_STATUS: + send_status("status=frobnicate"); + break; + case MODE_OID_ABORT: + send_status("status=abort"); + break; + case MODE_OID_BARE_STATUS: + /* No hunk-section flush: a protocol violation. */ + packet_write_fmt(1, "status=success\n"); + packet_flush(1); + break; + case MODE_OID_EMPTY_PACKET: + /* + * An empty packet is not a flush; inside the hunk + * section it is a protocol violation. + */ + if (write(1, "0004", 4) < 0) + die_errno("write empty packet"); + send_status("status=success"); + break; + case MODE_OID_CRASH: + packet_write_fmt(1, "hunk 5 2 5 2\n"); + exit(0); + case MODE_OID_GARBAGE: + if (write(1, "@@@@ not a pkt-line @@@@", 24) < 0) + die_errno("write garbage"); + exit(0); + default: + send_status("status=need-content"); + break; + } + + free(command); + free(pathname); + free(old_oid); + free(new_oid); + } +} + +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 */ + + if (mode != MODE_CAP_NONE) + packet_write_fmt(1, "capability=hunks-by-oid\n"); + packet_flush(1); +} + +static const char *const usage_str[] = { + "test-tool diff-process-backend --mode= [--log=]", + NULL +}; + +int cmd__diff_process_backend(int argc, const char **argv) +{ + const char *mode_str = NULL, *log_path = NULL; + enum mode mode = MODE_OID_FIXED; + struct option options[] = { + OPT_STRING(0, "mode", &mode_str, "mode", + "response shape (default oid-fixed);" + " 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 fd8bee01b5..96ac3f3550 100644 --- a/t/meson.build +++ b/t/meson.build @@ -520,6 +520,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', @@ -586,6 +587,7 @@ integration_tests = [ 't4217-log-limit.sh', 't4218-log-graph-indentation.sh', 't4219-log-follow-merge.sh', + 't4220-diff-hunks.sh', 't4252-am-options.sh', 't4253-am-keep-cr-dos.sh', 't4254-am-corrupt.sh', @@ -1162,6 +1164,7 @@ benchmarks = [ 'perf/p4205-log-pretty-formats.sh', 'perf/p4209-pickaxe.sh', 'perf/p4211-line-log.sh', + 'perf/p4218-diff-hunks.sh', 'perf/p4220-log-grep-engines.sh', 'perf/p4221-log-grep-engines-fixed.sh', 'perf/p5302-pack-index.sh', diff --git a/t/perf/p4218-diff-hunks.sh b/t/perf/p4218-diff-hunks.sh new file mode 100755 index 0000000000..f849e97832 --- /dev/null +++ b/t/perf/p4218-diff-hunks.sh @@ -0,0 +1,48 @@ +#!/bin/sh + +test_description='diff-hunks store performance' +. ./perf-lib.sh + +test_perf_default_repo + +# Pick a file to blame pseudo-randomly. The sort key is the blob +# hash, so it is stable. +test_expect_success 'select a file' ' + git ls-tree -r HEAD | grep ^100644 | + sort -k 3 | head -n 1 | cut -f 2 >filelist +' + +file=$(cat filelist) +export file + +# Warm the store the way an owner would: a stat walk with writing on. +test_perf 'warm the store' ' + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git log --all --stat >/dev/null +' + +test_expect_success 'ensure the store is warm for the timed reads' ' + GIT_DIFF_HUNKS_WRITE=1 git log --all --stat >/dev/null +' + +test_perf 'log --stat -1000 (store)' ' + git log --stat -1000 >/dev/null +' + +test_perf 'log --stat -1000 (no store)' ' + git -c core.diffhunks=false log --stat -1000 >/dev/null +' + +test_perf 'blame $file (store)' ' + git blame "$file" >/dev/null +' + +test_perf 'blame $file (no store)' ' + git -c core.diffhunks=false blame "$file" >/dev/null +' + +test_expect_success 'clean up store' ' + git diff-hunks clear +' + +test_done diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh new file mode 100755 index 0000000000..a8efa19416 --- /dev/null +++ b/t/t4080-diff-process.sh @@ -0,0 +1,593 @@ +#!/bin/sh + +test_description='diff..process: oid-only hunk requests' + +TEST_PASSES_SANITIZE_LEAK=true +. ./test-lib.sh + +# See t/helper/test-diff-process-backend.c for the process implementation +# and available --mode= options. + +BACKEND="test-tool diff-process-backend" + +test_expect_success 'setup' ' + echo "*.c diff=cdiff" >.gitattributes && + git add .gitattributes && + + # 10 lines, changes at 5-6 and 9-10 between the two commits. + cat >pair.c <<-\EOF && + line1 + line2 + line3 + line4 + original5 + original6 + line7 + line8 + line9 + line10 + EOF + git add pair.c && + git commit -m "add pair.c" && + + cat >pair.c <<-\EOF && + line1 + line2 + line3 + line4 + changed5 + changed6 + line7 + line8 + changed9 + changed10 + EOF + git add pair.c && + git commit -m "change pair.c" +' + +test_expect_success 'an oid-capable process answers blame by object names alone' ' + test_when_finished "rm -f backend.log" && + ORIG=$(git rev-parse --short HEAD~1) && + CHANGE=$(git rev-parse --short HEAD) && + # The process reports only lines 5-6 as changed, so blame attributes + # lines 9-10 to the original commit even though the builtin diff + # would show them as changed. + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + blame pair.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 && + test_grep "$CHANGE" line5 && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'an oid-capable process answers --numstat by object names alone' ' + test_when_finished "rm -f backend.log" && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + log -1 --format= --numstat -- pair.c >actual && + printf "2\t2\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'need-content falls through to the builtin diff' ' + test_when_finished "rm -f backend.log" && + git -c diff.cdiff.process="$BACKEND --mode=oid-need-content --log=backend.log" \ + log -1 --format= --numstat -- pair.c >actual && + printf "4\t4\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'a warmed hunk store does not override process hunks' ' + test_when_finished "git diff-hunks clear" && + ORIG=$(git rev-parse --short HEAD~1) && + GIT_DIFF_HUNKS_WRITE=1 git log -2 --stat -- pair.c >/dev/null && + + # Control: without a process, blame is served from the store. + git blame --show-stats pair.c >stats && + test_grep "num precomputed hits: 1" stats && + + # The store holds the builtin hunks, but a process-capable driver + # makes the process authoritative, so blame must reflect the + # process hunks (only lines 5-6), not a store hit. + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed" \ + blame pair.c >actual && + sed -n "9p" actual >line9 && + test_grep "$ORIG" line9 +' + +test_expect_success 'a worktree side is not asked by object names' ' + test_when_finished "rm -f backend.log && git checkout -- pair.c" && + echo "worktree edit" >>pair.c && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + diff --numstat -- pair.c >actual && + printf "1\t0\tpair.c\n" >expect && + test_cmp expect actual && + 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 --mode=oid-fixed --log=backend.log" \ + log -1 --format= --numstat --no-ext-diff -- pair.c >actual && + printf "4\t4\tpair.c\n" >expect && + test_cmp expect actual && + test_path_is_missing backend.log +' + +test_expect_success 'format-patch keeps its diffstat off the process' ' + test_when_finished "rm -f backend.log" && + # format-patch emits a diffstat, and a diffstat consults the + # process, but the gate keeps it builtin so a generated patch + # applies for recipients without the process. + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + format-patch -1 --stdout --stat -- pair.c >actual && + test_grep "^+changed9" actual && + test_path_is_missing backend.log +' + +test_expect_success 'format-patch --ext-diff keeps its diffstat off the process' ' + test_when_finished "rm -f backend.log" && + # The gate holds even when --ext-diff enables the external command. + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + format-patch -1 --stdout --ext-diff --stat -- pair.c >actual && + test_grep "^+changed9" actual && + test_path_is_missing backend.log +' + +test_expect_success 'diff process not consulted by plumbing diff commands' ' + test_when_finished "rm -f backend.log && git checkout -f HEAD -- pair.c" && + # diff-tree diffs the two commits, a real pair a defeated gate would + # consult the process for. + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + diff-tree --numstat HEAD >actual && + test_grep "pair.c" actual && + # diff-index needs a change to diff, or there is no pair and a + # missing log proves nothing; stage one and diff it against HEAD. + echo "staged change" >>pair.c && + git add pair.c && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + diff-index --cached --numstat HEAD -- pair.c >actual && + test_grep "pair.c" actual && + test_path_is_missing backend.log +' + +test_expect_success 'add -p stages from the builtin diff with a process configured' ' + test_when_finished "rm -f backend.log" && + cat >gate.c <<-\EOF && + int gate(void) { return 1; } + EOF + git add gate.c && + git commit -m "add gate.c" && + cat >gate.c <<-\EOF && + int gate(void) { return 2; } + EOF + # add -p builds its hunks from patch text, which is not a provider + # consumer today, so a configured process cannot shape what it + # offers. This pins that interactive patch stays builtin for the + # current consumers, rather than exercising the plumbing gate. + test_write_lines y | + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + add -p gate.c && + git diff --cached -- gate.c >staged && + test_grep "return 2" staged && + test_path_is_missing backend.log && + git commit -m "gate.c v2" +' + +test_expect_success 'blame withholds identity for the working-tree pair' ' + test_when_finished "rm -f backend.log && git checkout -- pair.c" && + echo "uncommitted" >>pair.c && + wt=$(git hash-object pair.c) && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + blame pair.c >actual && + # The dirty working-tree side is not a stored blob: no request + # may name its bytes by object id. + test_grep ! "new-oid=$wt" backend.log +' + +test_expect_success 'a replaced blob makes the process step aside' ' + new_blob=$(git rev-parse HEAD:pair.c) && + test_when_finished "rm -f backend.log && git replace -d $new_blob" && + # Replacing the new-side blob redirects the content the diff reads + # under the id the process would be sent, so a raw-id request would + # name bytes other than the ones diffed. Identity is withheld: the + # process is not consulted and the builtin computes the pair from + # the replaced content. + repl=$(printf "just one line\n" | git hash-object -w --stdin) && + git replace "$new_blob" "$repl" && + git log -1 --format= --numstat -- pair.c >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + log -1 --format= --numstat -- pair.c >actual && + test_cmp expect actual && + test_path_is_missing backend.log +' + +test_expect_success 'an equivalence answer omits the pair from the stat' ' + test_when_finished "rm -f backend.log" && + git -c diff.cdiff.process="$BACKEND --mode=oid-equal --log=backend.log" \ + log -1 --format= --numstat -- pair.c >actual && + # An equivalent pair sums to a zero-count entry, and the stat + # code omits zero-count modified entries, the same way a + # whitespace-only pair prints nothing under -w. The builtin + # diff would print nonzero counts here, and the log proves the + # process was consulted. + test_must_be_empty actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'blame passes equivalent pairs through to the boundary' ' + ORIG=$(git rev-parse --short ":/add pair.c") && + # The process asserts every consulted pair equal, so no line is + # ever treated as changed: every line passes through to the + # commit that added the file, marked as the blame boundary. + git -c diff.cdiff.process="$BACKEND --mode=oid-equal" \ + blame pair.c >actual && + sed -n "5p" actual >line5 && + test_grep "^\^$ORIG" line5 && + sed -n "10p" actual >line10 && + test_grep "^\^$ORIG" line10 +' + +test_expect_success 'a warming run records a pair the process defers' ' + test_when_finished "git diff-hunks clear" && + git diff-hunks clear && + # The process owns the path but defers this pair with + # need-content, so the pair gets the builtin diff; that is the + # result the store holds, so the warming run records it and a + # later read is served from the store. + GIT_DIFF_HUNKS_WRITE=1 git -c core.diffHunks=true \ + -c diff.cdiff.process="$BACKEND --mode=oid-need-content" \ + log -1 --format= --stat -- pair.c >/dev/null && + git -c core.diffHunks=true blame --show-stats pair.c >stats 2>&1 && + test_grep "num precomputed hits: 1" stats +' + +# The protocol error paths: each adversarial response shape must warn, +# fall back to the builtin output, and either keep the process alive +# (a per-pair rejection) or disable it for the rest of the command (a +# protocol error). The request log tells the two apart: the log walk +# below consults two pairs (gate.c first, then pair.c), so a disabled +# process shows one logged request and a live one shows two. + +test_expect_success 'a malformed hunk line disables the process for the command' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-malformed --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "disabling it for the remainder" err && + test_line_count = 1 backend.log +' + +test_expect_success 'coordinates past the blob size skip the pair, process stays alive' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-huge --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "past the end" err && + test_line_count = 2 backend.log +' + +test_expect_success 'a count that overflows long skips the pair, process stays alive' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-erange --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "out-of-range coordinates" err && + test_line_count = 2 backend.log +' + +test_expect_success 'overlapping hunks are rejected per pair' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-overlap --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "overlapping hunks" err && + test_line_count = 2 backend.log +' + +test_expect_success 'misaligned hunks are rejected per pair' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-misaligned --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "misaligned" err && + test_line_count = 2 backend.log +' + +test_expect_success 'a start of zero with a nonzero count is rejected per pair' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + # A start of 0 names an empty side, so a nonzero count beside it + # names no line; the coordinate is rejected and the pair falls back + # to the builtin diff while the process stays alive. + git -c diff.cdiff.process="$BACKEND --mode=oid-badstart --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "out-of-range coordinates" err && + test_line_count = 2 backend.log +' + +test_expect_success 'an unrecognized status disables the process for the command' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-unknown-status --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "unrecognized status .frobnicate." err && + test_line_count = 1 backend.log +' + +test_expect_success 'status=abort withdraws the capability without a warning' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-abort --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep ! "disabling" err && + test_line_count = 1 backend.log +' + +test_expect_success 'a bare status without the hunk-section flush is a protocol error' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-bare-status --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "disabling it for the remainder" err && + test_line_count = 1 backend.log +' + +test_expect_success 'an empty packet in the hunk section is a protocol error' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-empty-packet --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "disabling it for the remainder" err && + test_line_count = 1 backend.log +' + +test_expect_success 'a process that dies mid-response fails the command over to builtin' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-crash --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "disabling it for the remainder" err && + test_line_count = 1 backend.log +' + +test_expect_success 'garbage bytes on stdout fail the command over to builtin' ' + test_when_finished "rm -f backend.log err" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=oid-garbage --log=backend.log" \ + log --format= --numstat -- "*.c" >actual 2>err && + test_cmp expect actual && + test_grep "disabling it for the remainder" err && + test_line_count = 1 backend.log +' + +test_expect_success 'a process announcing no capability is never asked' ' + test_when_finished "rm -f backend.log" && + git log --format= --numstat -- "*.c" >expect && + git -c diff.cdiff.process="$BACKEND --mode=cap-none --log=backend.log" \ + log --format= --numstat -- "*.c" >actual && + test_cmp expect actual && + test_must_be_empty backend.log +' + +test_expect_success 'a trailing token on a hunk line is ignored' ' + test_when_finished "rm -f backend.log" && + git -c diff.cdiff.process="$BACKEND --mode=oid-trailing --log=backend.log" \ + log -1 --format= --numstat -- pair.c >actual && + printf "2\t2\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'a failed start warns once and the store may serve the path' ' + git init failrepo && + ( + cd failrepo && + echo "*.c diff=cdiff" >.gitattributes && + git add .gitattributes && + test_commit f1 f.c "one" && + test_commit f2 f.c "one +two" && + test_commit f3 f.c "one +two +three" && + GIT_DIFF_HUNKS_WRITE=1 git log --format= --stat -- f.c >/dev/null && + # The command has no shell metacharacters, so it fails at + # exec time; a shell-wrapped command would fail at the + # handshake, which the gentle handshake in the base + # (061a68e443) likewise degrades to the builtin diff. + git blame f.c >expect && + git -c diff.cdiff.process=/does-not-exist-diff-backend \ + blame f.c >actual 2>err && + test_cmp expect actual && + # One warning even though the blame consults two pairs. + test $(grep -c "failed to start" err) = 1 && + # A failure is a non-answer like any other: the request + # that observes it and every later request pass to the + # store, so the warmed store serves both pairs. + git -c diff.cdiff.process=/does-not-exist-diff-backend \ + blame --show-stats f.c >stats 2>&1 && + test_grep "num precomputed hits: 2" stats + ) +' + +test_expect_success 'the store serves a pair the process defers' ' + test_when_finished "git diff-hunks clear" && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git log --format= --stat -- pair.c >/dev/null && + git blame pair.c >expect && + # need-content defers the pair to the builtin diff, which is + # what the store holds, so the walk continues past the process + # and the store serves the pair. + git -c diff.cdiff.process="$BACKEND --mode=oid-need-content" \ + blame pair.c >actual && + test_cmp expect actual && + git -c diff.cdiff.process="$BACKEND --mode=oid-need-content" \ + blame --show-stats pair.c >stats 2>&1 && + test_grep "num precomputed hits: 1" stats +' + +test_expect_success 'git diff between commits consults the process' ' + test_when_finished "rm -f backend.log" && + ORIG=$(git rev-parse ":/add pair.c") && + CHANGE=$(git rev-parse ":/change pair.c") && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + diff --numstat $ORIG $CHANGE -- pair.c >actual && + printf "2\t2\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'git show consults the process' ' + test_when_finished "rm -f backend.log" && + CHANGE=$(git rev-parse ":/change pair.c") && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + show --format= --numstat $CHANGE -- pair.c >actual && + printf "2\t2\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'diff-tree --ext-diff consults the process' ' + test_when_finished "rm -f backend.log" && + CHANGE=$(git rev-parse ":/change pair.c") && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + diff-tree --ext-diff --no-commit-id --numstat $CHANGE >actual && + printf "2\t2\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success '--no-diff-process forbids consulting alone' ' + test_when_finished "rm -f backend.log" && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + log -1 --no-diff-process --format= --numstat -- pair.c >actual && + printf "4\t4\tpair.c\n" >expect && + test_cmp expect actual && + test_path_is_missing backend.log +' + +test_expect_success '--diff-process allows plumbing to consult' ' + test_when_finished "rm -f backend.log" && + CHANGE=$(git rev-parse ":/change pair.c") && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + diff-tree --diff-process --no-commit-id --numstat $CHANGE >actual && + printf "2\t2\tpair.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=pair.c" backend.log +' + +test_expect_success 'a forced blame diff algorithm bypasses the process' ' + test_when_finished "rm -f backend.log" && + CHANGE=$(git rev-parse --short ":/change pair.c") && + # The process would attribute lines 9-10 to the original commit + # (see the oid-fixed blame test above); a forced builtin + # algorithm must produce the builtin attribution and never start + # the process. + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + blame --histogram pair.c >actual && + sed -n "9p" actual >line9 && + test_grep "$CHANGE" line9 && + test_path_is_missing backend.log +' + +test_expect_success 'textconv output is never identified to the process' ' + test_when_finished "rm -f backend.log" && + echo "*.tcv diff=tcv" >>.gitattributes && + git add .gitattributes && + git commit -m tcv-attr && + test_config diff.tcv.textconv cat && + test_commit tcv1 file.tcv "alpha" && + test_commit tcv2 file.tcv "alpha +beta" && + git blame file.tcv >expect && + git -c diff.tcv.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + blame file.tcv >actual && + test_cmp expect actual && + test_path_is_missing backend.log +' + +test_expect_success 'a gitlink side is never identified to the process' ' + test_when_finished "rm -f backend.log" && + echo "sub diff=cdiff" >>.gitattributes && + git add .gitattributes && + git commit -m sub-attr && + C1=$(git rev-parse HEAD) && + C2=$(git rev-parse HEAD~1) && + git update-index --add --cacheinfo 160000,$C1,sub && + git commit -m sub-1 && + git update-index --add --cacheinfo 160000,$C2,sub && + git commit -m sub-2 && + git -c diff.cdiff.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + log -1 --format= --numstat -- sub >actual && + printf "1\t1\tsub\n" >expect && + test_cmp expect actual && + test_path_is_missing backend.log +' + +test_expect_success 'a relative diff consults by the repo-relative path' ' + test_when_finished "rm -f backend.log" && + echo "reldir/*.rel diff=rdrv" >>.gitattributes && + git add .gitattributes && + git commit -m rel-attr && + mkdir reldir && + test_write_lines line1 line2 line3 line4 original5 original6 \ + line7 line8 line9 line10 >reldir/x.rel && + git add reldir/x.rel && + git commit -m "add x.rel" && + test_write_lines line1 line2 line3 line4 changed5 changed6 \ + line7 line8 changed9 changed10 >reldir/x.rel && + git add reldir/x.rel && + git commit -m "change x.rel" && + # diff.relative strips the prefix from the displayed name; the + # driver lookup and the request pathname must still use the + # repo-relative path, or the directory-scoped attribute above + # would not match and the process would never be consulted. + # The process runs at the repository root, so its log lands there. + ( + cd reldir && + git -c diff.relative=true \ + -c diff.rdrv.process="$BACKEND --mode=oid-fixed --log=backend.log" \ + log -1 --format= --numstat -- x.rel + ) >actual && + printf "2\t2\tx.rel\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=reldir/x.rel" backend.log +' + +test_expect_success 'an empty file side is answered with a start of zero' ' + test_when_finished "rm -f backend.log" && + >empty.c && + git add empty.c && + git commit -m "add empty.c" && + printf "x\ny\nz\n" >empty.c && + git add empty.c && + git commit -m "fill empty.c" && + # The process addresses the empty old side with a start of 0 and a + # count of 0, and claims two of the three added lines. The answer is + # used as sent, so the stat shows the two lines it named, not the + # three the builtin would. + git -c diff.cdiff.process="$BACKEND --mode=oid-empty --log=backend.log" \ + log -1 --format= --numstat -- empty.c >actual && + printf "2\t0\tempty.c\n" >expect && + test_cmp expect actual && + test_grep "command=hunks-by-oid pathname=empty.c" backend.log +' + +test_done diff --git a/t/t4220-diff-hunks.sh b/t/t4220-diff-hunks.sh new file mode 100755 index 0000000000..086c53f651 --- /dev/null +++ b/t/t4220-diff-hunks.sh @@ -0,0 +1,819 @@ +#!/bin/sh + +test_description='precomputed diff hunks store (git diff-hunks) + +The store maps an (old blob, new blob, diff settings) key to the hunks of +diffing the pair. It is a cache: reading is on by default +(core.diffHunks), while writing is +off by default and enabled per run by GIT_DIFF_HUNKS_WRITE (or the +diffHunks.write config), so a diff or log warms the store only when the +owner opts in. These tests check that a warmed store never changes +output, that lookups honor the diff settings, and that a corrupt store is +read as absent while verify reports the corruption.' + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh + +STORE=.git/objects/info/diff-hunks + +# Warm the store the way a repository owner would: a stat walk with +# writing enabled. A --stat walk records one entry per trim-stable blob +# pair, serving blame and the summary formats alike. Extra arguments +# (e.g. -c options) are passed to git before "log". +warm () { + GIT_DIFF_HUNKS_WRITE=1 git "$@" log --all --stat >/dev/null +} + +# Run a command with the store disabled, for ground truth. +no_store () { + git -c core.diffhunks=false "$@" +} + +test_expect_success 'setup' ' + test_commit initial file.txt "line 1" && + test_commit second file.txt "line 1 +line 2" && + test_commit third file.txt "line 1 +line 2 +line 3" && + test_commit fourth file.txt "changed line 1 +line 2 +line 3 +line 4" +' + +test_expect_success 'ordinary commands do not create the store' ' + git log --stat >/dev/null && + git blame file.txt >/dev/null && + git diff --stat second third >/dev/null && + test_path_is_missing $STORE +' + +test_expect_success 'writing is gated by env and config, env wins' ' + test_when_finished "git diff-hunks clear" && + # The diffHunks.write config enables writing. + git -c diffHunks.write=true log --all --stat >/dev/null && + test_path_is_file $STORE && + git diff-hunks clear && + # GIT_DIFF_HUNKS_WRITE overrides the config: 0 disables it. + GIT_DIFF_HUNKS_WRITE=0 git -c diffHunks.write=true log --all --stat >/dev/null && + test_path_is_missing $STORE && + # and enables it without any config. + GIT_DIFF_HUNKS_WRITE=1 git log --all --stat >/dev/null && + test_path_is_file $STORE +' + +test_expect_success 'a warm builds a store that verifies' ' + warm && + test_path_is_file $STORE && + git diff-hunks verify +' + +test_expect_success 'a second warming run refreshes the store in place' ' + warm && + test_commit fifth file.txt "brand new line" && + warm && + git diff-hunks verify && + no_store log --stat >expect && + git log --stat >actual && + test_cmp expect actual +' + +test_expect_success 'core.diffhunks=false disables lookups' ' + warm && + git -c core.diffhunks=false blame --show-stats file.txt >out 2>&1 && + test_grep "num precomputed hits: 0" out +' + +# Writing seeds from the current store and merges into it, so a later +# warming run keeps the entries an earlier one recorded rather than +# rebuilding. Warm one pair, then a different pair, and confirm the first +# is still served. +test_expect_success 'a later warming run preserves earlier entries' ' + git init incr && + ( + cd incr && + test_commit a1 f.txt "1" && + test_commit a2 f.txt "1 +2" && + test_commit a3 f.txt "1 +2 +3" && + GIT_DIFF_HUNKS_WRITE=1 git diff --stat a1 a2 >/dev/null && + git diff-hunks verify && + GIT_DIFF_HUNKS_WRITE=1 git diff --stat a2 a3 >/dev/null && + git diff-hunks verify && + + # Blaming as of a2 diffs the a1..a2 pair. If seeding had + # dropped it when the a2..a3 pair was warmed, this would + # report zero precomputed hits. + git blame --show-stats a2 -- f.txt >out 2>&1 && + test_grep "num precomputed hits: [1-9]" out && + + # The second warm ADDED the a2..a3 pair; blaming a3 diffs + # both a2..a3 and a1..a2, so a hit on each shows the store + # gained the new pair while keeping the earlier one. + git blame --show-stats a3 -- f.txt >out3 2>&1 && + test_grep "num precomputed hits: 2" out3 && + + no_store log --stat >expect && + git log --stat >actual && + test_cmp expect actual + ) +' + +test_expect_success 'log --stat matches with and without the store' ' + no_store log --stat >expect && + warm && + git log --stat >actual && + test_cmp expect actual +' + +test_expect_success 'log --numstat and --shortstat match' ' + no_store log --numstat >expect_num && + no_store log --shortstat >expect_short && + warm && + git log --numstat >actual_num && + git log --shortstat >actual_short && + test_cmp expect_num actual_num && + test_cmp expect_short actual_short +' + +# A built store must reproduce diffstat output at every context +# length. Only trim-stable pairs are recorded, so one entry serves +# every context; a trim-divergent pair is never recorded and always +# computed. Zero context is where trim_common_tail runs, which is +# what makes the two diffs differ. +test_expect_success 'diffstat matches at several context lengths' ' + no_store log --stat >expect_def && + no_store log -U0 --stat >expect_u0 && + no_store log -U7 --stat >expect_u7 && + warm && + git log --stat >got_def && + git log -U0 --stat >got_u0 && + git log -U7 --stat >got_u7 && + test_cmp expect_def got_def && + test_cmp expect_u0 got_u0 && + test_cmp expect_u7 got_u7 +' + +test_expect_success 'store built at a nonzero context stays correct at that context' ' + no_store -c diff.context=5 log --stat >expect && + warm -c diff.context=5 && + git -c diff.context=5 log --stat >actual && + test_cmp expect actual +' + +# This blob pair (a real git test file being modernized) has different +# valid diffs at different contexts: at zero context, where +# trim_common_tail runs, "diff -U0" reports 9/6, while "diff -U3" +# reports 10/7. Such a trim-divergent pair is exactly what the writer +# must never record, since no single entry could serve both readers. +# A compact synthetic pair cannot show this count split: on small +# inputs xdiff produces minimal diffs, minimal diffs of one pair all +# add and delete the same number of lines, and trimming the common +# tail preserves minimality, so the counts agree by construction (a +# search over thousands of synthetic pairs up to 8 lines found no +# split). The split needs the cost-capping heuristics that only larger +# inputs trigger, so the pair is shipped as a fixture under t4220/. +test_expect_success 'a trim-divergent file is correct at each context' ' + cp "$TEST_DIRECTORY/t4220/trim-divergent-old" div.sh && + git add div.sh && + git commit -m divergent-old && + cp "$TEST_DIRECTORY/t4220/trim-divergent-new" div.sh && + git add div.sh && + git commit -m divergent-new && + no_store log -1 --format= --stat -- div.sh >expect_def && + no_store log -1 --format= -U0 --stat -- div.sh >expect_u0 && + warm && + git log -1 --format= --stat -- div.sh >got_def && + git log -1 --format= -U0 --stat -- div.sh >got_u0 && + test_cmp expect_def got_def && + test_cmp expect_u0 got_u0 && + # The fixture must actually diverge, or the test would pass without + # exercising the split; fail loudly if a diff change ever levels it. + ! test_cmp expect_def expect_u0 +' + +# A warming run displays the diffstat it computes. At zero context xdi_diff +# trims, so the displayed counts must be the trimmed ones (what a store-less +# run shows), not the untrimmed ones the writer compares against when it +# decides whether the pair is stable enough to record. +test_expect_success 'warming --stat at zero context matches a store-less run' ' + git init -q warm-u0 && + ( + cd warm-u0 && + cp "$TEST_DIRECTORY/t4220/trim-divergent-old" div.sh && + git add div.sh && git commit -q -m old && + cp "$TEST_DIRECTORY/t4220/trim-divergent-new" div.sh && + git add div.sh && git commit -q -m new && + git -c core.diffhunks=false log -1 --format= -U0 --stat -- div.sh >expect && + GIT_DIFF_HUNKS_WRITE=1 git log -1 --format= -U0 --stat -- div.sh >got && + test_cmp expect got + ) +' + +test_expect_success 'diff --stat matches with and without the store, both directions' ' + no_store diff --stat second fourth >expect_fwd && + no_store diff --stat fourth second >expect_rev && + warm && + git diff --stat second fourth >got_fwd && + git diff --stat fourth second >got_rev && + test_cmp expect_fwd got_fwd && + test_cmp expect_rev got_rev +' + +test_expect_success 'show and diff-tree --stat use the store' ' + test_when_finished "git diff-hunks clear" && + # diff_hunks_attach() runs for show and diff-tree: a write-enabled + # --stat records into the store (without the attach there is no + # writer, so nothing is written). + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git show --stat fourth >/dev/null && + test_path_is_file "$STORE" && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git diff-tree --stat fourth >/dev/null && + test_path_is_file "$STORE" && + # Reading never changes their output. + git diff-hunks clear && + no_store show --stat fourth >expect_show && + no_store diff-tree --stat fourth >expect_dt && + warm && + git show --stat fourth >got_show && + git diff-tree --stat fourth >got_dt && + test_cmp expect_show got_show && + test_cmp expect_dt got_dt +' + +test_expect_success 'log -R --stat matches (reversed pairs keyed apart)' ' + no_store log -R --stat >expect && + warm && + git log -R --stat >actual && + test_cmp expect actual +' + +# One warm serves both diffstat and blame: the blob pairs a blame +# walks are the same parent-child pairs the diffstat warm recorded. +test_expect_success 'a single warming run serves both blame and diffstat' ' + warm && + git blame --show-stats file.txt >out 2>&1 && + test_grep "num precomputed hits: [1-9][0-9]*" out +' + +# The diffstat read path produces identical output on a hit or a miss, so +# it emits a trace2 "read-hits" count to prove it consulted the store. +test_expect_success 'diffstat consults the store (trace shows read hits)' ' + warm && + GIT_TRACE2_EVENT="$PWD/trace_on.json" git log --stat >/dev/null && + test_grep read-hits trace_on.json && + test_env GIT_TRACE2_EVENT="$PWD/trace_off.json" no_store log --stat >/dev/null && + test_grep ! read-hits trace_off.json +' + +test_expect_success 'blame matches with and without the store' ' + no_store blame file.txt >expect && + warm && + git blame file.txt >actual && + test_cmp expect actual +' + +test_expect_success 'blame --porcelain and --incremental match' ' + no_store blame --porcelain file.txt >expect_p && + no_store blame --incremental file.txt >expect_i && + warm && + git blame --porcelain file.txt >got_p && + git blame --incremental file.txt >got_i && + test_cmp expect_p got_p && + test_cmp expect_i got_i +' + +# Diff settings that change hunks but are not part of the store key must +# bypass it in both directions, so output stays byte-identical to a +# store-less run. +test_expect_success 'setup ignore fixture' ' + git init ignore-repo && + ( + cd ignore-repo && + test_write_lines code keep "# c" >f && + git add f && + git commit -m c1 && + test_write_lines codeCH keep "# cX" >f && + git add f && + git commit -m c2 && + warm + ) +' + +# Output parity alone cannot prove the guard: served counts can +# coincide with computed ones, so each bypass below also asserts the +# consultation itself (no read hit with the option, a hit without it) +# and that a warming run under the option records nothing. +test_expect_success '-I bypasses the store in both directions' ' + ( + cd ignore-repo && + no_store diff -I"^#" --numstat HEAD~ HEAD >expect && + git diff -I"^#" --numstat HEAD~ HEAD >actual && + test_cmp expect actual && + # -I does not change the key, so only the ignore_regex + # guard keeps the warmed entry from serving here. + GIT_TRACE2_EVENT="$PWD/trace_i.json" \ + git diff -I"^#" --numstat HEAD~ HEAD >/dev/null && + test_grep ! read-hits trace_i.json && + GIT_TRACE2_EVENT="$PWD/trace_i_ctl.json" \ + git diff --numstat HEAD~ HEAD >/dev/null && + test_grep read-hits trace_i_ctl.json && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git diff -I"^#" --numstat HEAD~ HEAD >/dev/null && + test_path_is_missing .git/objects/info/diff-hunks && + # Restore the warmed fixture for the tests below. + warm + ) +' + +test_expect_success '-B bypasses the store in both directions' ' + git init break-repo && + ( + cd break-repo && + test_write_lines a b c d e f g h >f && + git add f && + git commit -m orig && + test_write_lines 1 2 3 4 5 6 7 8 >f && + git add f && + git commit -m rewrite && + warm && + no_store diff -B --stat HEAD~ HEAD >expect && + git diff -B --stat HEAD~ HEAD >actual && + test_cmp expect actual && + GIT_TRACE2_EVENT="$PWD/trace_b.json" \ + git diff -B --stat HEAD~ HEAD >/dev/null && + test_grep ! read-hits trace_b.json && + GIT_TRACE2_EVENT="$PWD/trace_ctl.json" \ + git diff --stat HEAD~ HEAD >/dev/null && + test_grep read-hits trace_ctl.json && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git diff -B --stat HEAD~ HEAD >/dev/null && + test_path_is_missing .git/objects/info/diff-hunks + ) +' + +test_expect_success '--anchored bypasses the store in both directions' ' + ( + cd ignore-repo && + no_store diff --stat --anchored=keep HEAD~ HEAD >expect && + git diff --stat --anchored=keep HEAD~ HEAD >actual && + test_cmp expect actual && + # Anchors do not change the key, so only the anchors guard + # keeps the warmed entry from serving here. + GIT_TRACE2_EVENT="$PWD/trace_anchor.json" \ + git diff --stat --anchored=keep HEAD~ HEAD >/dev/null && + test_grep ! read-hits trace_anchor.json && + GIT_TRACE2_EVENT="$PWD/trace_plain.json" \ + git diff --stat HEAD~ HEAD >/dev/null && + test_grep read-hits trace_plain.json && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 \ + git diff --stat --anchored=keep HEAD~ HEAD >/dev/null && + test_path_is_missing .git/objects/info/diff-hunks + ) +' + +test_expect_success '--ignore-blank-lines bypasses the store in both directions' ' + git init ibl-repo && + ( + cd ibl-repo && + printf "a\n\nx\ny\nb\n" >f && + git add f && + git commit -m v1 && + printf "a\nx\ny\nB\n" >f && + git add f && + git commit -m v2 && + warm && + no_store diff --stat --ignore-blank-lines HEAD~ HEAD >expect && + git diff --stat --ignore-blank-lines HEAD~ HEAD >actual && + test_cmp expect actual && + # The flag is an xdl_opts bit and thus part of the key; the + # stat consumer excludes it before consulting at all. + GIT_TRACE2_EVENT="$PWD/trace_ibl.json" \ + git diff --stat --ignore-blank-lines HEAD~ HEAD >/dev/null && + test_grep ! read-hits trace_ibl.json && + GIT_TRACE2_EVENT="$PWD/trace_plain.json" \ + git diff --stat HEAD~ HEAD >/dev/null && + test_grep read-hits trace_plain.json && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 \ + git diff --stat --ignore-blank-lines HEAD~ HEAD >/dev/null && + test_path_is_missing .git/objects/info/diff-hunks + ) +' + +test_expect_success 'a whitespace-ignoring diff is not served default entries' ' + git init ws-repo && + ( + cd ws-repo && + test_write_lines alpha beta gamma >f && + git add f && + git commit -m c1 && + test_write_lines " alpha" beta gamma delta >f && + git add f && + git commit -m c2 && + warm && + no_store diff -w --numstat HEAD~ HEAD >expect && + git diff -w --numstat HEAD~ HEAD >actual && + test_cmp expect actual + ) +' + +test_expect_success 'blame -w stays correct and does not hit default entries' ' + ( + cd ws-repo && + no_store blame -w f >expect && + git blame -w --show-stats f >out 2>&1 && + test_grep "num precomputed hits: 0" out && + git blame -w f >actual && + test_cmp expect actual + ) +' + +test_expect_success 'blame with indentHeuristic off stays correct and misses' ' + warm && + git -c diff.indentHeuristic=false blame --show-stats file.txt >out 2>&1 && + test_grep "num precomputed hits: 0" out && + no_store -c diff.indentHeuristic=false blame file.txt >expect && + git -c diff.indentHeuristic=false blame file.txt >actual && + test_cmp expect actual +' + +test_expect_success 'a driver algorithm override keeps output correct and keys apart' ' + git init driver-algo && + ( + cd driver-algo && + echo "file.foo diff=foo" >.gitattributes && + git add .gitattributes && + git commit -m attributes && + test_write_lines 1 2 3 4 5 >file.foo && + git add file.foo && + git commit -m one && + test_write_lines 1 2 X 4 5 6 >file.foo && + git add file.foo && + git commit -m two && + warm -c diff.foo.algorithm=histogram && + no_store -c diff.foo.algorithm=histogram log --stat >expect && + git -c diff.foo.algorithm=histogram log --stat >actual && + test_cmp expect actual && + # The driver algorithm is an xdl_opts key bit: entries + # warmed at the default settings must not serve a + # driver-forced histogram read, and output stays correct. + git diff-hunks clear && + warm && + no_store -c diff.foo.algorithm=histogram log --stat >expect2 && + git -c diff.foo.algorithm=histogram log --stat >actual2 && + test_cmp expect2 actual2 && + GIT_TRACE2_EVENT="$PWD/trace_algo.json" \ + git -c diff.foo.algorithm=histogram log --stat >/dev/null && + test_grep ! read-hits trace_algo.json && + GIT_TRACE2_EVENT="$PWD/trace_algo_ctl.json" \ + git log --stat >/dev/null && + test_grep read-hits trace_algo_ctl.json + ) +' + +test_expect_success 'blame --reverse never consults the store' ' + warm && + git blame --reverse HEAD~3..HEAD file.txt >actual 2>/dev/null && + no_store blame --reverse HEAD~3..HEAD file.txt >expect 2>/dev/null && + test_cmp expect actual && + # Reverse blame withholds the pair identity. Zero hits alone + # cannot prove that: reverse pairs are never warmed, so a + # consulted pair would miss, not hit. Zero misses is what shows + # the store was never consulted. + git blame --reverse --show-stats HEAD~3..HEAD file.txt \ + >stats 2>/dev/null && + test_grep "num precomputed hits: 0" stats && + test_grep "num precomputed misses: 0" stats +' + +test_expect_success 'blame with a textconv driver bypasses the store' ' + echo "tc.txt diff=tc" >>.gitattributes && + git add .gitattributes && + git commit -m tc-attr && + git config diff.tc.textconv "sed -e s/1/one/" && + test_commit tc1 tc.txt "line 1" && + test_commit tc2 tc.txt "line 1 +line 2" && + warm && + git blame --show-stats tc.txt >out 2>&1 && + test_grep "num precomputed hits: 0" out && + no_store blame tc.txt >expect && + git blame tc.txt >actual && + test_cmp expect actual +' + +test_expect_success 'a replaced blob makes the store step aside' ' + git init replace-repo && + ( + cd replace-repo && + test_commit r1 f.txt "a" && + test_commit r2 f.txt "a +b" && + warm && + # Control: without a replacement the pair is served. + GIT_TRACE2_EVENT="$PWD/trace_ctl.json" \ + git log -1 --format= --numstat -- f.txt >/dev/null && + test_grep read-hits trace_ctl.json && + # Replace r2 blob: the diff now reads different content + # (through OBJECT_INFO_LOOKUP_REPLACE) under the id the store + # keyed, so a served answer would be the pre-replacement diff. + # Identity is withheld, the store steps aside, and the builtin + # computes from the replaced content. + new_blob=$(git rev-parse HEAD:f.txt) && + repl=$(printf "a\nB\nC\nD\n" | git hash-object -w --stdin) && + git replace "$new_blob" "$repl" && + no_store log -1 --format= --numstat -- f.txt >expect && + git log -1 --format= --numstat -- f.txt >actual && + test_cmp expect actual && + GIT_TRACE2_EVENT="$PWD/trace_repl.json" \ + git log -1 --format= --numstat -- f.txt >/dev/null && + test_grep ! read-hits trace_repl.json + ) +' + +test_expect_success 'blame -M and -C stay correct with the store' ' + warm && + no_store blame -M file.txt >expect_m && + no_store blame -C file.txt >expect_c && + git blame -M file.txt >got_m && + git blame -C file.txt >got_c && + test_cmp expect_m got_m && + test_cmp expect_c got_c +' + +# Copy-detecting (and reverse) blame still diff blob pairs through +# pass_blame_to_parent, so they must use the real blame xdl_opts. A +# whitespace-only change is invisible under -w; if -w were dropped on +# these paths the -w and non-w results would coincide. +test_expect_success 'blame -C honors -w' ' + git init -q blame-cw && + ( + cd blame-cw && + printf "one\ntwo\nthree\n" >f && + git add f && git commit -q -m base && + printf "one\n two \nthree\n" >f && + git add f && git commit -q -m reindent && + git blame -C -w f >with_w && + git blame -C f >without_w && + ! test_cmp with_w without_w + ) +' + +# Cover the pair shapes an object walk encounters: binary and +# mode-only changes produce no text hunks to record. +test_expect_success 'binary and mode-only changes do not break the writer' ' + printf "\\000\\001\\002" >bin.dat && + git add bin.dat && + git commit -m binary-1 && + printf "\\000\\001\\003\\004" >bin.dat && + git add bin.dat && + git commit -m binary-2 && + echo "mode content" >mode.txt && + git add mode.txt && + git commit -m mode-1 && + test_chmod +x mode.txt && + git commit -m mode-2 && + no_store log --stat >expect && + warm && + git log --stat >actual && + test_cmp expect actual +' + +test_expect_success 'blame across a rename matches' ' + echo "original content" >rename-src.txt && + git add rename-src.txt && + git commit -m "add rename-src" && + echo "more" >>rename-src.txt && + git add rename-src.txt && + git commit -m "modify rename-src" && + git mv rename-src.txt rename-dst.txt && + git commit -m "rename" && + echo "post" >>rename-dst.txt && + git add rename-dst.txt && + git commit -m "modify after rename" && + no_store blame rename-dst.txt >expect && + warm && + git blame rename-dst.txt >actual && + test_cmp expect actual +' + +test_expect_success 'blame handles merge commits' ' + git checkout -b merge-side main~2 && + test_commit merge-change merge-file.txt "side content" && + git checkout main && + git merge --no-edit merge-side && + no_store blame merge-file.txt >expect && + warm && + git blame merge-file.txt >actual && + test_cmp expect actual +' + +test_expect_success 'distinct --contents against one revision do not collide' ' + warm && + test_write_lines "line 1" "appended line" >c1 && + test_write_lines "rewritten line" >c2 && + # Ground truth without the store. + no_store blame -s --contents=c2 file.txt initial >expect && + # With the store, an intervening c1 run must not poison the c2 lookup. + git blame -s --contents=c1 file.txt initial >/dev/null && + git blame -s --contents=c2 file.txt initial >actual && + test_cmp expect actual && + # The --contents side is a working-tree pseudo-commit (a null commit + # id), so its pairs withhold identity and never consult the store. + # Output parity alone cannot show that: a consulted unwarmed pair + # would miss, not hit, so zero misses is what proves the pair was + # never looked up. + git blame -s --show-stats --contents=c2 file.txt initial >stats 2>&1 && + test_grep "num precomputed hits: 0" stats && + test_grep "num precomputed misses: 0" stats +' + +test_expect_success 'blame --ignore-rev bypasses the store for ignored pairs' ' + git init ignore-rev-repo && + ( + cd ignore-rev-repo && + test_commit ir1 f.txt "base" && + test_commit ir2 f.txt "base +more" && + warm && + # Control: the ordinary pass is served, nothing is computed. + git blame --show-stats f.txt >ctl 2>&1 && + test_grep "num precomputed hits: 1" ctl && + test_grep "num get patch: 0" ctl && + no_store blame --ignore-rev ir2 f.txt >expect && + git blame --ignore-rev ir2 f.txt >actual && + test_cmp expect actual && + # The ignored revision adds a pass that withholds identity: + # it computes its diff (get patch rises) instead of being + # served or even counted as a store consultation. + git blame --ignore-rev ir2 --show-stats f.txt >stats 2>&1 && + test_grep "num precomputed hits: 1" stats && + test_grep "num precomputed misses: 0" stats && + test_grep "num get patch: 1" stats + ) +' + +test_expect_success 'blame counts misses for pairs the store does not hold' ' + ( + cd ignore-rev-repo && + test_commit ir3 f.txt "base +more +third" && + git blame --show-stats f.txt >stats 2>&1 && + test_grep "num precomputed hits: 1" stats && + test_grep "num precomputed misses: 1" stats + ) +' + +test_expect_success 'log -L --stat neither reads nor records' ' + warm && + GIT_TRACE2_EVENT="$PWD/trace_linelog.json" \ + git log -L1,1:file.txt --stat >/dev/null && + test_grep ! read-hits trace_linelog.json && + git diff-hunks clear && + GIT_DIFF_HUNKS_WRITE=1 git log -L1,1:file.txt --stat >/dev/null && + test_path_is_missing $STORE +' + +# Integrity: a structurally broken header is read as absent (the reader +# falls back to xdiff and stays correct); a checksum mismatch is caught +# by verify, which is when integrity is checked. +test_expect_success 'a truncated store is read as absent' ' + warm && + test_copy_bytes 20 <$STORE >truncated && + mv truncated $STORE && + no_store blame file.txt >expect && + git blame file.txt >actual && + test_cmp expect actual +' + +test_expect_success 'a corrupt signature is read as absent' ' + warm && + printf "XXXX" >corrupt && + tail -c +5 <$STORE >>corrupt && + mv corrupt $STORE && + no_store blame file.txt >expect && + git blame file.txt >actual && + test_cmp expect actual +' + +# Byte 6 of the header is the chunk count; a value larger than the file +# can hold must be rejected before the chunk table is walked. +test_expect_success 'an over-claimed chunk count is read as absent' ' + warm && + printf "\377" | dd of=$STORE bs=1 seek=6 count=1 conv=notrunc 2>/dev/null && + no_store blame file.txt >expect && + git blame file.txt >actual && + test_cmp expect actual +' + +# A record with no hunks would replay as an equivalence claim, which +# the writer never records; the reader must treat such a record as a +# miss and recompute, and verify must flag it. +test_expect_success 'a zero-hunk record is read as a miss and fails verify' ' + git init zero-hunk && + ( + cd zero-hunk && + test_commit z1 f.txt "base" && + test_commit z2 f.txt "base +more" && + warm && + # The store holds one entry of one hunk: a 4-byte count and + # one 16-byte hunk record, just before the trailing + # checksum. Zero the count to craft the record the writer + # refuses to produce. + rawsz=$(test_oid rawsz) && + fsize=$(test_file_size $STORE) && + printf "\\0\\0\\0\\0" | dd of=$STORE bs=1 \ + seek=$((fsize - rawsz - 20)) count=4 conv=notrunc \ + 2>/dev/null && + no_store blame f.txt >expect && + git blame --show-stats f.txt >stats 2>&1 && + test_grep "num precomputed hits: 0" stats && + git blame f.txt >actual && + test_cmp expect actual && + test_must_fail git diff-hunks verify + ) +' + +test_expect_success 'verify succeeds on a valid store and on an absent one' ' + warm && + git diff-hunks verify && + git diff-hunks clear && + test_path_is_missing $STORE && + git diff-hunks verify +' + +test_expect_success 'verify detects a checksum mismatch' ' + test_when_finished "git diff-hunks clear" && + warm && + fsize=$(test_file_size $STORE) && + mid=$((fsize / 2)) && + printf "\\377" | dd of=$STORE bs=1 seek=$mid count=1 conv=notrunc 2>/dev/null && + test_must_fail git diff-hunks verify +' + +test_expect_success 'a warm discards a corrupt store rather than seeding from it' ' + test_when_finished "git diff-hunks clear" && + warm && + # Corrupt the checksum: the next warm must not carry the corrupt + # entries forward into a fresh checksum-valid file; it discards + # them (with a warning) and rewrites a store that verifies. + fsize=$(test_file_size $STORE) && + printf "\\377" | dd of=$STORE bs=1 seek=$((fsize / 2)) count=1 conv=notrunc 2>/dev/null && + warm 2>err && + test_grep "failed its checksum" err && + git diff-hunks verify && + no_store log --stat >expect && + git log --stat >actual && + test_cmp expect actual +' + +# A generated patch must carry the builtin diffstat, not one served from +# the sender's local store, so its counts do not depend on whether the +# sender warmed the store. Poison the store so a served answer diverges +# from the builtin, then confirm format-patch shows the builtin counts. +test_expect_success 'format-patch keeps its diffstat off the store' ' + git init fp-repo && + ( + cd fp-repo && + test_commit p1 f.txt "a" && + test_commit p2 f.txt "a +b" && + warm && + # Bump the new-side count of the single recorded hunk. The + # record stays structurally valid, and a read skips the + # trailing checksum, so the store serves this poisoned count. + rawsz=$(test_oid rawsz) && + fsize=$(test_file_size .git/objects/info/diff-hunks) && + printf "\\0\\0\\0\\7" | dd of=.git/objects/info/diff-hunks bs=1 \ + seek=$((fsize - rawsz - 4)) count=4 conv=notrunc 2>/dev/null && + # The store now serves a divergent count, proving the poison + # is live and observable through a store consumer. + printf "7\t0\tf.txt\n" >poisoned && + git log -1 --format= --numstat -- f.txt >served && + test_cmp poisoned served && + # format-patch does not consult the store, so its output is + # identical with the store poisoned and with it disabled. + no_store format-patch -1 --stdout --stat -- f.txt >expect && + git format-patch -1 --stdout --stat -- f.txt >actual && + test_cmp expect actual + ) +' + +test_expect_success 'diff-hunks clear removes the store file' ' + warm && + test_path_is_file $STORE && + git diff-hunks clear && + test_path_is_missing $STORE +' + +test_done diff --git a/t/t4220/README b/t/t4220/README new file mode 100644 index 0000000000..b850fe7c61 --- /dev/null +++ b/t/t4220/README @@ -0,0 +1,55 @@ +t4220 diff-hunks test fixtures +============================== + +trim-divergent-old, trim-divergent-new +-------------------------------------- + +Two revisions of a single real file, used by t4220-diff-hunks.sh to +exercise a "trim-divergent" blob pair: one whose diff hunk counts change +with the amount of context, so the trimmed and untrimmed results +disagree. + +They are two versions of git's own t/t6002-rev-list-bisect.sh, taken from +git.git history around: + + 090af9957c ("t6002: fix use of `expr` with `set -e`", + Patrick Steinhardt, 2026-04-21) + +which rewrites `$(expr ...)` arithmetic as `$((...))` and reformats a few +test_expect_success blocks. + + trim-divergent-old = 090af9957c^:t/t6002-rev-list-bisect.sh (blob daa009c9a1) + trim-divergent-new = 090af9957c :t/t6002-rev-list-bisect.sh (blob f2de40b5ed) + +To regenerate them from any git.git checkout: + + git show 090af9957c^:t/t6002-rev-list-bisect.sh >trim-divergent-old + git show 090af9957c:t/t6002-rev-list-bisect.sh >trim-divergent-new + +Why this pair +------------- + +The diff-hunks store records only "trim-stable" pairs: those whose hunks +are identical whether or not xdiff trims the common head and tail (which +it does at zero context, in trim_common_tail). This pair is deliberately +NOT trim-stable: + + diff -U0 reports 9 added / 6 deleted + diff -U3 reports 10 added / 7 deleted + +Because the counts diverge with context, the writer must refuse to record +this pair and every command must recompute it from the blobs. t4220 uses +it to prove that the displayed counts stay correct at each context, and +that a divergent pair is never served from the store. See +t4220-diff-hunks.sh ("a trim-divergent file is correct at each context" +and the store-poison test). + +Why not a synthesized fixture +----------------------------- + +The divergence needs real content that makes xdiff's common-tail trimming +shift a hunk boundary while the added/deleted balance stays equal. A +minimal hand-written file that reliably triggers the -U0 vs -U3 count +disagreement has not been found yet; until one is, this real pair is kept +verbatim. If you synthesize a smaller equivalent, replace these two files +and delete this note. diff --git a/t/t4220/trim-divergent-new b/t/t4220/trim-divergent-new new file mode 100644 index 0000000000..f2de40b5ed --- /dev/null +++ b/t/t4220/trim-divergent-new @@ -0,0 +1,319 @@ +#!/bin/sh +# +# Copyright (c) 2005 Jon Seymour +# +test_description='Tests git rev-list --bisect functionality' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-t6000.sh # t6xxx specific functions + +# usage: test_bisection max-diff bisect-option head ^prune... +# +# e.g. test_bisection 1 --bisect l1 ^l0 +# +test_bisection_diff() +{ + _max_diff=$1 + _bisect_option=$2 + shift 2 + _bisection=$(git rev-list $_bisect_option "$@") + _list_size=$(git rev-list "$@" | wc -l) + _head=$1 + shift 1 + _bisection_size=$(git rev-list $_bisection "$@" | wc -l) + [ -n "$_list_size" -a -n "$_bisection_size" ] || + error "test_bisection_diff failed" + + # Test if bisection size is close to half of list size within + # tolerance. + # + _bisect_err=$(($_list_size - $_bisection_size * 2)) + if test "$_bisect_err" -lt 0 + then + _bisect_err=$((0 - $_bisect_err)) + fi + _bisect_err=$(($_bisect_err / 2)) ; # floor + + test_expect_success "bisection diff $_bisect_option $_head $* <= $_max_diff" ' + test $_bisect_err -le $_max_diff + ' +} + +date >path0 +git update-index --add path0 +save_tag tree git write-tree +on_committer_date "00:00" hide_error save_tag root unique_commit root tree +on_committer_date "00:01" save_tag l0 unique_commit l0 tree -p root +on_committer_date "00:02" save_tag l1 unique_commit l1 tree -p l0 +on_committer_date "00:03" save_tag l2 unique_commit l2 tree -p l1 +on_committer_date "00:04" save_tag a0 unique_commit a0 tree -p l2 +on_committer_date "00:05" save_tag a1 unique_commit a1 tree -p a0 +on_committer_date "00:06" save_tag b1 unique_commit b1 tree -p a0 +on_committer_date "00:07" save_tag c1 unique_commit c1 tree -p b1 +on_committer_date "00:08" save_tag b2 unique_commit b2 tree -p b1 +on_committer_date "00:09" save_tag b3 unique_commit b2 tree -p b2 +on_committer_date "00:10" save_tag c2 unique_commit c2 tree -p c1 -p b2 +on_committer_date "00:11" save_tag c3 unique_commit c3 tree -p c2 +on_committer_date "00:12" save_tag a2 unique_commit a2 tree -p a1 +on_committer_date "00:13" save_tag a3 unique_commit a3 tree -p a2 +on_committer_date "00:14" save_tag b4 unique_commit b4 tree -p b3 -p a3 +on_committer_date "00:15" save_tag a4 unique_commit a4 tree -p a3 -p b4 -p c3 +on_committer_date "00:16" save_tag l3 unique_commit l3 tree -p a4 +on_committer_date "00:17" save_tag l4 unique_commit l4 tree -p l3 +on_committer_date "00:18" save_tag l5 unique_commit l5 tree -p l4 +git update-ref HEAD $(tag l5) + + +# E +# / \ +# e1 | +# | | +# e2 | +# | | +# e3 | +# | | +# e4 | +# | | +# | f1 +# | | +# | f2 +# | | +# | f3 +# | | +# | f4 +# | | +# e5 | +# | | +# e6 | +# | | +# e7 | +# | | +# e8 | +# \ / +# F + + +on_committer_date "00:00" hide_error save_tag F unique_commit F tree +on_committer_date "00:01" save_tag e8 unique_commit e8 tree -p F +on_committer_date "00:02" save_tag e7 unique_commit e7 tree -p e8 +on_committer_date "00:03" save_tag e6 unique_commit e6 tree -p e7 +on_committer_date "00:04" save_tag e5 unique_commit e5 tree -p e6 +on_committer_date "00:05" save_tag f4 unique_commit f4 tree -p F +on_committer_date "00:06" save_tag f3 unique_commit f3 tree -p f4 +on_committer_date "00:07" save_tag f2 unique_commit f2 tree -p f3 +on_committer_date "00:08" save_tag f1 unique_commit f1 tree -p f2 +on_committer_date "00:09" save_tag e4 unique_commit e4 tree -p e5 +on_committer_date "00:10" save_tag e3 unique_commit e3 tree -p e4 +on_committer_date "00:11" save_tag e2 unique_commit e2 tree -p e3 +on_committer_date "00:12" save_tag e1 unique_commit e1 tree -p e2 +on_committer_date "00:13" save_tag E unique_commit E tree -p e1 -p f1 + +on_committer_date "00:00" hide_error save_tag U unique_commit U tree +on_committer_date "00:01" save_tag u0 unique_commit u0 tree -p U +on_committer_date "00:01" save_tag u1 unique_commit u1 tree -p u0 +on_committer_date "00:02" save_tag u2 unique_commit u2 tree -p u0 +on_committer_date "00:03" save_tag u3 unique_commit u3 tree -p u0 +on_committer_date "00:04" save_tag u4 unique_commit u4 tree -p u0 +on_committer_date "00:05" save_tag u5 unique_commit u5 tree -p u0 +on_committer_date "00:06" save_tag V unique_commit V tree -p u1 -p u2 -p u3 -p u4 -p u5 + +test_sequence() +{ + _bisect_option=$1 + + test_bisection_diff 0 $_bisect_option l0 ^root + test_bisection_diff 0 $_bisect_option l1 ^root + test_bisection_diff 0 $_bisect_option l2 ^root + test_bisection_diff 0 $_bisect_option a0 ^root + test_bisection_diff 0 $_bisect_option a1 ^root + test_bisection_diff 0 $_bisect_option a2 ^root + test_bisection_diff 0 $_bisect_option a3 ^root + test_bisection_diff 0 $_bisect_option b1 ^root + test_bisection_diff 0 $_bisect_option b2 ^root + test_bisection_diff 0 $_bisect_option b3 ^root + test_bisection_diff 0 $_bisect_option c1 ^root + test_bisection_diff 0 $_bisect_option c2 ^root + test_bisection_diff 0 $_bisect_option c3 ^root + test_bisection_diff 0 $_bisect_option E ^F + test_bisection_diff 0 $_bisect_option e1 ^F + test_bisection_diff 0 $_bisect_option e2 ^F + test_bisection_diff 0 $_bisect_option e3 ^F + test_bisection_diff 0 $_bisect_option e4 ^F + test_bisection_diff 0 $_bisect_option e5 ^F + test_bisection_diff 0 $_bisect_option e6 ^F + test_bisection_diff 0 $_bisect_option e7 ^F + test_bisection_diff 0 $_bisect_option f1 ^F + test_bisection_diff 0 $_bisect_option f2 ^F + test_bisection_diff 0 $_bisect_option f3 ^F + test_bisection_diff 0 $_bisect_option f4 ^F + test_bisection_diff 0 $_bisect_option E ^F + + test_bisection_diff 1 $_bisect_option V ^U + test_bisection_diff 0 $_bisect_option V ^U ^u1 ^u2 ^u3 + test_bisection_diff 0 $_bisect_option u1 ^U + test_bisection_diff 0 $_bisect_option u2 ^U + test_bisection_diff 0 $_bisect_option u3 ^U + test_bisection_diff 0 $_bisect_option u4 ^U + test_bisection_diff 0 $_bisect_option u5 ^U + +# +# the following illustrates Linus' binary bug blatt idea. +# +# assume the bug is actually at l3, but you don't know that - all you know is that l3 is broken +# and it wasn't broken before +# +# keep bisecting the list, advancing the "bad" head and accumulating "good" heads until +# the bisection point is the head - this is the bad point. +# + +test_output_expect_success "$_bisect_option l5 ^root" 'git rev-list $_bisect_option l5 ^root' <expect && + git rev-list --bisect >actual && + test_cmp expect actual +' + +test_expect_success 'rev-parse --bisect can default to good/bad refs' ' + git rev-parse c3 ^b1 ^c1 >expect && + git rev-parse --bisect >actual && + + # output order depends on the refnames, which in turn depends on + # the exact sha1s. We just want to make sure we have the same set + # of lines in any order. + sort expect.sorted && + sort actual.sorted && + test_cmp expect.sorted actual.sorted +' + +test_output_expect_success '--bisect --first-parent' 'git rev-list --bisect --first-parent E ^F' <expect.unsorted <<-EOF && + $(git rev-parse E) (tag: E, dist=0) + $(git rev-parse e1) (tag: e1, dist=1) + $(git rev-parse e2) (tag: e2, dist=2) + $(git rev-parse e3) (tag: e3, dist=3) + $(git rev-parse e4) (tag: e4, dist=4) + $(git rev-parse e5) (tag: e5, dist=4) + $(git rev-parse e6) (tag: e6, dist=3) + $(git rev-parse e7) (tag: e7, dist=2) + $(git rev-parse e8) (tag: e8, dist=1) + EOF + + # expect results to be ordered by distance (descending), + # commit hash (ascending) + sort -k4,4r -k1,1 expect.unsorted >expect && + git rev-list --bisect-all --first-parent E ^F >actual && + test_cmp expect actual +' + +test_expect_success '--bisect without any revisions' ' + git rev-list --bisect HEAD..HEAD >out && + test_must_be_empty out +' + +test_done diff --git a/t/t4220/trim-divergent-old b/t/t4220/trim-divergent-old new file mode 100644 index 0000000000..daa009c9a1 --- /dev/null +++ b/t/t4220/trim-divergent-old @@ -0,0 +1,316 @@ +#!/bin/sh +# +# Copyright (c) 2005 Jon Seymour +# +test_description='Tests git rev-list --bisect functionality' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-t6000.sh # t6xxx specific functions + +# usage: test_bisection max-diff bisect-option head ^prune... +# +# e.g. test_bisection 1 --bisect l1 ^l0 +# +test_bisection_diff() +{ + _max_diff=$1 + _bisect_option=$2 + shift 2 + _bisection=$(git rev-list $_bisect_option "$@") + _list_size=$(git rev-list "$@" | wc -l) + _head=$1 + shift 1 + _bisection_size=$(git rev-list $_bisection "$@" | wc -l) + [ -n "$_list_size" -a -n "$_bisection_size" ] || + error "test_bisection_diff failed" + + # Test if bisection size is close to half of list size within + # tolerance. + # + _bisect_err=$(expr $_list_size - $_bisection_size \* 2) + test "$_bisect_err" -lt 0 && _bisect_err=$(expr 0 - $_bisect_err) + _bisect_err=$(expr $_bisect_err / 2) ; # floor + + test_expect_success \ + "bisection diff $_bisect_option $_head $* <= $_max_diff" \ + 'test $_bisect_err -le $_max_diff' +} + +date >path0 +git update-index --add path0 +save_tag tree git write-tree +on_committer_date "00:00" hide_error save_tag root unique_commit root tree +on_committer_date "00:01" save_tag l0 unique_commit l0 tree -p root +on_committer_date "00:02" save_tag l1 unique_commit l1 tree -p l0 +on_committer_date "00:03" save_tag l2 unique_commit l2 tree -p l1 +on_committer_date "00:04" save_tag a0 unique_commit a0 tree -p l2 +on_committer_date "00:05" save_tag a1 unique_commit a1 tree -p a0 +on_committer_date "00:06" save_tag b1 unique_commit b1 tree -p a0 +on_committer_date "00:07" save_tag c1 unique_commit c1 tree -p b1 +on_committer_date "00:08" save_tag b2 unique_commit b2 tree -p b1 +on_committer_date "00:09" save_tag b3 unique_commit b2 tree -p b2 +on_committer_date "00:10" save_tag c2 unique_commit c2 tree -p c1 -p b2 +on_committer_date "00:11" save_tag c3 unique_commit c3 tree -p c2 +on_committer_date "00:12" save_tag a2 unique_commit a2 tree -p a1 +on_committer_date "00:13" save_tag a3 unique_commit a3 tree -p a2 +on_committer_date "00:14" save_tag b4 unique_commit b4 tree -p b3 -p a3 +on_committer_date "00:15" save_tag a4 unique_commit a4 tree -p a3 -p b4 -p c3 +on_committer_date "00:16" save_tag l3 unique_commit l3 tree -p a4 +on_committer_date "00:17" save_tag l4 unique_commit l4 tree -p l3 +on_committer_date "00:18" save_tag l5 unique_commit l5 tree -p l4 +git update-ref HEAD $(tag l5) + + +# E +# / \ +# e1 | +# | | +# e2 | +# | | +# e3 | +# | | +# e4 | +# | | +# | f1 +# | | +# | f2 +# | | +# | f3 +# | | +# | f4 +# | | +# e5 | +# | | +# e6 | +# | | +# e7 | +# | | +# e8 | +# \ / +# F + + +on_committer_date "00:00" hide_error save_tag F unique_commit F tree +on_committer_date "00:01" save_tag e8 unique_commit e8 tree -p F +on_committer_date "00:02" save_tag e7 unique_commit e7 tree -p e8 +on_committer_date "00:03" save_tag e6 unique_commit e6 tree -p e7 +on_committer_date "00:04" save_tag e5 unique_commit e5 tree -p e6 +on_committer_date "00:05" save_tag f4 unique_commit f4 tree -p F +on_committer_date "00:06" save_tag f3 unique_commit f3 tree -p f4 +on_committer_date "00:07" save_tag f2 unique_commit f2 tree -p f3 +on_committer_date "00:08" save_tag f1 unique_commit f1 tree -p f2 +on_committer_date "00:09" save_tag e4 unique_commit e4 tree -p e5 +on_committer_date "00:10" save_tag e3 unique_commit e3 tree -p e4 +on_committer_date "00:11" save_tag e2 unique_commit e2 tree -p e3 +on_committer_date "00:12" save_tag e1 unique_commit e1 tree -p e2 +on_committer_date "00:13" save_tag E unique_commit E tree -p e1 -p f1 + +on_committer_date "00:00" hide_error save_tag U unique_commit U tree +on_committer_date "00:01" save_tag u0 unique_commit u0 tree -p U +on_committer_date "00:01" save_tag u1 unique_commit u1 tree -p u0 +on_committer_date "00:02" save_tag u2 unique_commit u2 tree -p u0 +on_committer_date "00:03" save_tag u3 unique_commit u3 tree -p u0 +on_committer_date "00:04" save_tag u4 unique_commit u4 tree -p u0 +on_committer_date "00:05" save_tag u5 unique_commit u5 tree -p u0 +on_committer_date "00:06" save_tag V unique_commit V tree -p u1 -p u2 -p u3 -p u4 -p u5 + +test_sequence() +{ + _bisect_option=$1 + + test_bisection_diff 0 $_bisect_option l0 ^root + test_bisection_diff 0 $_bisect_option l1 ^root + test_bisection_diff 0 $_bisect_option l2 ^root + test_bisection_diff 0 $_bisect_option a0 ^root + test_bisection_diff 0 $_bisect_option a1 ^root + test_bisection_diff 0 $_bisect_option a2 ^root + test_bisection_diff 0 $_bisect_option a3 ^root + test_bisection_diff 0 $_bisect_option b1 ^root + test_bisection_diff 0 $_bisect_option b2 ^root + test_bisection_diff 0 $_bisect_option b3 ^root + test_bisection_diff 0 $_bisect_option c1 ^root + test_bisection_diff 0 $_bisect_option c2 ^root + test_bisection_diff 0 $_bisect_option c3 ^root + test_bisection_diff 0 $_bisect_option E ^F + test_bisection_diff 0 $_bisect_option e1 ^F + test_bisection_diff 0 $_bisect_option e2 ^F + test_bisection_diff 0 $_bisect_option e3 ^F + test_bisection_diff 0 $_bisect_option e4 ^F + test_bisection_diff 0 $_bisect_option e5 ^F + test_bisection_diff 0 $_bisect_option e6 ^F + test_bisection_diff 0 $_bisect_option e7 ^F + test_bisection_diff 0 $_bisect_option f1 ^F + test_bisection_diff 0 $_bisect_option f2 ^F + test_bisection_diff 0 $_bisect_option f3 ^F + test_bisection_diff 0 $_bisect_option f4 ^F + test_bisection_diff 0 $_bisect_option E ^F + + test_bisection_diff 1 $_bisect_option V ^U + test_bisection_diff 0 $_bisect_option V ^U ^u1 ^u2 ^u3 + test_bisection_diff 0 $_bisect_option u1 ^U + test_bisection_diff 0 $_bisect_option u2 ^U + test_bisection_diff 0 $_bisect_option u3 ^U + test_bisection_diff 0 $_bisect_option u4 ^U + test_bisection_diff 0 $_bisect_option u5 ^U + +# +# the following illustrates Linus' binary bug blatt idea. +# +# assume the bug is actually at l3, but you don't know that - all you know is that l3 is broken +# and it wasn't broken before +# +# keep bisecting the list, advancing the "bad" head and accumulating "good" heads until +# the bisection point is the head - this is the bad point. +# + +test_output_expect_success "$_bisect_option l5 ^root" 'git rev-list $_bisect_option l5 ^root' <expect && + git rev-list --bisect >actual && + test_cmp expect actual +' + +test_expect_success 'rev-parse --bisect can default to good/bad refs' ' + git rev-parse c3 ^b1 ^c1 >expect && + git rev-parse --bisect >actual && + + # output order depends on the refnames, which in turn depends on + # the exact sha1s. We just want to make sure we have the same set + # of lines in any order. + sort expect.sorted && + sort actual.sorted && + test_cmp expect.sorted actual.sorted +' + +test_output_expect_success '--bisect --first-parent' 'git rev-list --bisect --first-parent E ^F' <expect.unsorted <<-EOF && + $(git rev-parse E) (tag: E, dist=0) + $(git rev-parse e1) (tag: e1, dist=1) + $(git rev-parse e2) (tag: e2, dist=2) + $(git rev-parse e3) (tag: e3, dist=3) + $(git rev-parse e4) (tag: e4, dist=4) + $(git rev-parse e5) (tag: e5, dist=4) + $(git rev-parse e6) (tag: e6, dist=3) + $(git rev-parse e7) (tag: e7, dist=2) + $(git rev-parse e8) (tag: e8, dist=1) + EOF + + # expect results to be ordered by distance (descending), + # commit hash (ascending) + sort -k4,4r -k1,1 expect.unsorted >expect && + git rev-list --bisect-all --first-parent E ^F >actual && + test_cmp expect actual +' + +test_expect_success '--bisect without any revisions' ' + git rev-list --bisect HEAD..HEAD >out && + test_must_be_empty out +' + +test_done diff --git a/userdiff.c b/userdiff.c index 7129bf1482..0e1b9fd1fa 100644 --- a/userdiff.c +++ b/userdiff.c @@ -519,6 +519,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, diff --git a/write-or-die.h b/write-or-die.h index a045bdfaef..6e4ecb04fd 100644 --- a/write-or-die.h +++ b/write-or-die.h @@ -23,13 +23,15 @@ enum fsync_component { FSYNC_COMPONENT_INDEX = 1 << 4, FSYNC_COMPONENT_REFERENCE = 1 << 5, FSYNC_COMPONENT_OBJECT_MAP = 1 << 6, + FSYNC_COMPONENT_DIFF_HUNKS = 1 << 7, }; #define FSYNC_COMPONENTS_OBJECTS (FSYNC_COMPONENT_LOOSE_OBJECT | \ FSYNC_COMPONENT_PACK) #define FSYNC_COMPONENTS_DERIVED_METADATA (FSYNC_COMPONENT_PACK_METADATA | \ - FSYNC_COMPONENT_COMMIT_GRAPH) + FSYNC_COMPONENT_COMMIT_GRAPH | \ + FSYNC_COMPONENT_DIFF_HUNKS) #define FSYNC_COMPONENTS_DEFAULT ((FSYNC_COMPONENTS_OBJECTS | \ FSYNC_COMPONENTS_DERIVED_METADATA) & \ @@ -47,7 +49,8 @@ enum fsync_component { FSYNC_COMPONENT_COMMIT_GRAPH | \ FSYNC_COMPONENT_INDEX | \ FSYNC_COMPONENT_REFERENCE | \ - FSYNC_COMPONENT_OBJECT_MAP) + FSYNC_COMPONENT_OBJECT_MAP | \ + FSYNC_COMPONENT_DIFF_HUNKS) #ifndef FSYNC_COMPONENTS_PLATFORM_DEFAULT #define FSYNC_COMPONENTS_PLATFORM_DEFAULT FSYNC_COMPONENTS_DEFAULT diff --git a/xdiff-interface.h b/xdiff-interface.h index 71e5dffefb..23db1d2a46 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -6,6 +6,18 @@ struct object_database; +/* + * 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 + * storing hunks in this struct. + */ +struct xdl_hunk { + long old_start, old_count; + long new_start, new_count; +}; + /* * xdiff isn't equipped to handle content over a gigabyte; * we make the cutoff 1GB - 1MB to give some breathing