Re: git/git-scm.com GH Issue Tracker

Jeff King [off-list ref] writes:

[1] If we had a more permissive set of defaults, it would probably make
    sense to turn on fsckObjects by default. Some of the checks are
    security-relevant, like disallowing trees with ".GIT",
    "../../etc/passwd", etc. Those _should_ be handled sanely by the
    rest of Git, but it serves as a belt-and-suspenders check, and also
    protects anybody with a buggy Git downstream from you.

Yeah, we really should encourage people to turn it on.  Turning it
on by default is one way to do so, of course.

I think the reason why the transfer side is stricter than the local
checking [*1*] is because the problem in the local history is
already done and there is not much the user can do to fix it, while
objects that originate from outside world could be rejected to keep
the receiving repository clean.

    GitHub has had the feature turned on for ages, with a few caveats:

      - we loosened the zero-padded mode warning, because it was causing
	too many false positives

      - we loosened the timezone checks for the same reason; we've seen
	time zones that aren't exactly 4 characters before

      - we occasionally get complaints from people trying to push old
	histories with bogus committer idents. Usually a missing name or
	similar.

As long as we are sure that modern Git and its reimplementations no
longer create objects with this problems, I think loosening these
checks is perfectly sensible, as the only objects that trigger the
check will be the ones buried deep in the history made back when Git
was young.

Of course, that will make it easier to let broken objects created by
newer reimplementations of Git (and bugs in our code that cause us
to create such broken objects) go unnoticed, so we would want to
keep them at warn (not ignore) for the end-users.

     So those are the ones we'd probably need to loosen off the bat, and
     they're all pretty harmless. But it would be a potential irritating
     regression for somebody if they have a history with other minor
     flaws, and Git suddenly starts refusing to clone it.

[Footnote]

*1* ... which is what Jonathan Nieder originally wondered at
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=743227

On Mon, Feb 06, 2017 at 12:49:34PM -0800, Junio C Hamano wrote:

Of course, that will make it easier to let broken objects created by
newer reimplementations of Git (and bugs in our code that cause us
to create such broken objects) go unnoticed, so we would want to
keep them at warn (not ignore) for the end-users.

Yes, sorry if that wasn't clear. By "loosen" I just meant to warn,
not ignore.

So a viable path forward is perhaps:

  1. Add fetch.fsck.* (and probably transfer.fsck.*) to match the
     receive-pack options.

  2. Go over the current list of default warning/error settings and make
     sure they are sensible.

  3. Add a "light" mode to transfer.fsckObjects and friends that blocks
     only errors, not warnings. Maybe setting the config bool to
     "errors-only" instead of "true" or something.

  4. (Optional) Default transfer.fsckObjects to "errors-only".

     The escape hatch is to set fsckObjects to "false", or to downgrade
     your specific problem to a warning via transfer.fsck.*.

-Peff