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'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 }.
diff --git a/builtin/clone.c b/builtin/clone.c
index 52e50f17baf..000379eea7f 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1172,9 +1172,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
transport->cloning = 1;
if (is_bundle) {
- struct bundle_header header = { 0 };
+ struct bundle_header header = BUNDLE_HEADER_INIT;
int fd = read_bundle_header(path, &header);
- int has_filter = !!header.filter;
+ int has_filter = header.filter.choice != LOFC_DISABLED;
if (fd > 0)
close(fd);
diff --git a/bundle.c b/bundle.c
index 9ca6a7eb1c2..3846108f7a6 100644
--- a/bundle.c
+++ b/bundle.c
@@ -33,8 +33,7 @@ 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);
+ list_objects_filter_release(&header->filter);
}
static int parse_capability(struct bundle_header *header, const char *capability)
@@ -48,8 +47,7 @@ static int parse_capability(struct bundle_header *header, const char *capability
return 0;
}
if (skip_prefix(capability, "filter=", &arg)) {
- CALLOC_ARRAY(header->filter, 1);
- parse_list_objects_filter(header->filter, arg);
+ parse_list_objects_filter(&header->filter, arg);
return 0;
}
return error(_("unknown capability '%s'"), capability);
@@ -227,7 +225,7 @@ int verify_bundle(struct repository *r,
req_nr = revs.pending.nr;
setup_revisions(2, argv, &revs, NULL);
- revs.filter = header->filter;
+ revs.filter = &header->filter;
if (prepare_revision_walk(&revs))
die(_("revision walk setup failed"));
@@ -269,9 +267,9 @@ int verify_bundle(struct repository *r,
r->nr);
list_refs(r, 0, NULL);
- if (header->filter) {
+ if (header->filter.choice != LOFC_DISABLED) {
printf_ln("The bundle uses this filter: %s",
- list_objects_filter_spec(header->filter));
+ list_objects_filter_spec(&header->filter));
}
r = &header->prerequisites;
@@ -631,7 +629,7 @@ int unbundle(struct repository *r, struct bundle_header *header,
strvec_pushl(&ip.args, "index-pack", "--fix-thin", "--stdin", NULL);
/* If there is a filter, then we need to create the promisor pack. */
- if (header->filter)
+ if (header->filter.choice != LOFC_DISABLED)
strvec_push(&ip.args, "--promisor=from-bundle");
if (extra_index_pack_args) {
diff --git a/bundle.h b/bundle.h
index eb026153d56..7fef2108f43 100644
--- a/bundle.h
+++ b/bundle.h
@@ -4,15 +4,14 @@
#include "strvec.h"
#include "cache.h"
#include "string-list.h"
-
-struct list_objects_filter_options;
+#include "list-objects-filter-options.h"
struct bundle_header {
unsigned version;
struct string_list prerequisites;
struct string_list references;
const struct git_hash_algo *hash_algo;
- struct list_objects_filter_options *filter;
+ struct list_objects_filter_options filter;
};
#define BUNDLE_HEADER_INIT \
