Re: [GSoC][PATCH v9 5/9] builtin/refs: add verify subcommand
On Tue, Jul 09, 2024 at 05:30:42PM -0500, Justin Tobler wrote:
Show 8 quoted lines
> On 24/07/09 08:35PM, shejialuo wrote:
> > Introduce a new subcommand "verify" in git-refs(1) to allow the user to
> > check the reference database consistency.
>
> The next patch in the series uses `git-refs verify` to when running
> git-fsck(1). It might be worth mentioning here that this is also
> intended as the entry point for fscking refs.
>
Yes, I will improve the commit message, in the next patch.
Show 27 quoted lines
> >
> > Mentored-by: Patrick Steinhardt <ps@pks.im>
> > Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> > Signed-off-by: shejialuo <shejialuo@gmail.com>
> [snip]
> > @@ -58,15 +63,54 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix)
> > return err;
> > }
> >
> > +static int cmd_refs_verify(int argc, const char **argv, const char *prefix)
> > +{
> > + struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT;
> > + const char * const verify_usage[] = {
> > + REFS_VERIFY_USAGE,
> > + NULL,
> > + };
> > + unsigned int verbose = 0, strict = 0;
> > + struct option options[] = {
> > + OPT__VERBOSE(&verbose, N_("be verbose")),
> > + OPT_BOOL(0, "strict", &strict, N_("enable strict checking")),
> > + OPT_END(),
> > + };
> > + int ret = 0;
>
> nit: Being that we always assign a value to `ret`, defaulting here to
> zero is redundant.
> Show 23 quoted lines
> > +
> > + argc = parse_options(argc, argv, prefix, options, verify_usage, 0);
> > + if (argc)
> > + usage(_("too many arguments"));
> > +
> > + if (verbose)
> > + fsck_refs_options.verbose_refs = 1;
> > + if (strict)
> > + fsck_refs_options.strict = 1;
> > +
> > + git_config(git_fsck_config, &fsck_refs_options);
> > + prepare_repo_settings(the_repository);
> > +
> > + ret = refs_fsck(get_main_ref_store(the_repository), &fsck_refs_options);
> > +
> > + /*
> > + * Explicitly free the allocated array and "oid_skiplist"
>
> s/oid_skiplist/skip_oids
>
> I think we forgot to update this comment after the variable was renamed
> in a previous patch version.
> Yes, thanks for you remind. I have just forgotten here.