builtin/fsck: add `git refs verify` child process

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.

Then, introduce the option to allow the user to disable checking ref
database consistency. Put this function in the very first execution
sequence of "git-fsck(1)" due to that we don't want the existing code of
"git-fsck(1)" which would implicitly check the consistency of refs to
die the program.

Last, update the test to exercise the code.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 5b82e4605c2e91dc647330409af81728b665a80a..5e71a29c3b02d9dd29ec1ab16f261f8486cddd0f 100644 (file)
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
         [--[no-]full] [--strict] [--verbose] [--lost-found]
         [--[no-]dangling] [--[no-]progress] [--connectivity-only]
-        [--[no-]name-objects] [<object>...]
+        [--[no-]name-objects] [--[no-]references] [<object>...]
 
 DESCRIPTION
 -----------
@@ -104,6 +104,11 @@ care about this output and want to speed it up further.
        progress status even if the standard error stream is not
        directed to a terminal.
 
+--[no-]references::
+       Control whether to check the references database consistency
+       via 'git refs verify'. See linkgit:git-refs[1] for details.
+       The default is to check the references database.
+
 CONFIGURATION
 -------------
 

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0196c54eb68ee54c22de72d64b3f31602594e50b..620efe8368e1482e893bd78244dbef75c2a4462f 100644 (file)
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -50,6 +50,7 @@ static int verbose;
 static int show_progress = -1;
 static int show_dangling = 1;
 static int name_objects;
+static int check_references = 1;
 #define ERROR_OBJECT 01
 #define ERROR_REACHABLE 02
 #define ERROR_PACK 04
@@ -902,11 +903,37 @@ 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;
+
+       if (show_progress)
+               progress = start_progress(r, _("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"
           "         [--[no-]dangling] [--[no-]progress] [--connectivity-only]\n"
-          "         [--[no-]name-objects] [<object>...]"),
+          "         [--[no-]name-objects] [--[no-]references] [<object>...]"),
        NULL
 };
 
@@ -925,6 +952,7 @@ static struct option fsck_opts[] = {
                                N_("write dangling objects in .git/lost-found")),
        OPT_BOOL(0, "progress", &show_progress, N_("show progress")),
        OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")),
+       OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")),
        OPT_END(),
 };
 
@@ -967,6 +995,9 @@ int cmd_fsck(int argc,
        git_config(git_fsck_config, &fsck_obj_options);
        prepare_repo_settings(the_repository);
 
+       if (check_references)
+               fsck_refs(the_repository);
+
        if (connectivity_only) {
                for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
                for_each_packed_object(the_repository,

diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 767e2bd4a0640a2116ad221d6d378042f0a43c1d..9d1dc2144c4b7259bc578e9630923e96c1c98a3e 100755 (executable)
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -830,4 +830,43 @@ test_expect_success 'packed-ref without sorted trait should not be checked' '
        )
 '
 
+test_expect_success '--[no-]references option should apply to fsck' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       branch_dir_prefix=.git/refs/heads &&
+       (
+               cd repo &&
+               test_commit default &&
+               for trailing_content in " garbage" "    more garbage"
+               do
+                       printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage &&
+                       git fsck 2>err &&
+                       cat >expect <<-EOF &&
+                       warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\''
+                       EOF
+                       rm $branch_dir_prefix/branch-garbage &&
+                       test_cmp expect err || return 1
+               done &&
+
+               for trailing_content in " garbage" "    more garbage"
+               do
+                       printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage &&
+                       git fsck --references 2>err &&
+                       cat >expect <<-EOF &&
+                       warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\''
+                       EOF
+                       rm $branch_dir_prefix/branch-garbage &&
+                       test_cmp expect err || return 1
+               done &&
+
+               for trailing_content in " garbage" "    more garbage"
+               do
+                       printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage &&
+                       git fsck --no-references 2>err &&
+                       rm $branch_dir_prefix/branch-garbage &&
+                       test_must_be_empty err || return 1
+               done
+       )
+'
+
 test_done