revision: mark contents of an uninteresting tree uninteresting

"git rev-list --objects ^A^{tree} B^{tree}" ought to mean "I want a
list of objects inside B's tree, but please exclude the objects that
appear inside A's tree".

we see the top-level tree marked as uninteresting (i.e. ^A^{tree} in
the above example) and call mark_tree_uninteresting() on it; this
unfortunately prevents us from recursing into the tree and marking
the objects in the tree as uninteresting.

The reason why "git log ^A A" yields an empty set of commits,
i.e. we do not have a similar issue for commits, is because we call
mark_parents_uninteresting() after seeing an uninteresting commit.
The uninteresting-ness of the commit itself does not prevent its
parents from being marked as uninteresting.

Introduce mark_tree_contents_uninteresting() and structure the code
in handle_commit() in such a way that it makes it the responsibility
of the callchain leading to this function to mark commits, trees and
blobs as uninteresting, and also make it the responsibility of the
helpers called from this function to mark objects that are reachable
from them.

Note that this is a very old bug that probably dates back to the day
when "rev-list --objects" was introduced.  The line to clear
tree->object.parsed at the end of mark_tree_contents_uninteresting()
can be removed when this fix is merged to the codebase after
6e454b9a (clear parsed flag when we free tree buffers, 2013-06-05).

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

diff --git a/revision.c b/revision.c
index 7010aff817e3889dbdb6f99fdbdd8b18db8b6d2c..28449c514003075da0e6ee8ee98dbdcb4e77d90f 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -98,17 +98,12 @@ static void mark_blob_uninteresting(struct blob *blob)
        blob->object.flags |= UNINTERESTING;
 }
 
-void mark_tree_uninteresting(struct tree *tree)
+static void mark_tree_contents_uninteresting(struct tree *tree)
 {
        struct tree_desc desc;
        struct name_entry entry;
        struct object *obj = &tree->object;
 
-       if (!tree)
-               return;
-       if (obj->flags & UNINTERESTING)
-               return;
-       obj->flags |= UNINTERESTING;
        if (!has_sha1_file(obj->sha1))
                return;
        if (parse_tree(tree) < 0)
@@ -135,6 +130,19 @@ void mark_tree_uninteresting(struct tree *tree)
         */
        free(tree->buffer);
        tree->buffer = NULL;
+       tree->object.parsed = 0;
+}
+
+void mark_tree_uninteresting(struct tree *tree)
+{
+       struct object *obj = &tree->object;
+
+       if (!tree)
+               return;
+       if (obj->flags & UNINTERESTING)
+               return;
+       obj->flags |= UNINTERESTING;
+       mark_tree_contents_uninteresting(tree);
 }
 
 void mark_parents_uninteresting(struct commit *commit)
@@ -294,7 +302,8 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
                if (!revs->tree_objects)
                        return NULL;
                if (flags & UNINTERESTING) {
-                       mark_tree_uninteresting(tree);
+                       tree->object.flags |= UNINTERESTING;
+                       mark_tree_contents_uninteresting(tree);
                        return NULL;
                }
                add_pending_object(revs, object, "");
@@ -309,7 +318,7 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
                if (!revs->blob_objects)
                        return NULL;
                if (flags & UNINTERESTING) {
-                       mark_blob_uninteresting(blob);
+                       blob->object.flags |= UNINTERESTING;
                        return NULL;
                }
                add_pending_object(revs, object, "");

diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 15e3d6476c78ba3447504aa133e9e1d7ffbfd87d..9ad4971f8109f703b812580df3bb5cb3e37d5ddd 100755 (executable)
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -56,4 +56,10 @@ test_expect_success 'rev-list A..B and rev-list ^A B are the same' '
        test_cmp expect actual
 '
 
+test_expect_success 'propagate uninteresting flag down correctly' '
+       git rev-list --objects ^HEAD^{tree} HEAD^{tree} >actual &&
+       >expect &&
+       test_cmp expect actual
+'
+
 test_done