{"thread":{"id":"53073","subject":"[PATCH] Fix dir sep handling of GIT_ASKPASS on Windows","startedAt":"2020-03-23T21:13:15Z","lastAt":"2020-03-27T21:27:31Z","messageCount":13,"participants":["András Kucsma via GitGitGadget","Junio C Hamano","Torsten Bögershausen","András Kucsma","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"393820","messageId":"pull.587.git.1584997990694.gitgitgadget@gmail.com","threadId":"53073","inReplyTo":null,"subject":"[PATCH] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"András Kucsma via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-23T21:13:10Z","receivedAt":"2020-03-23T21:13:15Z","isPatch":true,"sender":{"key":"andras.kucsma@gmail.com","avatar":null},"body":"From: Andras Kucsma <r0maikx02b@gmail.com>\n\nOn Windows with git installed through cygwin, GIT_ASKPASS failed to run\nfor relative and absolute paths containing only backslashes as directory\nseparators.\n\nThe reason was that git assumed that if there are no forward slashes in\nthe executable path, it has to search for the executable on the PATH.\n\nThe fix is to look for OS specific directory separators, not just\nforward slashes.\n\nSigned-off-by: Andras Kucsma <r0maikx02b@gmail.com>\n---\n    Fix dir sep handling of GIT_ASKPASS on Windows\n    \n    On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n    for relative and absolute paths containing only backslashes as directory\n    separators.\n    \n    The reason was that git assumed that if there are no forward slashes in\n    the executable path, it has to search for the executable on the PATH.\n    \n    The fix is to look for OS specific directory separators, not just\n    forward slashes.\n    \n    Signed-off-by: Andras Kucsma r0maikx02b@gmail.com [r0maikx02b@gmail.com]\n    \n    CC: Torsten Bögershausen tboegi@web.de [tboegi@web.de]\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-587%2Fr0mai%2Ffix-prepare_cmd-windows-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-587/r0mai/fix-prepare_cmd-windows-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/587\n\n run-command.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex f5e1149f9b3..9fcc12ebf9c 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n \t}\n \n \t/*\n-\t * If there are no '/' characters in the command then perform a path\n-\t * lookup and use the resolved path as the command to exec.  If there\n-\t * are '/' characters, we have exec attempt to invoke the command\n-\t * directly.\n+\t * If there are no dir separator characters in the command then perform\n+\t * a path lookup and use the resolved path as the command to exec. If\n+\t * there are dir separator characters, we have exec attempt to invoke\n+\t * the command directly.\n \t */\n-\tif (!strchr(out->argv[1], '/')) {\n+\tif (find_last_dir_sep(out->argv[1]) == NULL) {\n \t\tchar *program = locate_in_PATH(out->argv[1]);\n \t\tif (program) {\n \t\t\tfree((char *)out->argv[1]);\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"393892","messageId":"xmqqlfnp1np6.fsf@gitster.c.googlers.com","threadId":"53073","inReplyTo":"pull.587.git.1584997990694.gitgitgadget@gmail.com","subject":"Re: [PATCH] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-24T20:51:17Z","receivedAt":"2020-03-24T20:51:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"András Kucsma via GitGitGadget\"  <gitgitgadget@gmail.com> writes:\n\n> From: Andras Kucsma <r0maikx02b@gmail.com>\n>\n> On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n> for relative and absolute paths containing only backslashes as directory\n> separators.\n>\n> The reason was that git assumed that if there are no forward slashes in\n> the executable path, it has to search for the executable on the PATH.\n\nAlso if I were reading the discussion correctly, there was a doubt\nabout locate_in_PATH() that may not work on Windows for at least two\nreasons.  Is it OK to ignore these issues, and if so why?\n\nI know if you have a full path, a broken locate_in_PATH() would be\nskipped and won't cause an immediate issue, but this change to make\nthe code realize that \"a\\\\b\" is not asking to search in %PATH% feels\njust a beginning of a fix, not the whole fix, at least to me.\n\n> The fix is to look for OS specific directory separators, not just\n> forward slashes.\n\nYes, but it is quite unfortunate that you would use a function that\nhas to scan the string to the end because it asks for the last one.\n\nPerhaps introduce \n\n--------------------------------------------------\n#ifndef has_dir_sep\nstatic inline int git_has_dir_sep(const char *path)\n{\n\treturn !!strchr(path, '/');\n}\n#define has_dir_sep(path) git_has_dir_sep(path)\n#endif\n--------------------------------------------------\n\nin <git-compat-util.h>, with a replacement definition in\n<compat/win32/path-utils.h> that may read\n\n--------------------------------------------------\n#define has_dir_sep(path) win32_has_dir_sep(path)\nstatic inline int has_dir_sep(const char *path)\n{\n        /* \n         * See how long the non-separator part of the given path is, and\n         * if and only if it covers the whole path (i.e. path[len] is NUL),\n         * there is no separator in the path---otherwise there is a separaptor.\n         */\n        size_t len = strcspn(path, \"/\\\\\");\n        return !!path[len];\n}\n--------------------------------------------------\n\nand use that instead?\n\n\n> diff --git a/run-command.c b/run-command.c\n> index f5e1149f9b3..9fcc12ebf9c 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n>  \t}\n>  \n>  \t/*\n> -\t * If there are no '/' characters in the command then perform a path\n> -\t * lookup and use the resolved path as the command to exec.  If there\n> -\t * are '/' characters, we have exec attempt to invoke the command\n> -\t * directly.\n> +\t * If there are no dir separator characters in the command then perform\n> +\t * a path lookup and use the resolved path as the command to exec. If\n> +\t * there are dir separator characters, we have exec attempt to invoke\n> +\t * the command directly.\n>  \t */\n> -\tif (!strchr(out->argv[1], '/')) {\n> +\tif (find_last_dir_sep(out->argv[1]) == NULL) {\n>  \t\tchar *program = locate_in_PATH(out->argv[1]);\n>  \t\tif (program) {\n>  \t\t\tfree((char *)out->argv[1]);\n>\n> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n"},{"id":"393964","messageId":"pull.587.v2.git.1585143910604.gitgitgadget@gmail.com","threadId":"53073","inReplyTo":"pull.587.git.1584997990694.gitgitgadget@gmail.com","subject":"[PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"András Kucsma via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-25T13:45:10Z","receivedAt":"2020-03-25T13:45:17Z","isPatch":true,"sender":{"key":"andras.kucsma@gmail.com","avatar":null},"body":"From: Andras Kucsma <r0maikx02b@gmail.com>\n\nOn Windows with git installed through cygwin, GIT_ASKPASS failed to run\nfor relative and absolute paths containing only backslashes as directory\nseparators.\n\nThe reason was that git assumed that if there are no forward slashes in\nthe executable path, it has to search for the executable on the PATH.\n\nThe fix is to look for OS specific directory separators, not just\nforward slashes.\n\nSigned-off-by: Andras Kucsma <r0maikx02b@gmail.com>\n---\n    Fix dir sep handling of GIT_ASKPASS on Windows\n    \n    On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n    for relative and absolute paths containing only backslashes as directory\n    separators.\n    \n    The reason was that git assumed that if there are no forward slashes in\n    the executable path, it has to search for the executable on the PATH.\n    \n    The fix is to look for OS specific directory separators, not just\n    forward slashes.\n    \n    Signed-off-by: Andras Kucsma r0maikx02b@gmail.com [r0maikx02b@gmail.com]\n    \n    Changes since v1:\n    \n     * Avoid scanning the whole path for a directory separator even if one\n       is found earlier as suggested by Junio C Hamano.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-587%2Fr0mai%2Ffix-prepare_cmd-windows-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-587/r0mai/fix-prepare_cmd-windows-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/587\n\nRange-diff vs v1:\n\n 1:  8fbfbec0d38 ! 1:  947931ac568 Fix dir sep handling of GIT_ASKPASS on Windows\n     @@ -14,6 +14,47 @@\n      \n          Signed-off-by: Andras Kucsma <r0maikx02b@gmail.com>\n      \n     + diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\n     + --- a/compat/win32/path-utils.h\n     + +++ b/compat/win32/path-utils.h\n     +@@\n     + \treturn ret;\n     + }\n     + #define find_last_dir_sep win32_find_last_dir_sep\n     ++static inline int win32_has_dir_sep(const char *path)\n     ++{\n     ++\t/*\n     ++\t * See how long the non-separator part of the given path is, and\n     ++\t * if and only if it covers the whole path (i.e. path[len] is NULL),\n     ++\t * there is no separator in the path---otherwise there is a separator.\n     ++\t */\n     ++\tsize_t len = strcspn(path, \"/\\\\\");\n     ++\treturn !!path[len];\n     ++}\n     ++#define has_dir_sep(path) win32_has_dir_sep(path)\n     + int win32_offset_1st_component(const char *path);\n     + #define offset_1st_component win32_offset_1st_component\n     + \n     +\n     + diff --git a/git-compat-util.h b/git-compat-util.h\n     + --- a/git-compat-util.h\n     + +++ b/git-compat-util.h\n     +@@\n     + #define find_last_dir_sep git_find_last_dir_sep\n     + #endif\n     + \n     ++#ifndef has_dir_sep\n     ++static inline int git_has_dir_sep(const char *path)\n     ++{\n     ++\treturn !!strchr(path, '/');\n     ++}\n     ++#define has_dir_sep(path) git_has_dir_sep(path)\n     ++#endif\n     ++\n     + #ifndef query_user_email\n     + #define query_user_email() NULL\n     + #endif\n     +\n       diff --git a/run-command.c b/run-command.c\n       --- a/run-command.c\n       +++ b/run-command.c\n     @@ -31,7 +72,7 @@\n      +\t * the command directly.\n       \t */\n      -\tif (!strchr(out->argv[1], '/')) {\n     -+\tif (find_last_dir_sep(out->argv[1]) == NULL) {\n     ++\tif (!has_dir_sep(out->argv[1])) {\n       \t\tchar *program = locate_in_PATH(out->argv[1]);\n       \t\tif (program) {\n       \t\t\tfree((char *)out->argv[1]);\n\n\n compat/win32/path-utils.h | 11 +++++++++++\n git-compat-util.h         |  8 ++++++++\n run-command.c             | 10 +++++-----\n 3 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\nindex f2e70872cd2..18eff7899e9 100644\n--- a/compat/win32/path-utils.h\n+++ b/compat/win32/path-utils.h\n@@ -20,6 +20,17 @@ static inline char *win32_find_last_dir_sep(const char *path)\n \treturn ret;\n }\n #define find_last_dir_sep win32_find_last_dir_sep\n+static inline int win32_has_dir_sep(const char *path)\n+{\n+\t/*\n+\t * See how long the non-separator part of the given path is, and\n+\t * if and only if it covers the whole path (i.e. path[len] is NULL),\n+\t * there is no separator in the path---otherwise there is a separator.\n+\t */\n+\tsize_t len = strcspn(path, \"/\\\\\");\n+\treturn !!path[len];\n+}\n+#define has_dir_sep(path) win32_has_dir_sep(path)\n int win32_offset_1st_component(const char *path);\n #define offset_1st_component win32_offset_1st_component\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex aed0b5d4f90..8ba576e81e3 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -389,6 +389,14 @@ static inline char *git_find_last_dir_sep(const char *path)\n #define find_last_dir_sep git_find_last_dir_sep\n #endif\n \n+#ifndef has_dir_sep\n+static inline int git_has_dir_sep(const char *path)\n+{\n+\treturn !!strchr(path, '/');\n+}\n+#define has_dir_sep(path) git_has_dir_sep(path)\n+#endif\n+\n #ifndef query_user_email\n #define query_user_email() NULL\n #endif\ndiff --git a/run-command.c b/run-command.c\nindex f5e1149f9b3..0f41af3b550 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n \t}\n \n \t/*\n-\t * If there are no '/' characters in the command then perform a path\n-\t * lookup and use the resolved path as the command to exec.  If there\n-\t * are '/' characters, we have exec attempt to invoke the command\n-\t * directly.\n+\t * If there are no dir separator characters in the command then perform\n+\t * a path lookup and use the resolved path as the command to exec. If\n+\t * there are dir separator characters, we have exec attempt to invoke\n+\t * the command directly.\n \t */\n-\tif (!strchr(out->argv[1], '/')) {\n+\tif (!has_dir_sep(out->argv[1])) {\n \t\tchar *program = locate_in_PATH(out->argv[1]);\n \t\tif (program) {\n \t\t\tfree((char *)out->argv[1]);\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"393984","messageId":"20200325163540.vdc7l72fke7yqryb@tb-raspi4","threadId":"53073","inReplyTo":"pull.587.v2.git.1585143910604.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2020-03-25T16:35:40Z","receivedAt":"2020-03-25T16:35:50Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Thanks for working on this. I have 1 or 2 nits/questions, please see below.\n\nOn Wed, Mar 25, 2020 at 01:45:10PM +0000, András Kucsma via GitGitGadget wrote:\n> From: Andras Kucsma <r0maikx02b@gmail.com>\n>\n> On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n\nMy understanding is, that git under cygwin needs this patch (so to say),\nbut isn't it so, that even Git for Windows has the same issue ?\nThe headline of the patch and the indicate so.\nHow about the following ?\n\nOn Windows GIT_ASKPASS failed to run for relative and absolute paths\ncontaining only backslashes as directory separators.\nThe reason was that Git assumed that if there are no forward slashes in\nthe executable path, it has to search for the executable on the PATH.\n\nThe fix is to look for OS specific directory separators, not just\nforward slashes, so introduce a helper function has_dir_sep() and use it\nin run-command.\n\n"},{"id":"393986","messageId":"CANPdQvKbgisOQatrwcY66Asodxi__feaVOvoJe2j9qvHcrbZBQ@mail.gmail.com","threadId":"53073","inReplyTo":"20200325163540.vdc7l72fke7yqryb@tb-raspi4","subject":"Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"András Kucsma","fromEmail":"r0maikx02b@gmail.com","sentAt":"2020-03-25T17:09:40Z","receivedAt":"2020-03-25T17:09:55Z","isPatch":true,"sender":{"key":"r0maikx02b@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6103818?v=4"},"body":"On Wed, Mar 25, 2020 at 5:35 PM Torsten Bögershausen <tboegi@web.de> wrote:\n>\n> Thanks for working on this. I have 1 or 2 nits/questions, please see below.\n>\n> On Wed, Mar 25, 2020 at 01:45:10PM +0000, András Kucsma via GitGitGadget wrote:\n> > From: Andras Kucsma <r0maikx02b@gmail.com>\n> >\n> > On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n>\n> My understanding is, that git under cygwin needs this patch (so to say),\n> but isn't it so, that even Git for Windows has the same issue ?\n> The headline of the patch and the indicate so.\n\nGit for Windows does not have this issue, because there\nGIT_WINDOWS_NATIVE is defined, which is not true under Cygwin:\nhttps://github.com/git/git/blob/274b9cc2/git-compat-util.h#L157-L165\n\nYou can see in start_command() that there are separate implementations\nbased on GIT_WINDOWS_NATIVE. The problematic code is in prepare_cmd,\nwhich is not called in the branch where GIT_WINDOWS_NATIVE is defined:\nhttps://github.com/git/git/blob/274b9cc2/run-command.c#L740\n\nThis means, that cygwin is running in the Unix-like branch of the\ncode, even though it supports backslashes in its paths.\n"},{"id":"394112","messageId":"xmqqmu82izt4.fsf@gitster.c.googlers.com","threadId":"53073","inReplyTo":"pull.587.v2.git.1585143910604.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-26T21:14:31Z","receivedAt":"2020-03-26T21:14:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"András Kucsma via GitGitGadget\"  <gitgitgadget@gmail.com> writes:\n\n> From: Andras Kucsma <r0maikx02b@gmail.com>\n>\n> On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n> for relative and absolute paths containing only backslashes as directory\n> separators.\n\n> Subject: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows\n\nIsn't it curious that there is nothing in the code that was touched\nthat is specific to GIT_ASKPASS?  We shouldn't have to see that in\nthe title.\n\nPerhaps\n\n    Subject: run-command: notice needs for PATH-lookup correctly on Cygwin\n\n    On Cygwin, the codepath for POSIX-like systems is taken in\n    run-command.c::start_command().  The prepare_cmd() helper\n    function is called to decide if the command needs to be looked\n    up in the $PATH, and the logic there is to do the PATH-lookup if\n    and only if it does not have any slash '/' in it.\n\n    Unfortunately, a end-user can give \"c:\\program files\\askpass\" or\n    \"a\\b\\c\" to be absolute or relative path to the command, but in\n    these strings there is no '/'.  We end up attempting to run the\n    command by appending the absoluter or relative path after each\n    colon-separated component of $PATH.\n\n    Instead, introduce a has_dir_sep(path) helper function to\n    abstract away the difference between true POSIX and Cygwin, and\n    use it to make the decision for PATH-lookup.\n\nHaving said all that, I am not sure if we need to change anything.\n\nAs Cygwin is about trying to mimicking UNIXy environment as much as\npossible, shouldn't \"GIT_ASKPASS=//c/program files/askpass\" the way\nend-users would expect to work, not the one that uses backslashes?\n\nAnd if the user pretends to be on UNIXy system by using Cygwin by\nusing slashes when specifying these commands run via the run_command\nAPI, the code makes the decision for PATH-lookup quite correctly,\nno?\n\nSo...\n"},{"id":"394113","messageId":"xmqqftduizp8.fsf@gitster.c.googlers.com","threadId":"53073","inReplyTo":"20200325163540.vdc7l72fke7yqryb@tb-raspi4","subject":"Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-26T21:16:51Z","receivedAt":"2020-03-26T21:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n>> On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n>\n> My understanding is, that git under cygwin needs this patch (so to say),\n> but isn't it so, that even Git for Windows has the same issue ?\n> The headline of the patch and the indicate so.\n\nYes, I agree that the commit is mistitled.  It is not specific to\nWindows (it only is Cygwin, as the support for native Windows goes a\nseparate codepath), and it is not specific to GIT_ASKPASS, either.\n"},{"id":"394130","messageId":"CANPdQvJk4kJSMitaV_cEk-xY3_a=o1VN4fAqFCPiXWJf2ahScw@mail.gmail.com","threadId":"53073","inReplyTo":"xmqqmu82izt4.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows","fromName":"András Kucsma","fromEmail":"r0maikx02b@gmail.com","sentAt":"2020-03-27T00:21:33Z","receivedAt":"2020-03-27T00:21:49Z","isPatch":true,"sender":{"key":"r0maikx02b@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6103818?v=4"},"body":"On Thu, Mar 26, 2020 at 10:14 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"András Kucsma via GitGitGadget\"  <gitgitgadget@gmail.com> writes:\n>\n> > From: Andras Kucsma <r0maikx02b@gmail.com>\n> >\n> > On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n> > for relative and absolute paths containing only backslashes as directory\n> > separators.\n>\n> > Subject: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows\n>\n> Isn't it curious that there is nothing in the code that was touched\n> that is specific to GIT_ASKPASS?  We shouldn't have to see that in\n> the title.\n\nYou're completely right, I'll rephrase the commit message based on\nyour suggestion and resubmit.\n\n> Having said all that, I am not sure if we need to change anything.\n>\n> As Cygwin is about trying to mimicking UNIXy environment as much as\n> possible, shouldn't \"GIT_ASKPASS=//c/program files/askpass\" the way\n> end-users would expect to work, not the one that uses backslashes?\n>\n> And if the user pretends to be on UNIXy system by using Cygwin by\n> using slashes when specifying these commands run via the run_command\n> API, the code makes the decision for PATH-lookup quite correctly,\n> no?\n>\n> So...\n\nCygwin provides a Unix like environment, while also maintaining\nWindows compatibility, at least as far as path handling is concerned.\nAs a quick test, fopen can handle forward slashes, backslashes too.\nThese four all work under cygwin:\n\nfopen(\"C:\\\\file.txt\", \"r\");\nfopen(\"C:/file.txt\", \"r\");\nfopen(\"/cygdrive/c/file.txt\", \"r\");\nfopen(\"/cygdrive\\\\c\\\\file.txt\", \"r\");\n\nThere seems to be a precedent to support Cygwin as a kind of \"hybrid\"\nplatform in the git codebase. In git-compat-util.h, the\ncompat/win32/path-utils.h header is included, but GIT_WINDOWS_NATIVE\nis not defined.\n\nhttps://github.com/git/git/blob/a7d14a442/git-compat-util.h#L204-L206\nhttps://github.com/git/git/blob/a7d14a442/git-compat-util.h#L157-L165\n\nThe compat/win32/path-utils.h header mostly provides utilities dealing\nwith directory separator related logic on Windows, but these utilities\nare being used in the Unixy code paths on Cygwin.\n\nThe current version of the patch fits into this pattern. It only\nchanges behaviour under Cygwin, not touching pure Windows and\nnon-Cygwin Unix variants at all.\n"},{"id":"394131","messageId":"pull.587.v3.git.1585269403947.gitgitgadget@gmail.com","threadId":"53073","inReplyTo":"pull.587.v2.git.1585143910604.gitgitgadget@gmail.com","subject":"[PATCH v3] run-command: trigger PATH lookup properly on Cygwin","fromName":"András Kucsma via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-27T00:36:43Z","receivedAt":"2020-03-27T00:36:49Z","isPatch":true,"sender":{"key":"andras.kucsma@gmail.com","avatar":null},"body":"From: Andras Kucsma <r0maikx02b@gmail.com>\n\nOn Cygwin, the codepath for POSIX-like systems is taken in\nrun-command.c::start_command(). The prepare_cmd() helper\nfunction is called to decide if the command needs to be looked\nup in the PATH. The logic there is to do the PATH-lookup if\nand only if it does not have any slash '/' in it. If this test\npasses we end up attempting to run the command by appending the\nstring after each colon-separated component of PATH.\n\nThe Cygwin environment supports both Windows and POSIX style\npaths, so both forwardslahes '/' and back slashes '\\' can be\nused as directory separators for any external program the user\nsupplies.\n\nExamples for path strings which are being incorrectly searched\nfor in the PATH instead of being executed as is:\n\n- \"C:\\Program Files\\some-program.exe\"\n- \"a\\b\\c.exe\"\n\nTo handle these, the PATH lookup detection logic in prepare_cmd()\nis taught to know about this Cygwin quirk, by introducing\nhas_dir_sep(path) helper function to abstract away the difference\nbetween true POSIX and Cygwin systems.\n\nSigned-off-by: Andras Kucsma <r0maikx02b@gmail.com>\n---\n    run-command: trigger PATH lookup properly on Cygwin\n    \n    On Cygwin, the codepath for POSIX-like systems is taken in\n    run-command.c::start_command(). The prepare_cmd() helper function is\n    called to decide if the command needs to be looked up in the PATH. The\n    logic there is to do the PATH-lookup if and only if it does not have any\n    slash '/' in it. If this test passes we end up attempting to run the\n    command by appending the string after each colon-separated component of\n    PATH.\n    \n    The Cygwin environment supports both Windows and POSIX style paths, so\n    both forwardslahes '/' and back slashes '' can be used as directory\n    separators for any external program the user supplies.\n    \n    Examples for path strings which are being incorrectly searched for in\n    the PATH instead of being executed as is:\n    \n     * \"C:\\Program Files\\some-program.exe\"\n     * \"a\\b\\c.exe\"\n    \n    To handle these, the PATH lookup detection logic in prepare_cmd() is\n    taught to know about this Cygwin quirk, by introducing has_dir_sep(path)\n    helper function to abstract away the difference between true POSIX and\n    Cygwin systems.\n    \n    Signed-off-by: Andras Kucsma r0maikx02b@gmail.com [r0maikx02b@gmail.com]\n    \n    Changes since v1:\n    \n     * Avoid scanning the whole path for a directory separator even if one\n       is found earlier as suggested by Junio C Hamano. Changes since v2:\n     * Rephrased the commit message based on Junio's suggestion.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-587%2Fr0mai%2Ffix-prepare_cmd-windows-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-587/r0mai/fix-prepare_cmd-windows-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/587\n\nRange-diff vs v2:\n\n 1:  947931ac568 ! 1:  fd3cbd51635 Fix dir sep handling of GIT_ASKPASS on Windows\n     @@ -1,16 +1,30 @@\n      Author: Andras Kucsma <r0maikx02b@gmail.com>\n      \n     -    Fix dir sep handling of GIT_ASKPASS on Windows\n     +    run-command: trigger PATH lookup properly on Cygwin\n      \n     -    On Windows with git installed through cygwin, GIT_ASKPASS failed to run\n     -    for relative and absolute paths containing only backslashes as directory\n     -    separators.\n     +    On Cygwin, the codepath for POSIX-like systems is taken in\n     +    run-command.c::start_command(). The prepare_cmd() helper\n     +    function is called to decide if the command needs to be looked\n     +    up in the PATH. The logic there is to do the PATH-lookup if\n     +    and only if it does not have any slash '/' in it. If this test\n     +    passes we end up attempting to run the command by appending the\n     +    string after each colon-separated component of PATH.\n      \n     -    The reason was that git assumed that if there are no forward slashes in\n     -    the executable path, it has to search for the executable on the PATH.\n     +    The Cygwin environment supports both Windows and POSIX style\n     +    paths, so both forwardslahes '/' and back slashes '\\' can be\n     +    used as directory separators for any external program the user\n     +    supplies.\n      \n     -    The fix is to look for OS specific directory separators, not just\n     -    forward slashes.\n     +    Examples for path strings which are being incorrectly searched\n     +    for in the PATH instead of being executed as is:\n     +\n     +    - \"C:\\Program Files\\some-program.exe\"\n     +    - \"a\\b\\c.exe\"\n     +\n     +    To handle these, the PATH lookup detection logic in prepare_cmd()\n     +    is taught to know about this Cygwin quirk, by introducing\n     +    has_dir_sep(path) helper function to abstract away the difference\n     +    between true POSIX and Cygwin systems.\n      \n          Signed-off-by: Andras Kucsma <r0maikx02b@gmail.com>\n      \n\n\n compat/win32/path-utils.h | 11 +++++++++++\n git-compat-util.h         |  8 ++++++++\n run-command.c             | 10 +++++-----\n 3 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\nindex f2e70872cd2..18eff7899e9 100644\n--- a/compat/win32/path-utils.h\n+++ b/compat/win32/path-utils.h\n@@ -20,6 +20,17 @@ static inline char *win32_find_last_dir_sep(const char *path)\n \treturn ret;\n }\n #define find_last_dir_sep win32_find_last_dir_sep\n+static inline int win32_has_dir_sep(const char *path)\n+{\n+\t/*\n+\t * See how long the non-separator part of the given path is, and\n+\t * if and only if it covers the whole path (i.e. path[len] is NULL),\n+\t * there is no separator in the path---otherwise there is a separator.\n+\t */\n+\tsize_t len = strcspn(path, \"/\\\\\");\n+\treturn !!path[len];\n+}\n+#define has_dir_sep(path) win32_has_dir_sep(path)\n int win32_offset_1st_component(const char *path);\n #define offset_1st_component win32_offset_1st_component\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex aed0b5d4f90..8ba576e81e3 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -389,6 +389,14 @@ static inline char *git_find_last_dir_sep(const char *path)\n #define find_last_dir_sep git_find_last_dir_sep\n #endif\n \n+#ifndef has_dir_sep\n+static inline int git_has_dir_sep(const char *path)\n+{\n+\treturn !!strchr(path, '/');\n+}\n+#define has_dir_sep(path) git_has_dir_sep(path)\n+#endif\n+\n #ifndef query_user_email\n #define query_user_email() NULL\n #endif\ndiff --git a/run-command.c b/run-command.c\nindex f5e1149f9b3..0f41af3b550 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n \t}\n \n \t/*\n-\t * If there are no '/' characters in the command then perform a path\n-\t * lookup and use the resolved path as the command to exec.  If there\n-\t * are '/' characters, we have exec attempt to invoke the command\n-\t * directly.\n+\t * If there are no dir separator characters in the command then perform\n+\t * a path lookup and use the resolved path as the command to exec. If\n+\t * there are dir separator characters, we have exec attempt to invoke\n+\t * the command directly.\n \t */\n-\tif (!strchr(out->argv[1], '/')) {\n+\tif (!has_dir_sep(out->argv[1])) {\n \t\tchar *program = locate_in_PATH(out->argv[1]);\n \t\tif (program) {\n \t\t\tfree((char *)out->argv[1]);\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"394182","messageId":"xmqqeetdhdxo.fsf@gitster.c.googlers.com","threadId":"53073","inReplyTo":"pull.587.v3.git.1585269403947.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] run-command: trigger PATH lookup properly on Cygwin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-27T18:04:35Z","receivedAt":"2020-03-27T18:04:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"András Kucsma via GitGitGadget\"  <gitgitgadget@gmail.com> writes:\n\n> Subject: Re: [PATCH v3] run-command: trigger PATH lookup properly on Cygwin\n\nYou phrased it much better than my earlier attempt.  Succinct,\naccurate and to the point.  Good.\n\n>  compat/win32/path-utils.h | 11 +++++++++++\n>  git-compat-util.h         |  8 ++++++++\n>  run-command.c             | 10 +++++-----\n>  3 files changed, 24 insertions(+), 5 deletions(-)\n>\n> diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\n> index f2e70872cd2..18eff7899e9 100644\n> --- a/compat/win32/path-utils.h\n> +++ b/compat/win32/path-utils.h\n> @@ -20,6 +20,17 @@ static inline char *win32_find_last_dir_sep(const char *path)\n>  \treturn ret;\n>  }\n>  #define find_last_dir_sep win32_find_last_dir_sep\n> +static inline int win32_has_dir_sep(const char *path)\n> +{\n> +\t/*\n> +\t * See how long the non-separator part of the given path is, and\n> +\t * if and only if it covers the whole path (i.e. path[len] is NULL),\n\nThe name of the ASCII character '\\0' is NUL, not NULL (I'll fix it\nwhile applying, so no need to resend if you do not have anything\nelse that needs updating).\n\nOtherwise, the patch looks good. \n\nThanks.\n"},{"id":"394183","messageId":"CANPdQvLiiMTnd9ZOJvroECgG6ZzrtxS2ew_FY=CbXq4npT02ow@mail.gmail.com","threadId":"53073","inReplyTo":"xmqqeetdhdxo.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3] run-command: trigger PATH lookup properly on Cygwin","fromName":"András Kucsma","fromEmail":"r0maikx02b@gmail.com","sentAt":"2020-03-27T18:10:57Z","receivedAt":"2020-03-27T18:11:11Z","isPatch":true,"sender":{"key":"r0maikx02b@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6103818?v=4"},"body":"On Fri, Mar 27, 2020 at 7:04 PM Junio C Hamano <gitster@pobox.com> wrote:\n> The name of the ASCII character '\\0' is NUL, not NULL (I'll fix it\n> while applying, so no need to resend if you do not have anything\n> else that needs updating).\n\nRight, sorry! I have no other updates.\n\n> Otherwise, the patch looks good.\n>\n> Thanks.\n\nThanks for the help!\n"},{"id":"394188","messageId":"877dz5bpyd.fsf@igel.home","threadId":"53073","inReplyTo":"xmqqeetdhdxo.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3] run-command: trigger PATH lookup properly on Cygwin","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2020-03-27T18:41:30Z","receivedAt":"2020-03-27T18:41:36Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Mär 27 2020, Junio C Hamano wrote:\n\n> The name of the ASCII character '\\0' is NUL, not NULL\n\nNUL is not a name, it is an abbreviation or acronym.  Its name is the\nNull character.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"394200","messageId":"xmqqd08xfpz4.fsf@gitster.c.googlers.com","threadId":"53073","inReplyTo":"877dz5bpyd.fsf@igel.home","subject":"Re: [PATCH v3] run-command: trigger PATH lookup properly on Cygwin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-27T21:27:27Z","receivedAt":"2020-03-27T21:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab <schwab@linux-m68k.org> writes:\n\n> On Mär 27 2020, Junio C Hamano wrote:\n>\n>> The name of the ASCII character '\\0' is NUL, not NULL\n>\n> NUL is not a name, it is an abbreviation or acronym.  Its name is the\n> Null character.\n\nOK, let's put it differently.\n\n> +\t * See how long the non-separator part of the given path is, and\n> +\t * if and only if it covers the whole path (i.e. path[len] is NULL),\n\nWhen referring to character '\\0' like so, write \"NUL\", not \"NULL\",\nas the latter is how you write a null pointer.\n"}]}