Subject: Re: [PATCH 18/18] git receive-pack: support excluding objects from fsck'ing

From: Junio C Hamano <[email-masked]>
Date: Wed, 10 Dec 2014 10:23:25 -0800

Johannes Schindelin <[email-masked]> writes:

> The optional new config option `receive.fsck.skip-list` specifies the path
> to a file listing the names, i.e. SHA-1s, one per line, of objects that
> are to be ignored by `git receive-pack` when `receive.fsckObjects = true`.
>
> This is extremely handy in case of legacy repositories where it would
> cause more pain to change incorrect objects than to live with them
> (e.g. a duplicate 'author' line in an early commit object).
>
> The intended use case is for server administrators to inspect objects
> that are reported by `git push` as being too problematic to enter the
> repository, and to add the objects' SHA-1 to a (preferably sorted) file
> when the objects are legitimate, i.e. when it is determined that those
> problematic objects should be allowed to enter the server.
>
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---
>  builtin/receive-pack.c          |  9 +++++++
>  fsck.c                          | 59 +++++++++++++++++++++++++++++++++++++++--
>  fsck.h                          |  2 ++
>  t/t5504-fetch-receive-strict.sh | 12 +++++++++
>  4 files changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 111e514..5169f1f 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -110,6 +110,15 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  
> +	if (starts_with(var, "receive.fsck.skip-list")) {

s/skip-list/skiplist/;

> +		const char *path = is_absolute_path(value) ?
> +			value : git_path("%s", value);
> +		if (fsck_strict_mode.len)
> +			strbuf_addch(&fsck_strict_mode, ',');
> +		strbuf_addf(&fsck_strict_mode, "skip-list=%s", path);
> +		return 0;
> +	}
> +
>  	if (starts_with(var, "receive.fsck.")) {
>  		if (fsck_strict_mode.len)
>  			strbuf_addch(&fsck_strict_mode, ',');
> diff --git a/fsck.c b/fsck.c
> index 154f361..00693f2 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -7,6 +7,7 @@
>  #include "tag.h"
>  #include "fsck.h"
>  #include "refs.h"
> +#include "sha1-array.h"
>  
>  #define FOREACH_MSG_ID(FUNC) \
>  	/* fatal errors */ \
> @@ -56,7 +57,9 @@
>  	FUNC(ZERO_PADDED_FILEMODE) \
>  	/* infos (reported as warnings, but ignored by default) */ \
>  	FUNC(INVALID_TAG_NAME) \
> -	FUNC(MISSING_TAGGER_ENTRY)
> +	FUNC(MISSING_TAGGER_ENTRY) \
> +	/* special value */ \
> +	FUNC(SKIP_LIST)

This feels like a kludge to me without comment on what "special
value" means.  Does it mean "this object has an error (which by
default is ignored) of being on the skip list?"  Should we be able
to optionally warn an object on the skip-list exists with the same
mechansim the rest of the series uses to tweak the error level?