[v2,0/3] git bisect visualize: find gitk on Windows again

Message

Elijah Newren via GitGitGadget Aug. 4, 2023, 4:08 a.m. UTC

Louis Strous reported a regression in git bisect visualize on Windows[1]
that caused git bisect visualize to use git log instead of gitk unless
explicitly called as git bisect visualize gitk. It was introduced during the
conversion of git bisect visualize to a builtin.

This patch series fixes that regression.

Changes since v1:

 * simplified patches 1 and 2 based on Junios feedback.
 * expanded the new wording in the documentation to clarify which variables
   users might encounter

[1]
https://lore.kernel.org/git/[e-mail]/

Matthias Aßhauer (3):
  run-command: conditionally define locate_in_PATH()
  compat/mingw: implement a native locate_in_PATH()
  docs: update when `git bisect visualize` uses `gitk`

 Documentation/git-bisect.txt | 11 ++++++++---
 compat/mingw.c               |  5 +++++
 compat/mingw.h               |  3 +++
 run-command.c                |  2 ++
 4 files changed, 18 insertions(+), 3 deletions(-)


base-commit: fb7d80edcae482f4fa5d4be0227dc3054734e5f3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1560%2Frimrul%2Fwin-bisect-visualize-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1560/rimrul/win-bisect-visualize-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1560

Range-diff vs v1:

 1:  6ed968e1288 < -:  ----------- compat: make path_lookup() available outside mingw.c
 2:  bf8b34aaef3 ! 1:  dc9c0812d20 run-command: teach locate_in_PATH about Windows
     @@ Metadata
      Author: Matthias Aßhauer <[e-mail]>
      
       ## Commit message ##
     -    run-command: teach locate_in_PATH about Windows
     +    run-command: conditionally define locate_in_PATH()
      
     -    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.
     +    This commit doesn't change any behaviour by itself, but allows us to easily
     +    define compat replacements for locate_in_PATH(). It prepares us for the next
     +    commit that adds a native Windows implementation of locate_in_PATH().
      
     -    Reported-by: Louis Strous <[e-mail]>
          Signed-off-by: Matthias Aßhauer <[e-mail]>
      
       ## run-command.c ##
      @@ run-command.c: 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;
     + 	return st.st_mode & S_IXUSR;
     + }
       
     ++#ifndef locate_in_PATH
     + /*
     +  * Search $PATH for a command.  This emulates the path search that
     +  * execvp would perform, without actually executing the command so it
      @@ run-command.c: static char *locate_in_PATH(const char *file)
     - 
       	strbuf_release(&buf);
       	return NULL;
     -+#else
     -+	return mingw_path_lookup(file,0);
     -+#endif
       }
     ++#endif
       
       int exists_in_PATH(const char *command)
     + {
 -:  ----------- > 2:  8b8c8c3f70a compat/mingw: implement a native locate_in_PATH()
 3:  c872431b608 ! 3:  04227199089 docs: update when `git bisect visualize` uses `gitk`
     @@ Documentation/git-bisect.txt: as an alternative to `visualize`):
      -If the `DISPLAY` environment variable is not set, 'git log' is used
      -instead.  You can also give command-line options such as `-p` and
      -`--stat`.
     -+If none of the environment variables `DISPLAY`, `SESSIONNAME`, `MSYSTEM` and
     -+`SECURITYSESSIONID` is set, 'git log' is used instead.  You can also give
     -+command-line options such as `-p` and `--stat`.
     ++Git detects a graphical environment through various environment variables:
     ++`DISPLAY`, which is set in X Window System environments on Unix systems.
     ++`SESSIONNAME`, which is set under Cygwin in interactive desktop sessions.
     ++`MSYSTEM`, which is set under Msys2 and Git for Windows.
     ++`SECURITYSESSIONID`, which is set on macOS in interactive desktop sessions.
     ++
     ++If none of these environment variables is set, 'git log' is used instead.
     ++You can also give command-line options such as `-p` and `--stat`.
       
       ------------
       $ git bisect visualize --stat