Re:Re: [PATCH v3 2/4] unbundle: introduce unbundle_fsck_flags for fsckobjects handling

From: Xing Xin <bupt_xingxin@163.com>
Date: May 29, 2024, 18:12 UTC
Message-ID: <1d097be.b642.18fc58dfb70.Coremail.bupt_xingxin@163.com>
In-Reply-To: <ZlXIDXfc_1lGb51J@tanuki>

At 2024-05-28 20:03:25, "Patrick Steinhardt" <ps@pks.im> wrote:

Show 21 quoted lines

>On Mon, May 27, 2024 at 03:41:55PM +0000, Xing Xin via GitGitGadget wrote:
>[snip]
>> diff --git a/bundle.h b/bundle.h
>> index 021adbdcbb3..cfa9daddda6 100644
>> --- a/bundle.h
>> +++ b/bundle.h
>> @@ -30,6 +30,11 @@ int create_bundle(struct repository *r, const char *path,
>>  		  int argc, const char **argv, struct strvec *pack_options,
>>  		  int version);
>>  
>> +enum unbundle_fsck_flags {
>> +	UNBUNDLE_FSCK_NEVER = 0,
>> +	UNBUNDLE_FSCK_ALWAYS,
>> +};
>> +
>>  enum verify_bundle_flags {
>>  	VERIFY_BUNDLE_VERBOSE = (1 << 0),
>>  	VERIFY_BUNDLE_QUIET = (1 << 1),
>
>Wouldn't this have been a natural fit for the new flag, e.g. via
>something like `VERIFY_BUNDLE_FSCK`?

It makes sense to me. Currently, verify_bundle_flags controls the amount of information displayed when checking a bundle's prerequisites. The newly added unbundle_fsck_flags is designed to check for broken objects during the unbundle process, which is essentially a form of bundle verification. I believe we should extend some object verification capabilities to the git bundle verify command as well, perhaps by adding a --fsck-objects option.

With this in mind, I support adding new options to verify_bundle_flags. Since bundle.c:unbundle needs to combine multiple options, we must define new options using bitwise shifting:

	enum verify_bundle_flags {
		VERIFY_BUNDLE_VERBOSE = (1 << 0),
		VERIFY_BUNDLE_QUIET = (1 << 1),
		VERIFY_BUNDLE_FSCK_OBJECTS_ALWAYS = (1 << 2),
		VERIFY_BUNDLE_FSCK_OBJECTS_FOLLOW_FETCH = (1 << 3),
	};

How about the naming? I'm not very good at naming :)

Xing Xin