blame: correctly handle a path that used to be a directory

When trying to see if the same path exists in the parent, we ran
"diff-tree" with pathspec set to the path we are interested in with the
parent, and expect either to have exactly one resulting filepair (either
"changed from the parent", "created when there was none") or nothing (when
there is no change from the parent).

If the path used to be a directory, however, we will also see unbounded
number of entries that talk about the files that used to exist underneath
the directory in question.  Correctly pick only the entry that describes
the path we are interested in in such a case (namely, the creation of the
path as a regular file).

Noticed by Ben Willard.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-blame.c b/builtin-blame.c
index cf74a926149668b7e67544aecd46a0e05d4f59d9..0afdb16cb0e14367994fac99b60f36c292aeaac9 100644 (file)
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -362,18 +362,28 @@ static struct origin *find_origin(struct scoreboard *sb,
                               "", &diff_opts);
        diffcore_std(&diff_opts);
 
                               "", &diff_opts);
        diffcore_std(&diff_opts);
 

-       /* It is either one entry that says "modified", or "created",
-        * or nothing.
-        */

        if (!diff_queued_diff.nr) {
                /* The path is the same as parent */
                porigin = get_origin(sb, parent, origin->path);
                hashcpy(porigin->blob_sha1, origin->blob_sha1);
        if (!diff_queued_diff.nr) {
                /* The path is the same as parent */
                porigin = get_origin(sb, parent, origin->path);
                hashcpy(porigin->blob_sha1, origin->blob_sha1);

-       }
-       else if (diff_queued_diff.nr != 1)
-               die("internal error in blame::find_origin");
-       else {
-               struct diff_filepair *p = diff_queued_diff.queue[0];
+       } else {
+               /*
+                * Since origin->path is a pathspec, if the parent
+                * commit had it as a directory, we will see a whole
+                * bunch of deletion of files in the directory that we
+                * do not care about.
+                */
+               int i;
+               struct diff_filepair *p = NULL;
+               for (i = 0; i < diff_queued_diff.nr; i++) {
+                       const char *name;
+                       p = diff_queued_diff.queue[i];
+                       name = p->one->path ? p->one->path : p->two->path;
+                       if (!strcmp(name, origin->path))
+                               break;
+               }
+               if (!p)
+                       die("internal error in blame::find_origin");

                switch (p->status) {
                default:
                        die("internal error in blame::find_origin (%c)",
                switch (p->status) {
                default:
                        die("internal error in blame::find_origin (%c)",

diff --git a/t/t8003-blame.sh b/t/t8003-blame.sh
index 966bb0a61a89ed63dec085338d3c45f766a7f777..13c25f1d528ca1ec90575e42e0393accff5d8f35 100755 (executable)
--- a/t/t8003-blame.sh
+++ b/t/t8003-blame.sh
@@ -129,4 +129,19 @@ test_expect_success 'blame wholesale copy and more' '
 
 '
 
 
 '
 

+test_expect_success 'blame path that used to be a directory' '
+       mkdir path &&
+       echo A A A A A >path/file &&
+       echo B B B B B >path/elif &&
+       git add path &&
+       test_tick &&
+       git commit -m "path was a directory" &&
+       rm -fr path &&
+       echo A A A A A >path &&
+       git add path &&
+       test_tick &&
+       git commit -m "path is a regular file" &&
+       git blame HEAD^.. -- path
+'
+

 test_done
 test_done