{"thread":{"id":"33424","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","startedAt":"2013-04-07T23:17:08Z","lastAt":"2013-04-09T22:41:52Z","messageCount":28,"participants":["Jonathan Nieder","Aaron Schrab","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"213489","messageId":"1365376629-16054-1-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":null,"subject":"[PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-07T23:17:08Z","receivedAt":"2013-04-07T23:17:08Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Do not report an argument to clone's --reference option is not a local\ndirectory.  Nothing checks for the actual directory so we have no way to\nknow if whether or not exists.  Telling the user that a directory doesn't\nexist when that isn't actually known may lead him or her on the wrong\npath to finding the problem.\n\nSigned-off-by: Aaron Schrab <aaron@schrab.com>\n---\n builtin/clone.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f9c380e..0a1e0bf 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\n \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git)))\n-\t\tdie(_(\"reference repository '%s' is not a local directory.\"),\n+\t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n \t\t    item->string);\n \n \tstrbuf_addf(&alternate, \"%s/objects\", ref_git);\n-- \n1.8.2.677.g7422c62\n"},{"id":"213488","messageId":"1365376629-16054-2-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"1365376629-16054-1-git-send-email-aaron@schrab.com","subject":"[PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-07T23:17:09Z","receivedAt":"2013-04-07T23:17:09Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Try reading gitfile files when processing --reference options to clone.\nThis will allow, among other things, using a submodule checked out with\na recent version of git as a reference repository without requiring the\nuser to have internal knowledge of submodule layout.\n\nSigned-off-by: Aaron Schrab <aaron@schrab.com>\n---\n builtin/clone.c            | 13 ++++++++++---\n t/t5700-clone-reference.sh |  7 +++++++\n 2 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 0a1e0bf..376ded8 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -231,12 +231,19 @@ static void strip_trailing_slashes(char *dir)\n \n static int add_one_reference(struct string_list_item *item, void *cb_data)\n {\n-\tchar *ref_git;\n+\tchar *ref_git, *repo;\n \tstruct strbuf alternate = STRBUF_INIT;\n \n-\t/* Beware: real_path() and mkpath() return static buffer */\n+\t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n \tref_git = xstrdup(real_path(item->string));\n-\tif (is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n+\n+\trepo = (char *)read_gitfile(mkpath(\"%s/.git\", ref_git));\n+\tif (repo) {\n+\t\tfree(ref_git);\n+\t\tref_git = xstrdup(repo);\n+\t}\n+\n+\tif (!repo && is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n \t\tchar *ref_git_git = mkpathdup(\"%s/.git\", ref_git);\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex 2a7b78b..7a9044c 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -185,4 +185,11 @@ test_expect_success 'fetch with incomplete alternates' '\n \t! grep \" want $tag_object\" \"$U.K\"\n '\n \n+test_expect_success 'clone using repo with gitfile as a reference' '\n+\tgit clone --separate-git-dir=L A M &&\n+\tgit clone --reference=M A N &&\n+\techo \"$base_dir/L/objects\" > expected &&\n+\ttest_cmp expected \"$base_dir/N/.git/objects/info/alternates\"\n+'\n+\n test_done\n-- \n1.8.2.677.g7422c62\n"},{"id":"213478","messageId":"20130407234810.GG19857@elie.Belkin","threadId":"33424","inReplyTo":"1365376629-16054-1-git-send-email-aaron@schrab.com","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-07T23:48:10Z","receivedAt":"2013-04-07T23:48:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Aaron,\n\nAaron Schrab wrote:\n\n> Do not report an argument to clone's --reference option is not a local\n> directory.  Nothing checks for the actual directory so we have no way to\n> know if whether or not exists.  Telling the user that a directory doesn't\n> exist when that isn't actually known may lead him or her on the wrong\n> path to finding the problem.\n\nI don't understand the above explanation.  Could you give an example?\n\n[...]\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n>  \t\tfree(ref_git);\n>  \t\tref_git = ref_git_git;\n>  \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git)))\n> -\t\tdie(_(\"reference repository '%s' is not a local directory.\"),\n> +\t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n\n\"is_directory\" calls stat and checks if its target is a directory.  Is\nthe problem that \"/path/to/repo.git\" might be a directory but\n\"/path/to/repo.git/objects\" may not?\n\nWould it make sense for the message to say something like the\nfollowing?\n\n\tfatal: alternate object store '/path/to/repo.git/objects' is not a local directory\n\nThanks and hope that helps,\nJonathan\n"},{"id":"213490","messageId":"20130407235112.GH19857@elie.Belkin","threadId":"33424","inReplyTo":"1365376629-16054-2-git-send-email-aaron@schrab.com","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-07T23:51:12Z","receivedAt":"2013-04-07T23:51:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Aaron Schrab wrote:\n\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -231,12 +231,19 @@ static void strip_trailing_slashes(char *dir)\n>  \n>  static int add_one_reference(struct string_list_item *item, void *cb_data)\n>  {\n> -\tchar *ref_git;\n> +\tchar *ref_git, *repo;\n[...]\n> +\trepo = (char *)read_gitfile(mkpath(\"%s/.git\", ref_git));\n\nWhy not make repo a \"const char *\" and avoid the cast?  The above\nwould seem to make it too tempting to treat the return value from\nread_gitfile() as a mutable buffer instead of a real_path string that\nshould be copied asap.\n\nHope that helps,\nJonathan\n"},{"id":"213480","messageId":"20130408000658.GG27178@pug.qqx.org","threadId":"33424","inReplyTo":"20130407234810.GG19857@elie.Belkin","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T00:06:58Z","receivedAt":"2013-04-08T00:06:58Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 16:48 -0700 07 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>Hi Aaron,\n\nThanks for the feedback.\n\n>Aaron Schrab wrote:\n>\n>> Do not report an argument to clone's --reference option is not a local\n>> directory.  Nothing checks for the actual directory so we have no way to\n>> know if whether or not exists.  Telling the user that a directory doesn't\n>> exist when that isn't actually known may lead him or her on the wrong\n>> path to finding the problem.\n>\n>I don't understand the above explanation.  Could you give an example?\n\nI originally noticed this while trying to use a submodule as a reference \nrepository.  Since that submodule was first checked out using a recent \nversion of git it used a .git file rather than having a .git directory.  \nThis caused the checks to fail, and the misleading error message had me \nchecking for a typo in the path which I'd supplied.\n\nI'll attempt to clarify that message in the next version.\n\n>> --- a/builtin/clone.c\n>> +++ b/builtin/clone.c\n>> @@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n>>  \t\tfree(ref_git);\n>>  \t\tref_git = ref_git_git;\n>>  \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git)))\n>> -\t\tdie(_(\"reference repository '%s' is not a local directory.\"),\n>> +\t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n>\n>\"is_directory\" calls stat and checks if its target is a directory.  Is\n>the problem that \"/path/to/repo.git\" might be a directory but\n>\"/path/to/repo.git/objects\" may not?\n\nIn my case the issue was that /path/to/repo is a directory, but \n/path/to/repo/.git/objects (which is checked shortly before the above \ncontext) didn't exist since /path/to/repo/.git is a file.\n\n>Would it make sense for the message to say something like the\n>following?\n>\n>\tfatal: alternate object store '/path/to/repo.git/objects' is not a local directory\n\nThat would also avoid lying to the user.  But if combined with the \nsecond patch in this series it could cause confusion for a different \nreason.  Once .git files are honored, the path reported there may have \nno relation to the path supplied by the user.\n"},{"id":"213485","messageId":"20130408000845.GH27178@pug.qqx.org","threadId":"33424","inReplyTo":"20130407235112.GH19857@elie.Belkin","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T00:08:45Z","receivedAt":"2013-04-08T00:08:45Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 16:51 -0700 07 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> -\tchar *ref_git;\n>> +\tchar *ref_git, *repo;\n>[...]\n>> +\trepo = (char *)read_gitfile(mkpath(\"%s/.git\", ref_git));\n>\n>Why not make repo a \"const char *\" and avoid the cast?  The above\n>would seem to make it too tempting to treat the return value from\n>read_gitfile() as a mutable buffer instead of a real_path string that\n>should be copied asap.\n\nGood catch.  I'll fix that in the next version.\n\nThanks.\n"},{"id":"213487","messageId":"20130408011103.GI27178@pug.qqx.org","threadId":"33424","inReplyTo":"20130408000658.GG27178@pug.qqx.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T01:11:03Z","receivedAt":"2013-04-08T01:11:03Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 20:06 -0400 07 Apr 2013, I wrote:\n>At 16:48 -0700 07 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>>Would it make sense for the message to say something like the\n>>following?\n>>\n>>\tfatal: alternate object store '/path/to/repo.git/objects' is not a local directory\n>\n>That would also avoid lying to the user.  But if combined with the \n>second patch in this series it could cause confusion for a different \n>reason.  Once .git files are honored, the path reported there may have \n>no relation to the path supplied by the user.\n\nThinking on this further, even without the companion patch there's \nanother issue.  The problem isn't just that \n/path/supplied/by/user/objects isn't a directory.  It's that neither \nthat nor /path/supplied/by/user/.git/objects is a directory.  And in \nmany cases it's the latter that the user would be expecting to have been \nused.  Reporting on just the last name checked isn't really a good \ndescription of what's going on.\n"},{"id":"213516","messageId":"7vhajh15w0.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"20130408000658.GG27178@pug.qqx.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T13:58:07Z","receivedAt":"2013-04-08T13:58:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab <aaron@schrab.com> writes:\n\n> At 16:48 -0700 07 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>\n>>> Do not report an argument to clone's --reference option is not a local\n>>> directory.  Nothing checks for the actual directory so we have no way to\n>>> know if whether or not exists.  Telling the user that a directory doesn't\n>>> exist when that isn't actually known may lead him or her on the wrong\n>>> path to finding the problem.\n>>\n>>I don't understand the above explanation.  Could you give an example?\n>\n> I originally noticed this while trying to use a submodule as a\n> reference repository.\n\nI do agree that it would be nice to dereference .git gitfile when we\ndeal with --reference argument, but you do not want to use in-tree\nrepository of a submodule working tree.  What happens when you have\nto check out a version of the containing superproject that did not\nhave the submodule you are borrowing from?  The directory will\ndisappear, leaving the borrowing repository still pointing at it\nwith its .git/objects/info/alternates file, no?\n"},{"id":"213520","messageId":"20130408145749.GJ27178@pug.qqx.org","threadId":"33424","inReplyTo":"7vhajh15w0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T14:57:49Z","receivedAt":"2013-04-08T14:57:49Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 06:58 -0700 08 Apr 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>I do agree that it would be nice to dereference .git gitfile when we\n>deal with --reference argument, but you do not want to use in-tree\n>repository of a submodule working tree.  What happens when you have\n>to check out a version of the containing superproject that did not\n>have the submodule you are borrowing from?  The directory will\n>disappear, leaving the borrowing repository still pointing at it\n>with its .git/objects/info/alternates file, no?\n\nNo, submodule directories don't get removed when you checkout a version \nwhich didn't contain that submodule.  I believe that there are plans to \nchange that for submodules which store the repository data under the \ncontaining project's .git directory; but removing the submodule working \ntree would not affect a repository using that submodule as a reference, \nsince the reading of the .git file is only done during the initial \nclone.  I don't think that the risk of such a repository being deleted \nor moved is substantially higher than for any other repository.\n"},{"id":"213523","messageId":"7vip3xyr8c.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"20130408145749.GJ27178@pug.qqx.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T15:30:43Z","receivedAt":"2013-04-08T15:30:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab <aaron@schrab.com> writes:\n\n> At 06:58 -0700 08 Apr 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>>I do agree that it would be nice to dereference .git gitfile when we\n>>deal with --reference argument, but you do not want to use in-tree\n>>repository of a submodule working tree.  What happens when you have\n>>to check out a version of the containing superproject that did not\n>>have the submodule you are borrowing from?  The directory will\n>>disappear, leaving the borrowing repository still pointing at it\n>>with its .git/objects/info/alternates file, no?\n>\n> No, submodule directories don't get removed when you checkout a\n> version which didn't contain that submodule.  \n\nIn the old world order, we did not use .git gitfile.\n\nThe version of the superproject had a submodule at dirA/ and\ndirA/.git used to be a real directory.  \"clone --reference\n/path/to/super/dirA/.git\" can borrow objects from there, and will\nwrite /path/to/super/dirA/.git/objects (which is a real object\nstore) to the resulting repository's objects/info/alternates.\n\nYou switch to a version of the superproject with a plain file at\ndirA/ or there is nothing at dirA.  The checkout will fail and you\nneed to manually rectify the situation [*1*], but after that is\ndone, you do not have any repository at /path/to/super/dirA/.git\nanymore.\n\nThat was the reason why I recommended against the practice.\n\nIn the new world order, we use dirA/.git gitfile.\n\n\"clone --reference /path/to/super/dirA/.git\" does not anticipate .git\ncould be a gitfile, but it can be fixed to dereference it and point\nat \"/path/to/super/.git/modules/moduleA\", which will stay there\nacross branch switching at the supermodule level.\n\n\"clone\" has to store /path/to/super/.git/modules/moduleA in\n$GIT_DIR/objects/info/alternates of the new repository by\ndereferencing the value given to --reference.  By doing so, what is\nin the working tree of the superproject would not matter at the time\nof access in the new repository.\n\nSo you are right that we do not remove in the new world order, but\nthen --reference can be given to point at the real location ;-)\n\n\n[Footnote]\n\n*1* ... for which fundamental fix was made to use dirA/.git gitfile\nin the submodule working tree in the new world order.\n"},{"id":"213528","messageId":"20130408161745.GK27178@pug.qqx.org","threadId":"33424","inReplyTo":"7vip3xyr8c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T16:17:46Z","receivedAt":"2013-04-08T16:17:46Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 08:30 -0700 08 Apr 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>You switch to a version of the superproject with a plain file at\n>dirA/ or there is nothing at dirA.  The checkout will fail and you\n>need to manually rectify the situation [*1*], but after that is\n>done, you do not have any repository at /path/to/super/dirA/.git\n>anymore.\n>\n>That was the reason why I recommended against the practice.\n\nSo you're essentially saying you don't want to support using a new-world \nsubmodule as a reference because using an old-world submodule as such is \nlikely to be problematic?  Even though the type of submodule that is \nactually likely to cause problems would currently be accepted as a \nreference repository?  That seems somewhat perverse to me.\n\nAlso, nothing in this series is strictly about submodules; that just \nhappens to be what I was working with when I noticed the issue.  It \nwould apply to any repository created with --separate-git-dir, although \nsubmodules are likely to be the most common occurrence by far.\n\n>So you are right that we do not remove in the new world order, but\n>then --reference can be given to point at the real location ;-)\n\nYes, that's definitely a possibility.  But I think that the location of \nthe work tree for a repository is much more likely to come to a user's \nmind than the location of a non-bare repository.  Especially when \ndealing with submodules where the repository location was decided for \nthe user, and is somewhat of an implementation detail that the user \nshouldn't need to care about.\n"},{"id":"213566","messageId":"7vli8sykf0.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"20130408161745.GK27178@pug.qqx.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T17:57:55Z","receivedAt":"2013-04-08T17:57:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab <aaron@schrab.com> writes:\n\n> At 08:30 -0700 08 Apr 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>>You switch to a version of the superproject with a plain file at\n>>dirA/ or there is nothing at dirA.  The checkout will fail and you\n>>need to manually rectify the situation [*1*], but after that is\n>>done, you do not have any repository at /path/to/super/dirA/.git\n>>anymore.\n>>\n>>That was the reason why I recommended against the practice.\n>\n> So you're essentially saying you don't want to support using a\n> new-world submodule as a reference because using an old-world\n> submodule as such is likely to be problematic?\n\nIn general I am in favor of resolving a gitfile given to --reference\nwhen clone interprets it, and have it use the location of the real\nunderlying object store when it grabs objects not in there from the\norigin and store the location of the real underlying object store in\nthe objects/info/alternates of the newly created repository.  But\nthat is not limited to the gitfile used at the root level of a\nsubmodule checkout.\n\nBlindly using .git at the root level of submodule checkout as a\nreference is what I was recommending against as a general\nprecaution.  You may be dealing with an old-style submodule\ncheckout.\n"},{"id":"213567","messageId":"7vehekykan.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"20130408000845.GH27178@pug.qqx.org","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T18:00:32Z","receivedAt":"2013-04-08T18:00:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[ADMINISTRIVIA: please do not redirect a direct reply to you to\nother people using Mail-Followup-To.]\n\nAaron Schrab <aaron@schrab.com> writes:\n\n> At 16:51 -0700 07 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>>> -\tchar *ref_git;\n>>> +\tchar *ref_git, *repo;\n>>[...]\n>>> +\trepo = (char *)read_gitfile(mkpath(\"%s/.git\", ref_git));\n>>\n>>Why not make repo a \"const char *\" and avoid the cast?  The above\n>>would seem to make it too tempting to treat the return value from\n>>read_gitfile() as a mutable buffer instead of a real_path string that\n>>should be copied asap.\n>\n> Good catch.  I'll fix that in the next version.\n\nThanks.  The patch otherwise looks good to me.\n"},{"id":"213583","messageId":"20130408185823.GL27178@pug.qqx.org","threadId":"33424","inReplyTo":"7vli8sykf0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T18:58:24Z","receivedAt":"2013-04-08T18:58:24Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 10:57 -0700 08 Apr 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>In general I am in favor of resolving a gitfile given to --reference\n>when clone interprets it, and have it use the location of the real\n>underlying object store when it grabs objects not in there from the\n>origin and store the location of the real underlying object store in\n>the objects/info/alternates of the newly created repository.  But\n>that is not limited to the gitfile used at the root level of a\n>submodule checkout.\n\nYes, I agree that it's not limited to submodules.  The commit message \nfor the second part of this series only mentioned submodules because I \nsuspect that is by far the most common use of gitfiles.  The commit \nmessage for the first didn't even mention submodules at all, they were \nonly brought up because I was asked about what lead to me having an \nissue.\n\n>Blindly using .git at the root level of submodule checkout as a\n>reference is what I was recommending against as a general\n>precaution.\n\nI agree with that.  But I still don't think it's relevant to this patch \nseries.\n\n>You may be dealing with an old-style submodule checkout.\n\nNo, the submodule in question was done with the new style.  If it were \nan old-style checkout my attempt to clone using that as a reference \nwould have worked without issue (at least at clone time).\n"},{"id":"213584","messageId":"20130408185957.GM27178@pug.qqx.org","threadId":"33424","inReplyTo":"7vehekykan.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T18:59:57Z","receivedAt":"2013-04-08T18:59:57Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 11:00 -0700 08 Apr 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>Aaron Schrab <aaron@schrab.com> writes:\n>> Good catch.  I'll fix that in the next version.\n>\n>Thanks.  The patch otherwise looks good to me.\n\nGreat, I'll plan to send version 2 of this series later today.\n"},{"id":"213589","messageId":"7vobdox2hz.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"20130408185823.GL27178@pug.qqx.org","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T19:10:16Z","receivedAt":"2013-04-08T19:10:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab <aaron@schrab.com> writes:\n\n>>You may be dealing with an old-style submodule checkout.\n>\n> No, the submodule in question was done with the new style.\n\nI know that and I wasn't talking about _your_ particular case.\n\nI just wanted to make sure people who are reading this thread from\nsidelines (or finding with search engine later) applied that pattern\nblindly.\n"},{"id":"213626","messageId":"1365461200-13509-1-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"20130408185957.GM27178@pug.qqx.org","subject":"[PATCH v2 0/2] Using gitfile repository with clone --reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T22:46:38Z","receivedAt":"2013-04-08T22:46:38Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Here's the promised second version of this series.\n\nThe diff in the first patch is unchanged, but I have made significant\nchanges to the commit message to hopefully to a better job of describing\nwhy I think the old error message is bad.\n\nFor the second patch I've eliminated the need to do a cast.\n\nAlthough I'm sending these as a series, the changes are independent both\ntextually and semantically.\n"},{"id":"213627","messageId":"1365461200-13509-2-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"1365461200-13509-1-git-send-email-aaron@schrab.com","subject":"[PATCH 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T22:46:39Z","receivedAt":"2013-04-08T22:46:39Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Do not report that an argument to clone's --reference option is not a\nlocal directory.  Nothing checks for the existence or type of the path\nas supplied by the user; checks are only done for particular contents of\nthe supposed directory, so we have no way to know the status of the\nsupplied path.  Telling the user that a directory doesn't exist when\nthat isn't actually known may lead him or her on the wrong path to\nfinding the problem.\n\nInstead just state that the entered path is not a local repository which\nis really all that is known about it.  It could be more helpful to state\nthe actual paths which were checked, but I believe that giving a good\ndescription of that would be too verbose for a simple error message and\nwould be too dependent on implementation details.\n\nSigned-off-by: Aaron Schrab <aaron@schrab.com>\n---\n builtin/clone.c |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f9c380e..0a1e0bf 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\n \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git)))\n-\t\tdie(_(\"reference repository '%s' is not a local directory.\"),\n+\t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n \t\t    item->string);\n \n \tstrbuf_addf(&alternate, \"%s/objects\", ref_git);\n-- \n1.7.10.4\n"},{"id":"213628","messageId":"1365461200-13509-3-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"1365461200-13509-1-git-send-email-aaron@schrab.com","subject":"[PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-08T22:46:40Z","receivedAt":"2013-04-08T22:46:40Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Try reading gitfile files when processing --reference options to clone.\nThis will allow, among other things, using a submodule checked out with\na recent version of git as a reference repository without requiring the\nuser to have internal knowledge of submodule layout.\n\nSigned-off-by: Aaron Schrab <aaron@schrab.com>\n---\n builtin/clone.c            |   12 ++++++++++--\n t/t5700-clone-reference.sh |    7 +++++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 0a1e0bf..0dc0791 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -232,11 +232,19 @@ static void strip_trailing_slashes(char *dir)\n static int add_one_reference(struct string_list_item *item, void *cb_data)\n {\n \tchar *ref_git;\n+\tconst char *repo;\n \tstruct strbuf alternate = STRBUF_INIT;\n \n-\t/* Beware: real_path() and mkpath() return static buffer */\n+\t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n \tref_git = xstrdup(real_path(item->string));\n-\tif (is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n+\n+\trepo = read_gitfile(mkpath(\"%s/.git\", ref_git));\n+\tif (repo) {\n+\t\tfree(ref_git);\n+\t\tref_git = xstrdup(repo);\n+\t}\n+\n+\tif (!repo && is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n \t\tchar *ref_git_git = mkpathdup(\"%s/.git\", ref_git);\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex 2a7b78b..7a9044c 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -185,4 +185,11 @@ test_expect_success 'fetch with incomplete alternates' '\n \t! grep \" want $tag_object\" \"$U.K\"\n '\n \n+test_expect_success 'clone using repo with gitfile as a reference' '\n+\tgit clone --separate-git-dir=L A M &&\n+\tgit clone --reference=M A N &&\n+\techo \"$base_dir/L/objects\" > expected &&\n+\ttest_cmp expected \"$base_dir/N/.git/objects/info/alternates\"\n+'\n+\n test_done\n-- \n1.7.10.4\n"},{"id":"213633","messageId":"20130409001817.GV30308@google.com","threadId":"33424","inReplyTo":"1365461200-13509-2-git-send-email-aaron@schrab.com","subject":"Re: [PATCH 1/2] clone: Fix error message for reference repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-09T00:18:17Z","receivedAt":"2013-04-09T00:18:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Aaron Schrab wrote:\n\n> Do not report that an argument to clone's --reference option is not a\n> local directory.  Nothing checks for the existence or type of the path\n> as supplied by the user; checks are only done for particular contents of\n> the supposed directory, so we have no way to know the status of the\n> supplied path.\n\nYes, makes sense.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nMy only remaining qualm is that a person could be confused by the\nmessage after trying to pass in --reference=file:///path/to/repo, but\nI guess that trial and error would eventually lead such a person in\nthe right direction.\n"},{"id":"213634","messageId":"20130409002456.GW30308@google.com","threadId":"33424","inReplyTo":"1365461200-13509-3-git-send-email-aaron@schrab.com","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-09T00:24:56Z","receivedAt":"2013-04-09T00:24:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Aaron Schrab wrote:\n\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -232,11 +232,19 @@ static void strip_trailing_slashes(char *dir)\n>  static int add_one_reference(struct string_list_item *item, void *cb_data)\n>  {\n>  \tchar *ref_git;\n> +\tconst char *repo;\n>  \tstruct strbuf alternate = STRBUF_INIT;\n>  \n> -\t/* Beware: real_path() and mkpath() return static buffer */\n> +\t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n>  \tref_git = xstrdup(real_path(item->string));\n> -\tif (is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n> +\n> +\trepo = read_gitfile(mkpath(\"%s/.git\", ref_git));\n[...]\n> +++ b/t/t5700-clone-reference.sh\n> @@ -185,4 +185,11 @@ test_expect_success 'fetch with incomplete alternates' '\n>  \t! grep \" want $tag_object\" \"$U.K\"\n>  '\n>  \n> +test_expect_success 'clone using repo with gitfile as a reference' '\n> +\tgit clone --separate-git-dir=L A M &&\n> +\tgit clone --reference=M A N &&\n\nWhat should happen if I pass --reference=M/.git?\n\n> +\techo \"$base_dir/L/objects\" > expected &&\n\nThe usual style in tests is to include no space after >redirection\noperators.\n\nWith those two changes,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"213669","messageId":"20130409163149.GA20752@pug.qqx.org","threadId":"33424","inReplyTo":"20130409002456.GW30308@google.com","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-09T16:31:49Z","receivedAt":"2013-04-09T16:31:49Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 17:24 -0700 08 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> +test_expect_success 'clone using repo with gitfile as a reference' '\n>> +\tgit clone --separate-git-dir=L A M &&\n>> +\tgit clone --reference=M A N &&\n>\n>What should happen if I pass --reference=M/.git?\n\nThat isn't supported and I wouldn't expect it to work.  The --reference \noption is documented as taking the location of a repository as the \nargument and I wouldn't consider a .git file to be a repository.  I also \ncan't think of a reason that it would be very useful since it should be \nsimple to just refer to the directory containing the .git file.  But if \nothers disagree, I could be convinced to add support for that.\n\nI also wouldn't consider it breakage if that use would start working, so \nI don't see a point in adding a test to check that that usage fails.\n\n>> +\techo \"$base_dir/L/objects\" > expected &&\n>\n>The usual style in tests is to include no space after >redirection\n>operators.\n\nFixed for the next version, pending further comments.\n"},{"id":"213670","messageId":"7v8v4rtzw4.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"20130409163149.GA20752@pug.qqx.org","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2013-04-09T16:47:07Z","receivedAt":"2013-04-09T16:47:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab <aaron@schrab.com> writes:\n\n> At 17:24 -0700 08 Apr 2013, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>>> +test_expect_success 'clone using repo with gitfile as a reference' '\n>>> +\tgit clone --separate-git-dir=L A M &&\n>>> +\tgit clone --reference=M A N &&\n>>\n>>What should happen if I pass --reference=M/.git?\n>\n> That isn't supported and I wouldn't expect it to work.  The\n> --reference option is documented as taking the location of a\n> repository as the argument and I wouldn't consider a .git file to be a\n> repository.  I also can't think of a reason that it would be very\n> useful since it should be simple to just refer to the directory\n> containing the .git file.  But if others disagree, I could be\n> convinced to add support for that.\n\nIf M/.git weren't a gitfile that points elsewhere, that request\nought to work, no?  A gitfile is the moral equilvalent of a symbolic\nlink, meant to help people on platforms and filesystems that lack\nsymbolic links, so in that sense, not supporting the case goes\nagainst the whole reason why we have added support for gitfile in\nthe first place, I think.\n"},{"id":"213671","messageId":"20130409165022.GB20752@pug.qqx.org","threadId":"33424","inReplyTo":"7v8v4rtzw4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-09T16:50:22Z","receivedAt":"2013-04-09T16:50:22Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 09:47 -0700 09 Apr 2013, Junio C Hamano <junio@pobox.com> wrote:\n>Aaron Schrab <aaron@schrab.com> writes:\n>> But if others disagree, I could be convinced to add support for that.\n>\n>If M/.git weren't a gitfile that points elsewhere, that request\n>ought to work, no?  A gitfile is the moral equilvalent of a symbolic\n>link, meant to help people on platforms and filesystems that lack\n>symbolic links, so in that sense, not supporting the case goes\n>against the whole reason why we have added support for gitfile in\n>the first place, I think.\n\nOK, I'm convinced.  I'll modify it to support that as well.\n"},{"id":"213729","messageId":"1365546120-22048-1-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"7v8v4rtzw4.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3 0/2] Using gitfile repository with clone --reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-09T22:21:58Z","receivedAt":"2013-04-09T22:21:58Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Here's the third version of my series for dealing with gitfiles in clone\n--reference.\n\nThe first patch is unchanged from the previous version except for the\naddition of a Reviewed-by line.\n\nThe second patch has been modified so that it now supports having a .git\nfile supplied as the argument to the option directly rather than only\ndealing with that if the containing directory was supplied.  This makes\nthe first patch from the series more important, since it would make even\nless sense to complain that the path isn't a directory when a\nnon-directory is acceptable.\n\nI've also fixed the minor style issue in the test script from the previous\nversions.\n"},{"id":"213730","messageId":"1365546120-22048-2-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"1365546120-22048-1-git-send-email-aaron@schrab.com","subject":"[PATCH v3 1/2] clone: Fix error message for reference repository","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-09T22:21:59Z","receivedAt":"2013-04-09T22:21:59Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Do not report that an argument to clone's --reference option is not a\nlocal directory.  Nothing checks for the existence or type of the path\nas supplied by the user; checks are only done for particular contents of\nthe supposed directory, so we have no way to know the status of the\nsupplied path.  Telling the user that a directory doesn't exist when\nthat isn't actually known may lead him or her on the wrong path to\nfinding the problem.\n\nInstead just state that the entered path is not a local repository which\nis really all that is known about it.  It could be more helpful to state\nthe actual paths which were checked, but I believe that giving a good\ndescription of that would be too verbose for a simple error message and\nwould be too dependent on implementation details.\n\nSigned-off-by: Aaron Schrab <aaron@schrab.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f9c380e..0a1e0bf 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\n \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git)))\n-\t\tdie(_(\"reference repository '%s' is not a local directory.\"),\n+\t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n \t\t    item->string);\n \n \tstrbuf_addf(&alternate, \"%s/objects\", ref_git);\n-- \n1.8.2.677.g9202ef0\n"},{"id":"213731","messageId":"1365546120-22048-3-git-send-email-aaron@schrab.com","threadId":"33424","inReplyTo":"1365546120-22048-1-git-send-email-aaron@schrab.com","subject":"[PATCH v3 2/2] clone: Allow repo using gitfile as a reference","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-09T22:22:00Z","receivedAt":"2013-04-09T22:22:00Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Try reading gitfile files when processing --reference options to clone.\nThis will allow, among other things, using a submodule checked out with\na recent version of git as a reference repository without requiring the\nuser to have internal knowledge of submodule layout.\n\nSigned-off-by: Aaron Schrab <aaron@schrab.com>\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 0a1e0bf..58fee98 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -232,11 +232,21 @@ static void strip_trailing_slashes(char *dir)\n static int add_one_reference(struct string_list_item *item, void *cb_data)\n {\n \tchar *ref_git;\n+\tconst char *repo;\n \tstruct strbuf alternate = STRBUF_INIT;\n \n-\t/* Beware: real_path() and mkpath() return static buffer */\n+\t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n \tref_git = xstrdup(real_path(item->string));\n-\tif (is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n+\n+\trepo = read_gitfile(ref_git);\n+\tif (!repo)\n+\t\trepo = read_gitfile(mkpath(\"%s/.git\", ref_git));\n+\tif (repo) {\n+\t\tfree(ref_git);\n+\t\tref_git = xstrdup(repo);\n+\t}\n+\n+\tif (!repo && is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n \t\tchar *ref_git_git = mkpathdup(\"%s/.git\", ref_git);\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex 2a7b78b..719d778 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -185,4 +185,17 @@ test_expect_success 'fetch with incomplete alternates' '\n \t! grep \" want $tag_object\" \"$U.K\"\n '\n \n+test_expect_success 'clone using repo with gitfile as a reference' '\n+\tgit clone --separate-git-dir=L A M &&\n+\tgit clone --reference=M A N &&\n+\techo \"$base_dir/L/objects\" >expected &&\n+\ttest_cmp expected \"$base_dir/N/.git/objects/info/alternates\"\n+'\n+\n+test_expect_success 'clone using repo pointed at by gitfile as reference' '\n+\tgit clone --reference=M/.git A O &&\n+\techo \"$base_dir/L/objects\" >expected &&\n+\ttest_cmp expected \"$base_dir/O/.git/objects/info/alternates\"\n+'\n+\n test_done\n-- \n1.8.2.677.g9202ef0\n"},{"id":"213733","messageId":"7v8v4rpbrj.fsf@alter.siamese.dyndns.org","threadId":"33424","inReplyTo":"1365546120-22048-1-git-send-email-aaron@schrab.com","subject":"Re: [PATCH v3 0/2] Using gitfile repository with clone --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-09T22:41:52Z","receivedAt":"2013-04-09T22:41:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab <aaron@schrab.com> writes:\n\n> Here's the third version of my series for dealing with gitfiles in clone\n> --reference.\n>\n> The first patch is unchanged from the previous version except for the\n> addition of a Reviewed-by line.\n>\n> The second patch has been modified so that it now supports having a .git\n> file supplied as the argument to the option directly rather than only\n> dealing with that if the containing directory was supplied.  This makes\n> the first patch from the series more important, since it would make even\n> less sense to complain that the path isn't a directory when a\n> non-directory is acceptable.\n>\n> I've also fixed the minor style issue in the test script from the previous\n> versions.\n\nThanks, will queue.\n"}]}