git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "András Kucsma via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Torsten Bögershausen" <tboegi@web.de>,
	"Junio C Hamano" <gitster@pobox.com>,
	"András Kucsma" <r0maikx02b@gmail.com>,
	"Andras Kucsma" <r0maikx02b@gmail.com>
Subject: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows
Date: Wed, 25 Mar 2020 13:45:10 +0000	[thread overview]
Message-ID: <pull.587.v2.git.1585143910604.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.587.git.1584997990694.gitgitgadget@gmail.com>

From: Andras Kucsma <r0maikx02b@gmail.com>

On Windows with git installed through cygwin, GIT_ASKPASS failed to run
for relative and absolute paths containing only backslashes as directory
separators.

The reason was that git assumed that if there are no forward slashes in
the executable path, it has to search for the executable on the PATH.

The fix is to look for OS specific directory separators, not just
forward slashes.

Signed-off-by: Andras Kucsma <r0maikx02b@gmail.com>
---
    Fix dir sep handling of GIT_ASKPASS on Windows
    
    On Windows with git installed through cygwin, GIT_ASKPASS failed to run
    for relative and absolute paths containing only backslashes as directory
    separators.
    
    The reason was that git assumed that if there are no forward slashes in
    the executable path, it has to search for the executable on the PATH.
    
    The fix is to look for OS specific directory separators, not just
    forward slashes.
    
    Signed-off-by: Andras Kucsma r0maikx02b@gmail.com [r0maikx02b@gmail.com]
    
    Changes since v1:
    
     * Avoid scanning the whole path for a directory separator even if one
       is found earlier as suggested by Junio C Hamano.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-587%2Fr0mai%2Ffix-prepare_cmd-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-587/r0mai/fix-prepare_cmd-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/587

Range-diff vs v1:

 1:  8fbfbec0d38 ! 1:  947931ac568 Fix dir sep handling of GIT_ASKPASS on Windows
     @@ -14,6 +14,47 @@
      
          Signed-off-by: Andras Kucsma <r0maikx02b@gmail.com>
      
     + diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
     + --- a/compat/win32/path-utils.h
     + +++ b/compat/win32/path-utils.h
     +@@
     + 	return ret;
     + }
     + #define find_last_dir_sep win32_find_last_dir_sep
     ++static inline int win32_has_dir_sep(const char *path)
     ++{
     ++	/*
     ++	 * See how long the non-separator part of the given path is, and
     ++	 * if and only if it covers the whole path (i.e. path[len] is NULL),
     ++	 * there is no separator in the path---otherwise there is a separator.
     ++	 */
     ++	size_t len = strcspn(path, "/\\");
     ++	return !!path[len];
     ++}
     ++#define has_dir_sep(path) win32_has_dir_sep(path)
     + int win32_offset_1st_component(const char *path);
     + #define offset_1st_component win32_offset_1st_component
     + 
     +
     + diff --git a/git-compat-util.h b/git-compat-util.h
     + --- a/git-compat-util.h
     + +++ b/git-compat-util.h
     +@@
     + #define find_last_dir_sep git_find_last_dir_sep
     + #endif
     + 
     ++#ifndef has_dir_sep
     ++static inline int git_has_dir_sep(const char *path)
     ++{
     ++	return !!strchr(path, '/');
     ++}
     ++#define has_dir_sep(path) git_has_dir_sep(path)
     ++#endif
     ++
     + #ifndef query_user_email
     + #define query_user_email() NULL
     + #endif
     +
       diff --git a/run-command.c b/run-command.c
       --- a/run-command.c
       +++ b/run-command.c
     @@ -31,7 +72,7 @@
      +	 * the command directly.
       	 */
      -	if (!strchr(out->argv[1], '/')) {
     -+	if (find_last_dir_sep(out->argv[1]) == NULL) {
     ++	if (!has_dir_sep(out->argv[1])) {
       		char *program = locate_in_PATH(out->argv[1]);
       		if (program) {
       			free((char *)out->argv[1]);


 compat/win32/path-utils.h | 11 +++++++++++
 git-compat-util.h         |  8 ++++++++
 run-command.c             | 10 +++++-----
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
index f2e70872cd2..18eff7899e9 100644
--- a/compat/win32/path-utils.h
+++ b/compat/win32/path-utils.h
@@ -20,6 +20,17 @@ static inline char *win32_find_last_dir_sep(const char *path)
 	return ret;
 }
 #define find_last_dir_sep win32_find_last_dir_sep
+static inline int win32_has_dir_sep(const char *path)
+{
+	/*
+	 * See how long the non-separator part of the given path is, and
+	 * if and only if it covers the whole path (i.e. path[len] is NULL),
+	 * there is no separator in the path---otherwise there is a separator.
+	 */
+	size_t len = strcspn(path, "/\\");
+	return !!path[len];
+}
+#define has_dir_sep(path) win32_has_dir_sep(path)
 int win32_offset_1st_component(const char *path);
 #define offset_1st_component win32_offset_1st_component
 
diff --git a/git-compat-util.h b/git-compat-util.h
index aed0b5d4f90..8ba576e81e3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -389,6 +389,14 @@ static inline char *git_find_last_dir_sep(const char *path)
 #define find_last_dir_sep git_find_last_dir_sep
 #endif
 
+#ifndef has_dir_sep
+static inline int git_has_dir_sep(const char *path)
+{
+	return !!strchr(path, '/');
+}
+#define has_dir_sep(path) git_has_dir_sep(path)
+#endif
+
 #ifndef query_user_email
 #define query_user_email() NULL
 #endif
diff --git a/run-command.c b/run-command.c
index f5e1149f9b3..0f41af3b550 100644
--- a/run-command.c
+++ b/run-command.c
@@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)
 	}
 
 	/*
-	 * If there are no '/' characters in the command then perform a path
-	 * lookup and use the resolved path as the command to exec.  If there
-	 * are '/' characters, we have exec attempt to invoke the command
-	 * directly.
+	 * If there are no dir separator characters in the command then perform
+	 * a path lookup and use the resolved path as the command to exec. If
+	 * there are dir separator characters, we have exec attempt to invoke
+	 * the command directly.
 	 */
-	if (!strchr(out->argv[1], '/')) {
+	if (!has_dir_sep(out->argv[1])) {
 		char *program = locate_in_PATH(out->argv[1]);
 		if (program) {
 			free((char *)out->argv[1]);

base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925
-- 
gitgitgadget

  parent reply	other threads:[~2020-03-25 13:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-23 21:13 [PATCH] Fix dir sep handling of GIT_ASKPASS on Windows András Kucsma via GitGitGadget
2020-03-24 20:51 ` Junio C Hamano
2020-03-25 13:45 ` András Kucsma via GitGitGadget [this message]
2020-03-25 16:35   ` [PATCH v2] " Torsten Bögershausen
2020-03-25 17:09     ` András Kucsma
2020-03-26 21:16     ` Junio C Hamano
2020-03-26 21:14   ` Junio C Hamano
2020-03-27  0:21     ` András Kucsma
2020-03-27  0:36   ` [PATCH v3] run-command: trigger PATH lookup properly on Cygwin András Kucsma via GitGitGadget
2020-03-27 18:04     ` Junio C Hamano
2020-03-27 18:10       ` András Kucsma
2020-03-27 18:41       ` Andreas Schwab
2020-03-27 21:27         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.587.v2.git.1585143910604.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=r0maikx02b@gmail.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).