Re: [PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

From: Jeff King <peff@peff.net>
Date: Jul 16, 2018, 18:30 UTC
Message-ID: <20180716183033.GA22298@sigill.intra.peff.net>
In-Reply-To: <xmqqwotvt5uz.fsf@gitster-ct.c.googlers.com>

On Mon, Jul 16, 2018 at 11:04:04AM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> >    site's support). And the broken .gitmodules may be too
> >    far back in history for rewriting to be feasible (again,
> >    this is an issue for cgit).
> 
> "again" but this is the first mention that hints cgit has some
> problem (but not exactly what problem).  Is that the "cgit has a
> file called .gitmodules that predates the submodule support on our
> side?" thing?

I think you missed it. In the paragraph above the one you quoted, I said:

   The cgit repository, for example, has a file named
   .gitmodules from a pre-submodule attempt at sharing code,
   but does not actually have any gitlinks.

Show 19 quoted lines

> > So we're being unnecessarily restrictive without actually
> > improving the security in a meaningful way. It would be more
> > convenient to downgrade this check to "info", which means
> > we'd still comment on it, but not reject a push. Site admins
> > can already do this via config, but we should ship sensible
> > defaults.
> > ...
> > Considering both sets of arguments, it makes sense to loosen
> > this check for now.
> >
> > Note that we have to tweak the test in t7415 since fsck will
> > no longer consider this a fatal error. But we still check
> > that it reports the warning, and that we don't get the
> > spurious error from the config code.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> 
> Thanks.

So I'm curious if you found the argument in my commit message compelling. :)

My recollection from the earlier discussion was that you were more in favor of keeping things tight. E.g.,:

  https://public-inbox.org/git/xmqqh8lgrz5c.fsf@gitster-ct.c.googlers.com/

but reading it again:

 - there we were talking about non-blob objects as
   .gitmodules

 - I think your main concern was that there be a way for the
   user to loosen/tighten, which there is.

-Peff