{"thread":{"id":"25043","subject":"git's use of mkdir(2)","startedAt":"2010-09-08T19:36:43Z","lastAt":"2010-09-09T19:38:43Z","messageCount":3,"participants":["der Mouse","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"150322","messageId":"201009081936.PAA07078@Sparkle.Rodents-Montreal.ORG","threadId":"25043","inReplyTo":null,"subject":"git's use of mkdir(2)","fromName":"der Mouse","fromEmail":"mouse@rodents-montreal.org","sentAt":"2010-09-08T19:36:43Z","receivedAt":"2010-09-08T19:36:43Z","isPatch":false,"sender":{"key":"mouse@rodents-montreal.org","avatar":null},"body":"I've been trying to convince git to run on some of the systems I use.\nOf particular relevance at the moment are two BSD systems which have an\nimportant behavioural difference.\n\nSpecifically, if /foo/bar does not exist but /foo does,\nmkdir(\"/foo/bar/\",...) works on one and fails showing ENOENT on the\nother. (Without the trailing slash, it works on both.)\n\ngit (at least in the version I've got here, which appears to be\n1.6.4.1) depends on this working, and breaks as a result on the second\nof the above two systems.\n\nI've been doing a little digging and I think I might be able to fix\nthis - but, before I charged ahead with it, I thought I'd float a query\nhere to ask (a) if this is a known issue and fixed in something more\nrecent (I had a look at 1.7.2 and a quick read of the code makes me\nthink it still does this, but I could have missed something) and (b) if\nthere would be any interest in such fixes if I do come up with them.\n\nThoughts?\n\n/~\\ The ASCII\t\t\t\t Mouse\n\\ / Ribbon Campaign\n X Against HTML\t\tmouse@rodents-montreal.org\n/ \\ Email!\t 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B\n"},{"id":"150384","messageId":"7v62yen3ts.fsf@alter.siamese.dyndns.org","threadId":"25043","inReplyTo":"201009081936.PAA07078@Sparkle.Rodents-Montreal.ORG","subject":"Re: git's use of mkdir(2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-09T18:56:31Z","receivedAt":"2010-09-09T18:56:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"der Mouse writes:\n\n> I've been trying to convince git to run on some of the systems I use.\n> Of particular relevance at the moment are two BSD systems which have an\n> important behavioural difference.\n>\n> Specifically, if /foo/bar does not exist but /foo does,\n> mkdir(\"/foo/bar/\",...) works on one and fails showing ENOENT on the\n> other. (Without the trailing slash, it works on both.)\n\nWhat vintage of BSD do you have that exhibits the problem? It smells like\na POSIX violation, considering what \"4.12 Pathname Resolution\" says on the\nmatter:\n\n A pathname that contains at least one non- character and that\n ends with one or more trailing characters shall not be resolved\n successfully unless the last pathname component before the trailing\n characters names an existing directory or a directory entry that\n is to be created for a directory immediately after the pathname is\n resolved.\n\nNot that I am saying that such a system does not deserve to be supported,\nbut I am curious to know how widespread the damage is.\n\n> here to ask (a) if this is a known issue and fixed in something more\n> recent (I had a look at 1.7.2 and a quick read of the code makes me\n> think it still does this, but I could have missed something)\n\nI don't think so---we seem to have a compat/ replacement \"mkdir(2)\" for\nMinGW (but that doesn't trim the trailing slash so I would imagine MinGW\ndoes not suffer from such a violation), but not for the flavor of BSD you\nhave. It shouldn't be hard to add one, though.\n"},{"id":"150401","messageId":"201009091938.PAA18955@Sparkle.Rodents-Montreal.ORG","threadId":"25043","inReplyTo":"7v62yen3ts.fsf@alter.siamese.dyndns.org","subject":"Re: git's use of mkdir(2)","fromName":"der Mouse","fromEmail":"mouse@rodents-montreal.org","sentAt":"2010-09-09T19:38:43Z","receivedAt":"2010-09-09T19:38:43Z","isPatch":false,"sender":{"key":"mouse@rodents-montreal.org","avatar":null},"body":">> Specifically, if /foo/bar does not exist but /foo does,\n>> mkdir(\"/foo/bar/\",...) works on one and fails showing ENOENT on the\n>> other. (Without the trailing slash, it works on both.)\n> What vintage of BSD do you have that exhibits the problem?\n\nNetBSD 1.4T. (About a decade old at this point.) It's unlikely that\nthe machine I most care about this on is going to switch versions\nanytime soon, though I may be able to convince its mkdir(2) to strip\ntrailing slashes - I should look at the other implementation....\n\n> It smells like a POSIX violation, considering what \"4.12 Pathname\n> Resolution\" says on the matter:\n\n> A pathname that contains at least one non- character and\n> that ends with one or more trailing characters shall not be\n> resolved successfully unless the last pathname component before the\n> trailing characters names an existing directory or a\n> directory entry that is to be created for a directory immediately\n> after the pathname is resolved.\n\nWell, notice that it does not say that it shall be successfully\nresolved in either of those exception cases, just that it shall not be\nsuccessfully resolved in other cases. I read this as \"you can append a\nslash to require that the last component be a directory\", with the\nsecond exception so that the mkdir-with-trailing-slash-works behaviour\nis permitted; I don't read it as mandating that mkdir work with a\ntrailing slash. Is there a rationale of any sort that elaborates on\nthis choice, or perhaps something about mkdir elsewhere?\n\n> Not that I am saying that such a system does not deserve to be\n> supported, but I am curious to know how widespread the damage is.\n\nI don't know. Of my own experience, I know one version that tolerates\nwhat git does and one version that doesn't; I haven't experimented to\ntry to determine how widespread either is - though I can infer that\nLinux falls in the trailing-slash-allowed camp.\n\nFor the moment, I bludgeoned it into working with the patch below, but\nI haven't really exercised it very much yet, so it's entirely possible\nit's going to trip over the same thing somewhere else - there are a lot\nof mkdir calls scattered around and I haven't looked at any outside\nbuiltin-init-db.c.\n\nPersonally, I'd tend to avoid depending on this detail of mkdir, when\nit's easy (which it appears to be in the case of builtin-init-db.c)\njust on general principles. But I don't know whether this is outside\nthe scope of what git cares about - I just know _I_ care about it. :)\n\nGiven the existing code, a mkdir wrapper which strips trailing slashes\nmight be the least-pain answer; it feels wrong to accrete wrappers like\nthat all over the place, but sometimes they are right answers....\n\n/~\\ The ASCII\t\t\t\t Mouse\n\\ / Ribbon Campaign\n X Against HTML\t\tmouse@rodents-montreal.org\n/ \\ Email!\t 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B\n\nHere's the patch I mentioned above. (This is relative to 1.6.4.1's\nbuiltin-init-db.c; so far I haven't tried to build anything newer. I\nhave a handful more patches which I apply as part of my build script,\nbut they're not relevant to this particular thread; I don't think any\nof them interact with this one.)\n\n--- OLD/builtin-init-db.c\tThu Jan 1 00:00:00 1970\n+++ NEW/builtin-init-db.c\tThu Jan 1 00:00:00 1970\n@@ -46,6 +46,8 @@\n \t * be really carefully chosen.\n \t */\n \tsafe_create_dir(path, 1);\n+\tpath[baselen++] = '/';\n+\ttemplate[template_baselen++] = '/';\n \twhile ((de = readdir(dir)) != NULL) {\n \t\tstruct stat st_git, st_template;\n \t\tint namelen;\n@@ -75,8 +77,6 @@\n \t\t\tint template_baselen_sub = template_baselen + namelen;\n \t\t\tif (!subdir)\n \t\t\t\tdie_errno(\"cannot opendir '%s'\", template);\n-\t\t\tpath[baselen_sub++] =\n-\t\t\t\ttemplate[template_baselen_sub++] = '/';\n \t\t\tpath[baselen_sub] =\n \t\t\t\ttemplate[template_baselen_sub] = 0;\n \t\t\tcopy_templates_1(path, baselen_sub,\n@@ -127,10 +127,6 @@\n \tif (PATH_MAX <= (template_len+strlen(\"/config\")))\n \t\tdie(\"insanely long template path %s\", template_dir);\n \tstrcpy(template_path, template_dir);\n-\tif (template_path[template_len-1] != '/') {\n-\t\ttemplate_path[template_len++] = '/';\n-\t\ttemplate_path[template_len] = 0;\n-\t}\n \tdir = opendir(template_path);\n \tif (!dir) {\n \t\twarning(\"templates not found %s\", template_dir);\n@@ -138,7 +134,7 @@\n \t}\n \n \t/* Make sure that template is from the correct vintage */\n-\tstrcpy(template_path + template_len, \"config\");\n+\tstrcpy(template_path + template_len, \"/config\");\n \trepository_format_version = 0;\n \tgit_config_from_file(check_repository_format_version,\n \t\t\t template_path, NULL);\n@@ -155,8 +151,6 @@\n \t}\n \n \tmemcpy(path, git_dir, len);\n-\tif (len && path[len - 1] != '/')\n-\t\tpath[len++] = '/';\n \tpath[len] = 0;\n \tcopy_templates_1(path, len,\n \t\t\t template_path, template_len,\n@@ -217,7 +211,7 @@\n \t * Create the default symlink from \".git/HEAD\" to the \"master\"\n \t * branch, if it does not exist yet.\n \t */\n-\tstrcpy(path + len, \"HEAD\");\n+\tstrcpy(path + len, \"/HEAD\");\n \treinit = (!access(path, R_OK)\n \t\t || readlink(path, junk, sizeof(junk)-1) != -1);\n \tif (!reinit) {\n@@ -230,7 +224,7 @@\n \tgit_config_set(\"core.repositoryformatversion\", repo_version_string);\n \n \tpath[len] = 0;\n-\tstrcpy(path + len, \"config\");\n+\tstrcpy(path + len, \"/config\");\n \n \t/* Check filemode trustability */\n \tfilemode = TEST_FILEMODE;\n@@ -259,7 +253,7 @@\n \tif (!reinit) {\n \t\t/* Check if symlink is supported in the work tree */\n \t\tpath[len] = 0;\n-\t\tstrcpy(path + len, \"tXXXXXX\");\n+\t\tstrcpy(path + len, \"/tXXXXXX\");\n \t\tif (!close(xmkstemp(path)) &&\n \t\t !unlink(path) &&\n \t\t !symlink(\"testing\", path) &&\n@@ -271,7 +265,7 @@\n \n \t\t/* Check if the filesystem is case-insensitive */\n \t\tpath[len] = 0;\n-\t\tstrcpy(path + len, \"CoNfIg\");\n+\t\tstrcpy(path + len, \"/CoNfIg\");\n \t\tif (!access(path, F_OK))\n \t\t\tgit_config_set(\"core.ignorecase\", \"true\");\n \t}\n"}]}