diff --git a/Documentation/config/diff.adoc b/Documentation/config/diff.adoc index 1135a62a0ad3de..ac0635bb3bee1c 100644 --- a/Documentation/config/diff.adoc +++ b/Documentation/config/diff.adoc @@ -218,6 +218,11 @@ endif::git-diff[] Set this option to `true` to make the diff driver cache the text conversion outputs. See linkgit:gitattributes[5] for details. +`diff..process`:: + The command to run as a long-running diff process that + provides hunks to Git's diff pipeline. + See linkgit:gitattributes[5] for details. + `diff.indentHeuristic`:: Set this option to `false` to disable the default heuristics that shift diff hunk boundaries to make patches easier to read. diff --git a/Documentation/diff-algorithm-option.adoc b/Documentation/diff-algorithm-option.adoc index 8e3a0b63d784d8..4d7e2ec35f97ea 100644 --- a/Documentation/diff-algorithm-option.adoc +++ b/Documentation/diff-algorithm-option.adoc @@ -18,3 +18,6 @@ For instance, if you configured the `diff.algorithm` variable to a non-default value and want to use the default one, then you have to use `--diff-algorithm=default` option. ++ +If you explicitly choose a diff algorithm, it also bypasses +`diff..process` (see linkgit:gitattributes[5]). diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc index 8a63b5e164114a..18b8b0ed242838 100644 --- a/Documentation/diff-options.adoc +++ b/Documentation/diff-options.adoc @@ -825,7 +825,9 @@ endif::git-format-patch[] to use this option with linkgit:git-log[1] and friends. `--no-ext-diff`:: - Disallow external diff drivers. + Disallow external diff helpers, including + `diff..command` and `diff..process` + (see linkgit:gitattributes[5]). `--textconv`:: `--no-textconv`:: diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc index f20041a323d174..1700bd8e974deb 100644 --- a/Documentation/gitattributes.adoc +++ b/Documentation/gitattributes.adoc @@ -821,6 +821,145 @@ 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. +Using an external diff process +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +If `diff..process` is defined, Git sends the old and new file +content to an external tool and receives back a list of changed +regions (pairs of line ranges in the old and new file). Git uses +these instead of its builtin diff algorithm, but still controls +all output formatting, so features like word diff, function context, +color, and blame work normally. This is achieved by using the +long-running process protocol (described in +Documentation/technical/long-running-process-protocol.adoc). +Unlike `diff..command`, which replaces Git's output entirely, +the diff process feeds results back into the standard pipeline. + +First, in `.gitattributes`, assign the `diff` attribute for paths. + +------------------------ +*.c diff=cdiff +------------------------ + +Then, define a "diff..process" configuration to specify +the diff process command. + +---------------------------------------------------------------- +[diff "cdiff"] + process = /path/to/diff-process-tool +---------------------------------------------------------------- + +When Git encounters the first file that needs to be diffed, it starts +the process and performs the handshake. In the handshake, the welcome +message sent by Git is "git-diff-client", only version 1 is supported, +and the supported capability is "hunks" (the changed regions +described below). + +For each file, Git sends a list of "key=value" pairs terminated with +a flush packet, followed by the old and new file content as packetized +data, each terminated with a flush packet. The pathname is relative +to the repository root. When `diff..textconv` is also set, +the tool receives the textconv-transformed content rather than the +raw blob. Git does not send binary files to the diff process. + +----------------------- +packet: git> command=hunks +packet: git> pathname=path/file.c +packet: git> 0000 +packet: git> OLD_CONTENT +packet: git> 0000 +packet: git> NEW_CONTENT +packet: git> 0000 +----------------------- + +The tool is expected to respond with zero or more hunk lines, +a flush packet, and a status packet terminated with a flush packet. +Each hunk line has the form: + + `hunk ` + +where `` and `` identify a range of lines in +the old file, and `` and `` identify the +replacement range in the new file. Start values are 1-based and +counts are non-negative. Ranges must not extend beyond the end of +the file. For example, `hunk 3 2 3 4` means that 2 lines starting +at line 3 in the old file were replaced by 4 lines starting at +line 3 in the new file. An `` of 0 means no lines were +removed (pure insertion); a `` of 0 means no lines were +added (pure deletion). + +Lines are delimited by newlines. A file `"foo\nbar\n"` and a +file `"foo\nbar"` both have 2 lines. + +Hunks must be listed in order and must not overlap. Any line +not covered by a hunk is treated as unchanged, so the total +number of unchanged lines must be the same on both sides. +For example, if the old file has 10 lines and the hunks cover +4 of them (`old_count` values summing to 4), then 6 old lines +are unchanged. The new file must also have exactly 6 lines +not covered by hunks, so the `new_count` values must sum to +`new_file_lines - 6`. + +----------------------- +packet: git< hunk 1 3 1 5 +packet: git< hunk 10 2 12 2 +packet: git< 0000 +packet: git< status=success +packet: git< 0000 +----------------------- + +If the tool responds with hunks and "success", Git marks those lines +as changed and feeds them into the standard diff pipeline. Patch +output features (word diff, function context, color) work normally. +Note that `--stat` and other summary formats use their own diff path +and are not affected by the diff process. + +If no hunk lines precede the flush, followed by "success", Git +treats the files as having no changes: `git diff` produces no output +and `git blame` skips the commit, attributing lines to earlier commits. + +----------------------- +packet: git< 0000 +packet: git< status=success +packet: git< 0000 +----------------------- + +If the tool returns invalid hunks (out of bounds, overlapping), Git +silently falls back to the builtin diff algorithm. + +In case the tool cannot or does not want to process the content, +it is expected to respond with an "error" status. Git warns and +falls back to the builtin diff algorithm for this file. The tool +remains available for subsequent files. + +----------------------- +packet: git< 0000 +packet: git< status=error +packet: git< 0000 +----------------------- + +In case the tool cannot or does not want to process the content as +well as any future content for the lifetime of the Git process, it +is expected to respond with an "abort" status. Git silently falls +back to the builtin diff algorithm for this file and does not send +further requests to the tool. + +----------------------- +packet: git< 0000 +packet: git< status=abort +packet: git< 0000 +----------------------- + +If the tool dies during the communication or does not adhere to the +protocol then Git will stop the process and fall back to the builtin +diff algorithm. Git warns once and does not restart the process for +subsequent files. + +Tools should ignore unknown keys in the per-file request to remain +forward-compatible. Future versions of Git may send additional +`command=` values; tools that receive an unrecognized command should +respond with `status=error` rather than terminating. + Defining a custom hunk-header ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/Makefile b/Makefile index cedc234173e377..22900368dd59b5 100644 --- a/Makefile +++ b/Makefile @@ -1142,6 +1142,7 @@ LIB_OBJS += diff-delta.o LIB_OBJS += diff-merges.o LIB_OBJS += diff-lib.o LIB_OBJS += diff-no-index.o +LIB_OBJS += diff-process.o LIB_OBJS += diff.o LIB_OBJS += diffcore-break.o LIB_OBJS += diffcore-delta.o diff --git a/blame.c b/blame.c index a3c49d132e4ae1..79f02735a45a95 100644 --- a/blame.c +++ b/blame.c @@ -19,6 +19,8 @@ #include "tag.h" #include "trace2.h" #include "blame.h" +#include "diff-process.h" +#include "xdiff-interface.h" #include "alloc.h" #include "commit-slab.h" #include "bloom.h" @@ -314,17 +316,25 @@ static struct commit *fake_working_tree_commit(struct repository *r, -static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, - xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts) +static int diff_hunks_xpp(mmfile_t *file_a, mmfile_t *file_b, + xdl_emit_hunk_consume_func_t hunk_func, + void *cb_data, xpparam_t *xpp) { - xpparam_t xpp = {0}; xdemitconf_t xecfg = {0}; xdemitcb_t ecb = {NULL}; - xpp.flags = xdl_opts; xecfg.hunk_func = hunk_func; ecb.priv = cb_data; - return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb); + return xdi_diff(file_a, file_b, xpp, &xecfg, &ecb); +} + +static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, + xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts) +{ + xpparam_t xpp = {0}; + + xpp.flags = xdl_opts; + return diff_hunks_xpp(file_a, file_b, hunk_func, cb_data, &xpp); } static const char *get_next_line(const char *start, const char *end) @@ -1943,6 +1953,7 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, struct blame_origin *parent, int ignore_diffs) { mmfile_t file_p, file_o; + xpparam_t xpp = {0}; struct blame_chunk_cb_data d; struct blame_entry *newdest = NULL; @@ -1961,10 +1972,21 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, &sb->num_read_blob, ignore_diffs); sb->num_get_patch++; - if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts)) - die("unable to generate diff (%s -> %s)", - oid_to_hex(&parent->commit->object.oid), - oid_to_hex(&target->commit->object.oid)); + xpp.flags = sb->xdl_opts; + /* + * If the diff process considers the files equivalent, + * skip the diff so blame looks past this commit. + */ + if (diff_process_fill_hunks(&sb->revs->diffopt, target->path, + &file_p, &file_o, &xpp) + != DIFF_PROCESS_EQUIVALENT) { + if (diff_hunks_xpp(&file_p, &file_o, blame_chunk_cb, + &d, &xpp)) + die("unable to generate diff (%s -> %s)", + oid_to_hex(&parent->commit->object.oid), + oid_to_hex(&target->commit->object.oid)); + } + free(xpp.external_hunks); /* The rest are the same as the parent */ blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, 0, parent, target, 0); diff --git a/builtin/log.c b/builtin/log.c index 8c0939dd42ada2..1ea520c12de951 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -2213,6 +2213,13 @@ int cmd_format_patch(int argc, if (argc > 1) die(_("unrecognized argument: %s"), argv[1]); + /* + * Disable diff..process so that patches generated by + * format-patch are always based on the builtin diff algorithm + * and can be applied reliably. + */ + rev.diffopt.flags.no_diff_process = 1; + if (rev.diffopt.output_format & DIFF_FORMAT_NAME) die(_("--name-only does not make sense")); if (rev.diffopt.output_format & DIFF_FORMAT_NAME_STATUS) diff --git a/diff-process.c b/diff-process.c new file mode 100644 index 00000000000000..d2ef9463d73461 --- /dev/null +++ b/diff-process.c @@ -0,0 +1,288 @@ +/* + * Diff process backend: communicates with a long-running external + * tool via the pkt-line protocol to obtain custom line-matching + * results. The tool controls which lines are marked as changed + * while the display shows the file content (after any textconv + * transformation, if configured). + * + * Protocol: pkt-line over stdin/stdout, following the pattern of + * the long-running filter process protocol (see convert.c). + * + * Handshake: + * git> git-diff-client / version=1 / flush + * tool< git-diff-server / version=1 / flush + * git> capability=hunks / flush + * tool< capability=hunks / flush + * + * Per-file: + * git> command=hunks / pathname= / flush + * git> / flush + * git> / flush + * tool< hunk + * tool< ... / flush + * tool< status=success / flush + * + * When the tool returns no hunks with status=success, it considers + * the files equivalent. Git will skip the diff for that file. + */ + +#include "git-compat-util.h" +#include "diff-process.h" +#include "diff.h" +#include "gettext.h" +#include "repository.h" +#include "sigchain.h" +#include "userdiff.h" +#include "sub-process.h" +#include "pkt-line.h" +#include "strbuf.h" +#include "xdiff/xdiff.h" + +#define CAP_HUNKS (1u << 0) + +struct diff_subprocess { + struct subprocess_entry subprocess; + unsigned int supported_capabilities; +}; + +static int start_diff_process_fn(struct subprocess_entry *subprocess) +{ + static int versions[] = { 1, 0 }; + static struct subprocess_capability capabilities[] = { + { "hunks", CAP_HUNKS }, + { NULL, 0 } + }; + struct diff_subprocess *entry = + container_of(subprocess, struct diff_subprocess, subprocess); + + return subprocess_handshake(subprocess, "git-diff", + versions, NULL, + capabilities, + &entry->supported_capabilities); +} + +static struct diff_subprocess *get_or_launch_process( + struct userdiff_driver *drv) +{ + struct diff_subprocess *entry; + + if (drv->diff_subprocess) + return drv->diff_subprocess; + + entry = xcalloc(1, sizeof(*entry)); + if (subprocess_start_command(&entry->subprocess, drv->process, + start_diff_process_fn)) { + free(entry); + drv->diff_process_failed = 1; + return NULL; + } + + drv->diff_subprocess = entry; + return entry; +} + +static int send_file_content(int fd, const char *buf, long size) +{ + int ret = 0; + + if (size < 0) + return -1; + if (size > 0) + ret = write_packetized_from_buf_no_flush(buf, size, fd); + if (ret) + return ret; + return packet_flush_gently(fd); +} + +static int parse_hunk_line(const char *line, struct xdl_hunk *hunk) +{ + char *end; + + /* + * Format: "hunk " + * All numbers must be non-negative decimal with no leading + * whitespace or sign characters. + */ + if (!skip_prefix(line, "hunk ", &line)) + return -1; + + if (!isdigit(*line)) + return -1; + errno = 0; + hunk->old_start = strtol(line, &end, 10); + if (errno || end == line || *end++ != ' ') + return -1; + line = end; + + if (!isdigit(*line)) + return -1; + errno = 0; + hunk->old_count = strtol(line, &end, 10); + if (errno || end == line || *end++ != ' ') + return -1; + line = end; + + if (!isdigit(*line)) + return -1; + errno = 0; + hunk->new_start = strtol(line, &end, 10); + if (errno || end == line || *end++ != ' ') + return -1; + line = end; + + if (!isdigit(*line)) + return -1; + errno = 0; + hunk->new_count = strtol(line, &end, 10); + if (errno || end == line || *end != '\0') + return -1; + + return 0; +} + +static enum diff_process_result get_hunks( + struct userdiff_driver *drv, + const char *path, + const char *old_buf, long old_size, + const char *new_buf, long new_size, + struct xdl_hunk **hunks_out, + size_t *nr_hunks_out) +{ + struct diff_subprocess *backend; + struct child_process *process; + int fd_in, fd_out; + struct strbuf status = STRBUF_INIT; + struct xdl_hunk *hunks = NULL; + struct xdl_hunk hunk; + size_t nr_hunks = 0, alloc_hunks = 0; + int len; + char *line; + + backend = get_or_launch_process(drv); + if (!backend) + return DIFF_PROCESS_ERROR; + + if (!(backend->supported_capabilities & CAP_HUNKS)) + return DIFF_PROCESS_SKIP; + + process = subprocess_get_child_process(&backend->subprocess); + fd_in = process->in; + fd_out = process->out; + + sigchain_push(SIGPIPE, SIG_IGN); + + /* Send request */ + if (packet_write_fmt_gently(fd_in, "command=hunks\n") || + packet_write_fmt_gently(fd_in, "pathname=%s\n", path) || + packet_flush_gently(fd_in)) + goto comm_error; + + /* Send old file content */ + if (send_file_content(fd_in, old_buf, old_size)) + goto comm_error; + + /* Send new file content */ + if (send_file_content(fd_in, new_buf, new_size)) + goto comm_error; + + /* Read hunks until flush packet */ + while ((len = packet_read_line_gently(fd_out, NULL, &line)) >= 0 && + line) { + if (parse_hunk_line(line, &hunk) < 0) + goto comm_error; + ALLOC_GROW(hunks, nr_hunks + 1, alloc_hunks); + hunks[nr_hunks++] = hunk; + } + if (len < 0) + goto comm_error; + + /* Read status */ + if (subprocess_read_status(fd_out, &status)) + goto comm_error; + + if (!strcmp(status.buf, "success")) { + *hunks_out = hunks; + *nr_hunks_out = nr_hunks; + strbuf_release(&status); + sigchain_pop(SIGPIPE); + return DIFF_PROCESS_OK; + } + + if (!strcmp(status.buf, "abort")) { + /* + * The tool voluntarily withdrew: stop sending requests + * but do not warn (this is not a failure). + */ + backend->supported_capabilities &= ~CAP_HUNKS; + free(hunks); + strbuf_release(&status); + sigchain_pop(SIGPIPE); + return DIFF_PROCESS_SKIP; + } + + /* status=error or unknown status */ + free(hunks); + strbuf_release(&status); + sigchain_pop(SIGPIPE); + return DIFF_PROCESS_ERROR; + +comm_error: + /* + * Communication failure (broken pipe, malformed response). + * Tear down the process and mark as failed so we do not + * retry on every subsequent file. + */ + drv->diff_process_failed = 1; + drv->diff_subprocess = NULL; + subprocess_stop_command(&backend->subprocess); + free(backend); + free(hunks); + strbuf_release(&status); + sigchain_pop(SIGPIPE); + return DIFF_PROCESS_ERROR; +} + +enum diff_process_result diff_process_fill_hunks( + struct diff_options *diffopt, + const char *path, + const mmfile_t *file_a, + const mmfile_t *file_b, + xpparam_t *xpp) +{ + struct userdiff_driver *drv; + struct xdl_hunk *ext_hunks = NULL; + size_t nr = 0; + enum diff_process_result res; + + if (!diffopt || !path) + return DIFF_PROCESS_SKIP; + if (diffopt->flags.no_diff_process || diffopt->ignore_driver_algorithm) + return DIFF_PROCESS_SKIP; + + drv = userdiff_find_by_path(diffopt->repo->index, path); + if (!drv || !drv->process) + return DIFF_PROCESS_SKIP; + if (drv->diff_process_failed) + return DIFF_PROCESS_SKIP; + + res = get_hunks(drv, path, + file_a->ptr, file_a->size, + file_b->ptr, file_b->size, + &ext_hunks, &nr); + if (res == DIFF_PROCESS_OK) { + if (!nr) { + free(ext_hunks); + return DIFF_PROCESS_EQUIVALENT; + } + xpp->external_hunks = ext_hunks; + xpp->external_hunks_nr = nr; + return DIFF_PROCESS_OK; + } + if (res == DIFF_PROCESS_ERROR) { + warning(_("diff process '%s' failed for '%s'," + " falling back to builtin diff"), + drv->process, path); + return DIFF_PROCESS_ERROR; + } + return DIFF_PROCESS_SKIP; +} diff --git a/diff-process.h b/diff-process.h new file mode 100644 index 00000000000000..d34b42f8116a04 --- /dev/null +++ b/diff-process.h @@ -0,0 +1,39 @@ +#ifndef DIFF_PROCESS_H +#define DIFF_PROCESS_H + +#include "xdiff/xdiff.h" + +struct diff_options; + +enum diff_process_result { + DIFF_PROCESS_ERROR = -1, /* tool failure: warned, fell back */ + DIFF_PROCESS_OK = 0, /* hunks populated in xpp */ + DIFF_PROCESS_SKIP, /* no process configured: use builtin */ + DIFF_PROCESS_EQUIVALENT, /* tool says files are equivalent */ +}; + +/* + * Consult the diff process configured for 'path' and populate + * xpp->external_hunks with the returned hunks. + * + * Handles driver lookup, flag checks (--no-ext-diff, + * --diff-algorithm), subprocess management, and error reporting. + * + * Returns DIFF_PROCESS_OK when hunks are populated in xpp. + * The caller owns xpp->external_hunks and must free() it. + * + * Returns DIFF_PROCESS_EQUIVALENT when the tool returns no hunks + * (files are considered identical); caller should skip diff/blame. + * Returns DIFF_PROCESS_SKIP when no process applies; caller + * should use the builtin diff algorithm. + * Returns DIFF_PROCESS_ERROR on tool failure (already warned); + * caller should fall back to the builtin diff algorithm. + */ +enum diff_process_result diff_process_fill_hunks( + struct diff_options *diffopt, + const char *path, + const mmfile_t *file_a, + const mmfile_t *file_b, + xpparam_t *xpp); + +#endif /* DIFF_PROCESS_H */ diff --git a/diff.c b/diff.c index 397e38b41cc6fa..38932db084ff91 100644 --- a/diff.c +++ b/diff.c @@ -25,6 +25,7 @@ #include "utf8.h" #include "odb.h" #include "userdiff.h" +#include "diff-process.h" #include "submodule.h" #include "hashmap.h" #include "mem-pool.h" @@ -4031,6 +4032,17 @@ static void builtin_diff(const char *name_a, xpp.ignore_regex_nr = o->ignore_regex_nr; xpp.anchors = o->anchors; xpp.anchors_nr = o->anchors_nr; + + if (diff_process_fill_hunks(o, name_a, + &mf1, &mf2, &xpp) + == DIFF_PROCESS_EQUIVALENT) { + if (textconv_one) + free(mf1.ptr); + if (textconv_two) + free(mf2.ptr); + goto free_ab_and_return; + } + xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; xecfg.flags = XDL_EMIT_FUNCNAMES; @@ -4111,6 +4123,7 @@ static void builtin_diff(const char *name_a, } else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume, &ecbdata, &xpp, &xecfg)) die("unable to generate diff for %s", one->path); + free(xpp.external_hunks); if (o->word_diff) free_diff_words_data(&ecbdata); if (textconv_one) @@ -5900,6 +5913,17 @@ static int diff_opt_submodule(const struct option *opt, return 0; } +static int diff_opt_ext_diff(const struct option *opt, + const char *arg, int unset) +{ + struct diff_options *options = opt->value; + + BUG_ON_OPT_ARG(arg); + options->flags.allow_external = !unset; + options->flags.no_diff_process = unset; + return 0; +} + static int diff_opt_textconv(const struct option *opt, const char *arg, int unset) { @@ -6228,8 +6252,9 @@ struct option *add_diff_options(const struct option *opts, N_("exit with 1 if there were differences, 0 otherwise")), OPT_BOOL(0, "quiet", &options->flags.quick, N_("disable all output of the program")), - OPT_BOOL(0, "ext-diff", &options->flags.allow_external, - N_("allow an external diff helper to be executed")), + OPT_CALLBACK_F(0, "ext-diff", options, NULL, + N_("allow an external diff helper to be executed"), + PARSE_OPT_NOARG, diff_opt_ext_diff), OPT_CALLBACK_F(0, "textconv", options, NULL, N_("run external text conversion filters when comparing binary files"), PARSE_OPT_NOARG, diff_opt_textconv), diff --git a/diff.h b/diff.h index 7eb84aadf40c41..71ba389f030180 100644 --- a/diff.h +++ b/diff.h @@ -173,6 +173,11 @@ struct diff_flags { */ unsigned allow_external; + /** + * Disables diff..process. Set by --no-ext-diff. + */ + unsigned no_diff_process; + /** * For communication between the calling program and the options parser; * tell the calling program to signal the presence of difference using diff --git a/meson.build b/meson.build index 11488623bfd8f8..8a7370b38f5c1f 100644 --- a/meson.build +++ b/meson.build @@ -328,6 +328,7 @@ libgit_sources = [ 'diff-merges.c', 'diff-lib.c', 'diff-no-index.c', + 'diff-process.c', 'diff.c', 'diffcore-break.c', 'diffcore-delta.c', diff --git a/sub-process.c b/sub-process.c index 83bf0a0e82e56d..33b0bbc831a10b 100644 --- a/sub-process.c +++ b/sub-process.c @@ -49,7 +49,7 @@ int subprocess_read_status(int fd, struct strbuf *status) return (len < 0) ? len : 0; } -void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry) +void subprocess_stop_command(struct subprocess_entry *entry) { if (!entry) return; @@ -57,7 +57,14 @@ void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry) entry->process.clean_on_exit = 0; kill(entry->process.pid, SIGTERM); finish_command(&entry->process); +} + +void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry) +{ + if (!entry) + return; + subprocess_stop_command(entry); hashmap_remove(hashmap, &entry->ent, NULL); } @@ -72,7 +79,7 @@ static void subprocess_exit_handler(struct child_process *process) finish_command(process); } -int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd, +int subprocess_start_command(struct subprocess_entry *entry, const char *cmd, subprocess_start_fn startfn) { int err; @@ -96,15 +103,27 @@ 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 bfc3959a1b4894..45f1b8e5e3212f 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); diff --git a/t/.gitattributes b/t/.gitattributes index 7664c6e027d0ed..de97920cabf97c 100644 --- a/t/.gitattributes +++ b/t/.gitattributes @@ -23,3 +23,4 @@ t[0-9][0-9][0-9][0-9]/* -whitespace /t8005/*.txt eol=lf /t9*/*.dump eol=lf /t0040*.sh whitespace=-indent-with-non-tab +/t4080-diff-process.sh whitespace=-indent-with-non-tab diff --git a/t/meson.build b/t/meson.build index 7528e5cda5fef0..f67208d7eee004 100644 --- a/t/meson.build +++ b/t/meson.build @@ -510,6 +510,7 @@ integration_tests = [ 't4072-diff-max-depth.sh', 't4073-diff-stat-name-width.sh', 't4074-diff-shifted-matched-group.sh', + 't4080-diff-process.sh', 't4100-apply-stat.sh', 't4101-apply-nonl.sh', 't4102-apply-rename.sh', diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh new file mode 100755 index 00000000000000..fdf6da1c341e67 --- /dev/null +++ b/t/t4080-diff-process.sh @@ -0,0 +1,660 @@ +#!/bin/sh + +test_description='diff process via long-running process' + +. ./test-lib.sh + +if test_have_prereq PYTHON +then + PYTHON_PATH=$(command -v python3) || PYTHON_PATH=$(command -v python) +fi + +# +# A single parametric diff process. +# Usage: diff-process-backend --mode= [--log=] +# +# Modes: +# whole-file - report all lines as changed (default) +# fixed-hunk - always report hunk 5 2 5 2 +# bad-hunk - report out-of-bounds hunk 999 1 999 1 +# bad-sync - report hunk with mismatched unchanged totals +# overlap - report two overlapping hunks +# no-hunks - return no hunks (files considered equivalent) +# error - return status=error for every request +# abort - return status=abort for every request +# crash - read one request then exit without responding +# +setup_backend () { + cat >"$TRASH_DIRECTORY/diff-process-backend.py" <<-\PYEOF + import sys, os + + def read_pkt(): + hdr = sys.stdin.buffer.read(4) + if len(hdr) < 4: return None + length = int(hdr, 16) + if length == 0: return "" + data = sys.stdin.buffer.read(length - 4) + return data.decode().rstrip("\n") + + def write_pkt(line): + data = (line + "\n").encode() + sys.stdout.buffer.write(f"{len(data)+4:04x}".encode() + data) + sys.stdout.buffer.flush() + + def write_flush(): + sys.stdout.buffer.write(b"0000") + sys.stdout.buffer.flush() + + def read_content(): + chunks = [] + while True: + hdr = sys.stdin.buffer.read(4) + if len(hdr) < 4: break + length = int(hdr, 16) + if length == 0: break + chunks.append(sys.stdin.buffer.read(length - 4)) + return b"".join(chunks) + + mode = "whole-file" + logfile = None + for arg in sys.argv[1:]: + if arg.startswith("--mode="): + mode = arg[7:] + elif arg.startswith("--log="): + logfile = open(arg[6:], "a") + + def log(msg): + if logfile: + logfile.write(msg + "\n") + logfile.flush() + + # Handshake + assert read_pkt() == "git-diff-client" + assert read_pkt() == "version=1" + read_pkt() + write_pkt("git-diff-server") + write_pkt("version=1") + write_flush() + while True: + p = read_pkt() + if p == "": break + write_pkt("capability=hunks") + write_flush() + + log("ready") + + while True: + cmd = None + pathname = None + while True: + p = read_pkt() + if p is None: sys.exit(0) + if p == "": break + if p.startswith("command="): cmd = p.split("=",1)[1] + if p.startswith("pathname="): pathname = p.split("=",1)[1] + if cmd is None: sys.exit(0) + old = read_content() + new = read_content() + old_first = old.split(b"\n")[0].decode(errors="replace") if old else "" + new_first = new.split(b"\n")[0].decode(errors="replace") if new else "" + log(f"command={cmd} pathname={pathname} old={old_first} new={new_first}") + + if mode == "error": + write_flush() + write_pkt("status=error") + write_flush() + continue + + if mode == "abort": + write_flush() + write_pkt("status=abort") + write_flush() + continue + + if mode == "crash": + sys.exit(1) + + if cmd == "hunks": + if mode == "fixed-hunk": + write_pkt("hunk 5 2 5 2") + elif mode == "bad-hunk": + write_pkt("hunk 999 1 999 1") + elif mode == "bad-sync": + write_pkt("hunk 1 2 1 1") + elif mode == "overlap": + write_pkt("hunk 1 5 1 5") + write_pkt("hunk 3 2 3 2") + elif mode == "no-hunks": + pass + else: + ol = old.count(b"\n") + nl = new.count(b"\n") + write_pkt(f"hunk 1 {ol} 1 {nl}") + write_flush() + write_pkt("status=success") + write_flush() + else: + write_flush() + write_pkt("status=error") + write_flush() + PYEOF + write_script diff-process-backend <<-SHEOF + exec "$PYTHON_PATH" "$TRASH_DIRECTORY/diff-process-backend.py" "\$@" + SHEOF +} + +BACKEND="./diff-process-backend" + +test_expect_success PYTHON 'setup' ' + setup_backend && + echo "*.c diff=cdiff" >.gitattributes && + git add .gitattributes && + + # boundary.c: 10 lines, changes at 5-6 and 9-10. + # Used by: hunk boundaries, error fallback, crash, bad hunks, overlap. + cat >boundary.c <<-\EOF && + line1 + line2 + line3 + line4 + OLD5 + OLD6 + line7 + line8 + OLD9 + OLD10 + EOF + git add boundary.c && + + # worddiff.c: single-line function, value changes 1 -> 999. + # Used by: word-diff, --diff-algorithm, --no-ext-diff, --stat. + cat >worddiff.c <<-\EOF && + int value(void) { return 1; } + EOF + git add worddiff.c && + + # newfile.c: single-line function, value changes 42 -> 99. + # Used by: new file, --exit-code, multiple drivers. + cat >newfile.c <<-\EOF && + int new_func(void) { return 42; } + EOF + git add newfile.c && + + # logtest.c: single-line function for log/format-patch tests. + # Needs two commits so log -1 has a diff. + cat >logtest.c <<-\EOF && + int logfunc(void) { return 1; } + EOF + git add logtest.c && + + # two.c/one.c: two-file pair for error/abort/startup-failure tests. + cat >one.c <<-\EOF && + int first(void) { return 1; } + EOF + cat >two.c <<-\EOF && + int second(void) { return 2; } + EOF + git add one.c two.c && + + git commit -m "initial" && + + # Second commit for logtest.c (so log -1 has something to show). + cat >logtest.c <<-\EOF && + int logfunc(void) { return 2; } + EOF + git add logtest.c && + git commit -m "change logtest.c" && + + # Working tree modifications (not committed). + cat >boundary.c <<-\EOF && + line1 + line2 + line3 + line4 + NEW5 + NEW6 + line7 + line8 + NEW9 + NEW10 + EOF + + cat >worddiff.c <<-\EOF && + int value(void) { return 999; } + EOF + + cat >newfile.c <<-\EOF && + int new_func(void) { return 99; } + EOF + + cat >one.c <<-\EOF && + int first(void) { return 10; } + EOF + + cat >two.c <<-\EOF + int second(void) { return 20; } + EOF +' + +# +# Core behavior: the tool controls which lines are marked as changed. +# + +test_expect_success PYTHON 'diff process hunk boundaries affect output' ' + # The file has changes at lines 5-6 and 9-10, but fixed-hunk + # only reports lines 5-6 as changed. Lines 9-10 should not + # appear as changed in the output. + git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk" \ + diff boundary.c >actual && + test_grep "^-OLD5" actual && + test_grep "^-OLD6" actual && + test_grep "^+NEW5" actual && + test_grep "^+NEW6" actual && + test_grep ! "^-OLD9" actual && + test_grep ! "^-OLD10" actual && + test_grep ! "^+NEW9" actual && + test_grep ! "^+NEW10" actual +' + +test_expect_success PYTHON 'diff process works with new file' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff -- newfile.c >actual 2>stderr && + test_grep "return 99" actual && + test_grep "pathname=newfile.c" backend.log && + test_must_be_empty stderr +' + +test_expect_success PYTHON 'diff process works with added file (empty old side)' ' + cat >added.c <<-\EOF && + int added(void) { return 1; } + EOF + git add added.c && + + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff --cached -- added.c >actual 2>stderr && + test_grep "added" actual && + test_grep "pathname=added.c" backend.log && + test_must_be_empty stderr +' + +test_expect_success PYTHON 'diff process skipped for binary files' ' + printf "\\0binary" >binary.c && + git add binary.c && + git commit -m "add binary" && + printf "\\0changed" >binary.c && + + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff -- binary.c >actual && + test_grep "Binary files" actual && + test_path_is_missing backend.log +' + +test_expect_success PYTHON 'diff process not consulted for unmatched driver' ' + echo "not tracked by cdiff" >unmatched.txt && + git add unmatched.txt && + git commit -m "add unmatched.txt" && + + echo "modified" >unmatched.txt && + + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff -- unmatched.txt >actual && + test_grep "modified" actual && + test_path_is_missing backend.log +' + +test_expect_success PYTHON 'multiple drivers use separate processes' ' + echo "*.h diff=hdiff" >>.gitattributes && + git add .gitattributes && + + cat >multi.h <<-\EOF && + int header(void) { return 1; } + EOF + git add multi.h && + git commit -m "add multi.h" && + + cat >multi.h <<-\EOF && + int header(void) { return 2; } + EOF + + rm -f backend-c.log backend-h.log && + git -c diff.cdiff.process="$BACKEND --log=backend-c.log" \ + -c diff.hdiff.process="$BACKEND --log=backend-h.log" \ + diff -- newfile.c multi.h >actual 2>stderr && + test_grep "pathname=newfile.c" backend-c.log && + test_grep "pathname=multi.h" backend-h.log && + test_must_be_empty stderr +' + +test_expect_success PYTHON 'diff process works alongside textconv' ' + write_script uppercase-filter <<-\EOF && + tr "a-z" "A-Z" <"$1" + EOF + + cat >textconv.c <<-\EOF && + hello world + EOF + git add textconv.c && + git commit -m "add textconv.c" && + + cat >textconv.c <<-\EOF && + goodbye world + EOF + + rm -f backend.log && + git -c diff.cdiff.textconv="./uppercase-filter" \ + -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff -- textconv.c >actual 2>stderr && + # The diff process receives textconv-transformed (uppercase) content. + test_grep "pathname=textconv.c" backend.log && + test_grep "old=HELLO WORLD" backend.log && + test_grep "new=GOODBYE WORLD" backend.log && + test_must_be_empty stderr +' + +# +# Downstream features: word diff, log, equivalent files, exit code. +# + +test_expect_success PYTHON 'diff process with --word-diff' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff --word-diff worddiff.c >actual 2>stderr && + test_grep "\[-1;-\]" actual && + test_grep "{+999;+}" actual && + test_grep "pathname=worddiff.c" backend.log && + test_must_be_empty stderr +' + +test_expect_success PYTHON 'diff process works with git log -p' ' + # With no-hunks mode, the tool says the files are equivalent, + # so log -p should show the commit but no diff content. + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \ + log -1 -p -- logtest.c >actual 2>stderr && + test_grep "change logtest.c" actual && + test_grep ! "return 2" actual && + test_grep "command=hunks pathname=logtest.c" backend.log && + test_must_be_empty stderr +' + +test_expect_success PYTHON 'diff process no hunks suppresses diff output' ' + cat >nohunks.c <<-\EOF && + int zero(void) { return 0; } + EOF + git add nohunks.c && + git commit -m "add nohunks.c" && + + cat >nohunks.c <<-\EOF && + int zero(void) { return 999; } + EOF + + git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \ + diff nohunks.c >actual && + test_must_be_empty actual +' + +test_expect_success PYTHON 'diff process no hunks with --exit-code returns success' ' + git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \ + diff --exit-code nohunks.c +' + +test_expect_success PYTHON 'diff process with --exit-code and hunks returns failure' ' + test_expect_code 1 git -c diff.cdiff.process="$BACKEND" \ + diff --exit-code newfile.c +' + +# +# Bypass mechanisms: flags and commands that skip the diff process. +# + +test_expect_success PYTHON 'diff process bypassed by --diff-algorithm' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff --diff-algorithm=patience worddiff.c >actual && + test_grep "return 999" actual && + test_path_is_missing backend.log +' + +test_expect_success PYTHON 'diff process bypassed by --no-ext-diff' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff --no-ext-diff worddiff.c >actual && + test_grep "return 999" actual && + test_path_is_missing backend.log +' + +test_expect_success PYTHON 'diff process not used by format-patch' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + format-patch -1 --stdout -- logtest.c >actual && + test_grep "return 2" actual && + test_path_is_missing backend.log +' + +test_expect_success PYTHON 'diff process not used by --stat' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --log=backend.log" \ + diff --stat worddiff.c >actual && + test_grep "worddiff.c" actual && + test_path_is_missing backend.log +' + +# +# Error handling and fallback. +# + +test_expect_success PYTHON 'diff process fallback on tool error status' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --mode=error --log=backend.log" \ + diff boundary.c >actual 2>stderr && + # Fallback produces the full builtin diff (both change regions). + test_grep "^-OLD5" actual && + test_grep "^+NEW5" actual && + test_grep "^-OLD9" actual && + test_grep "^+NEW9" actual && + # Tool was contacted (it replied with error, not crash). + test_grep "command=hunks pathname=boundary.c" backend.log && + test_grep "diff process.*failed" stderr +' + +test_expect_success PYTHON 'diff process error keeps tool available for next file' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --mode=error --log=backend.log" \ + diff -- one.c two.c >actual 2>stderr && + # Unlike abort, error keeps the tool available: both files + # are sent to the tool (and both fall back). + test_grep "pathname=one.c" backend.log && + test_grep "pathname=two.c" backend.log && + test_grep "return 10" actual && + test_grep "return 20" actual +' + +test_expect_success PYTHON 'diff process abort disables for session' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --mode=abort --log=backend.log" \ + diff -- one.c two.c >actual && + # Both files should still produce diff output via fallback. + test_grep "return 10" actual && + test_grep "return 20" actual && + # The tool aborts on the first file and git clears its + # capability. The second file never contacts the tool. + test_grep "pathname=one.c" backend.log && + test_grep ! "pathname=two.c" backend.log +' + +test_expect_success PYTHON 'diff process fallback on tool crash' ' + git -c diff.cdiff.process="$BACKEND --mode=crash" \ + diff boundary.c >actual 2>stderr && + test_grep "^-OLD5" actual && + test_grep "^+NEW5" actual && + test_grep "^-OLD9" actual && + test_grep "^+NEW9" actual && + # Crash is a communication failure, so a warning is emitted. + test_grep "diff process.*failed" stderr +' + +test_expect_success PYTHON 'diff process startup failure only warns once' ' + git -c diff.cdiff.process="/nonexistent/tool" \ + diff -- one.c two.c >actual 2>stderr && + # Both files produce diff output via fallback. + test_grep "return 10" actual && + test_grep "return 20" actual && + # Sentinel prevents repeated warnings: only one, not one per file. + test_grep "diff process.*failed" stderr >warnings && + test_line_count = 1 warnings +' + +test_expect_success PYTHON 'diff process fallback on bad hunks' ' + git -c diff.cdiff.process="$BACKEND --mode=bad-hunk" \ + diff boundary.c >actual 2>stderr && + test_grep "^-OLD5" actual && + test_grep "^+NEW5" actual && + test_grep "^-OLD9" actual && + test_grep "^+NEW9" actual && + # Invalid hunks are caught by xdiff validation, not the + # protocol layer, so no warning is emitted. + test_must_be_empty stderr +' + +test_expect_success PYTHON 'diff process fallback on mismatched unchanged totals' ' + cat >synctest.c <<-\EOF && + line1 + line2 + line3 + EOF + git add synctest.c && + git commit -m "add synctest.c" && + + cat >synctest.c <<-\EOF && + line1 + changed + line3 + EOF + + # bad-sync reports hunk 1 2 1 1: marks 2 old lines and 1 new + # line as changed, leaving 1 unchanged old vs 2 unchanged new. + # The synchronization invariant fails and git falls back. + git -c diff.cdiff.process="$BACKEND --mode=bad-sync" \ + diff synctest.c >actual 2>stderr && + test_grep "changed" actual +' + +test_expect_success PYTHON 'diff process fallback on overlapping hunks' ' + # boundary.c has 10 lines, so both hunks are in bounds + # but they overlap at lines 3-5, triggering the ordering check. + git -c diff.cdiff.process="$BACKEND --mode=overlap" \ + diff boundary.c >actual 2>stderr && + test_grep "NEW5" actual +' + +# +# Blame integration. +# + +test_expect_success PYTHON 'blame uses tool-provided hunks' ' + cat >blame-hunk.c <<-\EOF && + line1 + line2 + line3 + line4 + original5 + original6 + line7 + line8 + line9 + line10 + EOF + git add blame-hunk.c && + git commit -m "add blame-hunk.c" && + ORIG=$(git rev-parse --short HEAD) && + + cat >blame-hunk.c <<-\EOF && + line1 + line2 + line3 + line4 + changed5 + changed6 + line7 + line8 + changed9 + changed10 + EOF + git add blame-hunk.c && + git commit -m "change blame-hunk.c" && + CHANGE=$(git rev-parse --short HEAD) && + + # With fixed-hunk mode the tool reports only lines 5-6 as changed, + # so blame should attribute lines 9-10 to the original commit + # even though the builtin diff would show them as changed. + git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk" \ + blame blame-hunk.c >actual && + sed -n "9p" actual >line9 && + sed -n "10p" actual >line10 && + test_grep "$ORIG" line9 && + test_grep "$ORIG" line10 && + sed -n "5p" actual >line5 && + sed -n "6p" actual >line6 && + test_grep "$CHANGE" line5 && + test_grep "$CHANGE" line6 +' + +test_expect_success PYTHON 'blame skips commits with no hunks from diff process' ' + cat >blame.c <<-\EOF && + int main(void) + { + return 0; + } + EOF + git add blame.c && + git commit -m "add blame.c" && + ORIG_COMMIT=$(git rev-parse --short HEAD) && + + cat >blame.c <<-\EOF && + int main(void) + { + return 0; + } + EOF + git add blame.c && + git commit -m "reformat blame.c" && + BLAME_COMMIT=$(git rev-parse --short HEAD) && + + # Without no-hunks mode, blame attributes the change. + git blame blame.c >without && + test_grep "$BLAME_COMMIT" without && + + # With no-hunks mode, the process considers the files equivalent + # and blame skips the reformat commit, attributing to the original. + git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \ + blame blame.c >with && + test_grep ! "$BLAME_COMMIT" with && + test_grep "$ORIG_COMMIT" with +' + +test_expect_success PYTHON 'blame --no-ext-diff bypasses diff process' ' + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \ + blame --no-ext-diff blame.c >actual && + # Without the process, blame attributes the reformat commit normally. + test_grep "$BLAME_COMMIT" actual && + test_path_is_missing backend.log +' + +test_expect_success PYTHON 'blame --no-ext-diff uses builtin hunks' ' + # fixed-hunk mode would narrow blame to lines 5-6, but + # --no-ext-diff should bypass it and use the builtin diff. + rm -f backend.log && + git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk --log=backend.log" \ + blame --no-ext-diff blame-hunk.c >actual && + # Builtin diff attributes lines 9-10 to the change commit. + sed -n "9p" actual >line9 && + test_grep "$CHANGE" line9 && + test_path_is_missing backend.log +' + +test_done diff --git a/userdiff.c b/userdiff.c index fe710a68bfdfa6..81c0bebcce65e0 100644 --- a/userdiff.c +++ b/userdiff.c @@ -499,6 +499,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 827361b0bc9569..a98eabe3770cc6 100644 --- a/userdiff.h +++ b/userdiff.h @@ -3,6 +3,7 @@ #include "notes-cache.h" +struct diff_subprocess; struct index_state; struct repository; @@ -31,6 +32,10 @@ struct userdiff_driver { char *textconv_owned; struct notes_cache *textconv_cache; int textconv_want_cache; + const char *process; + char *process_owned; + struct diff_subprocess *diff_subprocess; + unsigned diff_process_failed : 1; }; enum userdiff_driver_type { USERDIFF_DRIVER_TYPE_BUILTIN = 1<<0, diff --git a/xdiff-interface.c b/xdiff-interface.c index f043330f2a12a0..9542c0bcc20f37 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -124,7 +124,12 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE) return -1; - if (!xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT)) + /* + * External hunks reference line numbers in the original content; + * trimming the tail would change line counts and invalidate them. + */ + if (!xpp->external_hunks && + !xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT)) trim_common_tail(&a, &b); return xdl_diff(&a, &b, xpp, xecfg, xecb); diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index dc370712e92860..a7e8502e4ca6ae 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -78,6 +78,15 @@ typedef struct s_mmbuffer { long size; } mmbuffer_t; +/* + * Hunk descriptor for externally computed diffs. + * Line numbers are 1-based, matching unified diff convention. + */ +struct xdl_hunk { + long old_start, old_count; + long new_start, new_count; +}; + typedef struct s_xpparam { unsigned long flags; @@ -88,6 +97,10 @@ typedef struct s_xpparam { /* See Documentation/diff-options.adoc. */ char **anchors; size_t anchors_nr; + + /* Externally computed hunks: bypass the diff algorithm. Owned by caller. */ + struct xdl_hunk *external_hunks; + size_t external_hunks_nr; } xpparam_t; typedef struct s_xdemitcb { diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 5455b4690d38ff..b0a01ca8568d18 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -1085,16 +1085,97 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe, } } +/* + * Populate the changed[] arrays from externally supplied hunks, + * bypassing the diff algorithm. Validates that hunks are in order, + * non-overlapping, and within bounds. + * + * Returns 0 on success, -1 on validation failure. + */ +static int xdl_populate_hunks_from_external(xdfenv_t *xe, + struct xdl_hunk *hunks, + size_t nr_hunks) +{ + size_t i; + long j, prev_old_end = 0, prev_new_end = 0; + long total_old = 0, total_new = 0; + + /* + * xdl_prepare_env() may dirty changed[] via xdl_cleanup_records(). + * Clear them so only the external hunks are marked. + */ + xdl_clear_changed(&xe->xdf1); + xdl_clear_changed(&xe->xdf2); + + for (i = 0; i < nr_hunks; i++) { + struct xdl_hunk *h = &hunks[i]; + + if (h->old_count < 0 || h->new_count < 0) + return -1; + if (h->old_start < 1 || h->new_start < 1) + return -1; + + /* + * Range must fit: start + count - 1 <= nrec, + * rewritten to avoid overflow. Same for both sides. + * + * When count is 0 (pure insert/delete) the check + * reduces to 0 > nrec - start + 1, which rejects + * start > nrec + 1 and allows start == nrec + 1 + * (the position after the last line). + */ + if (h->old_count > (long)xe->xdf1.nrec - h->old_start + 1) + return -1; + if (h->new_count > (long)xe->xdf2.nrec - h->new_start + 1) + return -1; + + /* Ordering: no overlap with previous hunk (adjacent is OK) */ + if (h->old_start < prev_old_end || + h->new_start < prev_new_end) + return -1; + + for (j = 0; j < h->old_count; j++) + xe->xdf1.changed[h->old_start - 1 + j] = true; + for (j = 0; j < h->new_count; j++) + xe->xdf2.changed[h->new_start - 1 + j] = true; + + prev_old_end = h->old_start + h->old_count; + prev_new_end = h->new_start + h->new_count; + total_old += h->old_count; + total_new += h->new_count; + } + + /* + * Synchronization invariant: unchanged line counts must match. + * Otherwise xdl_build_script() would walk off one array. + */ + if ((long)xe->xdf1.nrec - total_old != + (long)xe->xdf2.nrec - total_new) + return -1; + + return 0; +} + int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb) { xdchange_t *xscr; xdfenv_t xe; emit_func_t ef = xecfg->hunk_func ? xdl_call_hunk_func : xdl_emit_diff; - if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) { + if (xpp->external_hunks) { + if (xdl_prepare_env(mf1, mf2, xpp, &xe) < 0) + return -1; + if (xdl_populate_hunks_from_external(&xe, + xpp->external_hunks, + xpp->external_hunks_nr) == 0) + goto diff_done; + xdl_free_env(&xe); + } + if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) return -1; - } + +diff_done: if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 || xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 || xdl_build_script(&xe, &xscr) < 0) { diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index cd4fc405eb18fe..4645a9a74611aa 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -432,3 +432,13 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return 0; } + +/* + * Reset the changed[] array so that no lines are marked as changed. + * Also clears the sentinel slots at changed[-1] and changed[nrec] + * that xdl_change_compact() relies on during backward scans. + */ +void xdl_clear_changed(xdfile_t *xdf) +{ + memset(xdf->changed - 1, 0, (xdf->nrec + 2) * sizeof(bool)); +} diff --git a/xdiff/xprepare.h b/xdiff/xprepare.h index 947d9fc1bb8cf9..0413baf07bcc90 100644 --- a/xdiff/xprepare.h +++ b/xdiff/xprepare.h @@ -28,6 +28,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdfenv_t *xe); void xdl_free_env(xdfenv_t *xe); +void xdl_clear_changed(xdfile_t *xdf);