{"thread":{"id":"60059","subject":"[PATCH 0/3] git bisect visualize: find gitk on Windows again","startedAt":"2023-08-03T10:28:35Z","lastAt":"2023-08-04T16:58:16Z","messageCount":23,"participants":["Matthias Aßhauer via GitGitGadget","Junio C Hamano","Matthias Aßhauer","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"480115","messageId":"pull.1560.git.1691058498.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":null,"subject":"[PATCH 0/3] git bisect visualize: find gitk on Windows again","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-03T10:28:15Z","receivedAt":"2023-08-03T10:28:35Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"Louis Strous reported a regression in git bisect visualize on Windows[1]\nthat caused git bisect visualize to use git log instead of gitk unless\nexplicitly called as git bisect visualize gitk.\n\nThis patch series fixes that regression.\n\n[1]\nhttps://lore.kernel.org/git/VI1PR10MB2462F7B52FF2E3F59AFE94A7F500A@VI1PR10MB2462.EURPRD10.PROD.OUTLOOK.COM/\n\nMatthias Aßhauer (3):\n  compat: make path_lookup() available outside mingw.c\n  run-command: teach locate_in_PATH about Windows\n  docs: update when `git bisect visualize` uses `gitk`\n\n Documentation/git-bisect.txt |  6 +++---\n compat/mingw.c               | 20 ++++++++------------\n compat/mingw.h               |  6 ++++++\n run-command.c                |  8 ++++----\n 4 files changed, 21 insertions(+), 19 deletions(-)\n\n\nbase-commit: fb7d80edcae482f4fa5d4be0227dc3054734e5f3\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1560%2Frimrul%2Fwin-bisect-visualize-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1560/rimrul/win-bisect-visualize-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1560\n-- \ngitgitgadget\n"},{"id":"480116","messageId":"6ed968e128897ad75fb09a69395ee3571bb40677.1691058498.git.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.git.1691058498.gitgitgadget@gmail.com","subject":"[PATCH 1/3] compat: make path_lookup() available outside mingw.c","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-03T10:28:16Z","receivedAt":"2023-08-03T10:28:37Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nRename it to mingw_path_lookup() to avoid leading future contributors\nto believe this would be portable.\n\nThis is in preparation for a patch to teach locate_in_PATH() and\nexists_in_PATH() in run-command.c to work on windows.\n\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n compat/mingw.c | 20 ++++++++------------\n compat/mingw.h |  6 ++++++\n 2 files changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex d06cdc6254f..5d3368b1705 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1316,11 +1316,7 @@ static char *lookup_prog(const char *dir, int dirlen, const char *cmd,\n \treturn NULL;\n }\n \n-/*\n- * Determines the absolute path of cmd using the split path in path.\n- * If cmd contains a slash or backslash, no lookup is performed.\n- */\n-static char *path_lookup(const char *cmd, int exe_only)\n+char *mingw_path_lookup(const char *cmd, int exe_only)\n {\n \tconst char *path;\n \tchar *prog = NULL;\n@@ -1515,7 +1511,7 @@ static int is_msys2_sh(const char *cmd)\n \t\tif (ret >= 0)\n \t\t\treturn ret;\n \n-\t\tp = path_lookup(cmd, 0);\n+\t\tp = mingw_path_lookup(cmd, 0);\n \t\tif (!p)\n \t\t\tret = 0;\n \t\telse {\n@@ -1533,7 +1529,7 @@ static int is_msys2_sh(const char *cmd)\n \t\tstatic char *sh;\n \n \t\tif (!sh)\n-\t\t\tsh = path_lookup(\"sh\", 0);\n+\t\t\tsh = mingw_path_lookup(\"sh\", 0);\n \n \t\treturn !fspathcmp(cmd, sh);\n \t}\n@@ -1646,7 +1642,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen\n \n \tstrace_env = getenv(\"GIT_STRACE_COMMANDS\");\n \tif (strace_env) {\n-\t\tchar *p = path_lookup(\"strace.exe\", 1);\n+\t\tchar *p = mingw_path_lookup(\"strace.exe\", 1);\n \t\tif (!p)\n \t\t\treturn error(\"strace not found!\");\n \t\tif (xutftowcs_path(wcmd, p) < 0) {\n@@ -1801,7 +1797,7 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **deltaenv,\n \t\t     int fhin, int fhout, int fherr)\n {\n \tpid_t pid;\n-\tchar *prog = path_lookup(cmd, 0);\n+\tchar *prog = mingw_path_lookup(cmd, 0);\n \n \tif (!prog) {\n \t\terrno = ENOENT;\n@@ -1812,7 +1808,7 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **deltaenv,\n \n \t\tif (interpr) {\n \t\t\tconst char *argv0 = argv[0];\n-\t\t\tchar *iprog = path_lookup(interpr, 1);\n+\t\t\tchar *iprog = mingw_path_lookup(interpr, 1);\n \t\t\targv[0] = prog;\n \t\t\tif (!iprog) {\n \t\t\t\terrno = ENOENT;\n@@ -1841,7 +1837,7 @@ static int try_shell_exec(const char *cmd, char *const *argv)\n \n \tif (!interpr)\n \t\treturn 0;\n-\tprog = path_lookup(interpr, 1);\n+\tprog = mingw_path_lookup(interpr, 1);\n \tif (prog) {\n \t\tint exec_id;\n \t\tint argc = 0;\n@@ -1890,7 +1886,7 @@ int mingw_execv(const char *cmd, char *const *argv)\n \n int mingw_execvp(const char *cmd, char *const *argv)\n {\n-\tchar *prog = path_lookup(cmd, 0);\n+\tchar *prog = mingw_path_lookup(cmd, 0);\n \n \tif (prog) {\n \t\tmingw_execv(prog, argv);\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 209cf7cebad..af1ff4be320 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -626,3 +626,9 @@ void open_in_gdb(void);\n  * Used by Pthread API implementation for Windows\n  */\n int err_win_to_posix(DWORD winerr);\n+\n+/*\n+ * Determines the absolute path of cmd using the split path in path.\n+ * If cmd contains a slash or backslash, no lookup is performed.\n+ */\n+char *mingw_path_lookup(const char *cmd, int exe_only);\n-- \ngitgitgadget\n\n"},{"id":"480117","messageId":"c872431b608424007f72c69c8526f96d532aaca1.1691058498.git.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.git.1691058498.gitgitgadget@gmail.com","subject":"[PATCH 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-03T10:28:18Z","receivedAt":"2023-08-03T10:28:39Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nThis check has involved more environment variables than just `DISPLAY` since\n508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n2008-02-14), so let's update the documentation accordingly.\n\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n Documentation/git-bisect.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex fbb39fbdf5d..82b1d5ac6c5 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -204,9 +204,9 @@ as an alternative to `visualize`):\n $ git bisect visualize\n ------------\n \n-If the `DISPLAY` environment variable is not set, 'git log' is used\n-instead.  You can also give command-line options such as `-p` and\n-`--stat`.\n+If none of the environment variables `DISPLAY`, `SESSIONNAME`, `MSYSTEM` and\n+`SECURITYSESSIONID` is set, 'git log' is used instead.  You can also give\n+command-line options such as `-p` and `--stat`.\n \n ------------\n $ git bisect visualize --stat\n-- \ngitgitgadget\n"},{"id":"480118","messageId":"bf8b34aaef32a64b85f778ab219aeb41238f2bf2.1691058498.git.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.git.1691058498.gitgitgadget@gmail.com","subject":"[PATCH 2/3] run-command: teach locate_in_PATH about Windows","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-03T10:28:17Z","receivedAt":"2023-08-03T10:28:40Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nsince 5e1f28d206 (bisect--helper: reimplement `bisect_visualize()` shell\n function in C, 2021-09-13) `git bisect visualize` uses exists_in_PATH()\nto check wether it should call `gitk`, but exists_in_PATH() relies on\nlocate_in_PATH() which currently only understands POSIX-ish PATH variables\n(a list of paths, separated by colons) on native Windows executables\nwe encounter Windows PATH variables (a list of paths that often contain\ndrive letters (and thus colons), separated by semicolons). Luckily we do\nalready have a function that can lookup executables on windows PATHs:\nmingw_path_lookup(). Teach locate_in_PATH() to use mingw_path_lookup()\non Windows.\n\nReported-by: Louis Strous <Louis.Strous@intellimagic.com>\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n run-command.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 60c94198664..8f518e37e27 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -182,13 +182,10 @@ int is_executable(const char *name)\n  * Returns the path to the command, as found in $PATH or NULL if the\n  * command could not be found.  The caller inherits ownership of the memory\n  * used to store the resultant path.\n- *\n- * This should not be used on Windows, where the $PATH search rules\n- * are more complicated (e.g., a search for \"foo\" should find\n- * \"foo.exe\").\n  */\n static char *locate_in_PATH(const char *file)\n {\n+#ifndef GIT_WINDOWS_NATIVE\n \tconst char *p = getenv(\"PATH\");\n \tstruct strbuf buf = STRBUF_INIT;\n \n@@ -217,6 +214,9 @@ static char *locate_in_PATH(const char *file)\n \n \tstrbuf_release(&buf);\n \treturn NULL;\n+#else\n+\treturn mingw_path_lookup(file,0);\n+#endif\n }\n \n int exists_in_PATH(const char *command)\n-- \ngitgitgadget\n\n"},{"id":"480124","messageId":"xmqqo7jo5d5a.fsf@gitster.g","threadId":"60059","inReplyTo":"pull.1560.git.1691058498.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] git bisect visualize: find gitk on Windows again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-03T16:00:49Z","receivedAt":"2023-08-03T16:01:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> Louis Strous reported a regression in git bisect visualize on Windows[1]\n> that caused git bisect visualize to use git log instead of gitk unless\n> explicitly called as git bisect visualize gitk.\n>\n> This patch series fixes that regression.\n\nWonderful.  It would be nice to describe where in the release\nsequence the \"regression\" happened, if we know it.  Perhaps you've\nwritten about it in one of the patches?  We'll find out.\n\nThanks again.\n\n\n> [1]\n> https://lore.kernel.org/git/VI1PR10MB2462F7B52FF2E3F59AFE94A7F500A@VI1PR10MB2462.EURPRD10.PROD.OUTLOOK.COM/\n>\n> Matthias Aßhauer (3):\n>   compat: make path_lookup() available outside mingw.c\n>   run-command: teach locate_in_PATH about Windows\n>   docs: update when `git bisect visualize` uses `gitk`\n>\n>  Documentation/git-bisect.txt |  6 +++---\n>  compat/mingw.c               | 20 ++++++++------------\n>  compat/mingw.h               |  6 ++++++\n>  run-command.c                |  8 ++++----\n>  4 files changed, 21 insertions(+), 19 deletions(-)\n>\n>\n> base-commit: fb7d80edcae482f4fa5d4be0227dc3054734e5f3\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1560%2Frimrul%2Fwin-bisect-visualize-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1560/rimrul/win-bisect-visualize-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1560\n"},{"id":"480125","messageId":"xmqq7cqc5cjf.fsf@gitster.g","threadId":"60059","inReplyTo":"bf8b34aaef32a64b85f778ab219aeb41238f2bf2.1691058498.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] run-command: teach locate_in_PATH about Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-03T16:13:56Z","receivedAt":"2023-08-03T16:14:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> diff --git a/run-command.c b/run-command.c\n> index 60c94198664..8f518e37e27 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -182,13 +182,10 @@ int is_executable(const char *name)\n>   * Returns the path to the command, as found in $PATH or NULL if the\n>   * command could not be found.  The caller inherits ownership of the memory\n>   * used to store the resultant path.\n> - *\n> - * This should not be used on Windows, where the $PATH search rules\n> - * are more complicated (e.g., a search for \"foo\" should find\n> - * \"foo.exe\").\n>   */\n>  static char *locate_in_PATH(const char *file)\n>  {\n> +#ifndef GIT_WINDOWS_NATIVE\n>  \tconst char *p = getenv(\"PATH\");\n>  \tstruct strbuf buf = STRBUF_INIT;\n>  \n> @@ -217,6 +214,9 @@ static char *locate_in_PATH(const char *file)\n>  \n>  \tstrbuf_release(&buf);\n>  \treturn NULL;\n> +#else\n> +\treturn mingw_path_lookup(file,0);\n> +#endif\n>  }\n\nIt may be cleaner to make the above more like\n\n\t#ifndef locate_in_PATH\n\tstatic char *locate_in_PATH(const char *file)\n\t{\n\t    ... original implementation without any #ifdef ...\n\t}\n\t#endif\n\nand redo the [1/3] patch so that it does not rename or otherwise\ntouch path_lookup() in any way, and instead implements a\nmingw_locate_in_PATH() in terms of path_lookup() and make it public,\ndeclare it in <compat/mingw.h>, together with #define\nlocate_in_PATH(), i.e. [1/3] will essentially become something like:\n\n    (add to compat/mingw.c)\n    char *mingw_locate_in_PATH(const char *file)\n    {\n\treturn path_lookup(file, 0);\n    }\n\n    (add to compat/mingw.h)\n    extern char *mingw_locate_in_PATH(const char *);\n    #define locate_in_PATH(file) mingw_locate_in_PATH(file)\n\nThat way, the second non-UNIXy system can add its own way to locate\nan executable in PATH without having to touch the main part of the\nsystem, right?\n"},{"id":"480126","messageId":"xmqq35105c7h.fsf@gitster.g","threadId":"60059","inReplyTo":"c872431b608424007f72c69c8526f96d532aaca1.1691058498.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-03T16:21:06Z","receivedAt":"2023-08-03T16:21:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>\n> This check has involved more environment variables than just `DISPLAY` since\n> 508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n> 2008-02-14), so let's update the documentation accordingly.\n>\n> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n> ---\n>  Documentation/git-bisect.txt | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> index fbb39fbdf5d..82b1d5ac6c5 100644\n> --- a/Documentation/git-bisect.txt\n> +++ b/Documentation/git-bisect.txt\n> @@ -204,9 +204,9 @@ as an alternative to `visualize`):\n>  $ git bisect visualize\n>  ------------\n>  \n> -If the `DISPLAY` environment variable is not set, 'git log' is used\n> -instead.  You can also give command-line options such as `-p` and\n> -`--stat`.\n> +If none of the environment variables `DISPLAY`, `SESSIONNAME`, `MSYSTEM` and\n> +`SECURITYSESSIONID` is set, 'git log' is used instead.  You can also give\n> +command-line options such as `-p` and `--stat`.\n\nGood.  Would a casual (read: not working on) Git for Windows user\nknow if their environment has MSYSTEM variable?  The same question\napplies to the other variables with relevant platforms.  I think\nfolks working in GUI environment of X Window pedigree may be\nfamiliar enough with DISPLAY (but of course I am biased as I have\nused such an envionrment in the past) that the original description\nis permissible without extra explanation, but among these new ones\nsome may deserve an additional short description in parentheses,\ne.g.\n\n\t..., `MSYSTEM` (always set in Git for Windows) and ...\n\nOther than that, this is a very welcome addition.\n\nThanks.\n"},{"id":"480131","messageId":"xmqqy1is3qqn.fsf@gitster.g","threadId":"60059","inReplyTo":"xmqqo7jo5d5a.fsf@gitster.g","subject":"Re: [PATCH 0/3] git bisect visualize: find gitk on Windows again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-03T18:50:08Z","receivedAt":"2023-08-03T18:53:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\n> writes:\n>\n>> Louis Strous reported a regression in git bisect visualize on Windows[1]\n>> that caused git bisect visualize to use git log instead of gitk unless\n>> explicitly called as git bisect visualize gitk.\n>>\n>> This patch series fixes that regression.\n>\n> Wonderful.  It would be nice to describe where in the release\n> sequence the \"regression\" happened, if we know it.  Perhaps you've\n> written about it in one of the patches?  We'll find out.\n>\n> Thanks again.\n\nAnd the answer was in [2/3], if I am reading the proposed log\nmessages correctly.  When the \"bisect visualize\" was ported to C, we\nbroke it.\n\nThanks.  Will try to queue based on 'maint'.\n"},{"id":"480132","messageId":"DB9P250MB0692B33DE1EBFC26865D78C9A508A@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM","threadId":"60059","inReplyTo":"xmqqy1is3qqn.fsf@gitster.g","subject":"Re: [PATCH 0/3] git bisect visualize: find gitk on Windows again","fromName":"Matthias Aßhauer","fromEmail":"mha1993@live.de","sentAt":"2023-08-03T19:03:36Z","receivedAt":"2023-08-03T19:05:22Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"\n\nOn Thu, 3 Aug 2023, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> \"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\n>> writes:\n>>\n>>> Louis Strous reported a regression in git bisect visualize on Windows[1]\n>>> that caused git bisect visualize to use git log instead of gitk unless\n>>> explicitly called as git bisect visualize gitk.\n>>>\n>>> This patch series fixes that regression.\n>>\n>> Wonderful.  It would be nice to describe where in the release\n>> sequence the \"regression\" happened, if we know it.  Perhaps you've\n>> written about it in one of the patches?  We'll find out.\n>>\n>> Thanks again.\n>\n> And the answer was in [2/3], if I am reading the proposed log\n> messages correctly.  When the \"bisect visualize\" was ported to C, we\n> broke it.\n\nYes, that's correct.\n\n>\n> Thanks.  Will try to queue based on 'maint'.\n>\n\nPlease wait a moment, I'm currently preparing a V2 based on your feedback.\n\n"},{"id":"480133","messageId":"xmqqtttf528j.fsf@gitster.g","threadId":"60059","inReplyTo":"DB9P250MB0692B33DE1EBFC26865D78C9A508A@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM","subject":"Re: [PATCH 0/3] git bisect visualize: find gitk on Windows again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-03T19:56:28Z","receivedAt":"2023-08-03T19:56:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Aßhauer <mha1993@live.de> writes:\n\n>> And the answer was in [2/3], if I am reading the proposed log\n>> messages correctly.  When the \"bisect visualize\" was ported to C, we\n>> broke it.\n>\n> Yes, that's correct.\n>\n>>\n>> Thanks.  Will try to queue based on 'maint'.\n>>\n>\n> Please wait a moment, I'm currently preparing a V2 based on your feedback.\n\nOK.  Thanks.\n"},{"id":"480141","messageId":"dc9c0812d203a4eb777659bb54fda60022bf9650.1691122124.git.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.v2.git.1691122124.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] run-command: conditionally define locate_in_PATH()","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-04T04:08:42Z","receivedAt":"2023-08-04T04:08:53Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nThis commit doesn't change any behaviour by itself, but allows us to easily\ndefine compat replacements for locate_in_PATH(). It prepares us for the next\ncommit that adds a native Windows implementation of locate_in_PATH().\n\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n run-command.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 60c94198664..85fc1507288 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -170,6 +170,7 @@ int is_executable(const char *name)\n \treturn st.st_mode & S_IXUSR;\n }\n \n+#ifndef locate_in_PATH\n /*\n  * Search $PATH for a command.  This emulates the path search that\n  * execvp would perform, without actually executing the command so it\n@@ -218,6 +219,7 @@ static char *locate_in_PATH(const char *file)\n \tstrbuf_release(&buf);\n \treturn NULL;\n }\n+#endif\n \n int exists_in_PATH(const char *command)\n {\n-- \ngitgitgadget\n\n"},{"id":"480142","messageId":"pull.1560.v2.git.1691122124.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.git.1691058498.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] git bisect visualize: find gitk on Windows again","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-04T04:08:41Z","receivedAt":"2023-08-04T04:08:56Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"Louis Strous reported a regression in git bisect visualize on Windows[1]\nthat caused git bisect visualize to use git log instead of gitk unless\nexplicitly called as git bisect visualize gitk. It was introduced during the\nconversion of git bisect visualize to a builtin.\n\nThis patch series fixes that regression.\n\nChanges since v1:\n\n * simplified patches 1 and 2 based on Junios feedback.\n * expanded the new wording in the documentation to clarify which variables\n   users might encounter\n\n[1]\nhttps://lore.kernel.org/git/VI1PR10MB2462F7B52FF2E3F59AFE94A7F500A@VI1PR10MB2462.EURPRD10.PROD.OUTLOOK.COM/\n\nMatthias Aßhauer (3):\n  run-command: conditionally define locate_in_PATH()\n  compat/mingw: implement a native locate_in_PATH()\n  docs: update when `git bisect visualize` uses `gitk`\n\n Documentation/git-bisect.txt | 11 ++++++++---\n compat/mingw.c               |  5 +++++\n compat/mingw.h               |  3 +++\n run-command.c                |  2 ++\n 4 files changed, 18 insertions(+), 3 deletions(-)\n\n\nbase-commit: fb7d80edcae482f4fa5d4be0227dc3054734e5f3\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1560%2Frimrul%2Fwin-bisect-visualize-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1560/rimrul/win-bisect-visualize-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1560\n\nRange-diff vs v1:\n\n 1:  6ed968e1288 < -:  ----------- compat: make path_lookup() available outside mingw.c\n 2:  bf8b34aaef3 ! 1:  dc9c0812d20 run-command: teach locate_in_PATH about Windows\n     @@ Metadata\n      Author: Matthias Aßhauer <mha1993@live.de>\n      \n       ## Commit message ##\n     -    run-command: teach locate_in_PATH about Windows\n     +    run-command: conditionally define locate_in_PATH()\n      \n     -    since 5e1f28d206 (bisect--helper: reimplement `bisect_visualize()` shell\n     -     function in C, 2021-09-13) `git bisect visualize` uses exists_in_PATH()\n     -    to check wether it should call `gitk`, but exists_in_PATH() relies on\n     -    locate_in_PATH() which currently only understands POSIX-ish PATH variables\n     -    (a list of paths, separated by colons) on native Windows executables\n     -    we encounter Windows PATH variables (a list of paths that often contain\n     -    drive letters (and thus colons), separated by semicolons). Luckily we do\n     -    already have a function that can lookup executables on windows PATHs:\n     -    mingw_path_lookup(). Teach locate_in_PATH() to use mingw_path_lookup()\n     -    on Windows.\n     +    This commit doesn't change any behaviour by itself, but allows us to easily\n     +    define compat replacements for locate_in_PATH(). It prepares us for the next\n     +    commit that adds a native Windows implementation of locate_in_PATH().\n      \n     -    Reported-by: Louis Strous <Louis.Strous@intellimagic.com>\n          Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n      \n       ## run-command.c ##\n      @@ run-command.c: int is_executable(const char *name)\n     -  * Returns the path to the command, as found in $PATH or NULL if the\n     -  * command could not be found.  The caller inherits ownership of the memory\n     -  * used to store the resultant path.\n     -- *\n     -- * This should not be used on Windows, where the $PATH search rules\n     -- * are more complicated (e.g., a search for \"foo\" should find\n     -- * \"foo.exe\").\n     -  */\n     - static char *locate_in_PATH(const char *file)\n     - {\n     -+#ifndef GIT_WINDOWS_NATIVE\n     - \tconst char *p = getenv(\"PATH\");\n     - \tstruct strbuf buf = STRBUF_INIT;\n     + \treturn st.st_mode & S_IXUSR;\n     + }\n       \n     ++#ifndef locate_in_PATH\n     + /*\n     +  * Search $PATH for a command.  This emulates the path search that\n     +  * execvp would perform, without actually executing the command so it\n      @@ run-command.c: static char *locate_in_PATH(const char *file)\n     - \n       \tstrbuf_release(&buf);\n       \treturn NULL;\n     -+#else\n     -+\treturn mingw_path_lookup(file,0);\n     -+#endif\n       }\n     ++#endif\n       \n       int exists_in_PATH(const char *command)\n     + {\n -:  ----------- > 2:  8b8c8c3f70a compat/mingw: implement a native locate_in_PATH()\n 3:  c872431b608 ! 3:  04227199089 docs: update when `git bisect visualize` uses `gitk`\n     @@ Documentation/git-bisect.txt: as an alternative to `visualize`):\n      -If the `DISPLAY` environment variable is not set, 'git log' is used\n      -instead.  You can also give command-line options such as `-p` and\n      -`--stat`.\n     -+If none of the environment variables `DISPLAY`, `SESSIONNAME`, `MSYSTEM` and\n     -+`SECURITYSESSIONID` is set, 'git log' is used instead.  You can also give\n     -+command-line options such as `-p` and `--stat`.\n     ++Git detects a graphical environment through various environment variables:\n     ++`DISPLAY`, which is set in X Window System environments on Unix systems.\n     ++`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n     ++`MSYSTEM`, which is set under Msys2 and Git for Windows.\n     ++`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n     ++\n     ++If none of these environment variables is set, 'git log' is used instead.\n     ++You can also give command-line options such as `-p` and `--stat`.\n       \n       ------------\n       $ git bisect visualize --stat\n\n-- \ngitgitgadget\n"},{"id":"480143","messageId":"042271990895c4cfdedb20c3aed3d4141df610bd.1691122124.git.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.v2.git.1691122124.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-04T04:08:44Z","receivedAt":"2023-08-04T04:09:00Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nThis check has involved more environment variables than just `DISPLAY` since\n508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n2008-02-14), so let's update the documentation accordingly.\n\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n Documentation/git-bisect.txt | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex fbb39fbdf5d..bec8d2abb22 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -204,9 +204,14 @@ as an alternative to `visualize`):\n $ git bisect visualize\n ------------\n \n-If the `DISPLAY` environment variable is not set, 'git log' is used\n-instead.  You can also give command-line options such as `-p` and\n-`--stat`.\n+Git detects a graphical environment through various environment variables:\n+`DISPLAY`, which is set in X Window System environments on Unix systems.\n+`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n+`MSYSTEM`, which is set under Msys2 and Git for Windows.\n+`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n+\n+If none of these environment variables is set, 'git log' is used instead.\n+You can also give command-line options such as `-p` and `--stat`.\n \n ------------\n $ git bisect visualize --stat\n-- \ngitgitgadget\n"},{"id":"480144","messageId":"8b8c8c3f70a25f198335e36dfd501ffcb9d411c3.1691122124.git.gitgitgadget@gmail.com","threadId":"60059","inReplyTo":"pull.1560.v2.git.1691122124.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] compat/mingw: implement a native locate_in_PATH()","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-04T04:08:43Z","receivedAt":"2023-08-04T04:09:01Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nsince 5e1f28d (bisect--helper: reimplement `bisect_visualize()` shell\n function in C, 2021-09-13) `git bisect visualize` uses exists_in_PATH()\nto check wether it should call `gitk`, but exists_in_PATH() relies on\nlocate_in_PATH() which currently only understands POSIX-ish PATH variables\n(a list of paths, separated by colons) on native Windows executables\nwe encounter Windows PATH variables (a list of paths that often contain\ndrive letters (and thus colons), separated by semicolons). Luckily we do\nalready have a function that can lookup executables on windows PATHs:\npath_lookup(). Implement a small replacement for the existing\nlocate_in_PATH() based on path_lookup().\n\nReported-by: Louis Strous <Louis.Strous@intellimagic.com>\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n compat/mingw.c | 5 +++++\n compat/mingw.h | 3 +++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex d06cdc6254f..bc3669d2986 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1347,6 +1347,11 @@ static char *path_lookup(const char *cmd, int exe_only)\n \treturn prog;\n }\n \n+char *mingw_locate_in_PATH(const char *cmd)\n+{\n+\treturn path_lookup(cmd, 0);\n+}\n+\n static const wchar_t *wcschrnul(const wchar_t *s, wchar_t c)\n {\n \twhile (*s && *s != c)\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 209cf7cebad..b5262205965 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -175,6 +175,9 @@ pid_t waitpid(pid_t pid, int *status, int options);\n #define kill mingw_kill\n int mingw_kill(pid_t pid, int sig);\n \n+#define locate_in_PATH mingw_locate_in_PATH\n+char *mingw_locate_in_PATH(const char *cmd);\n+\n #ifndef NO_OPENSSL\n #include <openssl/ssl.h>\n static inline int mingw_SSL_set_fd(SSL *ssl, int fd)\n-- \ngitgitgadget\n\n"},{"id":"480145","messageId":"xmqqo7jn3073.fsf@gitster.g","threadId":"60059","inReplyTo":"dc9c0812d203a4eb777659bb54fda60022bf9650.1691122124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] run-command: conditionally define locate_in_PATH()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-04T04:23:28Z","receivedAt":"2023-08-04T04:23:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>\n> This commit doesn't change any behaviour by itself, but allows us to easily\n> define compat replacements for locate_in_PATH(). It prepares us for the next\n> commit that adds a native Windows implementation of locate_in_PATH().\n>\n> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n> ---\n>  run-command.c | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/run-command.c b/run-command.c\n> index 60c94198664..85fc1507288 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -170,6 +170,7 @@ int is_executable(const char *name)\n>  \treturn st.st_mode & S_IXUSR;\n>  }\n>  \n> +#ifndef locate_in_PATH\n>  /*\n>   * Search $PATH for a command.  This emulates the path search that\n>   * execvp would perform, without actually executing the command so it\n\nMicronit.  The comment should be shared across different platform\nimplementations of this interface, so \"#ifndef\" would want to come\nimmediately after this comment, not before, I would think.\n\nIt does not affect the correctness, of course ;-)\n\n> @@ -218,6 +219,7 @@ static char *locate_in_PATH(const char *file)\n>  \tstrbuf_release(&buf);\n>  \treturn NULL;\n>  }\n> +#endif\n>  \n>  int exists_in_PATH(const char *command)\n>  {\n"},{"id":"480146","messageId":"xmqqjzub306e.fsf@gitster.g","threadId":"60059","inReplyTo":"8b8c8c3f70a25f198335e36dfd501ffcb9d411c3.1691122124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/3] compat/mingw: implement a native locate_in_PATH()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-04T04:23:53Z","receivedAt":"2023-08-04T04:24:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>\n> since 5e1f28d (bisect--helper: reimplement `bisect_visualize()` shell\n>  function in C, 2021-09-13) `git bisect visualize` uses exists_in_PATH()\n> to check wether it should call `gitk`, but exists_in_PATH() relies on\n> locate_in_PATH() which currently only understands POSIX-ish PATH variables\n> (a list of paths, separated by colons) on native Windows executables\n> we encounter Windows PATH variables (a list of paths that often contain\n> drive letters (and thus colons), separated by semicolons). Luckily we do\n> already have a function that can lookup executables on windows PATHs:\n> path_lookup(). Implement a small replacement for the existing\n> locate_in_PATH() based on path_lookup().\n>\n> Reported-by: Louis Strous <Louis.Strous@intellimagic.com>\n> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n> ---\n>  compat/mingw.c | 5 +++++\n>  compat/mingw.h | 3 +++\n>  2 files changed, 8 insertions(+)\n\nMakes perfect sense ;-)\n\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index d06cdc6254f..bc3669d2986 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -1347,6 +1347,11 @@ static char *path_lookup(const char *cmd, int exe_only)\n>  \treturn prog;\n>  }\n>  \n> +char *mingw_locate_in_PATH(const char *cmd)\n> +{\n> +\treturn path_lookup(cmd, 0);\n> +}\n> +\n>  static const wchar_t *wcschrnul(const wchar_t *s, wchar_t c)\n>  {\n>  \twhile (*s && *s != c)\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 209cf7cebad..b5262205965 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -175,6 +175,9 @@ pid_t waitpid(pid_t pid, int *status, int options);\n>  #define kill mingw_kill\n>  int mingw_kill(pid_t pid, int sig);\n>  \n> +#define locate_in_PATH mingw_locate_in_PATH\n> +char *mingw_locate_in_PATH(const char *cmd);\n> +\n>  #ifndef NO_OPENSSL\n>  #include <openssl/ssl.h>\n>  static inline int mingw_SSL_set_fd(SSL *ssl, int fd)\n"},{"id":"480147","messageId":"xmqqfs4z2zsg.fsf@gitster.g","threadId":"60059","inReplyTo":"042271990895c4cfdedb20c3aed3d4141df610bd.1691122124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-04T04:32:15Z","receivedAt":"2023-08-04T04:32:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>\n> This check has involved more environment variables than just `DISPLAY` since\n> 508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n> 2008-02-14), so let's update the documentation accordingly.\n>\n> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n> ---\n>  Documentation/git-bisect.txt | 11 ++++++++---\n>  1 file changed, 8 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> index fbb39fbdf5d..bec8d2abb22 100644\n> --- a/Documentation/git-bisect.txt\n> +++ b/Documentation/git-bisect.txt\n> @@ -204,9 +204,14 @@ as an alternative to `visualize`):\n>  $ git bisect visualize\n>  ------------\n>  \n> -If the `DISPLAY` environment variable is not set, 'git log' is used\n> -instead.  You can also give command-line options such as `-p` and\n> -`--stat`.\n> +Git detects a graphical environment through various environment variables:\n> +`DISPLAY`, which is set in X Window System environments on Unix systems.\n> +`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n> +`MSYSTEM`, which is set under Msys2 and Git for Windows.\n> +`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n\nGreat.\n\n> +If none of these environment variables is set, 'git log' is used instead.\n> +You can also give command-line options such as `-p` and `--stat`.\n\nMicronit.  I think \"is set\" want to be \"are set\", as \"none\" in \"none\nof these\" is used for \"not any\" [*].\n\n>  \n>  ------------\n>  $ git bisect visualize --stat\n\n\n[Reference]\n\n* https://www.merriam-webster.com/video/is-none-singular-or-plural\n"},{"id":"480148","messageId":"CAPig+cTE__6B3RNbew8sHQQC3ELi9YAArYX5ofXRpMPBzZfmrw@mail.gmail.com","threadId":"60059","inReplyTo":"042271990895c4cfdedb20c3aed3d4141df610bd.1691122124.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-08-04T05:27:30Z","receivedAt":"2023-08-04T05:28:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 4, 2023 at 1:22 AM Matthias Aßhauer via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> This check has involved more environment variables than just `DISPLAY` since\n> 508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n> 2008-02-14), so let's update the documentation accordingly.\n>\n> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n> ---\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> @@ -204,9 +204,14 @@ as an alternative to `visualize`):\n> -If the `DISPLAY` environment variable is not set, 'git log' is used\n> -instead.  You can also give command-line options such as `-p` and\n> -`--stat`.\n> +Git detects a graphical environment through various environment variables:\n> +`DISPLAY`, which is set in X Window System environments on Unix systems.\n> +`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n> +`MSYSTEM`, which is set under Msys2 and Git for Windows.\n> +`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n\nMicronit: SECURITYSESSIONID is not universal on macOS[1]; some people\nreport its presence in iTerm2 and HyperTerm, and perhaps even Apple's\nown Terminal (although it's not defined for me in Terminal on High\nSierra). Perhaps just say \"may be set on macOS\".\n\nProbably not worth a reroll.\n\n[1]: https://github.com/vercel/hyper/issues/482\n"},{"id":"480149","messageId":"AS1P250MB0701991FBCA2E37BD5238497A509A@AS1P250MB0701.EURP250.PROD.OUTLOOK.COM","threadId":"60059","inReplyTo":"xmqqo7jn3073.fsf@gitster.g","subject":"Re: [PATCH v2 1/3] run-command: conditionally define locate_in_PATH()","fromName":"Matthias Aßhauer","fromEmail":"mha1993@live.de","sentAt":"2023-08-04T05:27:28Z","receivedAt":"2023-08-04T05:28:50Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"\n\nOn Thu, 3 Aug 2023, Junio C Hamano wrote:\n\n> \"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\n> writes:\n>\n>> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>>\n>> This commit doesn't change any behaviour by itself, but allows us to easily\n>> define compat replacements for locate_in_PATH(). It prepares us for the next\n>> commit that adds a native Windows implementation of locate_in_PATH().\n>>\n>> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n>> ---\n>>  run-command.c | 2 ++\n>>  1 file changed, 2 insertions(+)\n>>\n>> diff --git a/run-command.c b/run-command.c\n>> index 60c94198664..85fc1507288 100644\n>> --- a/run-command.c\n>> +++ b/run-command.c\n>> @@ -170,6 +170,7 @@ int is_executable(const char *name)\n>>  \treturn st.st_mode & S_IXUSR;\n>>  }\n>>\n>> +#ifndef locate_in_PATH\n>>  /*\n>>   * Search $PATH for a command.  This emulates the path search that\n>>   * execvp would perform, without actually executing the command so it\n>\n> Micronit.  The comment should be shared across different platform\n> implementations of this interface, so \"#ifndef\" would want to come\n> immediately after this comment, not before, I would think.\n\nI can see the first part applying to all implementations, but the last \npart about it not working on windows is specific to this implementation.\n\nI guess we could split the comment, if we wanted to make that clear.\n\n> It does not affect the correctness, of course ;-)\n>\n>> @@ -218,6 +219,7 @@ static char *locate_in_PATH(const char *file)\n>>  \tstrbuf_release(&buf);\n>>  \treturn NULL;\n>>  }\n>> +#endif\n>>\n>>  int exists_in_PATH(const char *command)\n>>  {\n>\n"},{"id":"480150","messageId":"DB9P250MB06922EB40B40F07DBAA1441EA509A@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM","threadId":"60059","inReplyTo":"CAPig+cTE__6B3RNbew8sHQQC3ELi9YAArYX5ofXRpMPBzZfmrw@mail.gmail.com","subject":"Re: [PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Matthias Aßhauer","fromEmail":"mha1993@live.de","sentAt":"2023-08-04T05:54:34Z","receivedAt":"2023-08-04T05:55:16Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"\n\nOn Fri, 4 Aug 2023, Eric Sunshine wrote:\n\n> On Fri, Aug 4, 2023 at 1:22 AM Matthias Aßhauer via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n>> This check has involved more environment variables than just `DISPLAY` since\n>> 508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n>> 2008-02-14), so let's update the documentation accordingly.\n>>\n>> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n>> ---\n>> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n>> @@ -204,9 +204,14 @@ as an alternative to `visualize`):\n>> -If the `DISPLAY` environment variable is not set, 'git log' is used\n>> -instead.  You can also give command-line options such as `-p` and\n>> -`--stat`.\n>> +Git detects a graphical environment through various environment variables:\n>> +`DISPLAY`, which is set in X Window System environments on Unix systems.\n>> +`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n>> +`MSYSTEM`, which is set under Msys2 and Git for Windows.\n>> +`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n>\n> Micronit: SECURITYSESSIONID is not universal on macOS[1]; some people\n> report its presence in iTerm2 and HyperTerm, and perhaps even Apple's\n> own Terminal (although it's not defined for me in Terminal on High\n> Sierra). Perhaps just say \"may be set on macOS\".\n\nI've just checked in Terminal on Ventura and it isn't set for me either.\nI'll reword it.\n\n> Probably not worth a reroll.\n>\n> [1]: https://github.com/vercel/hyper/issues/482\n>\n"},{"id":"480153","messageId":"xmqq5y5u3gg3.fsf@gitster.g","threadId":"60059","inReplyTo":"AS1P250MB0701991FBCA2E37BD5238497A509A@AS1P250MB0701.EURP250.PROD.OUTLOOK.COM","subject":"Re: [PATCH v2 1/3] run-command: conditionally define locate_in_PATH()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-04T16:44:44Z","receivedAt":"2023-08-04T16:44:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Aßhauer <mha1993@live.de> writes:\n\n> On Thu, 3 Aug 2023, Junio C Hamano wrote:\n>\n>> \"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\n>> writes:\n>>\n>>> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>>>\n>>> This commit doesn't change any behaviour by itself, but allows us to easily\n>>> define compat replacements for locate_in_PATH(). It prepares us for the next\n>>> commit that adds a native Windows implementation of locate_in_PATH().\n>>>\n>>> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n>>> ---\n>>>  run-command.c | 2 ++\n>>>  1 file changed, 2 insertions(+)\n>>>\n>>> diff --git a/run-command.c b/run-command.c\n>>> index 60c94198664..85fc1507288 100644\n>>> --- a/run-command.c\n>>> +++ b/run-command.c\n>>> @@ -170,6 +170,7 @@ int is_executable(const char *name)\n>>>  \treturn st.st_mode & S_IXUSR;\n>>>  }\n>>>\n>>> +#ifndef locate_in_PATH\n>>>  /*\n>>>   * Search $PATH for a command.  This emulates the path search that\n>>>   * execvp would perform, without actually executing the command so it\n>>\n>> Micronit.  The comment should be shared across different platform\n>> implementations of this interface, so \"#ifndef\" would want to come\n>> immediately after this comment, not before, I would think.\n>\n> I can see the first part applying to all implementations, but the last\n> part about it not working on windows is specific to this\n> implementation.\n>\n> I guess we could split the comment, if we wanted to make that clear.\n>\n>> It does not affect the correctness, of course ;-)\n\nLet's not bother immediately before -rc0; letting people use \"gitk\"\nduring \"git bisect\" without having to type 5 extra keystrokes in the\nupcoming release is a good outcome and we can touch up the in-code\ncomment later.\n\nThanks.\n"},{"id":"480154","messageId":"xmqq1qgi3ge7.fsf@gitster.g","threadId":"60059","inReplyTo":"DB9P250MB06922EB40B40F07DBAA1441EA509A@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM","subject":"Re: [PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-04T16:45:52Z","receivedAt":"2023-08-04T16:45:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Aßhauer <mha1993@live.de> writes:\n\n> On Fri, 4 Aug 2023, Eric Sunshine wrote:\n>\n>> On Fri, Aug 4, 2023 at 1:22 AM Matthias Aßhauer via GitGitGadget\n>> <gitgitgadget@gmail.com> wrote:\n>>> This check has involved more environment variables than just `DISPLAY` since\n>>> 508e84a790 (bisect view: check for MinGW32 and MacOSX in addition to X11,\n>>> 2008-02-14), so let's update the documentation accordingly.\n>>>\n>>> Signed-off-by: Matthias Aßhauer <mha1993@live.de>\n>>> ---\n>>> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n>>> @@ -204,9 +204,14 @@ as an alternative to `visualize`):\n>>> -If the `DISPLAY` environment variable is not set, 'git log' is used\n>>> -instead.  You can also give command-line options such as `-p` and\n>>> -`--stat`.\n>>> +Git detects a graphical environment through various environment variables:\n>>> +`DISPLAY`, which is set in X Window System environments on Unix systems.\n>>> +`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n>>> +`MSYSTEM`, which is set under Msys2 and Git for Windows.\n>>> +`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n>>\n>> Micronit: SECURITYSESSIONID is not universal on macOS[1]; some people\n>> report its presence in iTerm2 and HyperTerm, and perhaps even Apple's\n>> own Terminal (although it's not defined for me in Terminal on High\n>> Sierra). Perhaps just say \"may be set on macOS\".\n>\n> I've just checked in Terminal on Ventura and it isn't set for me either.\n> I'll reword it.\n\nI'll locally tweak \"which may be set on macOS\" for now.  It should\nbe good enough for the upcoming release, right?\n"},{"id":"480156","messageId":"CAPig+cQdJSo_VXE=fScv7CKM-8Au8gdx4PmZ=eBtCG+mEN8Rfg@mail.gmail.com","threadId":"60059","inReplyTo":"xmqq1qgi3ge7.fsf@gitster.g","subject":"Re: [PATCH v2 3/3] docs: update when `git bisect visualize` uses `gitk`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-08-04T16:57:46Z","receivedAt":"2023-08-04T16:58:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 4, 2023 at 12:45 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Matthias Aßhauer <mha1993@live.de> writes:\n> > On Fri, 4 Aug 2023, Eric Sunshine wrote:\n> >> On Fri, Aug 4, 2023 at 1:22 AM Matthias Aßhauer via GitGitGadget\n> >> <gitgitgadget@gmail.com> wrote:\n> >>> +Git detects a graphical environment through various environment variables:\n> >>> +`DISPLAY`, which is set in X Window System environments on Unix systems.\n> >>> +`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.\n> >>> +`MSYSTEM`, which is set under Msys2 and Git for Windows.\n> >>> +`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.\n> >>\n> >> Micronit: SECURITYSESSIONID is not universal on macOS[1]; some people\n> >> report its presence in iTerm2 and HyperTerm, and perhaps even Apple's\n> >> own Terminal (although it's not defined for me in Terminal on High\n> >> Sierra). Perhaps just say \"may be set on macOS\".\n> >\n> > I've just checked in Terminal on Ventura and it isn't set for me either.\n> > I'll reword it.\n>\n> I'll locally tweak \"which may be set on macOS\" for now.  It should\n> be good enough for the upcoming release, right?\n\nI would think so.\n"}]}