Re: [PATCH v4 0/8] rev-list: implement object type filter

Jeff King [off-list ref] writes:

Yeah, as somebody who has added or touched a lot of those paths, I've
often wondered this myself: what would break if you asked for blobs but
not trees? I think it does not work like you'd expect, because
list-objects.c:process_tree() will not recurse the trees at all (and
hence you'd never see any blob).

Ah, OK, that much I figured.  I was wondering if it is worth
"fixing", IOW, if we see blobs being asked without trees being
asked, perhaps process_tree() should be taught to descend---even if
it won't show the trees themselves, blobs contained within would.

quoted

And, the above led to the question---the patches in your series
apparently do a lot more (even if we discount the option parsing
part), and I was wondering if that is because the independence
between these three bits the existing code aspires to maintain is
broken.

...
Anyway. I do think this all could be done using the existing bits in
rev_info. But there is value in making it all part of the modular
filtering system.

Yes, I do not have a problem with the approach to add new features
as part of the "modular filtering system".  But that leads me to
wonder into a different direction---coalesce (tag|tree|blob)_objects
members into a single bit, say all_objects, have "--objects" and
friends set that single bit, and update places like these to check
that single bit.

The patch is for illustration purposes; I didn't even aim to cover
the entirety of these two files.

 builtin/rev-list.c | 9 ++++-----
 pack-bitmap.c      | 4 +---
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git c/builtin/rev-list.c w/builtin/rev-list.c
index 7677b1af5a..b6f0d7e9a3 100644
--- c/builtin/rev-list.c
+++ w/builtin/rev-list.c

@@ -424,8 +424,7 @@ static int try_bitmap_count(struct rev_info *revs,
 	 * commits to traverse, since we don't know which objects go with which
 	 * commit.
 	 */
-	if (revs->max_count >= 0 &&
-	    (revs->tag_objects || revs->tree_objects || revs->blob_objects))
+	if (revs->max_count >= 0 && revs->all_objects)
 		return -1;
 
 	/*

@@ -439,9 +438,9 @@ static int try_bitmap_count(struct rev_info *revs,
 		return -1;
 
 	count_bitmap_commit_list(bitmap_git, &commit_count,
-				 revs->tree_objects ? &tree_count : NULL,
-				 revs->blob_objects ? &blob_count : NULL,
-				 revs->tag_objects ? &tag_count : NULL);
+				 revs->all_objects ? &tree_count : NULL,
+				 revs->all_objects ? &blob_count : NULL,
+				 revs->all_objects ? &tag_count : NULL);
 	if (max_count >= 0 && max_count < commit_count)
 		commit_count = max_count;
 

diff --git c/pack-bitmap.c w/pack-bitmap.c
index d7e9f14f92..918c80a391 100644
--- c/pack-bitmap.c
+++ w/pack-bitmap.c

@@ -801,9 +801,7 @@ static void show_extended_objects(struct bitmap_index *bitmap_git,
 			continue;
 
 		obj = eindex->objects[i];
-		if ((obj->type == OBJ_BLOB && !revs->blob_objects) ||
-		    (obj->type == OBJ_TREE && !revs->tree_objects) ||
-		    (obj->type == OBJ_TAG && !revs->tag_objects))
+		if (!revs->all_objects && obj->type != OBJ_COMMIT)
 			continue;
 
 		show_reach(&obj->oid, obj->type, 0, eindex->hashes[i], NULL, 0);