Re: [PATCH 4/4] fetch: implement fetch.fsck.*
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 25, 2018, 04:09 UTC
- Message-ID
- <xmqqy3g8nzph.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180524193516.28713-5-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 8 quoted lines
> fsck.skipList:: > - Like `fsck.<msg-id>` this variable has a corresponding > - `receive.fsck.skipList` variant. > + Like `fsck.<msg-id>` this variable has corresponding > + `receive.fsck.skipList` and `fetch.fsck.skipList` variants. > + > The path to a sorted list of object names (i.e. one SHA-1 per line) > that are known to be broken in a non-fatal way and should be
I think I've said this already, but I tend to agree with Eric that this is the other way around. Perhaps that is because I consider fsck.<var> the most basic one people would want to understand first, and then corresponding <command>.fsck.<var> a mere specialization of it. So "Here is what fsck.skipList does" followed by "By the way, you can configure it only for the (internal) fsck run during the object transfer with transfer.fsck.skipList" feels more natural presentation order.
Show 27 quoted lines
> diff --git a/fetch-pack.c b/fetch-pack.c > index 490c38f833..9e4282788e 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -19,6 +19,7 @@ > #include "sha1-array.h" > #include "oidset.h" > #include "packfile.h" > +#include "fsck.h" > > static int transfer_unpack_limit = -1; > static int fetch_unpack_limit = -1; > @@ -33,6 +34,7 @@ static int agent_supported; > static int server_supports_filtering; > static struct lock_file shallow_lock; > static const char *alternate_shallow_file; > +static struct strbuf fsck_msg_types = STRBUF_INIT; > > /* Remember to update object flag allocation in object.h */ > #define COMPLETE (1U << 0) > @@ -935,7 +937,8 @@ static int get_pack(struct fetch_pack_args *args, > */ > argv_array_push(&cmd.args, "--fsck-objects"); > else > - argv_array_push(&cmd.args, "--strict"); > + argv_array_pushf(&cmd.args, "--strict%s", > + fsck_msg_types.buf);
This made a reader wonder what fsck_msg_types.buf[] has.
It is empty or a comma separated list of things, prefixed with =, that is constructed by repeated calls to fetch_pack_config_cb(), so syntactically what we feed index-pack looks like "--strict", "--strict=thing", or "--strict=thing1,thing2,....,thingn". And each "thing" is either "<msgtype>=<value>" or "skiplist=<objectname>".
The buffer that has both msgtype specification and object name should not be called fsck_msg_types, though. It is probably fsck_exception or something.
Show 17 quoted lines
> + 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;
> + }
> +
> + return git_default_config(var, value, cb);
> +}