{"thread":{"id":"32232","subject":"[PATCH] fsck: warn about \".git\" in trees","startedAt":"2012-11-28T21:35:29Z","lastAt":"2012-12-04T10:40:06Z","messageCount":4,"participants":["Jeff King","Torsten Bögershausen","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"204208","messageId":"20121128213529.GA16518@sigill.intra.peff.net","threadId":"32232","inReplyTo":null,"subject":"[PATCH] fsck: warn about \".git\" in trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-28T21:35:29Z","receivedAt":"2012-11-28T21:35:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2012 at 01:25:05PM -0800, Junio C Hamano wrote:\n\n> >> * jk/fsck-dot-in-trees (2012-11-28) 1 commit\n> >>  - fsck: warn about '.' and '..' in trees\n> >> \n> >>  Will merge to 'next'.\n> >\n> > Do you have an opinion on warning about '.git', as well? It probably\n> > would make more sense as a patch on top, but I thought I'd ask before\n> > this got merged to next.\n> \n> Yeah, it would make sense to reject what we would not record\n> ourselves when the tools are used in a sane manner.\n\nHere's the patch on top of jk/fsck-dot-in-trees.\n\n-- >8 --\nSubject: [PATCH] fsck: warn about \".git\" in trees\n\nHaving a \".git\" entry inside a tree can cause confusing\nresults on checkout. At the top-level, you could not\ncheckout such a tree, as it would complain about overwriting\nthe real \".git\" directory. In a subdirectory, you might\ncheck it out, but performing operations in the subdirectory\nwould confusingly consider the in-tree \".git\" directory as\nthe repository.\n\nThe regular git tools already make it hard to accidentally\nadd such an entry to a tree, and do not allow such entries\nto enter the index at all. Teaching fsck about it provides\nan additional safety check, and let's us avoid propagating\nany such bogosity when transfer.fsckObjects is on.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n fsck.c          |  5 +++++\n t/t1450-fsck.sh | 15 +++++++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/fsck.c b/fsck.c\nindex 31c9a51..99c0497 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -144,6 +144,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \tint has_empty_name = 0;\n \tint has_dot = 0;\n \tint has_dotdot = 0;\n+\tint has_dotgit = 0;\n \tint has_zero_pad = 0;\n \tint has_bad_modes = 0;\n \tint has_dup_entries = 0;\n@@ -174,6 +175,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \t\t\thas_dot = 1;\n \t\tif (!strcmp(name, \"..\"))\n \t\t\thas_dotdot = 1;\n+\t\tif (!strcmp(name, \".git\"))\n+\t\t\thas_dotgit = 1;\n \t\thas_zero_pad |= *(char *)desc.buffer == '0';\n \t\tupdate_tree_entry(&desc);\n \n@@ -227,6 +230,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \t\tretval += error_func(&item->object, FSCK_WARN, \"contains '.'\");\n \tif (has_dotdot)\n \t\tretval += error_func(&item->object, FSCK_WARN, \"contains '..'\");\n+\tif (has_dotgit)\n+\t\tretval += error_func(&item->object, FSCK_WARN, \"contains '.git'\");\n \tif (has_zero_pad)\n \t\tretval += error_func(&item->object, FSCK_WARN, \"contains zero-padded file modes\");\n \tif (has_bad_modes)\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 0b5c30b..d730734 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -253,4 +253,19 @@ test_expect_success 'fsck notices \".\" and \"..\" in trees' '\n \t)\n '\n \n+test_expect_success 'fsck notices \".git\" in trees' '\n+\t(\n+\t\tgit init dotgit &&\n+\t\tcd dotgit &&\n+\t\tblob=$(echo foo | git hash-object -w --stdin) &&\n+\t\ttab=$(printf \"\\\\t\") &&\n+\t\tgit mktree <<-EOF &&\n+\t\t100644 blob $blob$tab.git\n+\t\tEOF\n+\t\tgit fsck 2>out &&\n+\t\tcat out &&\n+\t\tgrep \"warning.*\\\\.git\" out\n+\t)\n+'\n+\n test_done\n-- \n1.8.0.207.gdf2154c\n"},{"id":"204356","messageId":"50B90E11.8090501@web.de","threadId":"32232","inReplyTo":"20121128213529.GA16518@sigill.intra.peff.net","subject":"Re: [PATCH] fsck: warn about \".git\" in trees","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-11-30T19:50:41Z","receivedAt":"2012-11-30T19:50:41Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"> Having a \".git\" entry inside a tree can cause confusing\n> results on checkout. At the top-level, you could not\n> checkout such a tree, as it would complain about overwriting\n> the real \".git\" directory. In a subdirectory, you might\n> check it out, but performing operations in the subdirectory\n> would confusingly consider the in-tree \".git\" directory as\n> the repository.\n[snip]\n> +\tint has_dotgit = 0;\n\nName like \".\" or \"..\" are handled as directories by the OS.\n\n\".git\" could be a file or a directory, at least in theory,\nand from the OS point of view,\nbut we want to have this as a reserved name.\n\nLooking at bad directory names, which gives trouble when checking out:\n\nShould we check for \"/\" or \"../blabla\" as well?\n"},{"id":"204357","messageId":"20121130195509.GA8591@sigill.intra.peff.net","threadId":"32232","inReplyTo":"50B90E11.8090501@web.de","subject":"Re: [PATCH] fsck: warn about \".git\" in trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T19:55:09Z","receivedAt":"2012-11-30T19:55:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 30, 2012 at 08:50:41PM +0100, Torsten Bögershausen wrote:\n\n> >Having a \".git\" entry inside a tree can cause confusing\n> >results on checkout. At the top-level, you could not\n> >checkout such a tree, as it would complain about overwriting\n> >the real \".git\" directory. In a subdirectory, you might\n> >check it out, but performing operations in the subdirectory\n> >would confusingly consider the in-tree \".git\" directory as\n> >the repository.\n> [snip]\n> >+\tint has_dotgit = 0;\n> \n> Name like \".\" or \"..\" are handled as directories by the OS.\n\nRight. In theory git could run on a system that does not treat them\nspecially, but in practice they are going to be problematic on most\nsystems.\n\n> \".git\" could be a file or a directory, at least in theory, and from\n> the OS point of view, but we want to have this as a reserved name.\n\nExactly.\n\n> Looking at bad directory names, which gives trouble when checking out:\n> \n> Should we check for \"/\" or \"../blabla\" as well?\n\nWe do already (the error is \"contains full pathnames\"). We also cover\nempty pathnames and some other cases.\n\n-Peff\n"},{"id":"204485","messageId":"50BDD306.30301@op5.se","threadId":"32232","inReplyTo":"50B90E11.8090501@web.de","subject":"Re: [PATCH] fsck: warn about \".git\" in trees","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2012-12-04T10:40:06Z","receivedAt":"2012-12-04T10:40:06Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 11/30/2012 08:50 PM, Torsten Bögershausen wrote:\n>> Having a \".git\" entry inside a tree can cause confusing\n>> results on checkout. At the top-level, you could not\n>> checkout such a tree, as it would complain about overwriting\n>> the real \".git\" directory. In a subdirectory, you might\n>> check it out, but performing operations in the subdirectory\n>> would confusingly consider the in-tree \".git\" directory as\n>> the repository.\n> [snip]\n>> +    int has_dotgit = 0;\n> \n> Name like \".\" or \"..\" are handled as directories by the OS.\n> \n\nThe patch is for the index, where they're handled as whatever the mode\nclaims it is. The patch doesn't touch those parts though.\n\n> \".git\" could be a file or a directory, at least in theory,\n> and from the OS point of view,\n> but we want to have this as a reserved name.\n> \n> Looking at bad directory names, which gives trouble when checking out:\n> \n> Should we check for \"/\" or \"../blabla\" as well?\n> \n\nApart from the checks already in place, checking for git's internal\ndirectory separator marker (which is '/') is enough to catch both,\nand that check is done.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"}]}