On 3/8/2022 4:25 AM, Ævar Arnfjörð Bjarmason wrote:
>
> On Mon, Mar 07 2022, Derrick Stolee via GitGitGadget wrote:
>
>> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>>
>> The v3 bundle format has capabilities, allowing newer versions of Git to
>> create bundles with newer features. Older versions that do not
>> understand these new capabilities will fail with a helpful warning.
>>
>> Create a new capability allowing Git to understand that the contained
>> pack-file is filtered according to some object filter. Typically, this
>> filter will be "blob:none" for a blobless partial clone.
>>
>> This change teaches Git to parse this capability, place its value in the
>> bundle header, and demonstrate this understanding by adding a message to
>> 'git bundle verify'.
>>
>> Since we will use gently_parse_list_objects_filter() outside of
>> list-objects-filter-options.c, make it an external method and move its
>> API documentation to before its declaration.
>> [...]
>> --- a/bundle.c
>> +++ b/bundle.c
>> @@ -11,7 +11,7 @@
>> #include "run-command.h"
>> #include "refs.h"
>> #include "strvec.h"
>> -
>> +#include "list-objects-filter-options.h"
>>
>> static const char v2_bundle_signature[] = "# v2 git bundle\n";
>> static const char v3_bundle_signature[] = "# v3 git bundle\n";
>> @@ -33,6 +33,8 @@ void bundle_header_release(struct bundle_header *header)
>> {
>> string_list_clear(&header->prerequisites, 1);
>> string_list_clear(&header->references, 1);
>> + list_objects_filter_release(header->filter);
>> + free(header->filter);
>> }
>>
>> static int parse_capability(struct bundle_header *header, const char *capability)
>> @@ -45,6 +47,11 @@ static int parse_capability(struct bundle_header *header, const char *capability
>> header->hash_algo = &hash_algos[algo];
>> return 0;
>> }
>> + if (skip_prefix(capability, "filter=", &arg)) {
>> + CALLOC_ARRAY(header->filter, 1);
>> + parse_list_objects_filter(header->filter, arg);
>> + return 0;
>> + }
>> return error(_("unknown capability '%s'"), capability);
>> }
>>
>
> Re the comment I had on the v1 about embedding this data in the struct
> instead:
> https://lore.kernel.org/git/220307.86y21lycne.gmgdl@xxxxxxxxxxxxxxxxxxx/
>
> The below diff passes all your tests, i.e. re using NULL as a marker I
> think you may have missed that the API already has a LOFC_DISABLED for
> this (and grepping reveals similar API use of it).
I did miss this LOFC_DISABLED use, which must be the correct way to
interpret an "empty" filter set (re: earlier concerns that a .nr == 0
was used as a BUG() statement in some places).
> I'm not 100% sure it's correct, but if it isn't that's also going to
> suggest missing test coverage in this series.
>
> In any case you want the BUNDLE_HEADER_INIT change, your version is
> buggy in making that header use NODUP strings by hardcoding { 0 }.
Thanks for pointing this out.
Thanks,
-Stolee
