Re: [PATCH v5 3/4] unbundle: extend options to support object verification
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jun 11, 2024, 09:11 UTC
- Message-ID
- <ZmgUrQ6D1xtQghPy@tanuki>
- In-Reply-To
- <0a18d7839be67d6c0be137c7e15dff9663a161a8.1718088127.git.gitgitgadget@gmail.com>
On Tue, Jun 11, 2024 at 06:42:05AM +0000, Xing Xin via GitGitGadget wrote:
Show 17 quoted lines
> From: Xing Xin <xingxin.xx@bytedance.com> > > This commit extends object verification support in `bundle.c:unbundle` > by adding two new options to `verify_bundle_flags`: > > - `VERIFY_BUNDLE_FSCK_ALWAYS` explicitly enables checks for broken > objects. It will be used to add "--fsck-objects" support for "git > bundle unbundle" in a separate series. > - `VERIFY_BUNDLE_FSCK_FOLLOW_FETCH` is designed to be used during fetch > operations, specifically for direct bundle fetches and _bundle-uri_ > enabled fetches. When enabled, `bundle.c:unbundle` invokes > `fetch-pack.c:fetch_pack_fsck_objects` to determine whether to enable > checks for broken objects. Passing this flag during fetching will be > implemented in a subsequent commit. > > Note that the option `VERIFY_BUNDLE_FSCK_ALWAYS` takes precedence over > `VERIFY_BUNDLE_FSCK_FOLLOW_FETCH`.
Thanks, the new sequence of commits is much easier to follow. It also shows that there is no user of `VERIFY_BUNDLE_FSCK_ALWAYS` at the end of this series. So maybe we should drop that flag?
If you do that, then I'd also propose to merge patches 2 and 3 into one given that both are quite trivial and related to each other.
Other than that this series looks good to me.
Patrick
Show 58 quoted lines
> Reviewed-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>
> ---
> bundle.c | 10 ++++++++++
> bundle.h | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/bundle.c b/bundle.c
> index 95367c2d0a0..53ac73834ea 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -17,6 +17,7 @@
> #include "list-objects-filter-options.h"
> #include "connected.h"
> #include "write-or-die.h"
> +#include "fetch-pack.h"
>
> static const char v2_bundle_signature[] = "# v2 git bundle\n";
> static const char v3_bundle_signature[] = "# v3 git bundle\n";
> @@ -615,6 +616,7 @@ int unbundle(struct repository *r, struct bundle_header *header,
> enum verify_bundle_flags flags)
> {
> struct child_process ip = CHILD_PROCESS_INIT;
> + int fsck_objects = 0;
>
> if (verify_bundle(r, header, flags))
> return -1;
> @@ -625,6 +627,14 @@ int unbundle(struct repository *r, struct bundle_header *header,
> if (header->filter.choice)
> strvec_push(&ip.args, "--promisor=from-bundle");
>
> + if (flags & VERIFY_BUNDLE_FSCK_ALWAYS)
> + fsck_objects = 1;
> + else if (flags & VERIFY_BUNDLE_FSCK_FOLLOW_FETCH)
> + fsck_objects = fetch_pack_fsck_objects();
> +
> + if (fsck_objects)
> + strvec_push(&ip.args, "--fsck-objects");
> +
> if (extra_index_pack_args) {
> strvec_pushv(&ip.args, extra_index_pack_args->v);
> strvec_clear(extra_index_pack_args);
> diff --git a/bundle.h b/bundle.h
> index 021adbdcbb3..a39d8ea1a7e 100644
> --- a/bundle.h
> +++ b/bundle.h
> @@ -33,6 +33,8 @@ int create_bundle(struct repository *r, const char *path,
> enum verify_bundle_flags {
> VERIFY_BUNDLE_VERBOSE = (1 << 0),
> VERIFY_BUNDLE_QUIET = (1 << 1),
> + VERIFY_BUNDLE_FSCK_ALWAYS = (1 << 2),
> + VERIFY_BUNDLE_FSCK_FOLLOW_FETCH = (1 << 3),
> };
>
> int verify_bundle(struct repository *r, struct bundle_header *header,
> --
> gitgitgadget
>