Re: [PATCH] rev-list-options: fix off-by-one in '--filter=blob:limit=<n>' explainer
From: Junio C Hamano <hidden>
Date: 2024-01-16 16:54:26
"Nikolay Edigaryev via GitGitGadget" [off-list ref] writes:
From: Nikolay Edigaryev <redacted> '--filter=blob:limit=<n>' was introduced in 25ec7bcac0 (list-objects: filter objects in traverse_commit_list, 2017-11-21) and later expanded to bitmaps in 84243da129 (pack-bitmap: implement BLOB_LIMIT filtering, 2020-02-14) The logic that was introduced in these commits (and that still persists to this day) omits blobs larger than _or equal_ to n bytes or units.
Good eyes. The former does this
if (object_length < filter_data->max_bytes)
goto include_it;
and the latter does this
if (!bitmap_get(tips, pos) &&
get_size_by_pos(bitmap_git, pos) >= limit)
bitmap_unset(to_filter, pos);
However, the documentation (Documentation/rev-list-options.txt) states:quoted
The form '--filter=blob:limit=<n>[kmg]' omits blobs larger than nbytes or units. n may be zero. Moreover, the t6113-rev-list-bitmap-filters.sh tests for exactly this logic, so it seems it is the documentation that needs fixing, not the code.
Yup. The mechanism is used for things like "we do not want a large blob, like 100MB", and a byte on the boundary does not matter all that much in such a countext, but it does not hurt to be more correct ;-)
The form '--filter=blob:none' omits all blobs. + -The form '--filter=blob:limit=<n>[kmg]' omits blobs larger than n bytes -or units. n may be zero. The suffixes k, m, and g can be used to name -units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same -as 'blob:limit=1024'. +The form '--filter=blob:limit=<n>[kmg]' omits blobs of size at least n +bytes or units. n may be zero. The suffixes k, m, and g can be used +to name units in KiB, MiB, or GiB. For example, 'blob:limit=1k' +is the same as 'blob:limit=1024'.
With unnecessary paragraph wrapping, it is a bit hard to compare the preimage and the postimage, but I manually checked that this only does "larger than" -> "of size at least" and nothing else, which is expected and in line with what the proposed commit message claimed to do. Good job. Will queue. Thanks.
+ The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects which are not of the requested type. base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d