From: Junio C Hamano <gitster@pobox.com>
To: "Matthias Aßhauer via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
"Louis Strous" <Louis.Strous@intellimagic.com>,
"Pranit Bauva" <pranit.bauva@gmail.com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Denton Liu" <liu.denton@gmail.com>,
"Tanushree Tumane" <tanushreetumane@gmail.com>,
"Jeff Hostetler" <jeffhost@microsoft.com>,
"Miriam Rubio" <mirucam@gmail.com>,
"Matthias Aßhauer" <mha1993@live.de>
Subject: Re: [PATCH 2/3] run-command: teach locate_in_PATH about Windows
Date: Thu, 03 Aug 2023 09:13:56 -0700 [thread overview]
Message-ID: <xmqq7cqc5cjf.fsf@gitster.g> (raw)
In-Reply-To: <bf8b34aaef32a64b85f778ab219aeb41238f2bf2.1691058498.git.gitgitgadget@gmail.com> ("Matthias Aßhauer via GitGitGadget"'s message of "Thu, 03 Aug 2023 10:28:17 +0000")
"Matthias Aßhauer via GitGitGadget" <gitgitgadget@gmail.com>
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?
next prev parent reply other threads:[~2023-08-03 16:14 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 10:28 [PATCH 0/3] git bisect visualize: find gitk on Windows again Matthias Aßhauer via GitGitGadget
2023-08-03 10:28 ` [PATCH 1/3] compat: make path_lookup() available outside mingw.c Matthias Aßhauer via GitGitGadget
2023-08-03 10:28 ` [PATCH 2/3] run-command: teach locate_in_PATH about Windows Matthias Aßhauer via GitGitGadget
2023-08-03 16:13 ` Junio C Hamano [this message]
2023-08-03 10:28 ` [PATCH 3/3] docs: update when `git bisect visualize` uses `gitk` Matthias Aßhauer via GitGitGadget
2023-08-03 16:21 ` Junio C Hamano
2023-08-03 16:00 ` [PATCH 0/3] git bisect visualize: find gitk on Windows again Junio C Hamano
2023-08-03 18:50 ` Junio C Hamano
2023-08-03 19:03 ` Matthias Aßhauer
2023-08-03 19:56 ` Junio C Hamano
2023-08-04 4:08 ` [PATCH v2 " Matthias Aßhauer via GitGitGadget
2023-08-04 4:08 ` [PATCH v2 1/3] run-command: conditionally define locate_in_PATH() Matthias Aßhauer via GitGitGadget
2023-08-04 4:23 ` Junio C Hamano
2023-08-04 5:27 ` Matthias Aßhauer
2023-08-04 16:44 ` Junio C Hamano
2023-08-04 4:08 ` [PATCH v2 2/3] compat/mingw: implement a native locate_in_PATH() Matthias Aßhauer via GitGitGadget
2023-08-04 4:23 ` Junio C Hamano
2023-08-04 4:08 ` [PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk` Matthias Aßhauer via GitGitGadget
2023-08-04 4:32 ` Junio C Hamano
2023-08-04 5:27 ` Eric Sunshine
2023-08-04 5:54 ` Matthias Aßhauer
2023-08-04 16:45 ` Junio C Hamano
2023-08-04 16:57 ` Eric Sunshine
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq7cqc5cjf.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=Louis.Strous@intellimagic.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=jeffhost@microsoft.com \
--cc=liu.denton@gmail.com \
--cc=mha1993@live.de \
--cc=mirucam@gmail.com \
--cc=pranit.bauva@gmail.com \
--cc=tanushreetumane@gmail.com \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.