{"thread":{"id":"44587","subject":"[RFC/PATCH] add diff-pairs tool","startedAt":"2016-12-01T20:40:50Z","lastAt":"2016-12-01T22:30:22Z","messageCount":8,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"306748","messageId":"20161201204042.6yslbyrg7l6ghhww@sigill.intra.peff.net","threadId":"44587","inReplyTo":null,"subject":"[RFC/PATCH] add diff-pairs tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-01T20:40:43Z","receivedAt":"2016-12-01T20:40:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is a small tool I cooked up for splitting the tree-diff out from\nthe generation of the patch text, and doing them in separate processes.\nIt's a little more expensive than doing it all in one process (besides\nthe process/pipe overhead, we may load blob content in the tree diff for\nrename detection, and then again to do the actual patch). But it gives a\nlot of flexibility when handling large diffs. You can save the tree-diff\nand then feed it back in chunks to generate the actual patches, and do\nso across many invocations (if you had, say, a website that showed diffs\nand wanted to show small bits of them and let the user progressively\nclick through to see more).\n\nOther things I tried but didn't work:\n\n  - feeding two blobs to git-diff doesn't use plumbing, and loses\n    information about filenames, etc. It also requires one process per\n    pair.\n\n  - feeding pathspecs back to git-diff-tree, like:\n\n       git diff-tree --name-only $a $b |\n       while read filename; do\n         git diff-tree $a $b -- $filename\n       done\n\n    mostly works, but loses rename information (and is also slightly\n    less efficient, as it has to walk the trees again).\n\nSo instead, I made a tool that takes the pairs generated by \"diff-tree\n--raw\" and formats them as diff-tree would have if it had been done\nin-process.\n\nIt's rather elegant, I think. But the main reason this is RFC is that\nI'm not sure it's actually _useful_ to other people. It's a pretty\nnarrow use case.\n\nThe other reason it's an RFC is that it has a few rough edges. It works\nwell for my narrow use case, but there are cases where it probably\ndoesn't:\n\n  - it expects \"diff-tree -z\" input, which keeps parsing simple. It\n    probably wouldn't be too hard to have it handle the normal output,\n    too.\n\n  - it should handle submodules OK, but I don't know what it would do if\n    you fed it things like dirty submodules from diff-files output.\n\n  - some option combinations between the initial tree-diff and the\n    followup diff do not make sense. For instance, if you do a\n    non-recursive tree-diff (or one with \"-t\"), you'll have tree entries\n    in your --raw output. Feeding that to \"diff-pairs -p\" will silently\n    ignore the tree entries, because there's no way to represent them as\n    a patch.\n\n    Mostly these fall under \"well, don't do that\", but possibly the tool\n    could be more aggressive about complaining, or coming up with some\n    sensible behavior.\n\nSo anyway. Consider this a rough cut. What I'm really looking for at\nthis stage is whether anybody is interested enough in this for me to\nbother polishing it.\n\n-- >8 --\nThis takes the output of `diff-tree -z --raw` and feeds it\nback to the later stages of the diff machinery to produce\ndiffs in other formats. Because the interim format contains\nany whole-tree copy/rename information, you can safely feed\nsegments of the tree diff to get progressive patch-format\ndiffs. So something like:\n\n  git diff-tree -r -z $a $b |\n  git diff-pairs -p\n\nshould give you the same output that `git diff-tree -p`\nwould have.  Likewise, feeding each pair individually works,\ntoo:\n\n  git diff-tree -r -z -M $a $b |\n  perl -0ne '\n\tmy $meta = $_;\n\tmy $path = <>;\n\t# only renames have an extra path\n\tmy $path2 = <> if $meta =~ /[RC]\\d+/;\n\n\tprint STDERR \"feeding one diff\\n\";\n\topen(my $fh, \"|git diff-pairs -p\");\n\tprint $fh $meta, $path, $path2;\n  '\n\nThe renames will still be shown just as if the diff had been\ndone in one process.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n .gitignore                       |   1 +\n Documentation/git-diff-pairs.txt |  60 +++++++++++++++\n Makefile                         |   1 +\n builtin.h                        |   1 +\n builtin/diff-pairs.c             | 162 +++++++++++++++++++++++++++++++++++++++\n git.c                            |   1 +\n t/t4063-diff-pairs.sh            |  82 ++++++++++++++++++++\n 7 files changed, 308 insertions(+)\n create mode 100644 Documentation/git-diff-pairs.txt\n create mode 100644 builtin/diff-pairs.c\n create mode 100755 t/t4063-diff-pairs.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 05cb58a3d..805751f2e 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -48,6 +48,7 @@\n /git-diff\n /git-diff-files\n /git-diff-index\n+/git-diff-pairs\n /git-diff-tree\n /git-difftool\n /git-difftool--helper\ndiff --git a/Documentation/git-diff-pairs.txt b/Documentation/git-diff-pairs.txt\nnew file mode 100644\nindex 000000000..4e3b6c909\n--- /dev/null\n+++ b/Documentation/git-diff-pairs.txt\n@@ -0,0 +1,60 @@\n+git-diff-pairs(1)\n+=================\n+\n+NAME\n+----\n+git-diff-pairs - Compare blob pairs generated by `diff-tree --raw`\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git diff-pairs' [diff_format_options]\n+\n+DESCRIPTION\n+-----------\n+\n+Given the output of `diff-tree -z` on its stdin, `diff-pairs` will\n+reformat that output into whatever format is requested on its command\n+line.  For example:\n+\n+-----------------------------\n+git diff-tree -z -M $a $b |\n+git diff-pairs -p\n+-----------------------------\n+\n+will compute the tree diff in one step (including renames), and then\n+`diff-pairs` will compute and format the blob-level diffs for each pair.\n+This can be used to modify the raw diff in the middle (without having to\n+parse or re-create more complicated formats like `--patch`), or to\n+compute diffs progressively over the course of multiple invocations of\n+`diff-pairs`.\n+\n+Each blob pair is fed to the diff machinery individually and the output\n+flushed immediately, meaning it is safe to interactively read and write\n+from `diff-pairs`.\n+\n+OPTIONS\n+-------\n+\n+All diff options below are accepted, but note that tree-wide options\n+like `-M` are effectively noops, as we consider only one pair at a time.\n+\n+include::diff-options.txt[]\n+\n+BUGS\n+----\n+\n+`diff-pairs` should handle any input generated by `diff-tree --raw -z`.\n+It may choke or otherwise misbehave on output from `diff-files`, etc.\n+\n+Here's an incomplete list of things that `diff-pairs` could do, but\n+doesn't (mostly in the name of simplicity):\n+\n+ - Only `-z` input is accepted, not normal `--raw` input.\n+\n+ - Abbreviated sha1s are rejected in the input from `diff-tree`; if you\n+   want to abbreviate the output, you can pass `--abbrev` to\n+   `diff-pairs`.\n+\n+ - Pathspecs are not handled by `diff-pairs`; you can limit the diff via\n+   the initial `diff-tree` invocation.\ndiff --git a/Makefile b/Makefile\nindex f53fcc90d..c7ee3cd61 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -886,6 +886,7 @@ BUILTIN_OBJS += builtin/credential.o\n BUILTIN_OBJS += builtin/describe.o\n BUILTIN_OBJS += builtin/diff-files.o\n BUILTIN_OBJS += builtin/diff-index.o\n+BUILTIN_OBJS += builtin/diff-pairs.o\n BUILTIN_OBJS += builtin/diff-tree.o\n BUILTIN_OBJS += builtin/diff.o\n BUILTIN_OBJS += builtin/fast-export.o\ndiff --git a/builtin.h b/builtin.h\nindex b9122bc5f..f62134ec1 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -59,6 +59,7 @@ extern int cmd_describe(int argc, const char **argv, const char *prefix);\n extern int cmd_diff_files(int argc, const char **argv, const char *prefix);\n extern int cmd_diff_index(int argc, const char **argv, const char *prefix);\n extern int cmd_diff(int argc, const char **argv, const char *prefix);\n+extern int cmd_diff_pairs(int argc, const char **argv, const char *prefix);\n extern int cmd_diff_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_fast_export(int argc, const char **argv, const char *prefix);\n extern int cmd_fetch(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/diff-pairs.c b/builtin/diff-pairs.c\nnew file mode 100644\nindex 000000000..f80c49fef\n--- /dev/null\n+++ b/builtin/diff-pairs.c\n@@ -0,0 +1,162 @@\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+#include \"revision.h\"\n+\n+static const char diff_pairs_usage[] =\n+\"git diff-pairs [diff-options]\\n\"\n+\"\\n\"\n+\"Reads pairs of blobs from stdin in 'diff-tree -z' syntax:\\n\"\n+\"\\n\"\n+\"  :<mode_a> <mode_b> <sha1_a> <sha1_b> <type>\\\\0<path>\\0[path2\\0]\\n\"\n+\"\\n\"\n+\"and outputs the diff for each a/b pair to stdout.\";\n+\n+static unsigned parse_mode(const char *mode, char **endp)\n+{\n+\tunsigned long ret;\n+\n+\terrno = 0;\n+\tret = strtoul(mode, endp, 8);\n+\tif (errno || *endp == mode || *(*endp)++ != ' ' || (unsigned)ret != ret)\n+\t\tdie(\"unable to parse mode: %s\", mode);\n+\treturn ret;\n+}\n+\n+static void parse_oid(char *p, char **endp, struct object_id *oid)\n+{\n+\t*endp = p + GIT_SHA1_HEXSZ;\n+\tif (get_oid_hex(p, oid) || *(*endp)++ != ' ')\n+\t\tdie(\"unable to parse object id: %s\", p);\n+}\n+\n+static unsigned short parse_score(char *score)\n+{\n+\tunsigned long ret;\n+\tchar *endp;\n+\n+\terrno = 0;\n+\tret = strtoul(score, &endp, 10);\n+\tret *= MAX_SCORE / 100;\n+\tif (errno || endp == score || *endp || (unsigned short)ret != ret)\n+\t\tdie(\"unable to parse rename/copy score: %s\", score);\n+\treturn ret;\n+}\n+\n+/*\n+ * The pair-creation is mostly done by diff_change and diff_addremove,\n+ * which queue the filepair without returning it. So we have to resort\n+ * to pulling it out of the global diff queue.\n+ */\n+static void set_pair_status(char status)\n+{\n+\t/*\n+\t * If we have no items in the queue, for some reason the pair wasn't\n+\t * worth queueing. This generally shouldn't happen (since it means\n+\t * dropping some parts of the diff), but the user can trigger it with\n+\t * things like --ignore-submodules. If they do, the only sensible thing\n+\t * is for us to play along and skip it.\n+\t */\n+\tif (!diff_queued_diff.nr)\n+\t\treturn;\n+\n+\tdiff_queued_diff.queue[0]->status = status;\n+}\n+\n+int cmd_diff_pairs(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct rev_info revs;\n+\tstruct strbuf meta = STRBUF_INIT;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf path_dst = STRBUF_INIT;\n+\n+\tinit_revisions(&revs, prefix);\n+\tgit_config(git_diff_basic_config, NULL);\n+\trevs.disable_stdin = 1;\n+\targc = setup_revisions(argc, argv, &revs, NULL);\n+\n+\t/* Don't allow pathspecs at all. */\n+\tif (argc > 1)\n+\t\tusage(diff_pairs_usage);\n+\n+\tif (!revs.diffopt.output_format)\n+\t\trevs.diffopt.output_format = DIFF_FORMAT_RAW;\n+\n+\twhile (1) {\n+\t\tunsigned mode_a, mode_b;\n+\t\tstruct object_id oid_a, oid_b;\n+\t\tchar status;\n+\t\tchar *p;\n+\n+\t\tif (strbuf_getline_nul(&meta, stdin) == EOF)\n+\t\t\tbreak;\n+\n+\t\tp = meta.buf;\n+\t\tif (*p == ':')\n+\t\t\tp++;\n+\n+\t\tmode_a = parse_mode(p, &p);\n+\t\tmode_b = parse_mode(p, &p);\n+\n+\t\tparse_oid(p, &p, &oid_a);\n+\t\tparse_oid(p, &p, &oid_b);\n+\n+\t\tstatus = *p++;\n+\n+\t\tif (strbuf_getline_nul(&path, stdin) == EOF)\n+\t\t\tdie(\"got EOF while reading path\");\n+\n+\t\tswitch (status) {\n+\t\tcase DIFF_STATUS_ADDED:\n+\t\t\tdiff_addremove(&revs.diffopt, '+',\n+\t\t\t\t       mode_b, oid_b.hash,\n+\t\t\t\t       1, path.buf, 0);\n+\t\t\tset_pair_status(status);\n+\t\t\tbreak;\n+\n+\t\tcase DIFF_STATUS_DELETED:\n+\t\t\tdiff_addremove(&revs.diffopt, '-',\n+\t\t\t\t       mode_a, oid_a.hash,\n+\t\t\t\t       1, path.buf, 0);\n+\t\t\tset_pair_status(status);\n+\t\t\tbreak;\n+\n+\t\tcase DIFF_STATUS_TYPE_CHANGED:\n+\t\tcase DIFF_STATUS_MODIFIED:\n+\t\t\tdiff_change(&revs.diffopt,\n+\t\t\t\t    mode_a, mode_b,\n+\t\t\t\t    oid_a.hash, oid_b.hash,\n+\t\t\t\t    1, 1, path.buf, 0, 0);\n+\t\t\tset_pair_status(status);\n+\t\t\tbreak;\n+\n+\t\tcase DIFF_STATUS_RENAMED:\n+\t\tcase DIFF_STATUS_COPIED:\n+\t\t\t{\n+\t\t\t\tstruct diff_filespec *a, *b;\n+\t\t\t\tstruct diff_filepair *pair;\n+\n+\t\t\t\tif (strbuf_getline_nul(&path_dst, stdin) == EOF)\n+\t\t\t\t\tdie(\"got EOF while reading secondary path\");\n+\n+\t\t\t\ta = alloc_filespec(path.buf);\n+\t\t\t\tb = alloc_filespec(path_dst.buf);\n+\t\t\t\tfill_filespec(a, oid_a.hash, 1, mode_a);\n+\t\t\t\tfill_filespec(b, oid_b.hash, 1, mode_b);\n+\n+\t\t\t\tpair = diff_queue(&diff_queued_diff, a, b);\n+\t\t\t\tpair->status = status;\n+\t\t\t\tpair->score = parse_score(p);\n+\t\t\t\tpair->renamed_pair = 1;\n+\t\t\t}\n+\t\t\tbreak;\n+\n+\t\tdefault:\n+\t\t\tdie(\"unknown diff status: %c\", status);\n+\t\t}\n+\n+\t\tdiff_flush(&revs.diffopt);\n+\t}\n+\n+\treturn 0;\n+}\ndiff --git a/git.c b/git.c\nindex dce529fcb..4063c5366 100644\n--- a/git.c\n+++ b/git.c\n@@ -423,6 +423,7 @@ static struct cmd_struct commands[] = {\n \t{ \"diff\", cmd_diff },\n \t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"diff-index\", cmd_diff_index, RUN_SETUP },\n+\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP },\n \t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP },\n \t{ \"fast-export\", cmd_fast_export, RUN_SETUP },\n \t{ \"fetch\", cmd_fetch, RUN_SETUP },\ndiff --git a/t/t4063-diff-pairs.sh b/t/t4063-diff-pairs.sh\nnew file mode 100755\nindex 000000000..a99ec6655\n--- /dev/null\n+++ b/t/t4063-diff-pairs.sh\n@@ -0,0 +1,82 @@\n+#!/bin/sh\n+\n+test_description='basic diff-pairs tests'\n+. ./test-lib.sh\n+\n+# This creates a diff with added, modified, deleted, renamed, copied, and\n+# typechange entries. That includes one in a subdirectory for non-recursive\n+# tests, and both exact and inexact similarity scores.\n+test_expect_success 'create commit with various diffs' '\n+\techo to-be-gone >deleted &&\n+\techo original >modified &&\n+\techo now-a-file >symlink &&\n+\ttest_seq 200 >two-hundred &&\n+\ttest_seq 201 500 >five-hundred &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m base &&\n+\tgit tag base &&\n+\n+\techo now-here >added &&\n+\techo new >modified &&\n+\trm deleted &&\n+\tmkdir subdir &&\n+\techo content >subdir/file &&\n+\tmv two-hundred renamed &&\n+\ttest_seq 201 500 | sed s/300/modified/ >copied &&\n+\trm symlink &&\n+\tgit add -A . &&\n+\ttest_ln_s_add dest symlink &&\n+\ttest_tick &&\n+\tgit commit -m new &&\n+\tgit tag new\n+'\n+\n+test_expect_success 'diff-pairs recreates --raw' '\n+\tgit diff-tree -r -M -C -C base new >expect &&\n+\t# note that diff-pairs uses the default abbrev,\n+\t# so we must tweak that for identical output\n+\tgit diff-tree -r -M -C -C -z base new |\n+\tgit diff-pairs --no-abbrev >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff-pairs can create -p output' '\n+\tgit diff-tree -p -M -C -C base new >expect &&\n+\tgit diff-tree -r -M -C -C -z base new |\n+\tgit diff-pairs -p >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'non-recursive --raw retains tree entry' '\n+\tgit diff-tree base new >expect &&\n+\tgit diff-tree -z base new |\n+\tgit diff-pairs --no-abbrev >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'split input across multiple diff-pairs' '\n+\twrite_script split-raw-diff \"$PERL_PATH\" <<-\\EOF &&\n+\t$/ = \"\\0\";\n+\twhile (<>) {\n+\t  my $meta = $_;\n+\t  my $path = <>;\n+\t  # renames have an extra path\n+\t  my $path2 = <> if $meta =~ /[RC]\\d+/;\n+\n+\t  open(my $fh, \">\", sprintf \"diff%03d\", $.);\n+\t  print $fh $meta, $path, $path2;\n+\t}\n+\tEOF\n+\n+\tgit diff-tree -p -M -C -C base new >expect &&\n+\n+\tgit diff-tree -r -z -M -C -C base new |\n+\t./split-raw-diff &&\n+\tfor i in diff*; do\n+\t\tgit diff-pairs -p <$i\n+\tdone >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.11.0.319.g1f4e1e0\n"},{"id":"306751","messageId":"20161201205504.flgaf7dwv3b3dkkd@sigill.intra.peff.net","threadId":"44587","inReplyTo":"xmqqbmwv8j9m.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-01T20:55:04Z","receivedAt":"2016-12-01T20:55:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 01, 2016 at 12:52:05PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > This takes the output of `diff-tree -z --raw` and feeds it\n> > back to the later stages of the diff machinery to produce\n> > diffs in other formats.\n> \n> A full circle.  This reminds me of the experiment done more than 10\n> years ago at the beginning of the \"diffcore transformations\" design.\n\nHeh, I didn't even think to dig for prior art on the list.\n\nIn a sense this is just bringing the full power of diffcore out to the\nscripting interface. The one missing component is that you can't\nactually call diffcore_std() in the middle. The full pipeline would I\nguess be something more like:\n\n  git diff-tree --raw -z $a $b |\n  git detect-renames |\n  git diff-pairs -p\n\nor something. In my model it's sufficient for the rename detection to\nhappen as part of the first diff-tree (since it's a whole-tree operation\nanyway, there's no benefit to breaking it up into chunks).\n\n-Peff\n"},{"id":"306752","messageId":"xmqqbmwv8j9m.fsf@gitster.mtv.corp.google.com","threadId":"44587","inReplyTo":"20161201204042.6yslbyrg7l6ghhww@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-01T20:52:05Z","receivedAt":"2016-12-01T20:57:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This takes the output of `diff-tree -z --raw` and feeds it\n> back to the later stages of the diff machinery to produce\n> diffs in other formats.\n\nA full circle.  This reminds me of the experiment done more than 10\nyears ago at the beginning of the \"diffcore transformations\" design.\n\n"},{"id":"306753","messageId":"xmqq7f7j8iz6.fsf@gitster.mtv.corp.google.com","threadId":"44587","inReplyTo":"20161201205504.flgaf7dwv3b3dkkd@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-01T20:58:21Z","receivedAt":"2016-12-01T20:58:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Dec 01, 2016 at 12:52:05PM -0800, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > This takes the output of `diff-tree -z --raw` and feeds it\n>> > back to the later stages of the diff machinery to produce\n>> > diffs in other formats.\n>> \n>> A full circle.  This reminds me of the experiment done more than 10\n>> years ago at the beginning of the \"diffcore transformations\" design.\n>\n> Heh, I didn't even think to dig for prior art on the list.\n\nIt took me a while to dig it up because the topic is so old, but\n\nhttps://public-inbox.org/git/Pine.LNX.4.58.0504251832480.18901@ppc970.osdl.org/\n\nis the thread I had in mind.  The idea of rename detection followed\nsoon afterwards.\n"},{"id":"306755","messageId":"xmqq37i78ip1.fsf@gitster.mtv.corp.google.com","threadId":"44587","inReplyTo":"xmqq7f7j8iz6.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-01T21:04:26Z","receivedAt":"2016-12-01T21:04:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> It took me a while to dig it up because the topic is so old, but\n>\n> https://public-inbox.org/git/Pine.LNX.4.58.0504251832480.18901@ppc970.osdl.org/\n>\n> is the thread I had in mind.  The idea of rename detection followed\n> soon afterwards.\n\n... which was this one:\n\nhttps://public-inbox.org/git/7vr7g4m0lz.fsf_-_@assigned-by-dhcp.cox.net/#t\n\n"},{"id":"306757","messageId":"20161201213019.qfkqd324ommikym2@sigill.intra.peff.net","threadId":"44587","inReplyTo":"xmqq7f7j8iz6.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-01T21:30:20Z","receivedAt":"2016-12-01T21:30:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 01, 2016 at 12:58:21PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Thu, Dec 01, 2016 at 12:52:05PM -0800, Junio C Hamano wrote:\n> >\n> >> Jeff King <peff@peff.net> writes:\n> >> \n> >> > This takes the output of `diff-tree -z --raw` and feeds it\n> >> > back to the later stages of the diff machinery to produce\n> >> > diffs in other formats.\n> >> \n> >> A full circle.  This reminds me of the experiment done more than 10\n> >> years ago at the beginning of the \"diffcore transformations\" design.\n> >\n> > Heh, I didn't even think to dig for prior art on the list.\n> \n> It took me a while to dig it up because the topic is so old, but\n> \n> https://public-inbox.org/git/Pine.LNX.4.58.0504251832480.18901@ppc970.osdl.org/\n> \n> is the thread I had in mind.  The idea of rename detection followed\n> soon afterwards.\n\nThanks for an interesting read. Your diff-tree-helper patch is very\nsimilar to what I wrote.\n\nI do think the right decision was made back then. The single-process\nmodel is much more efficient, and it was over 10 years until somebody\nactually wanted to expose the functionality to a script (and even now,\nI'm not convinced enough people want it to even merit inclusion).\n\n-Peff\n"},{"id":"306764","messageId":"xmqqtwan70i0.fsf@gitster.mtv.corp.google.com","threadId":"44587","inReplyTo":"20161201213019.qfkqd324ommikym2@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-01T22:22:47Z","receivedAt":"2016-12-01T22:22:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> It took me a while to dig it up because the topic is so old, but\n>> \n>> https://public-inbox.org/git/Pine.LNX.4.58.0504251832480.18901@ppc970.osdl.org/\n>> \n>> is the thread I had in mind.  The idea of rename detection followed\n>> soon afterwards.\n>\n> Thanks for an interesting read. Your diff-tree-helper patch is very\n> similar to what I wrote.\n>\n> I do think the right decision was made back then. The single-process\n> model is much more efficient, and it was over 10 years until somebody\n> actually wanted to expose the functionality to a script (and even now,\n> I'm not convinced enough people want it to even merit inclusion).\n\nWell, 10 years ago the person in the thread who argued \"who cares\nabout producing patches?  each step in plumbing should do one thing\nand one thing only and do so well\" was Linus, so your coming up with\nthe diff-tree-helper again may indicate that we may want to step\nback and retry the experiment again, perhaps?\n\n"},{"id":"306767","messageId":"20161201223016.vzxpzxxnqwq7n4fi@sigill.intra.peff.net","threadId":"44587","inReplyTo":"xmqqtwan70i0.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] add diff-pairs tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-01T22:30:16Z","receivedAt":"2016-12-01T22:30:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 01, 2016 at 02:22:47PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >> It took me a while to dig it up because the topic is so old, but\n> >> \n> >> https://public-inbox.org/git/Pine.LNX.4.58.0504251832480.18901@ppc970.osdl.org/\n> >> \n> >> is the thread I had in mind.  The idea of rename detection followed\n> >> soon afterwards.\n> >\n> > Thanks for an interesting read. Your diff-tree-helper patch is very\n> > similar to what I wrote.\n> >\n> > I do think the right decision was made back then. The single-process\n> > model is much more efficient, and it was over 10 years until somebody\n> > actually wanted to expose the functionality to a script (and even now,\n> > I'm not convinced enough people want it to even merit inclusion).\n> \n> Well, 10 years ago the person in the thread who argued \"who cares\n> about producing patches?  each step in plumbing should do one thing\n> and one thing only and do so well\" was Linus, so your coming up with\n> the diff-tree-helper again may indicate that we may want to step\n> back and retry the experiment again, perhaps?\n\nI think there are two questions, looking historically.\n\nOne is whether the functionality should be exposed to scripts at all.\n\nThe second is, assuming it should be exposed, in which order to do it.\nYou can write a series of small scripts, and then tie them together. Or\nyou can write tie it all together in C, and then make specific helpers\nto expose the various bits.\n\nThe advantage of the first technique is that the tools are used\nconsistently by all parts of the system, so you know they don't grow\nweird bugs or fail to handle corner cases. The advantage of the second\nis that most people want the \"tied-together\" functionality, and it can\nrun a lot faster in-process.\n\nSo mostly I was suggesting that the right decision 10 years ago was to\noptimize for speed in the common case, and let people worry later about\nwhether they wanted to expose the functionality in more flexible ways.\nAnd that brings us to today.\n\nIt sounds like you are in favor of adding diff-pairs (and certainly it\nshouldn't _hurt_ anybody if they are not interested in using it; you'll\nnotice the patch didn't need to touch the diff code at all).\n\n-Peff\n"}]}