Re: [PATCH v4 0/8] rev-list: implement object type filter
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 15, 2021, 17:53 UTC
- Message-ID
- <xmqqczuvsac6.fsf@gitster.g>
- In-Reply-To
- <YHgOCUI++AY/7zwX@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 5 quoted lines
> 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.
Show 9 quoted lines
>> 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);