Re: [PATCH v2 8/8] builtin/fsck: add `git refs verify` child process

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 3, 2025, 08:40 UTC
Message-ID: <Z6CBC5kyvIhBuLk6@pks.im>
In-Reply-To: <Z5r7NnzvirWEljwV@ArchLinux>

On Thu, Jan 30, 2025 at 12:08:22PM +0800, shejialuo wrote:

Show 17 quoted lines

> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 7a4dcb0716..9a8613d07f 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -905,6 +905,34 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)
>  	return res;
>  }
>  
> +static void fsck_refs(struct repository *r)
> +{
> +	struct child_process refs_verify = CHILD_PROCESS_INIT;
> +	struct progress *progress = NULL;
> +	uint64_t progress_num = 1;
> +
> +	if (show_progress)
> +		progress = start_progress(r, _("Checking ref database"),
> +					  progress_num);

Hm. I don't really think that this progress meter adds anything right now. It only shows either 0 or 1, so it basically only tells you when you're done. And that is something that the user can tell without a progress meter.

Show 27 quoted lines

> +
> +	if (verbose)
> +		fprintf_ln(stderr, _("Checking ref database"));
> +
> +	child_process_init(&refs_verify);
> +	refs_verify.git_cmd = 1;
> +	strvec_pushl(&refs_verify.args, "refs", "verify", NULL);
> +	if (verbose)
> +		strvec_push(&refs_verify.args, "--verbose");
> +	if (check_strict)
> +		strvec_push(&refs_verify.args, "--strict");
> +
> +	if (run_command(&refs_verify))
> +		errors_found |= ERROR_REFS;
> +
> +	display_progress(progress, 1);
> +	stop_progress(&progress);
> +}
> +
>  static char const * const fsck_usage[] = {
>  	N_("git fsck [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]\n"
>  	   "         [--[no-]full] [--strict] [--verbose] [--lost-found]\n"
> @@ -970,6 +998,8 @@ int cmd_fsck(int argc,
>  	git_config(git_fsck_config, &fsck_obj_options);
>  	prepare_repo_settings(the_repository);
>  
> +	fsck_refs(the_repository);

I think there needs to be a way to disable this. How about we add an option `--[no-]references` to do so? I was briefly wondering whether we also want to have `--only-references`, but if a user wants to do that they can simply execute `git refs verify` directly.

Patrick