[v4,2/4] unbundle: extend verify_bundle_flags to support fsck-objects

Commit Message

Xing Xin May 30, 2024, 8:21 a.m. UTC

From: Xing Xin <[e-mail]>

This commit extends `verify_bundle_flags` by adding a new option
`VERIFY_BUNDLE_FSCK_ALWAYS`, which enables checks for broken objects in
`bundle.c:unbundle`. This option is now used as the default for fetches
involving bundles, specifically by `transport.c:fetch_refs_from_bundle`
for direct bundle fetches and by `bundle-uri.c:unbundle_from_file` for
_bundle-uri_ enabled fetches.

Upcoming commits will introduce another option as a replacement that
fits better with fetch operations. `VERIFY_BUNDLE_FSCK_ALWAYS` will be
further used to add "--fsck-objects" support for "git bundle unbundle"
and "git bundle verify".

Reviewed-by: Patrick Steinhardt <[e-mail]>
Signed-off-by: Xing Xin <[e-mail]>
---
 bundle-uri.c | 2 +-
 bundle.c     | 3 +++
 bundle.h     | 1 +
 transport.c  | 2 +-
 4 files changed, 6 insertions(+), 2 deletions(-)

Comments

Patrick Steinhardt June 6, 2024, 12:06 p.m. UTC | #1

On Thu, May 30, 2024 at 08:21:28AM +0000, Xing Xin via GitGitGadget wrote:
> From: Xing Xin <[e-mail]>

Tiny nit: the important change in this commit is not that you wire up
the flag, but rather that we start to execute git-fsck(1) now. I'd thus
propose to adapt the commit title accordingly.

Patrick

Xing Xin June 11, 2024, 6:46 a.m. UTC | #2

At 2024-06-06 20:06:41, "Patrick Steinhardt" <[e-mail]> wrote:
>On Thu, May 30, 2024 at 08:21:28AM +0000, Xing Xin via GitGitGadget wrote:
>> From: Xing Xin <[e-mail]>
>
>Tiny nit: the important change in this commit is not that you wire up
>the flag, but rather that we start to execute git-fsck(1) now. I'd thus

To be precise, it is adding a "--fsck-objects" flag to "git-index-pack".

>propose to adapt the commit title accordingly.

This commit is mapped to [PATCH v5 3/4] due to some adjustments to the
commit order and implementation details. Hope the new title can better
describe the new patch.

Xing Xin

Patch

diff --git a/bundle-uri.c b/bundle-uri.c
index 65666a11d9c..066ff788104 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -373,7 +373,7 @@  static int unbundle_from_file(struct repository *r, const char *file)
 	 * the prerequisite commits.
 	 */
 	if ((result = unbundle(r, &header, bundle_fd, NULL,
-			       VERIFY_BUNDLE_QUIET)))
+			       VERIFY_BUNDLE_QUIET | VERIFY_BUNDLE_FSCK_ALWAYS)))
 		return 1;
 
 	/*
diff --git a/bundle.c b/bundle.c
index 95367c2d0a0..26574e74bdd 100644
--- a/bundle.c
+++ b/bundle.c
@@ -625,6 +625,9 @@  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)
+		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..cf23c8615d3 100644
--- a/bundle.h
+++ b/bundle.h
@@ -33,6 +33,7 @@  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),
 };
 
 int verify_bundle(struct repository *r, struct bundle_header *header,
diff --git a/transport.c b/transport.c
index 0ad04b77fd2..1b3d61ffcec 100644
--- a/transport.c
+++ b/transport.c
@@ -184,7 +184,7 @@  static int fetch_refs_from_bundle(struct transport *transport,
 	if (!data->get_refs_from_bundle_called)
 		get_refs_from_bundle_inner(transport);
 	ret = unbundle(the_repository, &data->header, data->fd,
-		       &extra_index_pack_args, 0);
+		       &extra_index_pack_args, VERIFY_BUNDLE_FSCK_ALWAYS);
 	transport->hash_algo = data->header.hash_algo;
 	return ret;
 }