line-log: consult diff process for range tracking

git log -L tracks line ranges by diffing each commit against its
parent in collect_diff().  This pass used the builtin diff while the
displayed diff (builtin_diff()) consults a configured
diff.<driver>.process, so the two could disagree: a reformat-only
commit selected by builtin tracking was then rendered with an empty
diff because the tool reported the files equivalent.

Consult the process in collect_diff() too, mirroring the blame
integration.  When the tool reports the files equivalent, collect no
ranges; the tracked range then maps across unchanged and the commit
drops out of the log, matching what is displayed.  Like the summary
formats, the tracking pass diffs raw content, so the tool is consulted
on the raw blobs here.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index 7cdede6b218f8979a6c237d497a13776432e0752..8021dc8e399e62f5f61a8aee84d489d611e8a0c3 100644 (file)
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -1037,12 +1037,16 @@ Features that ask "which lines changed" use the tool's hunks in place
 of the builtin algorithm:
 
 - `git diff` patch output, together with everything layered on it:
-  word diff, function context (`-W`), `--color-moved`, the `@@` hunk
-  headers, and the `-L` line-range display.  These operate on the
-  lines the patch step already emitted, so they reflect the tool's
-  hunks without any further negotiation.
+  word diff, function context (`-W`), `--color-moved`, and the `@@`
+  hunk headers.  These operate on the lines the patch step already
+  emitted, so they reflect the tool's hunks without any further
+  negotiation.
 - `git blame`: a commit whose change the tool reports as equivalent is
   skipped, and its lines are attributed to an earlier commit.
+- `git log -L`: both the line-range display and the underlying range
+  tracking consult the tool, so a commit it reports as equivalent is
+  dropped from the log (its tracked range maps across unchanged)
+  rather than selected and then shown with an empty diff.
 - `--stat`, `--numstat`, and `--shortstat`: the inserted and deleted
   counts come from the tool's hunks, so a file the tool calls
   equivalent contributes no stat line, matching the empty patch that
@@ -1079,11 +1083,9 @@ design:
 - `--raw`, `--name-only`, and `--name-status` compare object ids at
   the tree level and never run a line-level diff at all.
 
-Two cases ask "which lines changed" but still use the builtin
-algorithm, and may consult the process in a later change: `git log
--L`'s commit selection and parent range propagation (as distinct from
-its display, which is covered above), and combined diffs (`--cc` and
-merge diffs), whose protocol would have to be extended from a single
+Combined diffs (`--cc` and merge diffs) ask "which lines changed" but
+still use the builtin algorithm, and may consult the process in a
+later change; their protocol would have to be extended from a single
 old/new pair to one comparison per merge parent.
 
 `--no-ext-diff` and `--diff-algorithm` bypass the process entirely,

diff --git a/line-log.c b/line-log.c
index 0179f138f702888c8a8ba77a53b4ba2904139299..68b753ec56565c05d392592085f4a49f6db6c173 100644 (file)
--- a/line-log.c
+++ b/line-log.c
@@ -7,11 +7,11 @@
 #include "tag.h"
 #include "tree.h"
 #include "diff.h"
+#include "diff-process.h"
 #include "commit.h"
 #include "decorate.h"
 #include "repository.h"
 #include "revision.h"
-#include "xdiff-interface.h"
 #include "strbuf.h"
 #include "line-log.h"
 #include "setup.h"
@@ -330,12 +330,15 @@ static int collect_diff_cb(long start_a, long count_a,
        return 0;
 }
 
-static int collect_diff(mmfile_t *parent, mmfile_t *target, struct diff_ranges *out)
+static int collect_diff(struct diff_options *diffopt, const char *path,
+                       mmfile_t *parent, mmfile_t *target,
+                       struct diff_ranges *out)
 {
        struct collect_diff_cbdata cbdata = {NULL};
        xpparam_t xpp;
        xdemitconf_t xecfg;
        xdemitcb_t ecb;
+       int ret = 0;
 
        memset(&xpp, 0, sizeof(xpp));
        memset(&xecfg, 0, sizeof(xecfg));
@@ -345,7 +348,23 @@ static int collect_diff(mmfile_t *parent, mmfile_t *target, struct diff_ranges *
        xecfg.hunk_func = collect_diff_cb;
        memset(&ecb, 0, sizeof(ecb));
        ecb.priv = &cbdata;
-       return xdi_diff(parent, target, &xpp, &xecfg, &ecb);
+
+       /*
+        * Consult the diff process so range tracking agrees with the
+        * diff that will be shown.  When the tool reports the files as
+        * equivalent we collect no ranges, so the tracked range maps
+        * across unchanged and the commit drops out of the log, rather
+        * than being selected here but rendered with an empty diff by
+        * the process-aware builtin_diff().  Blob oids are not threaded to
+        * this path yet, so pass NULL and send no old-oid/new-oid (a later
+        * change can supply the pair, where they would let the tool cache
+        * across the range-tracking and display passes over the same
+        * commit).
+        */
+       if (xdi_diff_process(diffopt, path, parent, target,
+                            NULL, NULL, &xpp, &xecfg, &ecb) == DIFF_PROCESS_ERROR)
+               ret = -1;
+       return ret;
 }
 
 /*
@@ -927,7 +946,13 @@ static int process_diff_filepair(struct rev_info *rev,
        }
 
        diff_ranges_init(&diff);
-       if (collect_diff(&file_parent, &file_target, &diff))
+       /*
+        * Select the driver by the old (parent) path, as builtin_diff() does
+        * with name_a, so a renamed file resolves to the same driver for
+        * range tracking as for the diff that is shown.
+        */
+       if (collect_diff(&rev->diffopt, pair->one->path,
+                        &file_parent, &file_target, &diff))
                die("unable to generate diff for %s", pair->one->path);
 
        /* NEEDSWORK should apply some heuristics to prevent mismatches */

diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index 118d0f94648977d478cebccc36589b8e2e6c72bd..9584a458b1e542909c91352c3b82934d7387865e 100755 (executable)
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -960,4 +960,37 @@ test_expect_success 'blame -w bypasses diff process' '
        test_path_is_missing backend.log
 '
 
+#
+# Line-log (git log -L) range tracking.
+#
+
+test_expect_success 'diff process drops equivalent commit from log -L' '
+       test_when_finished "rm -f backend.log" &&
+       cat >linelog.c <<-\EOF &&
+       int tracked(void) { return 1; }
+       EOF
+       git add linelog.c &&
+       git commit -m "add linelog.c" &&
+
+       cat >linelog.c <<-\EOF &&
+       int tracked(void) { return 2; }
+       EOF
+       git commit -am "change tracked line" &&
+
+       # Builtin line tracking selects the change commit.
+       git log --no-ext-diff -L1,1:linelog.c --format="%s" >builtin &&
+       test_grep "change tracked line" builtin &&
+
+       # With the tool reporting the change as equivalent, tracking
+       # drops the commit (the range maps across unchanged) instead of
+       # selecting it and rendering an empty diff.
+       git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
+               log -L1,1:linelog.c --format="%s" >actual &&
+       test_grep ! "change tracked line" actual &&
+       # The creating commit still appears, so the change commit was
+       # selectively dropped rather than the whole log going empty.
+       test_grep "add linelog.c" actual &&
+       test_grep "command=hunks pathname=linelog.c" backend.log
+'
+
 test_done