{"thread":{"id":"50252","subject":"[PATCH 0/2] mingw: fix git clone \\\\server\\share","startedAt":"2019-01-17T20:14:50Z","lastAt":"2019-01-17T20:14:53Z","messageCount":3,"participants":["Johannes Schindelin via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"367019","messageId":"pull.94.git.gitgitgadget@gmail.com","threadId":"50252","inReplyTo":null,"subject":"[PATCH 0/2] mingw: fix git clone \\\\server\\share","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-01-17T20:14:46Z","receivedAt":"2019-01-17T20:14:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Due to quirks in the MSYS2 runtime intended to help interpret a command-line\nas if it had been pre-processed by a shell, the backslashes would be\nmisinterpreted as if they were escape characters. This showed most\nprominently when trying to clone from a network share, as the path would\nstart with two backslashes.\n\nThese patches have been carried in Git for Windows for more than\none-and-a-half years (in a slightly different, less readable form), and it\nis time to bring them home.\n\nJohannes Schindelin (2):\n  mingw (t5580): document bug when cloning from backslashed UNC paths\n  mingw: special-case arguments to `sh`\n\n compat/mingw.c            | 77 ++++++++++++++++++++++++++++++++++++++-\n t/t0061-run-command.sh    | 10 +++++\n t/t5580-clone-push-unc.sh |  5 +++\n 3 files changed, 91 insertions(+), 1 deletion(-)\n\n\nbase-commit: 77556354bb7ac50450e3b28999e3576969869068\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-94%2Fdscho%2Func-path-w-backslashes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-94/dscho/unc-path-w-backslashes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/94\n-- \ngitgitgadget\n"},{"id":"367020","messageId":"785b71df39c777f3ff182098a6961fceb629bfcf.1547756085.git.gitgitgadget@gmail.com","threadId":"50252","inReplyTo":"pull.94.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] mingw (t5580): document bug when cloning from backslashed UNC paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-01-17T20:14:47Z","receivedAt":"2019-01-17T20:14:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nDue to a quirk in Git's method to spawn git-upload-pack, there is a\nproblem when passing paths with backslashes in them: Git will force the\ncommand-line through the shell, which has different quoting semantics in\nGit for Windows (being an MSYS2 program) than regular Win32 executables\nsuch as git.exe itself.\n\nThe symptom is that the first of the two backslashes in UNC paths of the\nform \\\\myserver\\folder\\repository.git is *stripped off*.\n\nDocument this bug by introducing a test case.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t5580-clone-push-unc.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh\nindex ba548df4a9..c3703765f4 100755\n--- a/t/t5580-clone-push-unc.sh\n+++ b/t/t5580-clone-push-unc.sh\n@@ -40,6 +40,11 @@ test_expect_success clone '\n \tgit clone \"file://$UNCPATH\" clone\n '\n \n+test_expect_failure 'clone with backslashed path' '\n+\tBACKSLASHED=\"$(echo \"$UNCPATH\" | tr / \\\\\\\\)\" &&\n+\tgit clone \"$BACKSLASHED\" backslashed\n+'\n+\n test_expect_success push '\n \t(\n \t\tcd clone &&\n-- \ngitgitgadget\n\n"},{"id":"367021","messageId":"d58c3ca9f1f988e4ee17c1ddccdae3d739ab7818.1547756085.git.gitgitgadget@gmail.com","threadId":"50252","inReplyTo":"pull.94.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] mingw: special-case arguments to `sh`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-01-17T20:14:48Z","receivedAt":"2019-01-17T20:14:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe MSYS2 runtime does its best to emulate the command-line wildcard\nexpansion and de-quoting which would be performed by the calling Unix\nshell on Unix systems.\n\nThose Unix shell quoting rules differ from the quoting rules applying to\nWindows' cmd and Powershell, making it a little awkward to quote\ncommand-line parameters properly when spawning other processes.\n\nIn particular, git.exe passes arguments to subprocesses that are *not*\nintended to be interpreted as wildcards, and if they contain\nbackslashes, those are not to be interpreted as escape characters, e.g.\nwhen passing Windows paths.\n\nNote: this is only a problem when calling MSYS2 executables, not when\ncalling MINGW executables such as git.exe. However, we do call MSYS2\nexecutables frequently, most notably when setting the use_shell flag in\nthe child_process structure.\n\nThere is no elegant way to determine whether the .exe file to be\nexecuted is an MSYS2 program or a MINGW one. But since the use case of\npassing a command line through the shell is so prevalent, we need to\nwork around this issue at least when executing sh.exe.\n\nLet's introduce an ugly, hard-coded test whether argv[0] is \"sh\", and\nwhether it refers to the MSYS2 Bash, to determine whether we need to\nquote the arguments differently than usual.\n\nThat still does not fix the issue completely, but at least it is\nsomething.\n\nIncidentally, this also fixes the problem where `git clone \\\\server\\repo`\nfailed due to incorrect handling of the backslashes when handing the path\nto the git-upload-pack process.\n\nFurther, we need to take care to quote not only whitespace and\nbackslashes, but also curly brackets. As aliases frequently go through\nthe MSYS2 Bash, and as aliases frequently get parameters such as\nHEAD@{yesterday}, this is really important. As an early version of this\npatch broke this, let's make sure that this does not regress by adding a\ntest case for that.\n\nHelped-by: Kim Gybels <kgybels@infogroep.be>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c            | 77 ++++++++++++++++++++++++++++++++++++++-\n t/t0061-run-command.sh    | 10 +++++\n t/t5580-clone-push-unc.sh |  2 +-\n 3 files changed, 87 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex b459e1a291..0af8684019 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -7,6 +7,7 @@\n #include \"../cache.h\"\n #include \"win32/lazyload.h\"\n #include \"../config.h\"\n+#include \"dir.h\"\n \n #define HCAST(type, handle) ((type)(intptr_t)handle)\n \n@@ -1031,7 +1032,7 @@ char *mingw_getcwd(char *pointer, int len)\n  * See \"Parsing C++ Command-Line Arguments\" at Microsoft's Docs:\n  * https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments\n  */\n-static const char *quote_arg(const char *arg)\n+static const char *quote_arg_msvc(const char *arg)\n {\n \t/* count chars to quote */\n \tint len = 0, n = 0;\n@@ -1086,6 +1087,37 @@ static const char *quote_arg(const char *arg)\n \treturn q;\n }\n \n+#include \"quote.h\"\n+\n+static const char *quote_arg_msys2(const char *arg)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *p2 = arg, *p;\n+\n+\tfor (p = arg; *p; p++) {\n+\t\tint ws = isspace(*p);\n+\t\tif (!ws && *p != '\\\\' && *p != '\"' && *p != '{')\n+\t\t\tcontinue;\n+\t\tif (!buf.len)\n+\t\t\tstrbuf_addch(&buf, '\"');\n+\t\tif (p != p2)\n+\t\t\tstrbuf_add(&buf, p2, p - p2);\n+\t\tif (!ws && *p != '{')\n+\t\t\tstrbuf_addch(&buf, '\\\\');\n+\t\tp2 = p;\n+\t}\n+\n+\tif (p == arg)\n+\t\tstrbuf_addch(&buf, '\"');\n+\telse if (!buf.len)\n+\t\treturn arg;\n+\telse\n+\t\tstrbuf_add(&buf, p2, p - p2),\n+\n+\tstrbuf_addch(&buf, '\"');\n+\treturn strbuf_detach(&buf, 0);\n+}\n+\n static const char *parse_interpreter(const char *cmd)\n {\n \tstatic char buf[100];\n@@ -1317,6 +1349,47 @@ struct pinfo_t {\n static struct pinfo_t *pinfo = NULL;\n CRITICAL_SECTION pinfo_cs;\n \n+/* Used to match and chomp off path components */\n+static inline int match_last_path_component(const char *path, size_t *len,\n+\t\t\t\t\t    const char *component)\n+{\n+\tsize_t component_len = strlen(component);\n+\tif (*len < component_len + 1 ||\n+\t    !is_dir_sep(path[*len - component_len - 1]) ||\n+\t    fspathncmp(path + *len - component_len, component, component_len))\n+\t\treturn 0;\n+\t*len -= component_len + 1;\n+\t/* chomp off repeated dir separators */\n+\twhile (*len > 0 && is_dir_sep(path[*len - 1]))\n+\t\t(*len)--;\n+\treturn 1;\n+}\n+\n+static int is_msys2_sh(const char *cmd)\n+{\n+\tif (cmd && !strcmp(cmd, \"sh\")) {\n+\t\tstatic int ret = -1;\n+\t\tchar *p;\n+\n+\t\tif (ret >= 0)\n+\t\t\treturn ret;\n+\n+\t\tp = path_lookup(cmd, 0);\n+\t\tif (!p)\n+\t\t\tret = 0;\n+\t\telse {\n+\t\t\tsize_t len = strlen(p);\n+\n+\t\t\tret = match_last_path_component(p, &len, \"sh.exe\") &&\n+\t\t\t\tmatch_last_path_component(p, &len, \"bin\") &&\n+\t\t\t\tmatch_last_path_component(p, &len, \"usr\");\n+\t\t\tfree(p);\n+\t\t}\n+\t\treturn ret;\n+\t}\n+\treturn 0;\n+}\n+\n static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaenv,\n \t\t\t      const char *dir,\n \t\t\t      int prepend_cmd, int fhin, int fhout, int fherr)\n@@ -1328,6 +1401,8 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen\n \tunsigned flags = CREATE_UNICODE_ENVIRONMENT;\n \tBOOL ret;\n \tHANDLE cons;\n+\tconst char *(*quote_arg)(const char *arg) =\n+\t\tis_msys2_sh(*argv) ? quote_arg_msys2 : quote_arg_msvc;\n \n \tdo_unset_environment_variables();\n \ndiff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\nindex 99a614bc7c..9c7604dcab 100755\n--- a/t/t0061-run-command.sh\n+++ b/t/t0061-run-command.sh\n@@ -199,4 +199,14 @@ test_expect_success 'GIT_TRACE with environment variables' '\n \t)\n '\n \n+test_expect_success MINGW 'verify curlies are quoted properly' '\n+\t: force the rev-parse through the MSYS2 Bash &&\n+\tgit -c alias.r=\"!git rev-parse\" r -- a{b}c >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\ta{b}c\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh\nindex c3703765f4..217adf3a63 100755\n--- a/t/t5580-clone-push-unc.sh\n+++ b/t/t5580-clone-push-unc.sh\n@@ -40,7 +40,7 @@ test_expect_success clone '\n \tgit clone \"file://$UNCPATH\" clone\n '\n \n-test_expect_failure 'clone with backslashed path' '\n+test_expect_success 'clone with backslashed path' '\n \tBACKSLASHED=\"$(echo \"$UNCPATH\" | tr / \\\\\\\\)\" &&\n \tgit clone \"$BACKSLASHED\" backslashed\n '\n-- \ngitgitgadget\n"}]}