[2/3] run-command: teach locate_in_PATH about Windows

Commit Message

Matthias Aßhauer Aug. 3, 2023, 10:28 a.m. UTC

From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <[e-mail]>

since 5e1f28d206 (bisect--helper: reimplement `bisect_visualize()` shell
 function in C, 2021-09-13) `git bisect visualize` uses exists_in_PATH()
to check wether it should call `gitk`, but exists_in_PATH() relies on
locate_in_PATH() which currently only understands POSIX-ish PATH variables
(a list of paths, separated by colons) on native Windows executables
we encounter Windows PATH variables (a list of paths that often contain
drive letters (and thus colons), separated by semicolons). Luckily we do
already have a function that can lookup executables on windows PATHs:
mingw_path_lookup(). Teach locate_in_PATH() to use mingw_path_lookup()
on Windows.

Reported-by: Louis Strous <[e-mail]>
Signed-off-by: Matthias Aßhauer <[e-mail]>
---
 run-command.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Junio C Hamano Aug. 3, 2023, 4:13 p.m. UTC | #1

"Matthias Aßhauer via GitGitGadget"  <[e-mail]>
writes:

> diff --git a/run-command.c b/run-command.c
> index 60c94198664..8f518e37e27 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -182,13 +182,10 @@ int is_executable(const char *name)
>   * Returns the path to the command, as found in $PATH or NULL if the
>   * command could not be found.  The caller inherits ownership of the memory
>   * used to store the resultant path.
> - *
> - * This should not be used on Windows, where the $PATH search rules
> - * are more complicated (e.g., a search for "foo" should find
> - * "foo.exe").
>   */
>  static char *locate_in_PATH(const char *file)
>  {
> +#ifndef GIT_WINDOWS_NATIVE
>  	const char *p = getenv("PATH");
>  	struct strbuf buf = STRBUF_INIT;
>  
> @@ -217,6 +214,9 @@ static char *locate_in_PATH(const char *file)
>  
>  	strbuf_release(&buf);
>  	return NULL;
> +#else
> +	return mingw_path_lookup(file,0);
> +#endif
>  }

It may be cleaner to make the above more like

	#ifndef locate_in_PATH
	static char *locate_in_PATH(const char *file)
	{
	    ... original implementation without any #ifdef ...
	}
	#endif

and redo the [1/3] patch so that it does not rename or otherwise
touch path_lookup() in any way, and instead implements a
mingw_locate_in_PATH() in terms of path_lookup() and make it public,
declare it in <compat/mingw.h>, together with #define
locate_in_PATH(), i.e. [1/3] will essentially become something like:

    (add to compat/mingw.c)
    char *mingw_locate_in_PATH(const char *file)
    {
	return path_lookup(file, 0);
    }

    (add to compat/mingw.h)
    extern char *mingw_locate_in_PATH(const char *);
    #define locate_in_PATH(file) mingw_locate_in_PATH(file)

That way, the second non-UNIXy system can add its own way to locate
an executable in PATH without having to touch the main part of the
system, right?

Patch

diff --git a/run-command.c b/run-command.c
index 60c94198664..8f518e37e27 100644
--- a/run-command.c
+++ b/run-command.c
@@ -182,13 +182,10 @@  int is_executable(const char *name)
  * Returns the path to the command, as found in $PATH or NULL if the
  * command could not be found.  The caller inherits ownership of the memory
  * used to store the resultant path.
- *
- * This should not be used on Windows, where the $PATH search rules
- * are more complicated (e.g., a search for "foo" should find
- * "foo.exe").
  */
 static char *locate_in_PATH(const char *file)
 {
+#ifndef GIT_WINDOWS_NATIVE
 	const char *p = getenv("PATH");
 	struct strbuf buf = STRBUF_INIT;
 
@@ -217,6 +214,9 @@  static char *locate_in_PATH(const char *file)
 
 	strbuf_release(&buf);
 	return NULL;
+#else
+	return mingw_path_lookup(file,0);
+#endif
 }
 
 int exists_in_PATH(const char *command)