difflame improvements

From: Edmundo Carmona Antoranz <eantoranz@gmail.com>
Date: Mar 5, 2017, 16:18 UTC
Message-ID: <CAOc6etbwE3wyVEUKJq8WCPe=hYBobWo-RtDonH46XCHF157+BQ@mail.gmail.com>

Hi!

Since my last post the biggest improvement is the ability to detect that the user has requested a "reverse" analysis.

Under "normal" circumstances a user would ask difflame to get the diff from an ancestor (call "difflame treeish1 treeish2" so that merge-base of treeish1 treeish2 equals treeish1). In this case the blame result is done using straight blame output for added lines and additional analysis to detect where a line was deleted (analysis has improved a lot in this regard.... I haven't heard anything from Peff, though). But if the user requests the opposite (call "difflame treeish1 treeish2" so that merge-base of treeish1 treeish2 is treeish2) then the analysis has to be driven "in reverse".

Here's one example taken from difflame itself:

normal "forward" call (hope output doesn't get butchered):

$ ./difflame.py HEAD~3 HEAD~2

diff --git a/difflame.py b/difflame.py
index e70154a..04c7577 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
 e5b218e4 (Edmundo 2017-02-01 365)         # we already had the revision
 50528377 (Edmundo 2017-03-04 366)         return REVISIONS_ID_CACHE[revision]
 d1d11d8a (Edmundo 2017-02-02 367)     # fallback to get it from git
       b1a6693 use rev-list to get revision IDs
-b1a6693 (Edmundo 2017-03-04 368)     full_revision =
run_git_command(["show", "--pretty=%H", revision]).split("\n")[0]
+b1a66932 (Edmundo 2017-03-04 368)     full_revision =
run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
 50528377 (Edmundo 2017-03-04 369)     REVISIONS_ID_CACHE[revision] =
full_revision
 e5b218e4 (Edmundo 2017-02-01 370)     return full_revision
 91b7d3f5 (Edmundo 2017-01-31 371)

"reverse" call:
$ ./difflame.py HEAD~2 HEAD~3
diff --git a/difflame.py b/difflame.py
index 04c7577..e70154a 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
 e5b218e4 (Edmundo 2017-02-01 365)         # we already had the revision
 50528377 (Edmundo 2017-03-04 366)         return REVISIONS_ID_CACHE[revision]
 d1d11d8a (Edmundo 2017-02-02 367)     # fallback to get it from git
       b1a6693 use rev-list to get revision IDs
-b1a66932 (Edmundo 2017-03-04 368)     full_revision =
run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
+b1a6693 (Edmundo 2017-03-04 368)     full_revision =
run_git_command(["show", "--pretty=%H", revision]).split("\n")[0]
 50528377 (Edmundo 2017-03-04 369)     REVISIONS_ID_CACHE[revision] =
full_revision
 e5b218e4 (Edmundo 2017-02-01 370)     return full_revision
 91b7d3f5 (Edmundo 2017-01-31 371)

Notice how the revision reported in both difflame calls is the same:

$ git show b1a66932
commit b1a66932704245fd653f8d48c0a718f168f334a7
Author: Edmundo Carmona Antoranz <whocares@gmail.com>
Date:   Sat Mar 4 13:59:50 2017 -0600

   use rev-list to get revision IDs

diff --git a/difflame.py b/difflame.py
index e70154a..04c7577 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
        # we already had the revision
        return REVISIONS_ID_CACHE[revision]
    # fallback to get it from git
-    full_revision = run_git_command(["show", "--pretty=%H",
revision]).split("\n")[0]
+    full_revision = run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
    REVISIONS_ID_CACHE[revision] = full_revision
    return full_revision


If this "detection" to perform reverse analysis hadn't been done, then
there wouldn't be a lot of useful information because there are no
revisions in HEAD~2..HEAD~3 and so the output would have been
something like:

diff --git a/difflame.py b/difflame.py
index 04c7577..e70154a 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
 e5b218e4 (Edmundo 2017-02-01 365)         # we already had the revision
 50528377 (Edmundo 2017-03-04 366)         return REVISIONS_ID_CACHE[revision]
 d1d11d8a (Edmundo 2017-02-02 367)     # fallback to get it from git
       b1a6693 use rev-list to get revision IDs
%b1a6693 (Edmundo 2017-03-04 368)     full_revision =
run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
       e5b218e printing hints for deleted lines
+e5b218e4 (Edmundo 2017-02-01 368)     full_revision =
run_git_command(["show", "--pretty=%H", revision]).split("\n")[0]
 50528377 (Edmundo 2017-03-04 369)     REVISIONS_ID_CACHE[revision] =
full_revision
 e5b218e4 (Edmundo 2017-02-01 370)     return full_revision
 91b7d3f5 (Edmundo 2017-01-31 371)

Notice how both the added line and the deleted line are reporting the
_wrong_ revision. It should be b1a66932 in all cases.


One question that has been bugging me for a while is what to do in
cases where treeish1, treeish2 are not "direct" descendants" (as in
merge-base treeish1 treeish2 is something other than treeish1 or
treeish2). Suppose a line was added on an ancestor of treeish1 but it
hasn't been merged into treeish2. In this case if we diff
treeish1..treeish2 we will get a _deleted_ line. However analysis to
find a deleting revision in treeish1..treeish2 will fail. I'm
wondering if it would be ok in this case to blame the deleted line on
the ancestor if treeish1 where the line was _added_.

Another thing I added is the support to use tags.

Best regards!