diff mbox series

[10/10] builtin/fsck: add `git refs verify` child process

Message ID Z3qOM5M1ioZ0Px4T@ArchLinux (mailing list archive)
State New
Headers show
Series add more ref consistency checks | expand

Commit Message

shejialuo Jan. 5, 2025, 1:50 p.m. UTC
At now, we have already implemented the ref consistency checks for both
"files-backend" and "packed-backend". Although we would check some
redundant things, it won't cause trouble. So, let's integrate it into
the "git-fsck(1)" command to get feedback from the users. And also by
calling "git refs verify" in "git-fsck(1)", we make sure that the new
added checks don't break.

Introduce a new function "fsck_refs" that initializes and runs a child
process to execute the "git refs verify" command. In order to provide
the user interface create a progress which makes the total task be 1.
It's hard to know how many loose refs we will check now. We might
improve this later.

And we run this function in the first execution sequence of
"git-fsck(1)" because we don't want the existing code of "git-fsck(1)"
which implicitly checks the consistency of refs to die the program.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 builtin/fsck.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Junio C Hamano Jan. 6, 2025, 10:16 p.m. UTC | #1
shejialuo <shejialuo@gmail.com> writes:

>  builtin/fsck.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 0196c54eb6..a10e52b601 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -902,6 +902,32 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)
>  	return res;
>  }
>  
> +static void fsck_refs(void)
> +{
> +	struct child_process refs_verify = CHILD_PROCESS_INIT;
> +	struct progress *progress = NULL;
> +
> +	if (show_progress)
> +		progress = start_progress(_("Checking ref database"), 1);

This had an obvious semantic conflicts with a topic in flight.

I've resolved it in the latest integration after pushing out the
2.48-rc2 this morning, so there is no need to resend, but please
remember that it would be a possibility to rebase on top of an
updated 'master' *IF* the other topic graduates to 'master' a lot
earlier than this topic hits 'next' (IOW, until that happens there
is no need to rebase).

Thanks.
shejialuo Jan. 7, 2025, noon UTC | #2
On Mon, Jan 06, 2025 at 02:16:22PM -0800, Junio C Hamano wrote:
> shejialuo <shejialuo@gmail.com> writes:
> 
> >  builtin/fsck.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/builtin/fsck.c b/builtin/fsck.c
> > index 0196c54eb6..a10e52b601 100644
> > --- a/builtin/fsck.c
> > +++ b/builtin/fsck.c
> > @@ -902,6 +902,32 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)
> >  	return res;
> >  }
> >  
> > +static void fsck_refs(void)
> > +{
> > +	struct child_process refs_verify = CHILD_PROCESS_INIT;
> > +	struct progress *progress = NULL;
> > +
> > +	if (show_progress)
> > +		progress = start_progress(_("Checking ref database"), 1);
> 
> This had an obvious semantic conflicts with a topic in flight.
> 
> I've resolved it in the latest integration after pushing out the
> 2.48-rc2 this morning, so there is no need to resend, but please
> remember that it would be a possibility to rebase on top of an
> updated 'master' *IF* the other topic graduates to 'master' a lot
> earlier than this topic hits 'next' (IOW, until that happens there
> is no need to rebase).
> 

Thanks for the careful notification. I'll watch this.

> Thanks.

Thanks.
Junio C Hamano Jan. 7, 2025, 3:52 p.m. UTC | #3
shejialuo <shejialuo@gmail.com> writes:

>> I've resolved it in the latest integration after pushing out the
>> 2.48-rc2 this morning, so there is no need to resend, but please
>> remember that it would be a possibility to rebase on top of an
>> updated 'master' *IF* the other topic graduates to 'master' a lot
>> earlier than this topic hits 'next' (IOW, until that happens there
>> is no need to rebase).
>> 
>
> Thanks for the careful notification. I'll watch this.

For future reference and to help those who may be reading from the
sidelines, it is a good practice to see how your topic interacts
with other things in flight by making a trial merge to 'next' and to
'seen'.  It would give you an opportunity to learn about what other
people are actively doing in the project.

Thanks.
diff mbox series

Patch

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0196c54eb6..a10e52b601 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -902,6 +902,32 @@  static int check_pack_rev_indexes(struct repository *r, int show_progress)
 	return res;
 }
 
+static void fsck_refs(void)
+{
+	struct child_process refs_verify = CHILD_PROCESS_INIT;
+	struct progress *progress = NULL;
+
+	if (show_progress)
+		progress = start_progress(_("Checking ref database"), 1);
+
+	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"
@@ -967,6 +993,8 @@  int cmd_fsck(int argc,
 	git_config(git_fsck_config, &fsck_obj_options);
 	prepare_repo_settings(the_repository);
 
+	fsck_refs();
+
 	if (connectivity_only) {
 		for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
 		for_each_packed_object(the_repository,