rev-list-options: fix off-by-one in '--filter=blob:limit=<n>' explainer
PATCH 1/1 rev-list-options: fix off-by-one in '--filter=blob:limit=<n>' explainer
From: Nikolay Edigaryev <[email protected]>
'--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.
However, the documentation (Documentation/rev-list-options.txt) states:
Quoted 1 line
The form '--filter=blob:limit=<n>[kmg]' omits blobs larger than n
bytes 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.
This changes the explanation to be similar toDocumentation/git-clone.txt, which is correct.
---
rev-list-options: fix off-by-one in '--filter=blob:limit=' explainer
'--filter=blob:limit=' 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.
However, the documentation (Documentation/rev-list-options.txt) states:
> The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes > 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.
This changes the explanation to be similar to Documentation/git-clone.txt, which is correct.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1645%2Fedigaryev%2Ffix-blob-limit-off-by-one-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1645/edigaryev/fix-blob-limit-off-by-one-v1
Pull-Request: https://github.com/git/git/pull/1645
Documentation/rev-list-options.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Documentation/rev-list-options.txt +4 -4
@@ -947,10 +947,10 @@ ifdef::git-rev-list[] + 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'. + The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects which are not of the requested type.
base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Signature · 1 line
gitgitgadget
"Nikolay Edigaryev via GitGitGadget" <[email protected]>
Quoted 9 lines · writes:
From: Nikolay Edigaryev <[email protected]>
'--filter=blob:limit=<n>' was introduced in
25ec7bcac0(list-objects:
filter objects intraverse_commit_list, 2017-11-21) and later expanded to bitmaps in84243da129(pack-bitmap: implementBLOB_LIMITfiltering, 2020-02-14)The logic that was introduced in these commits (and that still persists to this day) omits blobs larger than
_orequal_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);Quoted 8 lines
However, the documentation (
Documentation/rev-list-options.txt) states:Quoted 1 line
The form '--filter=blob:limit=<n>[kmg]' omits blobs larger than n
bytes or units. n may be zero.
Moreover, the
t6113-rev-list-bitmap-filters.shtests 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 ;-)
Documentation/rev-list-options.txt ifdef::git-rev-list[] · line 953
Quoted 10 lines
The form '--filter=blob:none' omits all blobs.
+Documentation/rev-list-options.txt +4
-4-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.
Quoted 5 lines
+
The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects which are not of the requested type.base-commit:
564d0252ca632e0264ed670534a51d18a689ef5d