{"thread":{"id":"60743","subject":"[PATCH] rev-list-options: fix off-by-one in '--filter=blob:limit=' explainer","startedAt":"2024-01-14T19:50:54Z","lastAt":"2024-01-16T16:54:26Z","messageCount":2,"participants":["Nikolay Edigaryev via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"486779","messageId":"pull.1645.git.git.1705261850650.gitgitgadget@gmail.com","threadId":"60743","inReplyTo":null,"subject":"[PATCH] rev-list-options: fix off-by-one in '--filter=blob:limit=' explainer","fromName":"Nikolay Edigaryev via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-14T19:50:50Z","receivedAt":"2024-01-14T19:50:54Z","isPatch":true,"sender":{"key":"edigaryev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/85709?v=4"},"body":"From: Nikolay Edigaryev \n\n'--filter=blob:limit=' was introduced in 25ec7bcac0 (list-objects:\nfilter objects in traverse_commit_list, 2017-11-21) and later expanded\nto bitmaps in 84243da129 (pack-bitmap: implement BLOB_LIMIT filtering,\n2020-02-14)\n\nThe logic that was introduced in these commits (and that still persists\nto this day) omits blobs larger than _or equal_ to n bytes or units.\n\nHowever, the documentation (Documentation/rev-list-options.txt) states:\n\n>The form '--filter=blob:limit=[kmg]' omits blobs larger than n\nbytes or units. n may be zero.\n\nMoreover, the t6113-rev-list-bitmap-filters.sh tests for exactly this\nlogic, so it seems it is the documentation that needs fixing, not the\ncode.\n\nThis changes the explanation to be similar to\nDocumentation/git-clone.txt, which is correct.\n\nSigned-off-by: Nikolay Edigaryev \n---\n rev-list-options: fix off-by-one in '--filter=blob:limit=' explainer\n \n '--filter=blob:limit=' was introduced in 25ec7bcac0 (list-objects:\n filter objects in traverse_commit_list, 2017-11-21) and later expanded\n to bitmaps in 84243da129 (pack-bitmap: implement BLOB_LIMIT filtering,\n 2020-02-14)\n \n The logic that was introduced in these commits (and that still persists\n to this day) omits blobs larger than or equal to n bytes or units.\n \n However, the documentation (Documentation/rev-list-options.txt) states:\n \n > The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes\n > or units. n may be zero.\n \n Moreover, the t6113-rev-list-bitmap-filters.sh tests for exactly this\n logic, so it seems it is the documentation that needs fixing, not the\n code.\n \n This changes the explanation to be similar to\n Documentation/git-clone.txt, which is correct.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1645%2Fedigaryev%2Ffix-blob-limit-off-by-one-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1645/edigaryev/fix-blob-limit-off-by-one-v1\nPull-Request: https://github.com/git/git/pull/1645\n\n Documentation/rev-list-options.txt | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 2bf239ff030..a583b52c612 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -947,10 +947,10 @@ ifdef::git-rev-list[]\n +\n The form '--filter=blob:none' omits all blobs.\n +\n-The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes\n-or units. n may be zero. The suffixes k, m, and g can be used to name\n-units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same\n-as 'blob:limit=1024'.\n+The form '--filter=blob:limit=[kmg]' omits blobs of size at least n\n+bytes or units. n may be zero. The suffixes k, m, and g can be used\n+to name units in KiB, MiB, or GiB. For example, 'blob:limit=1k'\n+is the same as 'blob:limit=1024'.\n +\n The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects\n which are not of the requested type.\n\nbase-commit: 564d0252ca632e0264ed670534a51d18a689ef5d\n-- \ngitgitgadget\n"},{"id":"486851","messageId":"xmqqo7dl1ao0.fsf@gitster.g","threadId":"60743","inReplyTo":"pull.1645.git.git.1705261850650.gitgitgadget@gmail.com","subject":"Re: [PATCH] rev-list-options: fix off-by-one in '--filter=blob:limit=' explainer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-16T16:54:23Z","receivedAt":"2024-01-16T16:54:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nikolay Edigaryev via GitGitGadget\" \nwrites:\n\n> From: Nikolay Edigaryev \n>\n> '--filter=blob:limit=' was introduced in 25ec7bcac0 (list-objects:\n> filter objects in traverse_commit_list, 2017-11-21) and later expanded\n> to bitmaps in 84243da129 (pack-bitmap: implement BLOB_LIMIT filtering,\n> 2020-02-14)\n>\n> The logic that was introduced in these commits (and that still persists\n> to this day) omits blobs larger than _or equal_ to n bytes or units.\n\nGood eyes. The former does this\n\n\t\tif (object_length < filter_data->max_bytes)\n\t\t\tgoto include_it;\n\nand the latter does this\n\n\n if (!bitmap_get(tips, pos) &&\n get_size_by_pos(bitmap_git, pos) >= limit)\n bitmap_unset(to_filter, pos);\n\n> However, the documentation (Documentation/rev-list-options.txt) states:\n>\n>>The form '--filter=blob:limit=[kmg]' omits blobs larger than n\n> bytes or units. n may be zero.\n>\n> Moreover, the t6113-rev-list-bitmap-filters.sh tests for exactly this\n> logic, so it seems it is the documentation that needs fixing, not the\n> code.\n\nYup. The mechanism is used for things like \"we do not want a large\nblob, like 100MB\", and a byte on the boundary does not matter all\nthat much in such a countext, but it does not hurt to be more\ncorrect ;-)\n\n> The form '--filter=blob:none' omits all blobs.\n> +\n> -The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes\n> -or units. n may be zero. The suffixes k, m, and g can be used to name\n> -units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same\n> -as 'blob:limit=1024'.\n> +The form '--filter=blob:limit=[kmg]' omits blobs of size at least n\n> +bytes or units. n may be zero. The suffixes k, m, and g can be used\n> +to name units in KiB, MiB, or GiB. For example, 'blob:limit=1k'\n> +is the same as 'blob:limit=1024'.\n\nWith unnecessary paragraph wrapping, it is a bit hard to compare the\npreimage and the postimage, but I manually checked that this only\ndoes\n\n\t\"larger than\" -> \"of size at least\"\n\nand nothing else, which is expected and in line with what the\nproposed commit message claimed to do. Good job.\n\nWill queue. Thanks.\n\n> +\n> The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects\n> which are not of the requested type.\n>\n> base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d\n"}]}