fsck: reject misconfigured fsck.skipList

In Git, fsck operations can ignore known broken objects via the
`fsck.skipList` configuration. This option expects a path to a file with
the list of object names. When the configuration is specified without a
path, an error message is printed, but the command continues as if the
configuration was not set. Configuring `fsck.skipList` without a value
is a misconfiguration so config parsing should be more strict and reject
it.

Update `git_fsck_config()` to no longer ignore misconfiguration of
`fsck.skipList`. The same behavior is also present for
`fetch.fsck.skipList` and `receive.fsck.skipList` so the configuration
parsers for these are updated to ensure the related operations remain
consistent.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c2e9103f112c1e37e9f030308633daf49eec1ecf..0158faf537b7a785ec9a35c274d6d07357fc4512 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -174,7 +174,7 @@ static int receive_pack_config(const char *var, const char *value,
                char *path;
 
                if (git_config_pathname(&path, var, value))
-                       return 1;
+                       return -1;
                strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
                        fsck_msg_types.len ? ',' : '=', path);
                free(path);

diff --git a/fetch-pack.c b/fetch-pack.c
index 3a227721ed0935d1f9c40584c57f54043354c032..055e8c3643015cf39edafa4c1d90cbe6905145f9 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1867,7 +1867,7 @@ int fetch_pack_fsck_config(const char *var, const char *value,
                char *path ;
 
                if (git_config_pathname(&path, var, value))
-                       return 0;
+                       return -1;
                strbuf_addf(msg_types, "%cskiplist=%s",
                        msg_types->len ? ',' : '=', path);
                free(path);

diff --git a/fsck.c b/fsck.c
index 87ce999a49c5480714553ffc990bb2be4688d94f..9fc4c25ffd59bae97690296e5e4528109e71da21 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -1353,7 +1353,7 @@ int git_fsck_config(const char *var, const char *value,
                struct strbuf sb = STRBUF_INIT;
 
                if (git_config_pathname(&path, var, value))
-                       return 1;
+                       return -1;
                strbuf_addf(&sb, "skiplist=%s", path);
                free(path);
                fsck_set_msg_types(options, sb.buf);

diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 8212a70be8ae392f98992862c380c0a9e16f1134..e273ab29c7f88f97a091bf57e90e235fd4f68f29 100755 (executable)
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -167,6 +167,8 @@ test_expect_success 'fsck with unsorted skipList' '
 
 test_expect_success 'fsck with invalid or bogus skipList input' '
        git -c fsck.skipList=/dev/null -c fsck.missingEmail=ignore fsck &&
+       test_must_fail git -c fsck.skipList -c fsck.missingEmail=ignore fsck 2>err &&
+       test_grep "unable to parse '\'fsck.skiplist\'' from command-line config" err &&
        test_must_fail git -c fsck.skipList=does-not-exist -c fsck.missingEmail=ignore fsck 2>err &&
        test_grep "could not open.*: does-not-exist" err &&
        test_must_fail git -c fsck.skipList=.git/config -c fsck.missingEmail=ignore fsck 2>err &&
@@ -213,6 +215,11 @@ test_expect_success 'fsck with exhaustive accepted skipList input (various types
        test_must_be_empty err
 '
 
+test_expect_success 'receive-pack with missing receive.fsck.skipList path' '
+       test_must_fail git -c receive.fsck.skipList receive-pack dst 2>err &&
+       test_grep "unable to parse '\'receive.fsck.skiplist\'' from command-line config" err
+'
+
 test_expect_success 'push with receive.fsck.skipList' '
        git push . $commit:refs/heads/bogus &&
        rm -rf dst &&
@@ -255,6 +262,9 @@ test_expect_success 'fetch with fetch.fsck.skipList' '
        test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec &&
 
        # Invalid and/or bogus skipList input
+       test_must_fail git --git-dir=dst/.git -c fetch.fsck.skipList fetch \
+               "file://$(pwd)" $refspec 2>err &&
+       test_grep "unable to parse '\'fetch.fsck.skiplist\'' from command-line config" err &&
        git --git-dir=dst/.git config fetch.fsck.skipList /dev/null &&
        test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec &&
        git --git-dir=dst/.git config fetch.fsck.skipList does-not-exist &&