git/git-scm.com GH Issue Tracker

I've went through a bunch of open issues on the git/git-scm.com repo (specifically, everything after #600) and I think the bulk of them can be closed.

I've taken the liberty of classifying them as shown below.

- Sam

# Irrelevant but someone should take a look

693

# Irrelevant to git-scm.com and should be closed # Each of these had had someone comment on it saying as much

939 912 906 905 901 896 894 886 885 884 883 879 877 871 868 865 861 840 837 834 828 813 811 807 796 795 790 774 751 748 745 744 729 727 721 719 711 690 686 686 674 673 671 667 660 653 635 631 621 615 613 612 611 610 609 608

# Resolved, duplicate, or non-issue

936 875 863 858 856 839 786 785 761 760 754 752 736 723 720 704 684 683 675 663 662 661 657 651 649 640 637 634 633 628 623 616 614 605 602 601

# Relevant and should be kept open

929 890 859 855 854 826 812 808 804 787 777 768 747 715 703 701 695 694 678 668 665 649 646 639 620 617

Adding Peff to cc as he is the current maintainer of the git-scm.com site/repo.

On Mon, Feb 6, 2017 at 7:15 AM, Samuel Lijin <sxlijin@gmail.com> wrote:

As a community member who cares a lot about that site, thank you! I would love to contribute by reviewing your reviews, but personally can't find the time (focusing free time on podcast production from Git-Merge instead ;)).

On Mon, Feb 6, 2017 at 1:15 PM, Samuel Lijin <sxlijin@gmail.com> wrote:

To save people some time (and since i looked at it anyway), this is about whether "warning in tree xxx: contains zero-padded file modes: from fsck should be a warning or error. It is a warning now even though "git -c transfer.fsckobjects=true clone" treats it as an error. There are some discussions in the past [1] [2] about this.

There's also a question "And I failed to find in the documentation if transfer.fsckobjects could be disabled per repository, can you confirm it's not possible for now ?"

(sorry no answer from me)

[1] http://public-inbox.org/git/%3CCAEBDL5W3DL0v=TusuB7Vg-4bWdAJh5d2Psc1N0Qe+KK3bZH3=Q@mail.gmail.com%3E/ [2] http://public-inbox.org/git/%3C20100326215600.GA10910@spearce.org%3E/

On Mon, Feb 06, 2017 at 05:18:03PM +0700, Duy Nguyen wrote:

The bug that caused the trees is long-fixed. There's a question of how severity levels should be handled in transfer.fsckObjects. By default it treats everything as a reason to reject the object. Dscho added configurable levels a few versions ago. It may be a good idea to tweak the defaults to something more permissive[1].

I don't know why it wouldn't be, though note that it won't override the operation-specific {receive,fetch}.fsckObjects.

-Peff

[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.

    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.

     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.

Jeff King <peff@peff.net> writes:

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.

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.

[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:

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

On Mon, Feb 06, 2017 at 12:15:08AM -0600, Samuel Lijin wrote:

Thanks, this is incredibly helpful. I'll close the appropriate ones you identified.

-Peff

Finished going through and nailed the rest of the open issues!

# Irrelevant but it seems like someone should take a look

511 466

# Irrelevant to git-scm.com and should be closed

599 596 570 565 563 558 538 537 520 511 509 507 501 494 465

# Resolved, duplicate, or non-issue

596 593 592 588 587 585 583 576 575 573 572 547 546 543 540 539 529 521 516 515 504 503 502 496 491 490 476 473 470 467 463 460 456 454 451 413 377 265 257 95

# Relevant and should be kept open

597 595 591 586 578 544 532 518 513 512 500 493 466 448 416 410 381 379 140 13 12 11

That's all of them!

- Sam

On Mon, Feb 6, 2017 at 12:34 PM, Jeff King <peff@peff.net> wrote: