{"thread":{"id":"30627","subject":"[PATCH 0/3] clone --local fixes","startedAt":"2012-05-26T03:42:26Z","lastAt":"2012-05-30T23:33:10Z","messageCount":19,"participants":["Jeff King","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"192217","messageId":"20120526034226.GA14287@sigill.intra.peff.net","threadId":"30627","inReplyTo":null,"subject":"[PATCH 0/3] clone --local fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-26T03:42:26Z","receivedAt":"2012-05-26T03:42:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Emeric mentioned that he was confused by the behavior of\n\n  git clone --local file:///path/to/repo\n\nin that the --local is silently ignored. Looking over the documentation,\nit is quite unclear whether it is supposed to do anything or not. I\ndon't think it's an especially pressing use case, so one solution could\nbe to simply clarify the current behavior in the documentation.\n\nHowever, I figured this should just be a two-line fix to give it more\nsensible behavior, so why not just make it to do the sensible thing? And\nwhile my initial version was two lines, it ended up growing to handle\nall of the corner cases properly.  So I'm not super-excited about patch\n2, but given that it is written, I think it's a reasonable thing to do\n(and it is not _too_ complex).\n\nAnd then while I was on the subject, I did patch 3, which is something\nthat has bugged me for a long time.\n\n  [1/3]: t5701: modernize style\n  [2/3]: clone: make --local handle URLs\n  [3/3]: clone: allow --no-local to turn off local optimizations\n\n-Peff\n"},{"id":"192218","messageId":"20120526034253.GA14332@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120526034226.GA14287@sigill.intra.peff.net","subject":"[PATCH 1/3] t5701: modernize style","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-26T03:42:53Z","receivedAt":"2012-05-26T03:42:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This test is pretty old and did not follow some of our more\nmodern best practices. In particular:\n\n  1. It chdir'd all over the place, leaving later tests to\n     deal with the fallout. Do our chdirs in subshells\n     instead.\n\n  2. It did not use test_must_fail.\n\n  3. It did not use test_line_count.\n\n  4. It checked for the non-existence of a ref by looking in the\n     .git/refs directory (since we pack refs during clone\n     these days, this will always be succeed, making the\n     test useless).\n\n     Note that one call to \"-e .git/refs/...\" remains,\n     because it is checking for the existence of a symbolic\n     ref, not a ref itself.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI mostly wanted to pull out the repo_is_hardlinked logic for tests I was\nabout to add, but once I got cleaning, I couldn't stop. And who can\nargue with that diffstat?\n\n t/t5701-clone-local.sh | 73 ++++++++++++++------------------------------------\n 1 file changed, 20 insertions(+), 53 deletions(-)\n\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex 6972258..c6feca4 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -3,7 +3,10 @@\n test_description='test local clone'\n . ./test-lib.sh\n \n-D=`pwd`\n+repo_is_hardlinked() {\n+\tfind \"$1/objects\" -type f -links 1 >output &&\n+\ttest_line_count = 0 output\n+}\n \n test_expect_success 'preparing origin repository' '\n \t: >file && git add . && git commit -m1 &&\n@@ -19,105 +22,72 @@ test_expect_success 'preparing origin repository' '\n '\n \n test_expect_success 'local clone without .git suffix' '\n-\tcd \"$D\" &&\n \tgit clone -l -s a b &&\n-\tcd b &&\n+\t(cd b &&\n \ttest \"$(GIT_CONFIG=.git/config git config --bool core.bare)\" = false &&\n-\tgit fetch\n+\tgit fetch)\n '\n \n test_expect_success 'local clone with .git suffix' '\n-\tcd \"$D\" &&\n \tgit clone -l -s a.git c &&\n-\tcd c &&\n-\tgit fetch\n+\t(cd c && git fetch)\n '\n \n test_expect_success 'local clone from x' '\n-\tcd \"$D\" &&\n \tgit clone -l -s x y &&\n-\tcd y &&\n-\tgit fetch\n+\t(cd y && git fetch)\n '\n \n test_expect_success 'local clone from x.git that does not exist' '\n-\tcd \"$D\" &&\n-\tif git clone -l -s x.git z\n-\tthen\n-\t\techo \"Oops, should have failed\"\n-\t\tfalse\n-\telse\n-\t\techo happy\n-\tfi\n+\ttest_must_fail git clone -l -s x.git z\n '\n \n test_expect_success 'With -no-hardlinks, local will make a copy' '\n-\tcd \"$D\" &&\n \tgit clone --bare --no-hardlinks x w &&\n-\tcd w &&\n-\tlinked=$(find objects -type f ! -links 1 | wc -l) &&\n-\ttest 0 = $linked\n+\t! repo_is_hardlinked w\n '\n \n test_expect_success 'Even without -l, local will make a hardlink' '\n-\tcd \"$D\" &&\n \trm -fr w &&\n \tgit clone -l --bare x w &&\n-\tcd w &&\n-\tcopied=$(find objects -type f -links 1 | wc -l) &&\n-\ttest 0 = $copied\n+\trepo_is_hardlinked w\n '\n \n test_expect_success 'local clone of repo with nonexistent ref in HEAD' '\n-\tcd \"$D\" &&\n \techo \"ref: refs/heads/nonexistent\" > a.git/HEAD &&\n \tgit clone a d &&\n-\tcd d &&\n+\t(cd d &&\n \tgit fetch &&\n-\ttest ! -e .git/refs/remotes/origin/HEAD'\n+\ttest ! -e .git/refs/remotes/origin/HEAD)\n+'\n \n test_expect_success 'bundle clone without .bundle suffix' '\n-\tcd \"$D\" &&\n \tgit clone dir/b3 &&\n-\tcd b3 &&\n-\tgit fetch\n+\t(cd b3 && git fetch)\n '\n \n test_expect_success 'bundle clone with .bundle suffix' '\n-\tcd \"$D\" &&\n \tgit clone b1.bundle &&\n-\tcd b1 &&\n-\tgit fetch\n+\t(cd b1 && git fetch)\n '\n \n test_expect_success 'bundle clone from b4' '\n-\tcd \"$D\" &&\n \tgit clone b4 bdl &&\n-\tcd bdl &&\n-\tgit fetch\n+\t(cd bdl && git fetch)\n '\n \n test_expect_success 'bundle clone from b4.bundle that does not exist' '\n-\tcd \"$D\" &&\n-\tif git clone b4.bundle bb\n-\tthen\n-\t\techo \"Oops, should have failed\"\n-\t\tfalse\n-\telse\n-\t\techo happy\n-\tfi\n+\ttest_must_fail git clone b4.bundle bb\n '\n \n test_expect_success 'bundle clone with nonexistent HEAD' '\n-\tcd \"$D\" &&\n \tgit clone b2.bundle b2 &&\n-\tcd b2 &&\n+\t(cd b2 &&\n \tgit fetch &&\n-\ttest ! -e .git/refs/heads/master\n+\ttest_must_fail git rev-parse --verify refs/heads/master)\n '\n \n test_expect_success 'clone empty repository' '\n-\tcd \"$D\" &&\n \tmkdir empty &&\n \t(cd empty &&\n \t git init &&\n@@ -135,7 +105,6 @@ test_expect_success 'clone empty repository' '\n '\n \n test_expect_success 'clone empty repository, and then push should not segfault.' '\n-\tcd \"$D\" &&\n \trm -fr empty/ empty-clone/ &&\n \tmkdir empty &&\n \t(cd empty && git init) &&\n@@ -145,13 +114,11 @@ test_expect_success 'clone empty repository, and then push should not segfault.'\n '\n \n test_expect_success 'cloning non-existent directory fails' '\n-\tcd \"$D\" &&\n \trm -rf does-not-exist &&\n \ttest_must_fail git clone does-not-exist\n '\n \n test_expect_success 'cloning non-git directory fails' '\n-\tcd \"$D\" &&\n \trm -rf not-a-git-repo not-a-git-repo-clone &&\n \tmkdir not-a-git-repo &&\n \ttest_must_fail git clone not-a-git-repo not-a-git-repo-clone\n-- \n1.7.10.1.21.g62fda49.dirty\n"},{"id":"192219","messageId":"20120526034532.GB14332@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120526034226.GA14287@sigill.intra.peff.net","subject":"[PATCH 2/3] clone: make --local handle URLs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-26T03:45:32Z","receivedAt":"2012-05-26T03:45:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Once upon a time, the --local flag was used to inform git\nthat it should perform some optimizations when cloning a\nlocal repository. These days, we perform those optimizations\nautomatically, and --local is largely a no-op. The one\nexception is that we will complain when \"--local\" is given\nif hardlinking fails, whereas the default behavior is to\nsimply fallback to a copy.\n\nUsing --local with anything except a local path is also a\nno-op, because we never bother to look at the flag. So this:\n\n  git clone --local file:///path/to/repo\n\nsilently ignores the local flag, when a more sensible\nbehavior would be to treat it the same as:\n\n  git clone --local /path/to/repo\n\nLikewise, this:\n\n  git clone --local http://example.com/repo.git\n\nis nonsense, but git silently accepts it.\n\nThis patch causes --local to treat file:// URLs as local,\nand to flag an error when nonsensical combinations are\nrequested.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI had hoped this could just be:\n\n  if (opt_local && !prefixcmp(repo_name, \"file://\"))\n          repo_name += 7;\n\nbut we have to handle URL decoding. And did you know that file:// URLs\ncan have a hostname in them? How useless. I made sure this behaves the\nsame as the parser in connect.c, though I couldn't find a good way to\nshare the code without making both sides less readable.\n\n Documentation/git-clone.txt | 17 ++++++++++-------\n builtin/clone.c             | 26 ++++++++++++++++++++++++++\n t/t5550-http-fetch.sh       |  4 ++++\n t/t5701-clone-local.sh      | 15 +++++++++++++++\n 4 files changed, 55 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 6e22522..589f3ce 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -46,13 +46,16 @@ OPTIONS\n \tmechanism and clones the repository by making a copy of\n \tHEAD and everything under objects and refs directories.\n \tThe files under `.git/objects/` directory are hardlinked\n-\tto save space when possible.  This is now the default when\n-\tthe source repository is specified with `/path/to/repo`\n-\tsyntax, so it essentially is a no-op option.  To force\n-\tcopying instead of hardlinking (which may be desirable\n-\tif you are trying to make a back-up of your repository),\n-\tbut still avoid the usual \"git aware\" transport\n-\tmechanism, `--no-hardlinks` can be used.\n+\tto save space when possible.\n++\n+This is the default when the source repository is specified with\n+`/path/to/repo`, but not with a `file://` URL; specifying `--local` with\n+a file URL will bypass the regular transport mechanism as if a direct\n+path had been provided.\n++\n+To force copying instead of hardlinking (which may be desirable if you\n+are trying to make a back-up of your repository), but still avoid the\n+usual \"git aware\" transport mechanism, `--no-hardlinks` can be used.\n \n --no-hardlinks::\n \tOptimize the cloning process from a repository on a\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex a4d8d25..05944db 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -23,6 +23,7 @@\n #include \"branch.h\"\n #include \"remote.h\"\n #include \"run-command.h\"\n+#include \"url.h\"\n \n /*\n  * Overall FIXMEs:\n@@ -607,6 +608,22 @@ static void write_config(struct string_list *config)\n \t}\n }\n \n+static const char *url_to_local_path(const char *url)\n+{\n+\tconst char *decoded = url_decode(url);\n+\tconst char *p;\n+\n+\tdecoded = skip_prefix(decoded, \"file://\");\n+\tif (!decoded)\n+\t\treturn NULL;\n+\n+\tif (has_dos_drive_prefix(decoded))\n+\t\treturn decoded;\n+\n+\tp = strchr(decoded, '/');\n+\treturn p ? p : decoded;\n+}\n+\n int cmd_clone(int argc, const char **argv, const char *prefix)\n {\n \tint is_bundle = 0, is_local;\n@@ -666,6 +683,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\trepo = xstrdup(absolute_path(repo_name));\n \telse if (!strchr(repo_name, ':'))\n \t\tdie(_(\"repository '%s' does not exist\"), repo_name);\n+\telse if (option_local) {\n+\t\tconst char *url_path = url_to_local_path(repo_name);\n+\t\tif (!url_path)\n+\t\t\tdie(_(\"cannot use --local with a non-local URL\"));\n+\t\tpath = get_repo_path(url_path, &is_bundle);\n+\t\tif (!path)\n+\t\t\tdie(_(\"repository '%s' does not exist\"), url_path);\n+\t\trepo = xstrdup(absolute_path(url_path));\n+\t}\n \telse\n \t\trepo = repo_name;\n \tis_local = path && !is_bundle;\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex b06f817..b8d77b6 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -40,6 +40,10 @@ test_expect_success 'clone http repository' '\n \ttest_cmp file clone/file\n '\n \n+test_expect_success 'cloning http with --local fails' '\n+\ttest_must_fail git clone --local $HTTPD_URL/dumb/repo.git local-clone\n+'\n+\n test_expect_success 'create password-protected repository' '\n \tmkdir \"$HTTPD_DOCUMENT_ROOT_PATH/auth/\" &&\n \tcp -Rf \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" \\\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex c6feca4..d2e0165 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -124,4 +124,19 @@ test_expect_success 'cloning non-git directory fails' '\n \ttest_must_fail git clone not-a-git-repo not-a-git-repo-clone\n '\n \n+test_expect_success 'cloning file:// turns off local optimizations' '\n+\tgit clone --bare file://\"$PWD\"/a non-local &&\n+\t! repo_is_hardlinked non-local\n+'\n+\n+test_expect_success 'cloning file:// with --local uses hardlinks' '\n+\tgit clone --bare --local file://\"$PWD\"/a force-local &&\n+\trepo_is_hardlinked force-local\n+'\n+\n+test_expect_success 'cloning file:// with --local parses URL properly' '\n+\tgit clone --bare --local file://host/\"$PWD\"/%61 force-local-odd &&\n+\trepo_is_hardlinked force-local-odd\n+'\n+\n test_done\n-- \n1.7.10.1.21.g62fda49.dirty\n"},{"id":"192220","messageId":"20120526034550.GC14332@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120526034226.GA14287@sigill.intra.peff.net","subject":"[PATCH 3/3] clone: allow --no-local to turn off local optimizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-26T03:45:50Z","receivedAt":"2012-05-26T03:45:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is basically the same as using \"file://\", but is a\nlittle less subtle for the end user.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-clone.txt |  4 +++-\n builtin/clone.c             | 12 ++++++------\n t/t5701-clone-local.sh      |  5 +++++\n 3 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 589f3ce..f388374 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -51,7 +51,9 @@ OPTIONS\n This is the default when the source repository is specified with\n `/path/to/repo`, but not with a `file://` URL; specifying `--local` with\n a file URL will bypass the regular transport mechanism as if a direct\n-path had been provided.\n+path had been provided. Specifying `--no-local` will cancel any previous\n+`--local`, and will override the default when `/path/to/repo` is given,\n+using the regular git transport instead.\n +\n To force copying instead of hardlinking (which may be desirable if you\n are trying to make a back-up of your repository), but still avoid the\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 05944db..d004abb 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -39,7 +39,7 @@ static const char * const builtin_clone_usage[] = {\n };\n \n static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n-static int option_local, option_no_hardlinks, option_shared, option_recursive;\n+static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n static char *option_template, *option_depth;\n static char *option_origin = NULL;\n static char *option_branch = NULL;\n@@ -71,8 +71,8 @@ static struct option builtin_clone_options[] = {\n \t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n \tOPT_BOOLEAN(0, \"mirror\", &option_mirror,\n \t\t    \"create a mirror repository (implies bare)\"),\n-\tOPT_BOOLEAN('l', \"local\", &option_local,\n-\t\t    \"to clone from a local repository\"),\n+\tOPT_BOOL('l', \"local\", &option_local,\n+\t\t\"to clone from a local repository\"),\n \tOPT_BOOLEAN(0, \"no-hardlinks\", &option_no_hardlinks,\n \t\t    \"don't use local hardlinks, always copy\"),\n \tOPT_BOOLEAN('s', \"shared\", &option_shared,\n@@ -343,7 +343,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \t\tif (!option_no_hardlinks) {\n \t\t\tif (!link(src->buf, dest->buf))\n \t\t\t\tcontinue;\n-\t\t\tif (option_local)\n+\t\t\tif (option_local > 0)\n \t\t\t\tdie_errno(_(\"failed to create link '%s'\"), dest->buf);\n \t\t\toption_no_hardlinks = 1;\n \t\t}\n@@ -683,7 +683,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\trepo = xstrdup(absolute_path(repo_name));\n \telse if (!strchr(repo_name, ':'))\n \t\tdie(_(\"repository '%s' does not exist\"), repo_name);\n-\telse if (option_local) {\n+\telse if (option_local > 0) {\n \t\tconst char *url_path = url_to_local_path(repo_name);\n \t\tif (!url_path)\n \t\t\tdie(_(\"cannot use --local with a non-local URL\"));\n@@ -694,7 +694,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t}\n \telse\n \t\trepo = repo_name;\n-\tis_local = path && !is_bundle;\n+\tis_local = option_local != 0 && path && !is_bundle;\n \tif (is_local && option_depth)\n \t\twarning(_(\"--depth is ignored in local clones; use file:// instead.\"));\n \ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex d2e0165..64faff1 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -139,4 +139,9 @@ test_expect_success 'cloning file:// with --local parses URL properly' '\n \trepo_is_hardlinked force-local-odd\n '\n \n+test_expect_success 'cloning a local path with --no-local does not hardlink' '\n+\tgit clone --bare --no-local a force-nonlocal &&\n+\t! repo_is_hardlinked force-nonlocal\n+'\n+\n test_done\n-- \n1.7.10.1.21.g62fda49.dirty\n"},{"id":"192222","messageId":"20120526041135.GA28957@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120526034226.GA14287@sigill.intra.peff.net","subject":"[PATCH 4/3] clone: send diagnostic messages to stderr","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-26T04:11:35Z","receivedAt":"2012-05-26T04:11:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Putting messages like \"Cloning into..\" and \"done\" on stdout\nis un-Unix and uselessly clutters the stdout channel. Send\nthem to stderr.\n\nWe have to tweak two tests to accommodate this:\n\n  1. t5601 checks for doubled output due to forking, and\n     doesn't actually care where the output goes; adjust it\n     to check stderr.\n\n  2. t5702 is trying to test whether progress output was\n     sent to stderr, but naively does so by checking\n     whether stderr produced any output. Instead, have it\n     look for \"%\", a token found in progress output but not\n     elsewhere (and which lets us avoid hard-coding the\n     progress text in the test).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis one isn't really related to the other patches in the series, but\nwhile we're on the subject of extremely minor git-clone annoyances, I\nthought I'd throw it in as a bonus round.\n\nArguably the test in t5601 should just go away entirely. stderr tends to\nbe line-buffered anyway, so the thing it is testing for wouldn't happen.\nNot to mention that according to 2c3766f, which introduced it, the\nproblem was due to start_async() not flushing output before forking.\nBut we long ago switched start_async to use pthreads, so the bug it is\ntesting for wouldn't even be detectable on any modern platform.\n\n builtin/clone.c          | 6 +++---\n t/t5601-clone.sh         | 2 +-\n t/t5702-clone-options.sh | 4 ++--\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex d004abb..08470ed 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -371,7 +371,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \t}\n \n \tif (0 <= option_verbosity)\n-\t\tprintf(_(\"done.\\n\"));\n+\t\tfprintf(stderr, _(\"done.\\n\"));\n }\n \n static const char *junk_work_tree;\n@@ -751,9 +751,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tif (0 <= option_verbosity) {\n \t\tif (option_bare)\n-\t\t\tprintf(_(\"Cloning into bare repository '%s'...\\n\"), dir);\n+\t\t\tfprintf(stderr, _(\"Cloning into bare repository '%s'...\\n\"), dir);\n \t\telse\n-\t\t\tprintf(_(\"Cloning into '%s'...\\n\"), dir);\n+\t\t\tfprintf(stderr, _(\"Cloning into '%s'...\\n\"), dir);\n \t}\n \tinit_db(option_template, INIT_DB_QUIET);\n \twrite_config(&option_config);\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 67869b4..aa9f991 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -36,7 +36,7 @@ test_expect_success 'clone with excess parameters (2)' '\n \n test_expect_success C_LOCALE_OUTPUT 'output from clone' '\n \trm -fr dst &&\n-\tgit clone -n \"file://$(pwd)/src\" dst >output &&\n+\tgit clone -n \"file://$(pwd)/src\" dst >output 2>&1 &&\n \ttest $(grep Clon output | wc -l) = 1\n '\n \ndiff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh\nindex 02cb024..67e170e 100755\n--- a/t/t5702-clone-options.sh\n+++ b/t/t5702-clone-options.sh\n@@ -22,14 +22,14 @@ test_expect_success 'clone -o' '\n test_expect_success 'redirected clone' '\n \n \tgit clone \"file://$(pwd)/parent\" clone-redirected >out 2>err &&\n-\ttest ! -s err\n+\t! grep % err\n \n '\n test_expect_success 'redirected clone -v' '\n \n \tgit clone --progress \"file://$(pwd)/parent\" clone-redirected-progress \\\n \t\t>out 2>err &&\n-\ttest -s err\n+\tgrep % err\n \n '\n \n-- \n1.7.10.1.21.g62fda49.dirty\n"},{"id":"192258","messageId":"7vsjemp20j.fsf@alter.siamese.dyndns.org","threadId":"30627","inReplyTo":"20120526034226.GA14287@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] clone --local fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-27T06:32:44Z","receivedAt":"2012-05-27T06:32:44Z","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> Emeric mentioned that he was confused by the behavior of\n>\n>   git clone --local file:///path/to/repo\n>\n> in that the --local is silently ignored. Looking over the documentation,\n> it is quite unclear whether it is supposed to do anything or not.\n\nAccording to 23d5335 (git clone: do not issue warning while cloning\nlocally across filesystems, 2007-08-20) and 3d5c418 (git-clone:\naggressively optimize local clone behaviour., 2007-08-01), in the very\nearly days we only used the local optimization when the end user\nexplicitly asked for it with the '--local' option.\n\nAfter 3d5c418f, the option has been made a no-op, because the local-ness\nof the $repository parameter, where the definition of the local-ness is\n\"does 'cd $repository' succeed?\", is all that is needed to trigger the\ncodepath.\n\nThe logic to choose the local optimization has since been \"is it a path to\na local directory---if so, use the optimizaiton.  Otherwise do the usual\nURL based thing\".  And that is why we tell people when they want to try\ncloning without local optimization to add \"file://\" in front of the path.\n\nSo your example \"file:///\" should *not* work even if --local is given,\nunless you happen to have a directory called \"file:\" in your current\ndirectory and it has path/to/repo subdirectory, which is a git repository.\nSpecifically, the repository at /path/to/repo is not what the command line\nis naming.\n\nI think we probably should stop advertising --local in the documentation\nand help text.\n"},{"id":"192306","messageId":"20120528053602.GA11174@sigill.intra.peff.net","threadId":"30627","inReplyTo":"7vsjemp20j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] clone --local fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-28T05:36:03Z","receivedAt":"2012-05-28T05:36:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 26, 2012 at 11:32:44PM -0700, Junio C Hamano wrote:\n\n> So your example \"file:///\" should *not* work even if --local is given,\n> unless you happen to have a directory called \"file:\" in your current\n> directory and it has path/to/repo subdirectory, which is a git repository.\n> Specifically, the repository at /path/to/repo is not what the command line\n> is naming.\n\nI think it depends on the definition of \"--local\". If it means \"when we\nare cloning without a URL, turn on the local optimizations\", then yes,\n\"file://\" should not work. If it means \"turn on local optimizations if\nthis destination supports it\", then it should.\n\nThe current behavior is ambiguous as to whether it is the first case, or\nwhether it is the second, and it was simply buggy. The history you gave\nargues that the original intent was the former. But to me that is much\nless important than what is useful and least surprising to users.\n\nThere are basically three sane behaviors for \"git clone --local\nfile://\":\n\n  1. --local is silently ignored, because it means \"if we are local,\n     turn on optimizations\" (though it has a horrible name, in that case)\n\n  2. it's an error; you should not use --local with a URL\n\n  3. it should use local optimizations (because file:// is local)\n\nIs there a compelling reason not to do (3)? It seems to be the\nfriendliest and least surprising to me.\n\nI'll admit I don't care too much about this use case. People don't\ntend to type \"file://...\" when they could type the simpler thing, so I\ndoubt anyone is hurting. I just don't see a reason not to make it work,\nbesides the usual \"it is a non-zero amount of code\".\n\n> I think we probably should stop advertising --local in the documentation\n> and help text.\n\nI kind of agree, and that was what I was going to do originally (instead\nof making it work). But I do think that \"--no-local\" (from my patch 3)\nis actually useful in practice, so I'd rather not drop the option from\nthe documentation entirely.\n\n-Peff\n"},{"id":"192389","messageId":"4FC3C46D.5070704@kdbg.org","threadId":"30627","inReplyTo":"20120526034532.GB14332@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] clone: make --local handle URLs","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-28T18:31:09Z","receivedAt":"2012-05-28T18:31:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.05.2012 05:45, schrieb Jeff King:\n> And did you know that file:// URLs\n> can have a hostname in them? How useless.\n...\n> +test_expect_success 'cloning file:// turns off local optimizations' '\n> +\tgit clone --bare file://\"$PWD\"/a non-local &&\n> +\t! repo_is_hardlinked non-local\n> +'\n> +\n> +test_expect_success 'cloning file:// with --local uses hardlinks' '\n> +\tgit clone --bare --local file://\"$PWD\"/a force-local &&\n> +\trepo_is_hardlinked force-local\n> +'\n> +\n> +test_expect_success 'cloning file:// with --local parses URL properly' '\n> +\tgit clone --bare --local file://host/\"$PWD\"/%61 force-local-odd &&\n> +\trepo_is_hardlinked force-local-odd\n> +'\n\nI'm pretty certain that we must use file://c:/path/to/repo on Windows to\nmake these work. This means we need $(pwd) rather than $PWD. But what\ndoes this mean w.r.t. parsing the URL in the strict sense? Is \"c:\" the\nhost part?\n\n-- Hannes\n"},{"id":"192392","messageId":"20120528191030.GB2478@sigill.intra.peff.net","threadId":"30627","inReplyTo":"4FC3C46D.5070704@kdbg.org","subject":"Re: [PATCH 2/3] clone: make --local handle URLs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-28T19:10:30Z","receivedAt":"2012-05-28T19:10:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 28, 2012 at 08:31:09PM +0200, Johannes Sixt wrote:\n\n> Am 26.05.2012 05:45, schrieb Jeff King:\n> > And did you know that file:// URLs\n> > can have a hostname in them? How useless.\n> ...\n> > +test_expect_success 'cloning file:// turns off local optimizations' '\n> > +\tgit clone --bare file://\"$PWD\"/a non-local &&\n> > +\t! repo_is_hardlinked non-local\n> > +'\n> > +\n> > +test_expect_success 'cloning file:// with --local uses hardlinks' '\n> > +\tgit clone --bare --local file://\"$PWD\"/a force-local &&\n> > +\trepo_is_hardlinked force-local\n> > +'\n> > +\n> > +test_expect_success 'cloning file:// with --local parses URL properly' '\n> > +\tgit clone --bare --local file://host/\"$PWD\"/%61 force-local-odd &&\n> > +\trepo_is_hardlinked force-local-odd\n> > +'\n> \n> I'm pretty certain that we must use file://c:/path/to/repo on Windows to\n> make these work. This means we need $(pwd) rather than $PWD.\n\nAh, sorry, I always forget your pwd woes.\n\n> But what does this mean w.r.t. parsing the URL in the strict sense? Is\n> \"c:\" the host part?\n\nTechnically, yes. But there is magic in connect.c to handle\nfile://c:/path/to/repo, and include \"c:/\" as part of the path. And I\nreplicated that logic in my patch (it's the  call to\nhas_dos_drive_prefix in url_to_local_path).\n\n-Peff\n"},{"id":"192418","messageId":"7vhauy6fy3.fsf@alter.siamese.dyndns.org","threadId":"30627","inReplyTo":"20120528053602.GA11174@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] clone --local fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-29T17:43:32Z","receivedAt":"2012-05-29T17:43:32Z","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> I think it depends on the definition of \"--local\". If it means \"when we\n> are cloning without a URL, turn on the local optimizations\", then yes,\n> \"file://\" should not work. If it means \"turn on local optimizations if\n> this destination supports it\", then it should.\n\nIt has meant the former since the day --local was introduced, and\nthe semi-deprecation at 3d5c418 (git-clone: aggressively optimize\nlocal clone behaviour., 2007-08-01) didn't change it, either.\n\n> The current behavior is ambiguous as to whether it is the first case, or\n> whether it is the second, and it was simply buggy. The history you gave\n> argues that the original intent was the former. But to me that is much\n> less important than what is useful and least surprising to users.\n\nChanging it would make it even more confusing to people who started\nusing Git before mid 2007, though.  That is why I am for deprecating\n(and eventually removing) \"--local\".\n\nIt is not like we do not have a way to force \"local\" behaviour.\nJust give a local pathname and \"local\" behaviour will kick in; if\nyou do not want \"local\" behaviour, give network URL or file:// URL.\nIf there were no old scripts, there is no reason to have \"--local\"\nin the supported options list of the command.\n\nHaving said all that, because the conclusion above is \"we need to\nkeep --local for now for old scripts, and its behaviour for sane\ncase should not change\", there are some glitches we may want to\naddress for people who try to use \"--local\" without knowing that it\nno longer is necessary to use it if you live in modern age.  For\nexample:\n\n $ git clone --local $URL\n\ndoes not error out, when $URL does not name a repository in the\nlocal filesystem. I think that *can* be changed without breaking old\nscripts.  We usually probe to see if $URL is a pathname of a local\nrepository, and then fall back to transports, but we should error\nout when --local is explicitly asked for, instead of falling back.\n\n> There are basically three sane behaviors for \"git clone --local\n> file://\":\n>\n>   1. --local is silently ignored, because it means \"if we are local,\n>      turn on optimizations\" (though it has a horrible name, in that case)\n\nThat is my first preference; \"--local\" started as an opt-in\nexperiment until it turns out to be useful and became no-op because\nof 3d5c418, and for the purpose of that experiment, \"--local\" was\nperfectly a good name.  It is a no-op now.\n\n>   2. it's an error; you should not use --local with a URL\n\nAnd this is my first preference for a longer time deprecation plan\n(see above).\n\n>   3. it should use local optimizations (because file:// is local)\n\nHrm.  \"file://\" has always been a way to say \"do not use local\noptimization\".  Doesn't it feel simply insane to invent \"--local\nfile://\" as a way to say \"even though the second word on this\ncommand line tells you not to use the local optimization, by adding\nthis newly redefined --local option, I am telling you to use the\nlocal codepath after all\"?  Instead, I would prefer to see us just\nfinish the deprecation that started long time ago.\n"},{"id":"192472","messageId":"20120530110305.GA13445@sigill.intra.peff.net","threadId":"30627","inReplyTo":"7vhauy6fy3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] clone --local fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-30T11:03:05Z","receivedAt":"2012-05-30T11:03:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 29, 2012 at 10:43:32AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I think it depends on the definition of \"--local\". If it means \"when we\n> > are cloning without a URL, turn on the local optimizations\", then yes,\n> > \"file://\" should not work. If it means \"turn on local optimizations if\n> > this destination supports it\", then it should.\n> \n> It has meant the former since the day --local was introduced, and\n> the semi-deprecation at 3d5c418 (git-clone: aggressively optimize\n> local clone behaviour., 2007-08-01) didn't change it, either.\n\nRight, but my argument was that since nobody probably ever cared about\nthe distinction, it is more important to do the least surprising thing.\n\n> > The current behavior is ambiguous as to whether it is the first case, or\n> > whether it is the second, and it was simply buggy. The history you gave\n> > argues that the original intent was the former. But to me that is much\n> > less important than what is useful and least surprising to users.\n> \n> Changing it would make it even more confusing to people who started\n> using Git before mid 2007, though.  That is why I am for deprecating\n> (and eventually removing) \"--local\".\n\nYes, it would technically be a regression. But I highly doubt that\nsomebody is relying on the fact that \"--local\" with file:// is a silent\nno-op. And other than that, the behavior remains the same (note that my\npatch explicitly did not try to turn on --local when it sees file://).\n\nAnyway, I do not really care about this part of the series. It was not\n\"this is a needed feature\" but rather \"this is less surprising and easy\nto do, so why not?\". We can drop it and replace it with a documentation\nupdate.\n\nI would still like to keep the --no-local patch, for two reasons:\n\n  1. The fact that using file:// overrides the local optimizations is\n     somewhat non-obvious if you do not already know that it is the\n     case.\n\n  2. File URLs require absolute paths[1]. You can't do the equivalent of\n     \"git clone --no-local foo.git\" without resorting to $PWD.\n\nSo here's an updated series (I see you took the test cleanups already).\n\n  [1/2]: docs/clone: mention that --local may be ignored\n  [2/2]: clone: allow --no-local to turn off local optimizations\n\nI didn't bother with deprecation or erroring out on URLs. So far we have\nhad exactly one report on this, and I think the improved documentation\nwould have solved this particular case.\n\n-Peff\n"},{"id":"192473","messageId":"20120530110819.GB13445@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120530110305.GA13445@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] clone --local fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-30T11:08:19Z","receivedAt":"2012-05-30T11:08:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 30, 2012 at 07:03:05AM -0400, Jeff King wrote:\n\n>   2. File URLs require absolute paths[1]. You can't do the equivalent of\n>      \"git clone --no-local foo.git\" without resorting to $PWD.\n\nI forgot to include my footnote, which was:\n\n  You can actually do \"git clone file://foo\" and we will treat that as\n  the path \"foo\". Which does not match the definition of the \"file\"\n  scheme, which would interpret that as \"host 'foo', no path\", but does\n  match what some other software does. However, if there _is_ a slash,\n  then we correctly throw away the host bit, so that \"file://junk/foo\"\n  _also_ points to the path \"foo\". Which means there is no equivalent\n  for:\n\n    git clone --no-local subdir/foo.git\n\n-Peff\n"},{"id":"192474","messageId":"20120530110907.GA15550@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120530110305.GA13445@sigill.intra.peff.net","subject":"[PATCH 1/2] docs/clone: mention that --local may be ignored","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-30T11:09:08Z","receivedAt":"2012-05-30T11:09:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The --local flag is not \"treat this like a local\nrepository\", but rather \"if we are local, turn on\noptimizations\". Therefore it does nothing in the case of:\n\n  git clone --local file:///path/to/repo\n\nLet's make that more clear in the documentation.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-clone.txt | 17 ++++++++++-------\n 1 file changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 6e22522..1d267f4 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -46,13 +46,16 @@ OPTIONS\n \tmechanism and clones the repository by making a copy of\n \tHEAD and everything under objects and refs directories.\n \tThe files under `.git/objects/` directory are hardlinked\n-\tto save space when possible.  This is now the default when\n-\tthe source repository is specified with `/path/to/repo`\n-\tsyntax, so it essentially is a no-op option.  To force\n-\tcopying instead of hardlinking (which may be desirable\n-\tif you are trying to make a back-up of your repository),\n-\tbut still avoid the usual \"git aware\" transport\n-\tmechanism, `--no-hardlinks` can be used.\n+\tto save space when possible.\n++\n+If the repository is specified as a local path (e.g., `/path/to/repo`),\n+this is the default, and --local is essentially a no-op.  If the\n+repository is specified as a URL, then this flag is ignored (and we\n+never use the local optimizations).\n++\n+To force copying instead of hardlinking (which may be desirable if you\n+are trying to make a back-up of your repository), but still avoid the\n+usual \"git aware\" transport mechanism, `--no-hardlinks` can be used.\n \n --no-hardlinks::\n \tOptimize the cloning process from a repository on a\n-- \n1.7.11.rc0.12.g6048c92\n"},{"id":"192476","messageId":"20120530111016.GB15550@sigill.intra.peff.net","threadId":"30627","inReplyTo":"20120530110305.GA13445@sigill.intra.peff.net","subject":"[PATCH 2/2] clone: allow --no-local to turn off local optimizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-30T11:10:16Z","receivedAt":"2012-05-30T11:10:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is basically the same as using \"file://\", but is a\nlittle less subtle for the end user. It also allows relative\npaths to be specified.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-clone.txt |  4 +++-\n builtin/clone.c             | 10 +++++-----\n t/t5701-clone-local.sh      | 10 ++++++++++\n 3 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 1d267f4..c1ddd4c 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -51,7 +51,9 @@ OPTIONS\n If the repository is specified as a local path (e.g., `/path/to/repo`),\n this is the default, and --local is essentially a no-op.  If the\n repository is specified as a URL, then this flag is ignored (and we\n-never use the local optimizations).\n+never use the local optimizations).  Specifying `--no-local` will\n+override the default when `/path/to/repo` is given, using the regular\n+git transport instead.\n +\n To force copying instead of hardlinking (which may be desirable if you\n are trying to make a back-up of your repository), but still avoid the\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex a4d8d25..7f3b982 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -38,7 +38,7 @@ static const char * const builtin_clone_usage[] = {\n };\n \n static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n-static int option_local, option_no_hardlinks, option_shared, option_recursive;\n+static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n static char *option_template, *option_depth;\n static char *option_origin = NULL;\n static char *option_branch = NULL;\n@@ -70,8 +70,8 @@ static struct option builtin_clone_options[] = {\n \t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n \tOPT_BOOLEAN(0, \"mirror\", &option_mirror,\n \t\t    \"create a mirror repository (implies bare)\"),\n-\tOPT_BOOLEAN('l', \"local\", &option_local,\n-\t\t    \"to clone from a local repository\"),\n+\tOPT_BOOL('l', \"local\", &option_local,\n+\t\t\"to clone from a local repository\"),\n \tOPT_BOOLEAN(0, \"no-hardlinks\", &option_no_hardlinks,\n \t\t    \"don't use local hardlinks, always copy\"),\n \tOPT_BOOLEAN('s', \"shared\", &option_shared,\n@@ -342,7 +342,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \t\tif (!option_no_hardlinks) {\n \t\t\tif (!link(src->buf, dest->buf))\n \t\t\t\tcontinue;\n-\t\t\tif (option_local)\n+\t\t\tif (option_local > 0)\n \t\t\t\tdie_errno(_(\"failed to create link '%s'\"), dest->buf);\n \t\t\toption_no_hardlinks = 1;\n \t\t}\n@@ -668,7 +668,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"repository '%s' does not exist\"), repo_name);\n \telse\n \t\trepo = repo_name;\n-\tis_local = path && !is_bundle;\n+\tis_local = option_local != 0 && path && !is_bundle;\n \tif (is_local && option_depth)\n \t\twarning(_(\"--depth is ignored in local clones; use file:// instead.\"));\n \ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex c6feca4..2d6121f 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -124,4 +124,14 @@ test_expect_success 'cloning non-git directory fails' '\n \ttest_must_fail git clone not-a-git-repo not-a-git-repo-clone\n '\n \n+test_expect_success 'cloning file:// does not hardlink' '\n+\tgit clone --bare file://\"$(pwd)\"/a non-local &&\n+\t! repo_is_hardlinked non-local\n+'\n+\n+test_expect_success 'cloning a local path with --no-local does not hardlink' '\n+\tgit clone --bare --no-local a force-nonlocal &&\n+\t! repo_is_hardlinked force-nonlocal\n+'\n+\n test_done\n-- \n1.7.11.rc0.12.g6048c92\n"},{"id":"192498","messageId":"7vzk8pziuk.fsf@alter.siamese.dyndns.org","threadId":"30627","inReplyTo":"20120530111016.GB15550@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] clone: allow --no-local to turn off local optimizations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-30T17:20:19Z","receivedAt":"2012-05-30T17:20:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Makes sense; thanks.\n\nBy the way, a somewhat funny exercise I found last night is this:\n\n\t$ cd /var/tmp\n        $ mkdir -p foo.xz:pub/git.git\n        $ cd foo.xz:pub/git.git\n        $ git init\n        $ git commit --allow-empty -m initial\n        $ cd /var/tmp\n        $ git clone foo.xz:pub/git.git x\n\nThis does not work as the transfer phase wants to run ssh on it.\n"},{"id":"192518","messageId":"20120530215912.GC3237@sigill.intra.peff.net","threadId":"30627","inReplyTo":"7vzk8pziuk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] clone: allow --no-local to turn off local optimizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-30T21:59:12Z","receivedAt":"2012-05-30T21:59:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 30, 2012 at 10:20:19AM -0700, Junio C Hamano wrote:\n\n> By the way, a somewhat funny exercise I found last night is this:\n> \n> \t$ cd /var/tmp\n>         $ mkdir -p foo.xz:pub/git.git\n>         $ cd foo.xz:pub/git.git\n>         $ git init\n>         $ git commit --allow-empty -m initial\n>         $ cd /var/tmp\n>         $ git clone foo.xz:pub/git.git x\n> \n> This does not work as the transfer phase wants to run ssh on it.\n\nYeah, you would have to do file://$PWD/foo.xz:pub/git.git, I think. Of\ncourse, there would be no way to specify \"--local\" while doing so... :)\n\nSimilarly, I find it a little odd that \"git clone file:///foo.git\" will\nactually find a file named \"file:/foo.git\" before checking the URL (IOW,\nthe argument is a path first, and then fallback to URL). I suspect\nnobody actually cares about either, as they are very unlikely corner\ncases.\n\n-Peff\n"},{"id":"192519","messageId":"7vmx4pxqua.fsf@alter.siamese.dyndns.org","threadId":"30627","inReplyTo":"20120530215912.GC3237@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] clone: allow --no-local to turn off local optimizations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-30T22:10:37Z","receivedAt":"2012-05-30T22:10:37Z","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> Similarly, I find it a little odd that \"git clone file:///foo.git\" will\n> actually find a file named \"file:/foo.git\" before checking the URL (IOW,\n> the argument is a path first, and then fallback to URL). I suspect\n> nobody actually cares about either, as they are very unlikely corner\n> cases.\n\nYeah, if anything, I would have expected --no-local to mean \"I might\nhave a local file that happens to be the same as this URL, but I am\nnot cloning from there; just go straight to the URL using transports\".\n"},{"id":"192524","messageId":"20120530232116.GA5007@sigill.intra.peff.net","threadId":"30627","inReplyTo":"7vmx4pxqua.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] clone: allow --no-local to turn off local optimizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-30T23:21:16Z","receivedAt":"2012-05-30T23:21:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 30, 2012 at 03:10:37PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Similarly, I find it a little odd that \"git clone file:///foo.git\" will\n> > actually find a file named \"file:/foo.git\" before checking the URL (IOW,\n> > the argument is a path first, and then fallback to URL). I suspect\n> > nobody actually cares about either, as they are very unlikely corner\n> > cases.\n> \n> Yeah, if anything, I would have expected --no-local to mean \"I might\n> have a local file that happens to be the same as this URL, but I am\n> not cloning from there; just go straight to the URL using transports\".\n\nBut that would not be the opposite of \"--local\", which you have just\nargued is not about interpreting the URL syntax at all, but is about\nturning off the local optimization code path when the origin repo is\nlocal.\n\nInterestingly, it seems that we don't handle this case well at all,\nanyway. We notice that \"file:///foo.git\" is resolvable as a path, set\nis_local, and set the path to \"$PWD/file:///foo.git\". But we still feed\nthat to the transport code to get the list of refs (just not to fetch\nthem). And the transport code barfs and says \"I don't understand the URL\nscheme $PWD/file\".\n\nSo I'm not sure what the actual rules were meant to be, and if we\nactually follow them. Or whether there are even intentional rules, and\nit is not \"what happens to work\". Again, these are such silly corner\ncases that I suspect it is simply the case that nobody has run into them\nor cared.\n\n-Peff\n"},{"id":"192525","messageId":"7vipfd8csp.fsf@alter.siamese.dyndns.org","threadId":"30627","inReplyTo":"20120530232116.GA5007@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] clone: allow --no-local to turn off local optimizations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-30T23:33:10Z","receivedAt":"2012-05-30T23:33:10Z","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> On Wed, May 30, 2012 at 03:10:37PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > Similarly, I find it a little odd that \"git clone file:///foo.git\" will\n>> > actually find a file named \"file:/foo.git\" before checking the URL (IOW,\n>> > the argument is a path first, and then fallback to URL). I suspect\n>> > nobody actually cares about either, as they are very unlikely corner\n>> > cases.\n>> \n>> Yeah, if anything, I would have expected --no-local to mean \"I might\n>> have a local file that happens to be the same as this URL, but I am\n>> not cloning from there; just go straight to the URL using transports\".\n>\n> But that would not be the opposite of \"--local\", which you have just\n> argued is not about interpreting the URL syntax at all, but is about\n> turning off the local optimization code path when the origin repo is\n> local.\n\nWhat I meant to say was that promoting \"--local\" like your original\nseries did and giving it a new meaning did not make much sense in\nthe context of the current semantics (i.e. if the path exists, it is\na path and you do not have to tell \"--local\" about it), but the\nsemantics _instead_ needs \"--no-local\" to be complete; without\n\"--no-local\" that is defined as such, the funny corner case that a\npath with the same as $URL prevents you from going to where you want\nto go.\n\n> Interestingly, it seems that we don't handle this case well at all,\n\nYes, isn't it interesting?  It is not just we feed it to transport,\nbut we also store it in the config so later \"git fetch\" will also do\nsomething inconsistent.  \"<scheme>://\", primarily because it has\ndoubled slashes, I wouldn't worry too much about them, but I would\nnot be surprised if somebody saw scp-style <host>:<path> conflict\nwith a local path and wished we handled such a case a bit more\nsanely.\n\n> ... Again, these are such silly corner\n> cases that I suspect it is simply the case that nobody has run into them\n> or cared.\n\n;-)\n"}]}