| Message ID | cf49115db4e8dcd406a17c946659c2eef3ec6045.1732229420.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pack-bitmap.c: typofix in `find_boundary_objects()` | expand |
Taylor Blau <me@ttaylorr.com> writes: > Fix that by correctly assigning the value of 'revs->tag_objects' to the > 'tmp_tags' field. Makes sense. This breakage would make no difference in practice right now, as {type}_objects members of the rev_info structure have always been all flipped on together since their inception at 3c90f03d (Prepare git-rev-list for tracking tag objects too, 2005-06-29), so the original value of the tag_objects member is always the same as that of the blob_objects member. A possible alternative "fix" for this typo could be to unify these {type}_objects members into a single .non_commit_objects member in the rev_info structure; given that we (as far as I remember) never utilized the "feature" that, say, we can ask only for trees but not blobs for the past ~20 years, nobody knows if the apparent "support" for that feature is subtly broken in multiple ways (and one of them you just fixed with this patch) and the "feature" may not be worth keeping. But neverhteless, this is a correct thing to do unless we decide to rip out the support for individual types. Will queue. Thanks. > > Signed-off-by: Taylor Blau <me@ttaylorr.com> > --- > Noticed while I was looking for an example of this pattern somewhere in > the codebase and was surprised when I found this typo ;-). > > pack-bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pack-bitmap.c b/pack-bitmap.c > index 4fa9dfc771..683f467051 100644 > --- a/pack-bitmap.c > +++ b/pack-bitmap.c > @@ -1270,7 +1270,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, > > tmp_blobs = revs->blob_objects; > tmp_trees = revs->tree_objects; > - tmp_tags = revs->blob_objects; > + tmp_tags = revs->tag_objects; > revs->blob_objects = 0; > revs->tree_objects = 0; > revs->tag_objects = 0; > > base-commit: 4083a6f05206077a50af7658bedc17a94c54607d > -- > 2.47.0.237.gc601277f4c4
diff --git a/pack-bitmap.c b/pack-bitmap.c index 4fa9dfc771..683f467051 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -1270,7 +1270,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, tmp_blobs = revs->blob_objects; tmp_trees = revs->tree_objects; - tmp_tags = revs->blob_objects; + tmp_tags = revs->tag_objects; revs->blob_objects = 0; revs->tree_objects = 0; revs->tag_objects = 0;
In the boundary-based bitmap traversal, we use the given 'rev_info' structure to first do a commit-only walk in order to determine the boundary between interesting and uninteresting objects. That walk only looks at commit objects, regardless of the state of revs->blob_objects, revs->tree_objects, and so on. In order to do this, we store the state of these variables in temporary fields before setting them back to zero, performing the traversal, and then setting them back. But there is a typo here that dates back to b0afdce5da (pack-bitmap.c: use commit boundary during bitmap traversal, 2023-05-08), where we incorrectly store the value of the "tags" field as "revs->blob_objects". This could lead to problems later on if, say, the caller wants tag objects but *not* blob objects. In the pre-image behavior, we'd set revs->tag_objects back to the old value of revs->blob_objects, thus emitting fewer objects than expected back to the caller. Fix that by correctly assigning the value of 'revs->tag_objects' to the 'tmp_tags' field. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- Noticed while I was looking for an example of this pattern somewhere in the codebase and was surprised when I found this typo ;-). pack-bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 4083a6f05206077a50af7658bedc17a94c54607d -- 2.47.0.237.gc601277f4c4