{"thread":{"id":"14105","subject":"Git clone behaviour change in 1.5.6 (vs 1.5.5.1)","startedAt":"2008-06-23T19:51:15Z","lastAt":"2008-06-25T06:11:35Z","messageCount":10,"participants":["Kalle Vahlman","Daniel Barkalow","Jeff King","Brandon Casey","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"80747","messageId":"177e83dd0806231251u223717e4s2149f69313787ead@mail.gmail.com","threadId":"14105","inReplyTo":null,"subject":"Git clone behaviour change in 1.5.6 (vs 1.5.5.1)","fromName":"Kalle Vahlman","fromEmail":"kalle.vahlman@gmail.com","sentAt":"2008-06-23T19:51:15Z","receivedAt":"2008-06-23T19:51:15Z","isPatch":false,"sender":{"key":"kalle.vahlman@gmail.com","avatar":null},"body":"Hi!\n\nSwitching to the 1.5.6 release from 1.5.5.1, I found out that the\nrewritten git-clone command changed its behaviour wrt cloning to a\nnon-existing destination directory structure. In the shell version the\ndestination (work tree) is created with 'mkdir -p' but in the C\nversion with just the mkdir() call which doesn't create the parent\ndirectories.\n\nI can't find any indication that this would be intended in the repo\nhistory nor in the mailing list, so I'm left thinking that this is an\nunwanted regression. Could someone confirm this?\n\n-- \nKalle Vahlman, zuh@iki.fi\nPowered by http://movial.fi\nInteresting stuff at http://syslog.movial.fi\n"},{"id":"80750","messageId":"alpine.LNX.1.00.0806231554380.19665@iabervon.org","threadId":"14105","inReplyTo":"177e83dd0806231251u223717e4s2149f69313787ead@mail.gmail.com","subject":"Re: Git clone behaviour change in 1.5.6 (vs 1.5.5.1)","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-06-23T19:56:41Z","receivedAt":"2008-06-23T19:56:41Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 23 Jun 2008, Kalle Vahlman wrote:\n\n> Hi!\n> \n> Switching to the 1.5.6 release from 1.5.5.1, I found out that the\n> rewritten git-clone command changed its behaviour wrt cloning to a\n> non-existing destination directory structure. In the shell version the\n> destination (work tree) is created with 'mkdir -p' but in the C\n> version with just the mkdir() call which doesn't create the parent\n> directories.\n> \n> I can't find any indication that this would be intended in the repo\n> history nor in the mailing list, so I'm left thinking that this is an\n> unwanted regression. Could someone confirm this?\n\nIt wasn't an intentional change, anyway.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"80760","messageId":"20080623203835.GA8105@sigill.intra.peff.net","threadId":"14105","inReplyTo":"alpine.LNX.1.00.0806231554380.19665@iabervon.org","subject":"Re: Git clone behaviour change in 1.5.6 (vs 1.5.5.1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-23T20:38:36Z","receivedAt":"2008-06-23T20:38:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 23, 2008 at 03:56:41PM -0400, Daniel Barkalow wrote:\n\n> > Switching to the 1.5.6 release from 1.5.5.1, I found out that the\n> > rewritten git-clone command changed its behaviour wrt cloning to a\n> > non-existing destination directory structure. In the shell version the\n> > destination (work tree) is created with 'mkdir -p' but in the C\n> > version with just the mkdir() call which doesn't create the parent\n> > directories.\n> > \n> > I can't find any indication that this would be intended in the repo\n> > history nor in the mailing list, so I'm left thinking that this is an\n> > unwanted regression. Could someone confirm this?\n> \n> It wasn't an intentional change, anyway.\n\nHere is a partial fix. For --bare repo creation, we follow a different\ncodepath, and I think the fix is likely to be a bit tricky. We actually\nfail in make_absolute_path, before we try creating the directory. But I\nsuspect we can't just create the directory ourselves because init_db\nexpects it not to exist.\n\n-- >8 --\nclone: create intermediate directories of repo destination\n\nThe shell version used to use \"mkdir -p\" to create the repo\npath, but the C version just calls \"mkdir\". Let's replicate\nthe old behavior.\n---\n builtin-clone.c  |   40 +++++++++++++++++++++++++++++++++++++++-\n t/t5601-clone.sh |   14 ++++++++++++++\n 2 files changed, 53 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 5c5acb4..758d02c 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -168,6 +168,44 @@ static void setup_reference(const char *repo)\n \tfree(ref_git_copy);\n }\n \n+static int mkdirp_recurse(char *path, int mode)\n+{\n+\tint n;\n+\tchar tmp;\n+\tint r;\n+\n+\tif (mkdir(path, mode) == 0)\n+\t\treturn 0;\n+\tif (errno != ENOENT)\n+\t\treturn -1;\n+\n+\tfor (n = strlen(path); n > 0 && path[n] != '/'; n--)\n+\t\t;\n+\tif (n == 0)\n+\t\treturn -1;\n+\ttmp = path[n];\n+\tpath[n] = '\\0';\n+\tr = mkdirp_recurse(path, mode);\n+\tpath[n] = tmp;\n+\tif (r < 0)\n+\t\treturn -1;\n+\treturn mkdir(path, mode);\n+}\n+\n+static int mkdirp(const char *path, int mode)\n+{\n+\tchar buf[PATH_MAX];\n+\tint n;\n+\n+\tn = strlen(path);\n+\tif (n >= PATH_MAX) {\n+\t\terrno = ENAMETOOLONG;\n+\t\treturn -1;\n+\t}\n+\tmemcpy(buf, path, n+1);\n+\treturn mkdirp_recurse(buf, mode);\n+}\n+\n static void copy_or_link_directory(char *src, char *dest)\n {\n \tstruct dirent *de;\n@@ -404,7 +442,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tif (!option_bare) {\n \t\tjunk_work_tree = work_tree;\n-\t\tif (mkdir(work_tree, 0755))\n+\t\tif (mkdirp(work_tree, 0755))\n \t\t\tdie(\"could not create work tree dir '%s'.\", work_tree);\n \t\tset_git_work_tree(work_tree);\n \t}\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 593d1a3..c2c83f0 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -30,4 +30,18 @@ test_expect_success 'clone checks out files' '\n \n '\n \n+test_expect_success 'clone creates intermediate directories' '\n+\n+\tgit clone src long/path/to/dst &&\n+\ttest -f long/path/to/dst/file\n+\n+'\n+\n+test_expect_failure 'clone creates intermediate directories for bare repo' '\n+\n+\tgit clone --bare src long/path/to/bare/dst &&\n+\ttest -f long/path/to/bare/dst/config\n+\n+'\n+\n test_done\n-- \n1.5.6.105.gceec6.dirty\n"},{"id":"80761","messageId":"0YbSdnKH0_SP30-YMV0Y1aH4SO46yc6x_2EA1VGwPtV_CltGk-7wdg@cipher.nrlssc.navy.mil","threadId":"14105","inReplyTo":"20080623203835.GA8105@sigill.intra.peff.net","subject":"Re: Git clone behaviour change in 1.5.6 (vs 1.5.5.1)","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-06-23T21:03:47Z","receivedAt":"2008-06-23T21:03:47Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Jeff King wrote:\n> On Mon, Jun 23, 2008 at 03:56:41PM -0400, Daniel Barkalow wrote:\n> \n>>> Switching to the 1.5.6 release from 1.5.5.1, I found out that the\n>>> rewritten git-clone command changed its behaviour wrt cloning to a\n>>> non-existing destination directory structure. In the shell version the\n>>> destination (work tree) is created with 'mkdir -p' but in the C\n>>> version with just the mkdir() call which doesn't create the parent\n>>> directories.\n\n> Here is a partial fix.\n\nThere is a mkdir_p() in builtin-merge-recursive.c which calls\nsafe_create_leading_directories() in sha1_file.c\n\nCan these functions be reused?\n\n\n-brandon\n"},{"id":"80835","messageId":"20080624055022.GC19224@sigill.intra.peff.net","threadId":"14105","inReplyTo":"0YbSdnKH0_SP30-YMV0Y1aH4SO46yc6x_2EA1VGwPtV_CltGk-7wdg@cipher.nrlssc.navy.mil","subject":"[PATCH] clone: create intermediate directories of destination repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-24T05:50:22Z","receivedAt":"2008-06-24T05:50:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The shell version used to use \"mkdir -p\" to create the repo\npath, but the C version just calls \"mkdir\". Let's replicate\nthe old behavior. In this case we can simply create the\ndirectories leading up to the git dir. If it's a bare repo,\nthen that is everything that init_db wants ahead of time. If\nit isn't bare, then the worktree contains the git dir, so we\ncreate the worktree.\n\nWe can reuse safe_create_leading_directories, but we need to\nmake a copy of our const buffer to do so. Since\nmerge-recursive uses the same pattern, we can factor this\nout into a global function. This has two other cleanup\nadvantages for merge-recursive:\n\n  1. mkdir_p wasn't a very good name. \"mkdir -p foo/bar\" actually\n     creates bar, but this function just creates the leading\n     directories.\n\n  2. mkdir_p took a mode argument, but it was completely\n     ignored.\n---\nOn Mon, Jun 23, 2008 at 04:03:47PM -0500, Brandon Casey wrote:\n\n> There is a mkdir_p() in builtin-merge-recursive.c which calls\n> safe_create_leading_directories() in sha1_file.c\n> \n> Can these functions be reused?\n\nThanks, I hadn't noticed those for some reason. This version uses the\nexisting code, and correctly handles both the bare and worktree cases.\n\nThe big difference is that safe_create_leading_directories will do an\nadjust_shared_perm on the result. I don't think that should be a\nproblem, but it is a difference.\n\n builtin-clone.c           |    5 +++--\n builtin-merge-recursive.c |   13 ++-----------\n cache.h                   |    1 +\n sha1_file.c               |    9 +++++++++\n t/t5601-clone.sh          |   14 ++++++++++++++\n 5 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 7190952..e951911 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -398,10 +398,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tgit_dir = xstrdup(mkpath(\"%s/.git\", dir));\n \t}\n \n+\tif (safe_create_leading_directories_const(git_dir) < 0)\n+\t\tdie(\"could not create leading directories of '%s'\", git_dir);\n+\n \tif (!option_bare) {\n \t\tjunk_work_tree = work_tree;\n-\t\tif (mkdir(work_tree, 0755))\n-\t\t\tdie(\"could not create work tree dir '%s'.\", work_tree);\n \t\tset_git_work_tree(work_tree);\n \t}\n \tjunk_git_dir = git_dir;\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 4aa28a1..43bf6aa 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -481,15 +481,6 @@ static char *unique_path(const char *path, const char *branch)\n \treturn newpath;\n }\n \n-static int mkdir_p(const char *path, unsigned long mode)\n-{\n-\t/* path points to cache entries, so xstrdup before messing with it */\n-\tchar *buf = xstrdup(path);\n-\tint result = safe_create_leading_directories(buf);\n-\tfree(buf);\n-\treturn result;\n-}\n-\n static void flush_buffer(int fd, const char *buf, unsigned long size)\n {\n \twhile (size > 0) {\n@@ -512,7 +503,7 @@ static int make_room_for_path(const char *path)\n \tint status;\n \tconst char *msg = \"failed to create path '%s'%s\";\n \n-\tstatus = mkdir_p(path, 0777);\n+\tstatus = safe_create_leading_directories_const(path);\n \tif (status) {\n \t\tif (status == -3) {\n \t\t\t/* something else exists */\n@@ -583,7 +574,7 @@ static void update_file_flags(const unsigned char *sha,\n \t\t\tclose(fd);\n \t\t} else if (S_ISLNK(mode)) {\n \t\t\tchar *lnk = xmemdupz(buf, size);\n-\t\t\tmkdir_p(path, 0777);\n+\t\t\tsafe_create_leading_directories_const(path);\n \t\t\tunlink(path);\n \t\t\tsymlink(lnk, path);\n \t\t\tfree(lnk);\ndiff --git a/cache.h b/cache.h\nindex d12ee7d..49f20dd 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -517,6 +517,7 @@ enum sharedrepo {\n int git_config_perm(const char *var, const char *value);\n int adjust_shared_perm(const char *path);\n int safe_create_leading_directories(char *path);\n+int safe_create_leading_directories_const(const char *path);\n char *enter_repo(char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 191f814..71be0c9 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -116,6 +116,15 @@ int safe_create_leading_directories(char *path)\n \treturn 0;\n }\n \n+int safe_create_leading_directories_const(const char *path)\n+{\n+\t/* path points to cache entries, so xstrdup before messing with it */\n+\tchar *buf = xstrdup(path);\n+\tint result = safe_create_leading_directories(buf);\n+\tfree(buf);\n+\treturn result;\n+}\n+\n char *sha1_to_hex(const unsigned char *sha1)\n {\n \tstatic int bufno;\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 593d1a3..c2c83f0 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -30,4 +30,18 @@ test_expect_success 'clone checks out files' '\n \n '\n \n+test_expect_success 'clone creates intermediate directories (non-bare)' '\n+\n+\tgit clone src long/path/to/dst &&\n+\ttest -f long/path/to/dst/file\n+\n+'\n+\n+test_expect_success 'clone creates intermediate directories (bare)' '\n+\n+\tgit clone --bare src long/path/to/bare/dst &&\n+\ttest -f long/path/to/bare/dst/config\n+\n+'\n+\n test_done\n-- \n1.5.6.50.gae760.dirty\n"},{"id":"80842","messageId":"7v3an3e0xv.fsf@gitster.siamese.dyndns.org","threadId":"14105","inReplyTo":"20080624055022.GC19224@sigill.intra.peff.net","subject":"Re: [PATCH] clone: create intermediate directories of destination repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-24T07:39:40Z","receivedAt":"2008-06-24T07:39:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The shell version used to use \"mkdir -p\" to create the repo path, but\n> the C version just calls \"mkdir\". Let's replicate the old behavior. In\n> this case we can simply create the directories leading up to the git\n> dir. If it's a bare repo, then that is everything that init_db wants\n> ahead of time. If it isn't bare, then the worktree contains the git dir,\n> so we create the worktree.\n\nClever ;-)\n\n> The big difference is that safe_create_leading_directories will do an\n> adjust_shared_perm on the result. I don't think that should be a\n> problem, but it is a difference.\n\nThis early in the code you would not have read anything that triggers\n\"shared\" so it probably is not a problem, I would think.\n"},{"id":"80846","messageId":"20080624080437.GA2581@sigill.intra.peff.net","threadId":"14105","inReplyTo":"7v3an3e0xv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] clone: create intermediate directories of destination repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-24T08:04:38Z","receivedAt":"2008-06-24T08:04:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 24, 2008 at 12:39:40AM -0700, Junio C Hamano wrote:\n\n> > The shell version used to use \"mkdir -p\" to create the repo path, but\n> > the C version just calls \"mkdir\". Let's replicate the old behavior. In\n> > this case we can simply create the directories leading up to the git\n> > dir. If it's a bare repo, then that is everything that init_db wants\n> > ahead of time. If it isn't bare, then the worktree contains the git dir,\n> > so we create the worktree.\n> \n> Clever ;-)\n\nI am worried that it is too clever. I didn't see an obvious way for\nwork_tree and git_dir to not have that property, but I think it is still\nworth somebody double-checking.\n\n-Peff\n"},{"id":"80910","messageId":"alpine.LNX.1.00.0806241113360.19665@iabervon.org","threadId":"14105","inReplyTo":"20080624080437.GA2581@sigill.intra.peff.net","subject":"Re: [PATCH] clone: create intermediate directories of destination repo","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-06-24T15:20:07Z","receivedAt":"2008-06-24T15:20:07Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 24 Jun 2008, Jeff King wrote:\n\n> On Tue, Jun 24, 2008 at 12:39:40AM -0700, Junio C Hamano wrote:\n> \n> > > The shell version used to use \"mkdir -p\" to create the repo path, but\n> > > the C version just calls \"mkdir\". Let's replicate the old behavior. In\n> > > this case we can simply create the directories leading up to the git\n> > > dir. If it's a bare repo, then that is everything that init_db wants\n> > > ahead of time. If it isn't bare, then the worktree contains the git dir,\n> > > so we create the worktree.\n> > \n> > Clever ;-)\n> \n> I am worried that it is too clever. I didn't see an obvious way for\n> work_tree and git_dir to not have that property, but I think it is still\n> worth somebody double-checking.\n\nI think you can specify git_dir and work_tree on the command line, and set \nthem to whatever you want, although I now don't remember whether they're \nactually both followed for clone (I tried to match the shell version, \nwhose behavior didn't make too much sense to me).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"81074","messageId":"20080625054134.GB2209@sigill.intra.peff.net","threadId":"14105","inReplyTo":"alpine.LNX.1.00.0806241113360.19665@iabervon.org","subject":"Re: [PATCH] clone: create intermediate directories of destination repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-25T05:41:34Z","receivedAt":"2008-06-25T05:41:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 24, 2008 at 11:20:07AM -0400, Daniel Barkalow wrote:\n\n> > I am worried that it is too clever. I didn't see an obvious way for\n> > work_tree and git_dir to not have that property, but I think it is still\n> > worth somebody double-checking.\n> \n> I think you can specify git_dir and work_tree on the command line, and set \n> them to whatever you want, although I now don't remember whether they're \n> actually both followed for clone (I tried to match the shell version, \n> whose behavior didn't make too much sense to me).\n\nThe git_dir variable is always set from the directory provided on the\ncommand line. However, the work_tree can be overridden by the\nenvironment. For some reason I missed the giant 'work_tree =\ngetenv(\"GIT_WORK_TREE\")' when I looked before. But that means that they\nare not necessarily related.\n\nSo my original patch was too clever. Here is the less clever patch, with\nan extra test that would break with the clever version.\n\n-- >8 --\nclone: create intermediate directories of destination repo\n\nThe shell version used to use \"mkdir -p\" to create the repo\npath, but the C version just calls \"mkdir\". Let's replicate\nthe old behavior. We have to create the git and worktree\nleading dirs separately; while most of the time, the\nworktree dir contains the git dir (as .git), the user can\noverride this using GIT_WORK_TREE.\n\nWe can reuse safe_create_leading_directories, but we need to\nmake a copy of our const buffer to do so. Since\nmerge-recursive uses the same pattern, we can factor this\nout into a global function. This has two other cleanup\nadvantages for merge-recursive:\n\n  1. mkdir_p wasn't a very good name. \"mkdir -p foo/bar\" actually\n     creates bar, but this function just creates the leading\n     directories.\n\n  2. mkdir_p took a mode argument, but it was completely\n     ignored.\n---\n builtin-clone.c           |    5 +++++\n builtin-merge-recursive.c |   13 ++-----------\n cache.h                   |    1 +\n sha1_file.c               |    9 +++++++++\n t/t5601-clone.sh          |   22 ++++++++++++++++++++++\n 5 files changed, 39 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 7190952..b2dfe1a 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -400,6 +400,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tif (!option_bare) {\n \t\tjunk_work_tree = work_tree;\n+\t\tif (safe_create_leading_directories_const(work_tree) < 0)\n+\t\t\tdie(\"could not create leading directories of '%s'\",\n+\t\t\t\t\twork_tree);\n \t\tif (mkdir(work_tree, 0755))\n \t\t\tdie(\"could not create work tree dir '%s'.\", work_tree);\n \t\tset_git_work_tree(work_tree);\n@@ -410,6 +413,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tsetenv(CONFIG_ENVIRONMENT, xstrdup(mkpath(\"%s/config\", git_dir)), 1);\n \n+\tif (safe_create_leading_directories_const(git_dir) < 0)\n+\t\tdie(\"could not create leading directories of '%s'\", git_dir);\n \tset_git_dir(make_absolute_path(git_dir));\n \n \tfprintf(stderr, \"Initialize %s\\n\", git_dir);\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 4aa28a1..43bf6aa 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -481,15 +481,6 @@ static char *unique_path(const char *path, const char *branch)\n \treturn newpath;\n }\n \n-static int mkdir_p(const char *path, unsigned long mode)\n-{\n-\t/* path points to cache entries, so xstrdup before messing with it */\n-\tchar *buf = xstrdup(path);\n-\tint result = safe_create_leading_directories(buf);\n-\tfree(buf);\n-\treturn result;\n-}\n-\n static void flush_buffer(int fd, const char *buf, unsigned long size)\n {\n \twhile (size > 0) {\n@@ -512,7 +503,7 @@ static int make_room_for_path(const char *path)\n \tint status;\n \tconst char *msg = \"failed to create path '%s'%s\";\n \n-\tstatus = mkdir_p(path, 0777);\n+\tstatus = safe_create_leading_directories_const(path);\n \tif (status) {\n \t\tif (status == -3) {\n \t\t\t/* something else exists */\n@@ -583,7 +574,7 @@ static void update_file_flags(const unsigned char *sha,\n \t\t\tclose(fd);\n \t\t} else if (S_ISLNK(mode)) {\n \t\t\tchar *lnk = xmemdupz(buf, size);\n-\t\t\tmkdir_p(path, 0777);\n+\t\t\tsafe_create_leading_directories_const(path);\n \t\t\tunlink(path);\n \t\t\tsymlink(lnk, path);\n \t\t\tfree(lnk);\ndiff --git a/cache.h b/cache.h\nindex a68866c..a9c14da 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -517,6 +517,7 @@ enum sharedrepo {\n int git_config_perm(const char *var, const char *value);\n int adjust_shared_perm(const char *path);\n int safe_create_leading_directories(char *path);\n+int safe_create_leading_directories_const(const char *path);\n char *enter_repo(char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 9330bc4..0d52e59 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -116,6 +116,15 @@ int safe_create_leading_directories(char *path)\n \treturn 0;\n }\n \n+int safe_create_leading_directories_const(const char *path)\n+{\n+\t/* path points to cache entries, so xstrdup before messing with it */\n+\tchar *buf = xstrdup(path);\n+\tint result = safe_create_leading_directories(buf);\n+\tfree(buf);\n+\treturn result;\n+}\n+\n char *sha1_to_hex(const unsigned char *sha1)\n {\n \tstatic int bufno;\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 593d1a3..b642fb2 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -30,4 +30,26 @@ test_expect_success 'clone checks out files' '\n \n '\n \n+test_expect_success 'clone respects GIT_WORK_TREE' '\n+\n+\tGIT_WORK_TREE=worktree git clone src bare &&\n+\ttest -f bare/config &&\n+\ttest -f worktree/file\n+\n+'\n+\n+test_expect_success 'clone creates intermediate directories' '\n+\n+\tgit clone src long/path/to/dst &&\n+\ttest -f long/path/to/dst/file\n+\n+'\n+\n+test_expect_success 'clone creates intermediate directories for bare repo' '\n+\n+\tgit clone --bare src long/path/to/bare/dst &&\n+\ttest -f long/path/to/bare/dst/config\n+\n+'\n+\n test_done\n-- \n1.5.6.57.g56b3.dirty\n"},{"id":"81080","messageId":"alpine.LNX.1.00.0806250211000.19665@iabervon.org","threadId":"14105","inReplyTo":"20080625054134.GB2209@sigill.intra.peff.net","subject":"Re: [PATCH] clone: create intermediate directories of destination repo","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-06-25T06:11:35Z","receivedAt":"2008-06-25T06:11:35Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 25 Jun 2008, Jeff King wrote:\n\n> On Tue, Jun 24, 2008 at 11:20:07AM -0400, Daniel Barkalow wrote:\n> \n> > > I am worried that it is too clever. I didn't see an obvious way for\n> > > work_tree and git_dir to not have that property, but I think it is still\n> > > worth somebody double-checking.\n> > \n> > I think you can specify git_dir and work_tree on the command line, and set \n> > them to whatever you want, although I now don't remember whether they're \n> > actually both followed for clone (I tried to match the shell version, \n> > whose behavior didn't make too much sense to me).\n> \n> The git_dir variable is always set from the directory provided on the\n> command line. However, the work_tree can be overridden by the\n> environment. For some reason I missed the giant 'work_tree =\n> getenv(\"GIT_WORK_TREE\")' when I looked before. But that means that they\n> are not necessarily related.\n> \n> So my original patch was too clever. Here is the less clever patch, with\n> an extra test that would break with the clever version.\n\nLooks good, thanks.\n\nAcked-by: Daniel Barkalow <barkalow@iabervon.org>\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}