shejialuo <shejialuo@xxxxxxxxx> writes: > Subject: Re: [GSoC][PATCH v10 06/10] builtin/refs: add verify subcommand and verbose_refs for "fsck_options" Just saying git refs: add verify subcommand would be clearer. If you really want to talk about two modes, you could say git refs: add "verify [--strict|--verbose]" subcommand but that may be too much. > Introduce a new subcommand "verify" in git-refs(1) to allow the user to > check the reference database consistency and also this subcommand will > be used as the entry point of checking refs for "git-fsck(1)". Last, add > "verbose_refs" field into "fsck_options" to indicate whether we should > print verbose messages when checking refs consistency. Is there a reason why this has to be verbose_refs and not a simple verbose bit? When people see how it is useful to ask for the verbose output while checking refs, wouldn't people wish to add the same "--verbose" support while checking objects, and at that point, wouldn't it be awkward to add verbose_objs member to the struct and having to flip both bits on? > Mentored-by: Patrick Steinhardt <ps@xxxxxx> > Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx> > Signed-off-by: shejialuo <shejialuo@xxxxxxxxx> > --- > Documentation/git-refs.txt | 13 +++++++++++ > builtin/refs.c | 44 ++++++++++++++++++++++++++++++++++++++ > fsck.h | 1 + > 3 files changed, 58 insertions(+) > > diff --git a/Documentation/git-refs.txt b/Documentation/git-refs.txt > index 5b99e04385..1244a85b64 100644 > --- a/Documentation/git-refs.txt > +++ b/Documentation/git-refs.txt > @@ -10,6 +10,7 @@ SYNOPSIS > -------- > [verse] > 'git refs migrate' --ref-format=<format> [--dry-run] > +'git refs verify' [--strict] [--verbose] > > DESCRIPTION > ----------- > @@ -22,6 +23,9 @@ COMMANDS > migrate:: > Migrate ref store between different formats. > > +verify:: > + Verify reference database consistency. > + The error reporting function for refs consistency check was still about reporting a problem for a single ref. I am wondering how consistency violations that are not about a single ref should be handled. For example, if refs/packed-backend.c:packed_fsck() finds that the file is not sorted properly or has some unparseable garbage in it, it is not something you can report as "refs/heads/main is broken", but those who are interested in seeing the "reference database consistency" verified, it is very much what they want the tool to notice. How would detection of such a breakage that is not attributed to a single ref fit in this "ref consistency check infrastructure" that was introduced by [05/10]? > + argc = parse_options(argc, argv, prefix, options, verify_usage, 0); > + if (argc) > + usage(_("too many arguments")); I do not think we want to change this line in this topic, but because I noticed that the issue is widespread, let me make a note here that we may want to clean up all the commands that give this message as a #leftoverbit item: $ git cmd foo baz usage: too many arguments is very unfriendly in that it is not immediately obvious to users which arguments are excess. Should they have given "git cmd<RET>"? Is it "git cmd foo" that does not take any argument? If you said something like $ git refs verify baz error: 'git refs verify' takes no arguments or even $ git refs verify baz error: unknown argument 'baz' given to 'git refs verify' it would be much more helpful. Thanks.