Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
> > There are legacy repositories out there whose older commits and tags
> > have issues that prevent pushing them when 'receive.fsckObjects' is set.
> > One real-life example is a commit object that has been hand-crafted to
> > list two authors.
> >
> > Often, it is not possible to fix those issues without disrupting the
> > work with said repositories, yet it is still desirable to perform checks
> > by setting `receive.fsckObjects = true`. This commit is the first step
> > to allow demoting specific fsck issues to mere warnings.
> >
> > The function added by this commit parses a list of settings in the form:
> >
> > missing-email=warn,bad-name=warn,...
> >
> > Unfortunately, the FSCK_WARN/FSCK_ERROR flag is only really heeded by
> > git fsck so far, but other call paths (e.g. git index-pack --strict)
> > error out *always* no matter what type was specified. Therefore, we
> > need to take extra care to default to all FSCK_ERROR in those cases.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > fsck.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > fsck.h | 7 +++++--
> > 2 files changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git a/fsck.c b/fsck.c
> > index 05b146c..9e6d70f 100644
> > --- a/fsck.c
> > +++ b/fsck.c
> > @@ -97,9 +97,63 @@ static int parse_msg_id(const char *text, int len)
> >
> > int fsck_msg_type(enum fsck_msg_id msg_id, struct fsck_options *options)
> > {
> > + if (options->strict_mode && msg_id >= 0 && msg_id < FSCK_MSG_MAX)
> > + return options->strict_mode[msg_id];
> > + if (options->strict)
> > + return FSCK_ERROR;
> > return msg_id < FIRST_WARNING ? FSCK_ERROR : FSCK_WARN;
> > }
>
> Hmm, if you are later going to allow demoting (hopefully also promoting)
> error to warn, etc., would the comparison between msg_id and FIRST_WARNING
> make much sense?
A later patch indeed adds that option. The reason the comparison still
makes sense is that the pure infos do not return at all so far, but all of
the reported warnings are fatal in strict mode (i.e. when
receive.fsckObjects = true). In another later patch it is made possible to
promote even infos (such as 'missing tagger') to warnings or even errors,
and that is when the "return FSCK_ERROR" is changed to "return msg_id <
FIRST_INFO ? FSCK_ERROR : FSCK_WARN".
> In other words, at some point wouldn't we be better off with
> something like this
>
> struct {
> enum id;
> const char *id_string;
> enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
> } possible_fsck_errors[];
I considered that, and Michael Haggerty also suggested that in a private
mail. However, I find that there is a clear hierarchy in the default
messages: fatal errors, errors, warnings and infos. This should be
reflected by the order IMHO.
But I guess it would make a lot of sense to insert those levels as special
enum values to make it harder to forget to adjust, say, "#define
FIRST_WARNING FSCK_MSG_BAD_FILEMODE" when introducing another warning that
sorts before said ID alphabetically. In other words, I think that we can
really afford to put something like
...
FUNC(UNKNOWN_TYPE) \
FUNC(ZERO_PADDED_DATE) \
FUNC(___WARNINGS) \
FUNC(BAD_FILEMODE) \
FUNC(EMPTY_NAME) \
...
at the price of making the parsing a little more complicated and wasting a
slight bit of more space for the msg_id_str array.
What do you think?
Ciao,
Dscho
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html