[PATCH 0/4] fsck: doc fixes & fetch.fsck.* implementation

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 24, 2018, 19:35 UTC
Message-ID: <20180524193516.28713-1-avarab@gmail.com>
In-Reply-To: <20180524190214.GA21354@sigill.intra.peff.net>

On Thu, May 24, 2018 at 9:02 PM, Jeff King <peff@peff.net> wrote:

Show 34 quoted lines

> On Thu, May 24, 2018 at 07:04:04PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> That doesn't work, because that's for the server-side, but I need the
>> fetch.fsck.* that doesn't exist. This works (I'll send a better patch
>> with tests / docs etc. soon):
>
> Yeah, I think this is the right direction.
>
>> +static int fetch_pack_config_cb(const char *var, const char *value, void *cb)
>> +{
>> +     if (strcmp(var, "fetch.fsck.skiplist") == 0) {
>> +             const char *path;
>> +
>> +             if (git_config_pathname(&path, var, value))
>> +                     return 1;
>> +             strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
>> +                     fsck_msg_types.len ? ',' : '=', path);
>> +             free((char *)path);
>> +             return 0;
>> +     }
>> +
>> +     if (skip_prefix(var, "fetch.fsck.", &var)) {
>> +             if (is_valid_msg_type(var, value))
>> +                     strbuf_addf(&fsck_msg_types, "%c%s=%s",
>> +                             fsck_msg_types.len ? ',' : '=', var, value);
>> +             else
>> +                     warning("Skipping unknown msg id '%s'", var);
>> +             return 0;
>> +     }
>
> This matches what's in receive-pack, though the way we stuff all of the
> options into a flat string is kind of nasty. I also wonder if we'd
> eventually run up against command-line limits if somebody had a
> complicated fsck config.

Yeah, but I'm leaving this for the future. I doubt that it's going to happen in practice, although if you have a huge skip-list...

> I wonder if we should simply be telling index-pack "please read fsck
> config from the prefix 'fetch.fsck'" instead.

I think this whole notion of reading the same config from two places sucks, and now with my patches it's three. But I can't think of a reasonable way to make it better without even more complexity, and maybe it's better to split it up anyway, i.e. the stuff you want to accept is different that fsck and fetch.

> I dunno, maybe I am just creating work. Certainly I don't think it
> should be a blocker for adding fetch.fsck support. But if you want to
> think about it while you are here or write a patch, I wouldn't complain. :)

Sorry, too late. I already wrote all of this :)

Ævar Arnfjörð Bjarmason (4):
  config doc: don't describe *.fetchObjects twice
  config doc: unify the description of fsck.* and receive.fsck.*
  config doc: elaborate on what transfer.fsckObjects does
  fetch: implement fetch.fsck.*

 Documentation/config.txt        | 113 ++++++++++++++++++++------------
 fetch-pack.c                    |  32 ++++++++-
 t/t5504-fetch-receive-strict.sh |  46 +++++++++++++
 3 files changed, 148 insertions(+), 43 deletions(-)

-- 
2.17.0.290.gded63e768a