All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Michael Montalbo <mmontalbo@gmail.com>,
	Michael Montalbo <mmontalbo@gmail.com>
Subject: [PATCH v5 2/9] xdiff: support external hunks via xpparam_t
Date: Wed, 15 Jul 2026 21:01:55 +0000	[thread overview]
Message-ID: <20045025497f08f3859646de8506d8f702bfc75f.1784149323.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2120.v5.git.1784149323.gitgitgadget@gmail.com>

From: Michael Montalbo <mmontalbo@gmail.com>

Add two new xpparam_t fields (external_hunks, external_hunks_nr)
that let callers supply pre-computed hunks.  When set, xdl_diff()
populates the changed[] arrays from these hunks instead of running
the diff algorithm, then continues through compaction and emission
as usual.

Validate supplied hunks before use.  Out-of-bounds line numbers,
overlapping or out-of-order hunks, and misaligned unchanged runs are
treated as a malformed tool response: xdl_populate_hunks_from_external()
warns, returns -1, and xdl_diff() falls back to the builtin diff
algorithm for that file.  The run of unchanged lines between two hunks
(and before the first and after the last) must be the same length on
both sides; xdl_build_script() walks the two files in lockstep over
unchanged lines, so a balanced total is not enough.  Non-negative
counts and 1-based starts are instead caller preconditions, checked
with BUG(), since the caller normalizes hunks before this point.

On rejection xdl_diff() frees the environment it prepared and falls
through to xdl_do_diff(), which prepares a fresh one for the builtin
pass.

Skip trim_common_tail() in xdi_diff() when external hunks are
present, since external hunks reference line numbers in the
original content.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 xdiff-interface.c |  7 +++-
 xdiff/xdiff.h     | 16 +++++++++
 xdiff/xdiffi.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++--
 xdiff/xprepare.c  | 10 ++++++
 xdiff/xprepare.h  |  1 +
 5 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/xdiff-interface.c b/xdiff-interface.c
index db6938689f..1fa16af668 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -124,7 +124,12 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
 	if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE)
 		return -1;
 
-	if (!xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
+	/*
+	 * External hunks reference line numbers in the original content;
+	 * trimming the tail would change line counts and invalidate them.
+	 */
+	if (!xpp->external_hunks &&
+	    !xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
 		trim_common_tail(&a, &b);
 
 	return xdl_diff(&a, &b, xpp, xecfg, xecb);
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index dc370712e9..4736bcdb07 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -78,6 +78,18 @@ typedef struct s_mmbuffer {
 	long size;
 } mmbuffer_t;
 
+/*
+ * Hunk descriptor for externally computed diffs, in xdiff's own
+ * coordinates: line numbers are 1-based and a hunk's start is the
+ * first line it covers.  A caller translates any external "empty side"
+ * idiom (such as git diff's start-0/count-0) to a 1-based start before
+ * handing hunks over.
+ */
+struct xdl_hunk {
+	long old_start, old_count;
+	long new_start, new_count;
+};
+
 typedef struct s_xpparam {
 	unsigned long flags;
 
@@ -88,6 +100,10 @@ typedef struct s_xpparam {
 	/* See Documentation/diff-options.adoc. */
 	char **anchors;
 	size_t anchors_nr;
+
+	/* Externally computed hunks: bypass the diff algorithm.  Owned by caller. */
+	struct xdl_hunk *external_hunks;
+	size_t external_hunks_nr;
 } xpparam_t;
 
 typedef struct s_xdemitcb {
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index c5a892f91e..73a456f5dd 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -1085,16 +1085,96 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,
 	}
 }
 
+/*
+ * Populate the changed[] arrays from externally supplied hunks,
+ * bypassing the diff algorithm.  The caller normalizes and validates
+ * the hunks first (order, overlap, and lockstep alignment), so this
+ * only marks lines changed after asserting the memory-safety
+ * preconditions it depends on: non-negative counts and 1-based starts
+ * (checked with BUG()), and an in-bounds range (a silent -1 so the
+ * caller can fall back to the builtin diff rather than index changed[]
+ * out of range).  Keeping this diagnostic-free leaves user-facing
+ * messages to the git layer.
+ *
+ * Returns 0 on success, -1 if a hunk is out of range.
+ */
+static int xdl_populate_hunks_from_external(xdfenv_t *xe,
+					    struct xdl_hunk *hunks,
+					    size_t nr_hunks)
+{
+	size_t i;
+	long j;
+
+	/*
+	 * xdl_prepare_env() may dirty changed[] via xdl_cleanup_records().
+	 * Clear them so only the external hunks are marked.
+	 */
+	xdl_clear_changed(&xe->xdf1);
+	xdl_clear_changed(&xe->xdf2);
+
+	for (i = 0; i < nr_hunks; i++) {
+		struct xdl_hunk *h = &hunks[i];
+
+		/*
+		 * Non-negative counts and 1-based starts are caller
+		 * preconditions (it normalizes hunks into xdiff coordinates
+		 * before this point), so a violation is a bug, not a bad
+		 * tool response.
+		 */
+		if (h->old_count < 0 || h->new_count < 0)
+			BUG("external hunk %"PRIuMAX": "
+				"negative count (old=%ld, new=%ld)",
+				(uintmax_t)(i + 1),
+				h->old_count, h->new_count);
+		if (h->old_start < 1 || h->new_start < 1)
+			BUG("external hunk %"PRIuMAX": "
+				"start not 1-based (old=%ld, new=%ld)",
+				(uintmax_t)(i + 1),
+				h->old_start, h->new_start);
+
+		/*
+		 * The caller validates ordering, overlap and lockstep
+		 * alignment (and diagnoses a bad response).  This is only a
+		 * silent in-bounds guard so the marking loop cannot index
+		 * changed[] out of range: start + count - 1 <= nrec,
+		 * rewritten to avoid overflow.  A count of 0 (pure
+		 * insert/delete) allows start == nrec + 1, the position
+		 * after the last line.  On a miss, return -1 and let the
+		 * caller fall back to the builtin diff.
+		 */
+		if (h->old_count > (long)xe->xdf1.nrec - h->old_start + 1 ||
+		    h->new_count > (long)xe->xdf2.nrec - h->new_start + 1)
+			return -1;
+
+		for (j = 0; j < h->old_count; j++)
+			xe->xdf1.changed[h->old_start - 1 + j] = true;
+		for (j = 0; j < h->new_count; j++)
+			xe->xdf2.changed[h->new_start - 1 + j] = true;
+	}
+
+	return 0;
+}
+
 int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 	     xdemitconf_t const *xecfg, xdemitcb_t *ecb) {
 	xdchange_t *xscr;
 	xdfenv_t xe;
 	emit_func_t ef = xecfg->hunk_func ? xdl_call_hunk_func : xdl_emit_diff;
 
-	if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) {
+	if (xpp->external_hunks) {
+		if (xdl_prepare_env(mf1, mf2, xpp, &xe) < 0)
+			return -1;
+		if (xdl_populate_hunks_from_external(&xe,
+						     xpp->external_hunks,
+						     xpp->external_hunks_nr) == 0)
+			goto diff_done;
+		xdl_free_env(&xe);
+	}
 
+	if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0)
 		return -1;
-	}
+
+diff_done:
 	if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
 	    xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 ||
 	    xdl_build_script(&xe, &xscr) < 0) {
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 11bada2608..f4ab935332 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -471,3 +471,13 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 
 	return 0;
 }
