[PATCH v2 1/2] revision: mark contents of an uninteresting tree uninteresting

2014-01-15 Thread Junio C Hamano
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 gits...@pobox.com
---
 revision.c   | 25 +
 t/t6000-rev-list-misc.sh |  6 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/revision.c b/revision.c
index 7010aff..28449c5 100644
--- 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 15e3d64..9ad4971 100755
--- 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
-- 
1.8.5.3-493-gb139ac2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] revision: mark contents of an uninteresting tree uninteresting

2014-01-15 Thread Jonathan Nieder
Junio C Hamano wrote:

 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.

So the tree is marked uninteresting twice --- once by setting in the
UNINTERESTING flag in handle_revision_arg() and a second attempted
time in mark_tree_uninteresting()?   Makes sense.

Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] revision: mark contents of an uninteresting tree uninteresting

2014-01-15 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Junio C Hamano wrote:

 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.

 So the tree is marked uninteresting twice --- once by setting in the
 UNINTERESTING flag in handle_revision_arg() and a second attempted
 time in mark_tree_uninteresting()?   Makes sense.

It is that the original code, the setting of the mark on the object
itself was inconsistent.  For commits, we did that ourselves; for
trees, we let the mark_tree_uninteresting() do so.

And mark_tree_uninteresting() has this quirk that it refuses to
recurse into the given tree, if the tree is already marked as
uninteresting by the caller.

We did not have the same problem on commits, because we make a
similar call to mark-parents-uninteresting but the function does not
care if the commit itself is already marked as uninteresting.

The distinction does not matter when tags are not involved.  But
once we start propagating the flags from a tag to objects that the
tag points at, it starts to matter.  The caller will mark the object
uninteresting in the loop that peels the tag, and the resulting
object is uninteresting.  It is not a problem for commits.  It was a
problem for trees, which used mark_tree_uninteresting() to mark all
the objects inside the tree uninteresting, including the tree itself.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html