{"thread":{"id":"45057","subject":"git/git-scm.com GH Issue Tracker","startedAt":"2017-02-06T06:16:11Z","lastAt":"2017-02-08T00:34:18Z","messageCount":8,"participants":["Samuel Lijin","Thomas Ferris Nicolaisen","Duy Nguyen","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"310900","messageId":"CAJZjrdX_8tjMhRac9QQOW8m_S2DprFPV=uZp8mFT+g6bASVd-w@mail.gmail.com","threadId":"45057","inReplyTo":null,"subject":"git/git-scm.com GH Issue Tracker","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-02-06T06:15:08Z","receivedAt":"2017-02-06T06:16:11Z","isPatch":false,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"I've went through a bunch of open issues on the git/git-scm.com repo\n(specifically, everything after #600) and I think the bulk of them can\nbe closed.\n\nI've taken the liberty of classifying them as shown below.\n\n- Sam\n\n\n# Irrelevant but someone should take a look\n\n693\n\n\n# Irrelevant to git-scm.com and should be closed\n# Each of these had had someone comment on it saying as much\n\n939 912 906 905 901 896 894 886 885 884 883 879 877 871 868 865 861 840\n837 834 828 813 811 807 796 795 790 774 751 748 745 744 729 727 721 719\n711 690 686 686 674 673 671 667 660 653 635 631 621 615 613 612 611 610\n609 608\n\n\n# Resolved, duplicate, or non-issue\n\n936 875 863 858 856 839 786 785 761 760 754 752 736 723 720 704 684 683\n675 663 662 661 657 651 649 640 637 634 633 628 623 616 614 605 602 601\n\n\n# Relevant and should be kept open\n\n929 890 859 855 854 826 812 808 804 787 777 768 747 715 703 701 695 694\n678 668 665 649 646 639 620 617\n"},{"id":"310902","messageId":"CAEcj5uWZSZ+ujSSoVzpAzmEks1v9eQ1c_Sc8Sc4fUM4cKzM_nA@mail.gmail.com","threadId":"45057","inReplyTo":"CAJZjrdX_8tjMhRac9QQOW8m_S2DprFPV=uZp8mFT+g6bASVd-w@mail.gmail.com","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Thomas Ferris Nicolaisen","fromEmail":"tfnico@gmail.com","sentAt":"2017-02-06T08:59:40Z","receivedAt":"2017-02-06T09:02:51Z","isPatch":false,"sender":{"key":"tfnico@gmail.com","avatar":"https://gravatar.com/avatar/628cf28a25ca4c596c7284562100f70f0ef908bcbfadd4da1eb3d48c23658d01?d=mp&s=160"},"body":"Adding Peff to cc as he is the current maintainer of the git-scm.com site/repo.\n\nOn Mon, Feb 6, 2017 at 7:15 AM, Samuel Lijin wrote:\n>\n> I've taken the liberty of classifying them as shown below.\n>\n\nAs a community member who cares a lot about that site, thank you! I\nwould love to contribute by reviewing your reviews, but personally\ncan't find the time (focusing free time on podcast production from\nGit-Merge instead ;)).\n"},{"id":"310904","messageId":"CACsJy8BCTY=T9f2ac7HUuHA-_RzjaxPHZNPQs9ecBhmsnDuRVQ@mail.gmail.com","threadId":"45057","inReplyTo":"CAJZjrdX_8tjMhRac9QQOW8m_S2DprFPV=uZp8mFT+g6bASVd-w@mail.gmail.com","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-06T10:18:03Z","receivedAt":"2017-02-06T10:18:39Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 6, 2017 at 1:15 PM, Samuel Lijin wrote:\n> # Irrelevant but someone should take a look\n>\n> 693\n\nTo save people some time (and since i looked at it anyway), this is\nabout whether \"warning in tree xxx: contains zero-padded file modes:\nfrom fsck should be a warning or error. It is a warning now even\nthough \"git -c transfer.fsckobjects=true clone\" treats it as an error.\nThere are some discussions in the past [1] [2] about this.\n\nThere's also a question \"And I failed to find in the documentation if\ntransfer.fsckobjects could be disabled per repository, can you confirm\nit's not possible for now ?\"\n\n(sorry no answer from me)\n\n[1] http://public-inbox.org/git/%3CCAEBDL5W3DL0v=TusuB7Vg-4bWdAJh5d2Psc1N0Qe+KK3bZH3=Q@mail.gmail.com%3E/\n[2] http://public-inbox.org/git/%3C20100326215600.GA10910@spearce.org%3E/\n-- \nDuy\n"},{"id":"310932","messageId":"20170206183429.sd7255b3ovvg746c@sigill.intra.peff.net","threadId":"45057","inReplyTo":"CAJZjrdX_8tjMhRac9QQOW8m_S2DprFPV=uZp8mFT+g6bASVd-w@mail.gmail.com","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T18:34:30Z","receivedAt":"2017-02-06T18:34:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 06, 2017 at 12:15:08AM -0600, Samuel Lijin wrote:\n\n> I've went through a bunch of open issues on the git/git-scm.com repo\n> (specifically, everything after #600) and I think the bulk of them can\n> be closed.\n> \n> I've taken the liberty of classifying them as shown below.\n\nThanks, this is incredibly helpful. I'll close the appropriate ones you\nidentified.\n\n-Peff\n"},{"id":"310933","messageId":"20170206184951.vwq6ib2qzxc542i4@sigill.intra.peff.net","threadId":"45057","inReplyTo":"CACsJy8BCTY=T9f2ac7HUuHA-_RzjaxPHZNPQs9ecBhmsnDuRVQ@mail.gmail.com","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T18:49:51Z","receivedAt":"2017-02-06T18:50:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 06, 2017 at 05:18:03PM +0700, Duy Nguyen wrote:\n\n> On Mon, Feb 6, 2017 at 1:15 PM, Samuel Lijin wrote:\n> > # Irrelevant but someone should take a look\n> >\n> > 693\n> \n> To save people some time (and since i looked at it anyway), this is\n> about whether \"warning in tree xxx: contains zero-padded file modes:\n> from fsck should be a warning or error. It is a warning now even\n> though \"git -c transfer.fsckobjects=true clone\" treats it as an error.\n> There are some discussions in the past [1] [2] about this.\n\nThe bug that caused the trees is long-fixed. There's a question of\nhow severity levels should be handled in transfer.fsckObjects. By\ndefault it treats everything as a reason to reject the object. Dscho\nadded configurable levels a few versions ago. It may be a good idea to\ntweak the defaults to something more permissive[1].\n\n> There's also a question \"And I failed to find in the documentation if\n> transfer.fsckobjects could be disabled per repository, can you confirm\n> it's not possible for now ?\"\n\nI don't know why it wouldn't be, though note that it won't override\nthe operation-specific {receive,fetch}.fsckObjects.\n\n-Peff\n\n[1] If we had a more permissive set of defaults, it would probably make\n sense to turn on fsckObjects by default. Some of the checks are\n security-relevant, like disallowing trees with \".GIT\",\n \"../../etc/passwd\", etc. Those _should_ be handled sanely by the\n rest of Git, but it serves as a belt-and-suspenders check, and also\n protects anybody with a buggy Git downstream from you.\n\n GitHub has had the feature turned on for ages, with a few caveats:\n\n - we loosened the zero-padded mode warning, because it was causing\n\ttoo many false positives\n\n - we loosened the timezone checks for the same reason; we've seen\n\ttime zones that aren't exactly 4 characters before\n\n - we occasionally get complaints from people trying to push old\n\thistories with bogus committer idents. Usually a missing name or\n\tsimilar.\n\n So those are the ones we'd probably need to loosen off the bat, and\n they're all pretty harmless. But it would be a potential irritating\n regression for somebody if they have a history with other minor\n flaws, and Git suddenly starts refusing to clone it.\n"},{"id":"310943","messageId":"xmqqlgtj11up.fsf@gitster.mtv.corp.google.com","threadId":"45057","inReplyTo":"20170206184951.vwq6ib2qzxc542i4@sigill.intra.peff.net","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-06T20:49:34Z","receivedAt":"2017-02-06T20:49:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> [1] If we had a more permissive set of defaults, it would probably make\n> sense to turn on fsckObjects by default. Some of the checks are\n> security-relevant, like disallowing trees with \".GIT\",\n> \"../../etc/passwd\", etc. Those _should_ be handled sanely by the\n> rest of Git, but it serves as a belt-and-suspenders check, and also\n> protects anybody with a buggy Git downstream from you.\n\nYeah, we really should encourage people to turn it on. Turning it\non by default is one way to do so, of course.\n\nI think the reason why the transfer side is stricter than the local\nchecking [*1*] is because the problem in the local history is\nalready done and there is not much the user can do to fix it, while\nobjects that originate from outside world could be rejected to keep\nthe receiving repository clean.\n\n\n> GitHub has had the feature turned on for ages, with a few caveats:\n>\n> - we loosened the zero-padded mode warning, because it was causing\n> \ttoo many false positives\n>\n> - we loosened the timezone checks for the same reason; we've seen\n> \ttime zones that aren't exactly 4 characters before\n>\n> - we occasionally get complaints from people trying to push old\n> \thistories with bogus committer idents. Usually a missing name or\n> \tsimilar.\n\nAs long as we are sure that modern Git and its reimplementations no\nlonger create objects with this problems, I think loosening these\nchecks is perfectly sensible, as the only objects that trigger the\ncheck will be the ones buried deep in the history made back when Git\nwas young.\n\nOf course, that will make it easier to let broken objects created by\nnewer reimplementations of Git (and bugs in our code that cause us\nto create such broken objects) go unnoticed, so we would want to\nkeep them at warn (not ignore) for the end-users.\n\n> So those are the ones we'd probably need to loosen off the bat, and\n> they're all pretty harmless. But it would be a potential irritating\n> regression for somebody if they have a history with other minor\n> flaws, and Git suddenly starts refusing to clone it.\n\n[Footnote]\n\n*1* ... which is what Jonathan Nieder originally wondered at\n https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=743227\n"},{"id":"310946","messageId":"20170206214507.7l5haqbimmwdwfm3@sigill.intra.peff.net","threadId":"45057","inReplyTo":"xmqqlgtj11up.fsf@gitster.mtv.corp.google.com","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T21:45:08Z","receivedAt":"2017-02-06T21:45:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 06, 2017 at 12:49:34PM -0800, Junio C Hamano wrote:\n\n> Of course, that will make it easier to let broken objects created by\n> newer reimplementations of Git (and bugs in our code that cause us\n> to create such broken objects) go unnoticed, so we would want to\n> keep them at warn (not ignore) for the end-users.\n\nYes, sorry if that wasn't clear. By \"loosen\" I just meant to warn,\nnot ignore.\n\nSo a viable path forward is perhaps:\n\n 1. Add fetch.fsck.* (and probably transfer.fsck.*) to match the\n receive-pack options.\n\n 2. Go over the current list of default warning/error settings and make\n sure they are sensible.\n\n 3. Add a \"light\" mode to transfer.fsckObjects and friends that blocks\n only errors, not warnings. Maybe setting the config bool to\n \"errors-only\" instead of \"true\" or something.\n\n 4. (Optional) Default transfer.fsckObjects to \"errors-only\".\n\n The escape hatch is to set fsckObjects to \"false\", or to downgrade\n your specific problem to a warning via transfer.fsck.*.\n\n-Peff\n"},{"id":"311019","messageId":"CAJZjrdU-UsWpNvjQ=eZi1wMUSWxRXLr5YNsgAce=z7Zt3yCTSA@mail.gmail.com","threadId":"45057","inReplyTo":"20170206183429.sd7255b3ovvg746c@sigill.intra.peff.net","subject":"Re: git/git-scm.com GH Issue Tracker","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-02-08T00:33:29Z","receivedAt":"2017-02-08T00:34:18Z","isPatch":false,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Finished going through and nailed the rest of the open issues!\n\n\n# Irrelevant but it seems like someone should take a look\n\n511 466\n\n\n# Irrelevant to git-scm.com and should be closed\n\n599 596 570 565 563 558 538 537 520 511 509 507 501 494 465\n\n\n# Resolved, duplicate, or non-issue\n\n596 593 592 588 587 585 583 576 575 573 572 547 546 543 540 539 529 521\n516 515 504 503 502 496 491 490 476 473 470 467 463 460 456 454 451 413\n377 265 257 95\n\n\n\n# Relevant and should be kept open\n\n597 595 591 586 578 544 532 518 513 512 500 493 466 448 416 410 381 379\n140 13 12 11\n\n\nThat's all of them!\n\n- Sam\n\nOn Mon, Feb 6, 2017 at 12:34 PM, Jeff King wrote:\n> On Mon, Feb 06, 2017 at 12:15:08AM -0600, Samuel Lijin wrote:\n>\n>> I've went through a bunch of open issues on the git/git-scm.com repo\n>> (specifically, everything after #600) and I think the bulk of them can\n>> be closed.\n>>\n>> I've taken the liberty of classifying them as shown below.\n>\n> Thanks, this is incredibly helpful. I'll close the appropriate ones you\n> identified.\n>\n> -Peff\n"}]}