+
+/*
+ * Reset the changed[] array so that no lines are marked as changed.
+ * Also clears the sentinel slots at changed[-1] and changed[nrec]
+ * that xdl_change_compact() relies on during backward scans.
+ */
+void xdl_clear_changed(xdfile_t *xdf)
+{
+	memset(xdf->changed - 1, 0, (xdf->nrec + 2) * sizeof(bool));
+}
diff --git a/xdiff/xprepare.h b/xdiff/xprepare.h
index 947d9fc1bb..0413baf07b 100644
--- a/xdiff/xprepare.h
+++ b/xdiff/xprepare.h
@@ -28,6 +28,7 @@
 int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 		    xdfenv_t *xe);
 void xdl_free_env(xdfenv_t *xe);
+void xdl_clear_changed(xdfile_t *xdf);
 
 
 
-- 
gitgitgadget


  parent reply	other threads:[~2026-07-15 21:02 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-22  2:11 [PATCH 0/5] [RFC] diff: add diff.<driver>.process for external hunk providers Michael Montalbo via GitGitGadget
2026-05-22  2:11 ` [PATCH 1/5] xdiff: support external hunks via xpparam_t Michael Montalbo via GitGitGadget
2026-05-22  5:29   ` Junio C Hamano
2026-05-22 19:06     ` Michael Montalbo
2026-05-24  8:50       ` Junio C Hamano
2026-05-24 18:01         ` Michael Montalbo
2026-05-22  2:11 ` [PATCH 2/5] userdiff: add diff.<driver>.process config Michael Montalbo via GitGitGadget
2026-05-22  2:11 ` [PATCH 3/5] diff: add long-running diff process via diff.<driver>.process Michael Montalbo via GitGitGadget
2026-05-22  2:11 ` [PATCH 4/5] blame: consult diff process for zero-hunk detection Michael Montalbo via GitGitGadget
2026-05-22  2:11 ` [PATCH 5/5] diff-process-normalize: add built-in whitespace normalizer Michael Montalbo via GitGitGadget
2026-05-22  5:29 ` [PATCH 0/5] [RFC] diff: add diff.<driver>.process for external hunk providers Junio C Hamano
2026-05-22 17:19   ` Michael Montalbo
2026-05-25 18:29 ` [PATCH v2 0/4] " Michael Montalbo via GitGitGadget
2026-05-25 18:29   ` [PATCH v2 1/4] xdiff: support external hunks via xpparam_t Michael Montalbo via GitGitGadget
2026-05-25 18:29   ` [PATCH v2 2/4] userdiff: add diff.<driver>.process config Michael Montalbo via GitGitGadget
2026-05-25 18:29   ` [PATCH v2 3/4] diff: add long-running diff process via diff.<driver>.process Michael Montalbo via GitGitGadget
2026-05-26  1:56     ` Junio C Hamano
2026-05-29  0:51       ` Michael Montalbo
2026-05-26  2:26     ` Junio C Hamano
2026-05-29  0:55       ` Michael Montalbo
2026-05-25 18:29   ` [PATCH v2 4/4] blame: consult diff process for zero-hunk detection Michael Montalbo via GitGitGadget
2026-05-29 20:48   ` [PATCH v3 0/6] [RFC] diff: add diff.<driver>.process for external hunk providers Michael Montalbo via GitGitGadget
2026-05-29 20:48     ` [PATCH v3 1/6] xdiff: support external hunks via xpparam_t Michael Montalbo via GitGitGadget
2026-05-29 20:48     ` [PATCH v3 2/6] userdiff: add diff.<driver>.process config Michael Montalbo via GitGitGadget
2026-05-29 20:48     ` [PATCH v3 3/6] sub-process: separate process lifecycle from hashmap management Michael Montalbo via GitGitGadget
2026-05-29 20:48     ` [PATCH v3 4/6] diff: add long-running diff process via diff.<driver>.process Michael Montalbo via GitGitGadget
2026-06-07 14:36       ` Johannes Schindelin
2026-06-07 17:04         ` Michael Montalbo
2026-06-08 12:26           ` Junio C Hamano
2026-06-07 20:36         ` Michael Montalbo
2026-06-08 17:19           ` Junio C Hamano
2026-06-08 12:06         ` Junio C Hamano
2026-05-29 20:48     ` [PATCH v3 5/6] diff: bypass diff process with --no-ext-diff and in format-patch Michael Montalbo via GitGitGadget
2026-05-29 20:48     ` [PATCH v3 6/6] blame: consult diff process for no-hunk detection Michael Montalbo via GitGitGadget
2026-05-31 10:44     ` [PATCH v3 0/6] [RFC] diff: add diff.<driver>.process for external hunk providers Junio C Hamano
2026-06-01  4:28       ` Michael Montalbo
2026-06-14 18:59     ` [PATCH v4 " Michael Montalbo via GitGitGadget
2026-06-14 18:59       ` [PATCH v4 1/6] xdiff: support external hunks via xpparam_t Michael Montalbo via GitGitGadget
2026-06-14 18:59       ` [PATCH v4 2/6] userdiff: add diff.<driver>.process config Michael Montalbo via GitGitGadget
2026-06-14 18:59       ` [PATCH v4 3/6] sub-process: separate process lifecycle from hashmap management Michael Montalbo via GitGitGadget
2026-06-14 18:59       ` [PATCH v4 4/6] diff: add long-running diff process via diff.<driver>.process Michael Montalbo via GitGitGadget
2026-06-14 18:59       ` [PATCH v4 5/6] diff: bypass diff process with --no-ext-diff and in format-patch Michael Montalbo via GitGitGadget
2026-06-14 18:59       ` [PATCH v4 6/6] blame: consult diff process for no-hunk detection Michael Montalbo via GitGitGadget
2026-07-15 21:01       ` [PATCH v5 0/9] [RFC] diff: add diff.<driver>.process for external hunk providers Michael Montalbo via GitGitGadget
2026-07-15 21:01         ` [PATCH v5 1/9] gitattributes: document how external diff drivers relate to diff features Michael Montalbo via GitGitGadget
2026-07-15 21:01         ` Michael Montalbo via GitGitGadget [this message]
2026-07-15 21:01         ` [PATCH v5 3/9] userdiff: add diff.<driver>.process config Michael Montalbo via GitGitGadget
2026-07-15 21:01         ` [PATCH v5 4/9] sub-process: separate process lifecycle from hashmap management Michael Montalbo via GitGitGadget
2026-07-15 21:01         ` [PATCH v5 5/9] diff: add long-running diff process via diff.<driver>.process Michael Montalbo via GitGitGadget
2026-07-15 21:01         ` [PATCH v5 6/9] diff: bypass diff process with --no-ext-diff and in format-patch Michael Montalbo via GitGitGadget
2026-07-15 21:02         ` [PATCH v5 7/9] blame: consult diff process for no-hunk detection Michael Montalbo via GitGitGadget
2026-07-15 21:02         ` [PATCH v5 8/9] diff: consult diff process for --stat counts Michael Montalbo via GitGitGadget
2026-07-15 21:02         ` [PATCH v5 9/9] line-log: consult diff process for range tracking Michael Montalbo via GitGitGadget
     [not found]     ` <pull.2120.v4.git.1781463332.gitgitgadget@gmail.com>
2026-06-15 21:14       ` [PREVIEW v4 0/6] [RFC] diff: add diff.<driver>.process for external hunk providers Michael Montalbo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20045025497f08f3859646de8506d8f702bfc75f.1784149323.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=mmontalbo@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.