Re: [PATCH v4 2/4] fsck: force core.checksumindex=1

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 3, 2017, 20:31 UTC
Message-ID: <CACBZZX7DFuLia8RzB908EG7+oWQiFGkFEq14bzT77A75msM98Q@mail.gmail.com>
In-Reply-To: <20170403185306.36164-3-git@jeffhostetler.com>

On Mon, Apr 3, 2017 at 8:53 PM,  <git@jeffhostetler.com> wrote:

> Teach fsck to override core.checksumindex and always verify
> the index checksum when reading the index.

Sorry to only chime in about this at v4.

I think this patch & the documentation you added for core.checksumindex in 1/4 would be much less confusing if you made this a on-by-default command-line option like e.g. "full".

With this patch nothing amends the documentation to indicate that the core.checksumindex is magically overridden when fsck runs, I think something like this (needs amending to integrate) on top would make this clearer:

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index b9f060e3b2..19b45b1b6f 100644
--- 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-]checksum-index] [<object>*]

 DESCRIPTION
 -----------
@@ -61,6 +61,11 @@ index file, all SHA-1 references in `refs`
namespace, and all reflogs
        object pools.  This is now default; you can turn it off
        with --no-full.

+--[no-]checksum-index:
+       Validate the checksum at the end of the index file, on by
+       default, locally overrides any "core.checksumIndex" setting
+       unless negated. See linkgit:git-config[1].
+
 --connectivity-only::
        Check only the connectivity of tags, commits and tree objects. By
        avoiding to unpack blobs, this speeds up the operation, at the
diff --git a/builtin/fsck.c b/builtin/fsck.c
index f76e4163ab..8fe8ec1775 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -24,6 +24,7 @@ static int show_tags;
 static int show_unreachable;
 static int include_reflogs = 1;
 static int check_full = 1;
+static int fsck_opt_checksum_index = 1;
 static int connectivity_only;
 static int check_strict;
 static int keep_cache_objects;
@@ -656,6 +657,8 @@ static struct option fsck_opts[] = {
        OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index
objects head nodes")),
        OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head
nodes (default)")),
        OPT_BOOL(0, "full", &check_full, N_("also consider packs and
alternate objects")),
+       OPT_BOOL(0, "checksum-index", &fsck_opt_checksum_index,
+                N_("validate the checksum at the end of the index file")),
        OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check
only connectivity")),
        OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")),
        OPT_BOOL(0, "lost-found", &write_lost_and_found,
@@ -681,6 +684,9 @@ int cmd_fsck(int argc, const char **argv, const
char *prefix)
        if (check_strict)
                fsck_obj_options.strict = 1;

+       if (fsck_opt_checksum_index)
+               force_core_checksum_index = 1;
+
        if (show_progress == -1)
                show_progress = isatty(2);
        if (verbose)