git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 2/3] builtin: introduce diff-pairs command

From
Karthik Nayak <karthik.188@gmail.com>
Date
Feb 27, 2025, 09:35 UTC
Message-ID
<CAOLa=ZS6yqquxAL_vCUf0FRrMbA6OLeY1VjqMv=pnxAo_17zLA@mail.gmail.com>
In-Reply-To
<20250225233925.1345086-3-jltobler@gmail.com>
Justin Tobler <jltobler@gmail.com> writes:
Show 16 quoted lines
> Through git-diff(1), a single diff can be generated from a pair of blob
> revisions directly. Unfortunately, there is not a mechanism to compute
> batches of specific file pair diffs in a single process. Such a feature
> is particularly useful on the server-side where diffing between a large
> set of changes is not feasible all at once due to timeout concerns.
>
> To facilitate this, introduce git-diff-pairs(1) which acts as a backend
> passing its NUL-terminated raw diff format input from stdin through diff
> machinery to produce various forms of output such as patch or raw.
>
> The raw format was originally designed as an interchange format and
> represents the contents of the diff_queue_diff list making it possible
> to break the diff pipeline into separate stages. For example,
> git-diff-tree(1) can be used as a frontend to compute file pairs to
> queue and feed its raw output to git-diff-pairs(1) to compute patches.
> With this, batches of diffs can be progessively generated without having
s/progessively/progressively
Show 12 quoted lines
> to recompute rename detection or retrieve object context. Something like
> the following:
>
> 	git diff-tree -r -z -M $old $new |
> 	git diff-pairs -p -z
>
> should generate the same output as `git diff-tree -p -M`. Furthermore,
> each line of raw diff formatted input can also be individually fed to a
> separate git-diff-pairs(1) process and still produce the same output.
>
> Based-on-patch-by: Jeff King <peff@peff.net>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
[snip]
Show 18 quoted lines
> diff --git a/builtin/diff-pairs.c b/builtin/diff-pairs.c
> new file mode 100644
> index 0000000000..9472b10461
> --- /dev/null
> +++ b/builtin/diff-pairs.c
> @@ -0,0 +1,193 @@
> +#include "builtin.h"
> +#include "commit.h"
> +#include "config.h"
> +#include "diff.h"
> +#include "diffcore.h"
> +#include "gettext.h"
> +#include "hex.h"
> +#include "object.h"
> +#include "parse-options.h"
> +#include "revision.h"
> +#include "strbuf.h"
> +
Nit: I could also compile without some of these headers, do we still
need them all?
    diff --git a/builtin/diff-pairs.c b/builtin/diff-pairs.c
    index 86e59a7e3a..1aea2ee726 100644
    --- a/builtin/diff-pairs.c
    +++ b/builtin/diff-pairs.c
    @@ -1,14 +1,9 @@
     #include "builtin.h"
    -#include "commit.h"
     #include "config.h"
    -#include "diff.h"
     #include "diffcore.h"
    -#include "gettext.h"
     #include "hex.h"
    -#include "object.h"
     #include "parse-options.h"
     #include "revision.h"
    -#include "strbuf.h"
     static unsigned parse_mode_or_die(const char *mode, const char **endp)
     {
Show 13 quoted lines
> +static unsigned parse_mode_or_die(const char *mode, const char **endp)
> +{
> +	uint16_t ret;
> +
> +	*endp = parse_mode(mode, &ret);
> +	if (!*endp)
> +		die(_("unable to parse mode: %s"), mode);
> +	return ret;
> +}
> +
> +static void parse_oid_or_die(const char *p, struct object_id *oid,
> +			     const char **endp, const struct git_hash_algo *algop)
>
Nit: without double checking, I couldn't tell what 'p' was, can we
rename the variables here to be consistent with `parse_oid_hex_algop()`?
Show 131 quoted lines
> +{
> +	if (parse_oid_hex_algop(p, oid, endp, algop) || *(*endp)++ != ' ')
> +		die(_("unable to parse object id: %s"), p);
> +}
> +
> +static void flush_diff_queue(struct diff_options *options)
> +{
> +	/*
> +	 * If rename detection is not requested, use rename information from the
> +	 * raw diff formatted input. Setting found_follow ensures diffcore_std()
> +	 * does not mess with rename information already present in queued
> +	 * filepairs.
> +	 */
> +	if (!options->detect_rename)
> +		options->found_follow = 1;
> +	diffcore_std(options);
> +	diff_flush(options);
> +}
> +
> +int cmd_diff_pairs(int argc, const char **argv, const char *prefix,
> +		   struct repository *repo)
> +{
> +	struct strbuf path_dst = STRBUF_INIT;
> +	struct strbuf path = STRBUF_INIT;
> +	struct strbuf meta = STRBUF_INIT;
> +	struct rev_info revs;
> +	int ret;
> +
> +	const char * const usage[] = {
> +		N_("git diff-pairs -z [<diff-options>]"),
> +		NULL
> +	};
> +	struct option options[] = {
> +		OPT_END()
> +	};
> +	struct option *parseopts = add_diff_options(options, &revs.diffopt);
> +
> +	show_usage_with_options_if_asked(argc, argv, usage, parseopts);
> +
> +	repo_init_revisions(repo, &revs, prefix);
> +	repo_config(repo, git_diff_basic_config, NULL);
> +	revs.disable_stdin = 1;
> +	revs.abbrev = 0;
> +	revs.diff = 1;
> +
> +	if (setup_revisions(argc, argv, &revs, NULL) > 1)
> +		usage_with_options(usage, parseopts);
> +
> +	/*
> +	 * With the -z option, both command input and raw output are
> +	 * NUL-delimited (this mode does not effect patch output). At present
> +	 * only NUL-delimited raw diff formatted input is supported.
> +	 */
> +	if (revs.diffopt.line_termination) {
> +		error(_("working without -z is not supported"));
> +		usage_with_options(usage, parseopts);
> +	}
> +
> +	if (revs.prune_data.nr) {
> +		error(_("pathspec arguments not supported"));
> +		usage_with_options(usage, parseopts);
> +	}
> +
> +	if (revs.pending.nr || revs.max_count != -1 ||
> +	    revs.min_age != (timestamp_t)-1 ||
> +	    revs.max_age != (timestamp_t)-1) {
> +		error(_("revision arguments not allowed"));
> +		usage_with_options(usage, parseopts);
> +	}
> +
> +	if (!revs.diffopt.output_format)
> +		revs.diffopt.output_format = DIFF_FORMAT_PATCH;
> +
> +	while (1) {
> +		struct object_id oid_a, oid_b;
> +		struct diff_filepair *pair;
> +		unsigned mode_a, mode_b;
> +		const char *p;
> +		char status;
> +
> +		if (strbuf_getline_nul(&meta, stdin) == EOF)
> +			break;
> +
> +		p = meta.buf;
> +		if (*p != ':')
> +			die(_("invalid raw diff input"));
> +		p++;
> +
> +		mode_a = parse_mode_or_die(p, &p);
> +		mode_b = parse_mode_or_die(p, &p);
> +
> +		if (S_ISDIR(mode_a) || S_ISDIR(mode_b))
> +			die(_("tree objects not supported"));
> +
> +		parse_oid_or_die(p, &oid_a, &p, repo->hash_algo);
> +		parse_oid_or_die(p, &oid_b, &p, repo->hash_algo);
> +
> +		status = *p++;
> +
> +		if (strbuf_getline_nul(&path, stdin) == EOF)
> +			die(_("got EOF while reading path"));
> +
> +		switch (status) {
> +		case DIFF_STATUS_ADDED:
> +			pair = diff_queue_addremove(&diff_queued_diff,
> +						    &revs.diffopt, '+', mode_b,
> +						    &oid_b, 1, path.buf, 0);
> +			if (pair)
> +				pair->status = status;
> +			break;
> +
> +		case DIFF_STATUS_DELETED:
> +			pair = diff_queue_addremove(&diff_queued_diff,
> +						    &revs.diffopt, '-', mode_a,
> +						    &oid_a, 1, path.buf, 0);
> +			if (pair)
> +				pair->status = status;
> +			break;
> +
> +		case DIFF_STATUS_TYPE_CHANGED:
> +		case DIFF_STATUS_MODIFIED:
> +			pair = diff_queue_change(&diff_queued_diff, &revs.diffopt,
> +						 mode_a, mode_b, &oid_a, &oid_b,
> +						 1, 1, path.buf, 0, 0);
> +			if (pair)
> +				pair->status = status;
> +			break;
> +
> +		case DIFF_STATUS_RENAMED:
> +		case DIFF_STATUS_COPIED:
> +			{

style: The general rule followed is to open the braces in the same line as the case statement. So `case DIFF_STATUS_COPIED: {`

Show 38 quoted lines
> +				struct diff_filespec *a, *b;
> +				unsigned int score;
> +
> +				if (strbuf_getline_nul(&path_dst, stdin) == EOF)
> +					die(_("got EOF while reading destination path"));
> +
> +				a = alloc_filespec(path.buf);
> +				b = alloc_filespec(path_dst.buf);
> +				fill_filespec(a, &oid_a, 1, mode_a);
> +				fill_filespec(b, &oid_b, 1, mode_b);
> +
> +				pair = diff_queue(&diff_queued_diff, a, b);
> +
> +				if (strtoul_ui(p, 10, &score))
> +					die(_("unable to parse rename/copy score: %s"), p);
> +
> +				pair->score = score * MAX_SCORE / 100;
> +				pair->status = status;
> +				pair->renamed_pair = 1;
> +			}
> +			break;
> +
> +		default:
> +			die(_("unknown diff status: %c"), status);
> +		}
> +	}
> +
> +	flush_diff_queue(&revs.diffopt);
> +	ret = diff_result_code(&revs);
> +
> +	strbuf_release(&path_dst);
> +	strbuf_release(&path);
> +	strbuf_release(&meta);
> +	release_revisions(&revs);
> +	FREE_AND_NULL(parseopts);
> +
> +	return ret;
> +}
[snip]
Previous: Justin ToblerNext: Justin Tobler
Message 55 of 78 in “batch blob diff generation”
  1. 0/3 batch blob diff generationJustin Tobler, Dec 13, 2024
  2. 1/3 builtin: introduce diff-blob commandJustin Tobler, Dec 13, 2024
  3. 2/3 builtin/diff-blob: add "--stdin" optionJustin Tobler, Dec 13, 2024
  4. 3/3 builtin/diff-blob: Add "-z" optionJustin Tobler, Dec 13, 2024
  5. Jeff KingDec 13, 2024
  6. Junio C HamanoDec 13, 2024
  7. Jeff KingDec 13, 2024
  8. Junio C HamanoDec 15, 2024
  9. Junio C HamanoDec 15, 2024
  10. Jeff KingDec 16, 2024
  11. Junio C HamanoDec 16, 2024
  12. Jeff KingDec 18, 2024
  13. Junio C HamanoDec 18, 2024
  14. Jeff KingDec 20, 2024
  15. Jeff KingDec 20, 2024
  16. Justin ToblerDec 13, 2024
  17. Jeff KingDec 16, 2024
  18. Junio C HamanoDec 13, 2024
  19. Junio C HamanoDec 15, 2024
  20. Jeff KingDec 16, 2024
  21. 0/3 batch blob diff generationJustin Tobler, Feb 12, 2025
  22. 1/3 diff: return diff_filepair from diff queue helpersJustin Tobler, Feb 12, 2025
  23. Karthik NayakFeb 12, 2025
  24. Justin ToblerFeb 12, 2025
  25. Patrick SteinhardtFeb 12, 2025
  26. Justin ToblerFeb 12, 2025
  27. Patrick SteinhardtFeb 13, 2025
  28. 2/3 builtin: introduce diff-pairs commandJustin Tobler, Feb 12, 2025
  29. Patrick SteinhardtFeb 12, 2025
  30. Karthik NayakFeb 12, 2025
  31. Justin ToblerFeb 25, 2025
  32. Jean-Noël AvilaFeb 12, 2025
  33. Junio C HamanoFeb 12, 2025
  34. Justin ToblerFeb 19, 2025
  35. Junio C HamanoFeb 19, 2025
  36. Junio C HamanoFeb 19, 2025
  37. Justin ToblerFeb 20, 2025
  38. Justin ToblerFeb 20, 2025
  39. Junio C HamanoFeb 20, 2025
  40. Phillip WoodFeb 17, 2025
  41. Justin ToblerFeb 19, 2025
  42. Junio C HamanoFeb 19, 2025
  43. Justin ToblerFeb 19, 2025
  44. Phillip WoodFeb 26, 2025
  45. 3/3 builtin/diff-pairs: allow explicit diff queue flushJustin Tobler, Feb 12, 2025
  46. Patrick SteinhardtFeb 12, 2025
  47. Phillip WoodFeb 17, 2025
  48. Justin ToblerFeb 19, 2025
  49. 0/3 batch blob diff generationJustin Tobler, Feb 25, 2025
  50. 1/3 diff: return diff_filepair from diff queue helpersJustin Tobler, Feb 25, 2025
  51. Junio C HamanoFeb 26, 2025
  52. 2/3 builtin: introduce diff-pairs commandJustin Tobler, Feb 25, 2025
  53. Junio C HamanoFeb 26, 2025
  54. Justin ToblerFeb 27, 2025
  55. Karthik NayakFeb 27, 2025
  56. Justin ToblerFeb 27, 2025
  57. Patrick SteinhardtFeb 27, 2025
  58. Justin ToblerFeb 27, 2025
  59. 3/3 builtin/diff-pairs: allow explicit diff queue flushJustin Tobler, Feb 25, 2025
  60. phillip.wood123@gmail.comFeb 26, 2025
  61. Justin ToblerFeb 27, 2025
  62. 0/4 batch blob diff generationJustin Tobler, Feb 28, 2025
  63. 1/4 diff: return diff_filepair from diff queue helpersJustin Tobler, Feb 28, 2025
  64. 2/4 diff: add option to skip resolving diff statusesJustin Tobler, Feb 28, 2025
  65. Patrick SteinhardtFeb 28, 2025
  66. Justin ToblerFeb 28, 2025
  67. 3/4 builtin: introduce diff-pairs commandJustin Tobler, Feb 28, 2025
  68. Patrick SteinhardtFeb 28, 2025
  69. Justin ToblerFeb 28, 2025
  70. 4/4 builtin/diff-pairs: allow explicit diff queue flushJustin Tobler, Feb 28, 2025
  71. 0/4 batch blob diff generationJustin Tobler, Feb 28, 2025
  72. 1/4 diff: return diff_filepair from diff queue helpersJustin Tobler, Feb 28, 2025
  73. Junio C HamanoMar 3, 2025
  74. 2/4 diff: add option to skip resolving diff statusesJustin Tobler, Feb 28, 2025
  75. Junio C HamanoMar 3, 2025
  76. 3/4 builtin: introduce diff-pairs commandJustin Tobler, Feb 28, 2025
  77. Junio C HamanoMar 3, 2025
  78. 4/4 builtin/diff-pairs: allow explicit diff queue flushJustin Tobler, Feb 28, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.