magit/with-editor

`git` not found, despite `magit-git-executable` being set.

joostkremers opened this issue · 15 comments

I'm in the process of setting up Emacs on Windows (have been a Linux user for years) and I ran into something that might be a bug with Magit and/or with-editor.

I installed Git for Windows and set magit-git-executable to the git.exe it provides. I did not add the directory containing git.exe to the system's Path (or to Emacs' exec-path). I was advised against that because Git for Windows is an MSYS2 binary, while Emacs is a MinGW binary. (I ran into different issues because I was mixing the two.)

With magit-git-executable set, magit-status works fine, and so does staging files, fetching and pulling, etc. What does not work, however, is writing a commit message. Instead, I get a warning and the suggestion to hit $. This is the error shown in the log buffer:

Waiting for Emacs...

*ERROR*: Searching for program: No such file or directory, git
error: There was a problem with the editor '"c:/msys64/mingw64/bin/emacsclient.exe"'.
Please supply the message using either -m or -F option.

This is actually the same error I get when I do git commit from a command line, but in that case it makes sense to me, because the path that Emacs sees does not contain git. However, when committing from Magit, I kinda expect git to be found, given that I've explicitly set magit-git-executable.

There is a workaround: Git for Windows puts git.exe an a few other binaries it needs in a separate directory that can be added to exec-path or the system Path without too much risk, so perhaps this issue isn't much of a concern. But I thought I'd bring it up just in case.

Can you get a backtrace using M-x toggle-debug-on-error or maybe M-x toggle-debug-on-error?

The error passes through:

./lib-src/emacsclient.c:2164: fprintf (stderr, "*ERROR*: %s", str);

I am guessing that we fail to use the appropriate emacs server and that either another server is being used or a new emacs instance is started. The output of with-editor-debug might help diagnose that.

Can you get a backtrace using M-x toggle-debug-on-error

I tried toggle-debug-on-error, but there actually no Emacs error, so there's no backtrace.

or maybe M-x toggle-debug-on-error?

Not sure what you meant with the second toggle-debug-on-error. 😃

I am guessing that we fail to use the appropriate emacs server and that either another server is being used or a new emacs instance is started. The output of with-editor-debug might help diagnose that.

The output of M-x with-editor-debug:

with-editor: c:/Users/joost.kremers/.emacs.d/elpa/with-editor-20220130.1942/with-editor.el
emacs: c:/msys64/mingw64/bin/emacs (27.2)
system:
  system-type: windows-nt
  system-configuration: x86_64-w64-mingw32
  system-configuration-options: --prefix=/mingw64 --build=x86_64-w64-mingw32 --with-modules --without-dbus --without-compress-install 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe' CPPFLAGS=-D__USE_MINGW_ANSI_STDIO=1 'LDFLAGS=-pipe -Wl,--dynamicbase,--high-entropy-va,--nxcompat,--default-image-base-high'
server:
  server-running-p: t
  server-process: #<process server>
  server-use-tcp: t
  server-name: server
  server-socket-dir: nil
    WARNING: not an accessible directory
  server-auth-dir: ~/.emacs.d/server/
    server
with-editor-emacsclient-executable:
 value:   c:/msys64/mingw64/bin/emacsclient.exe (27.2)
 default: c:/msys64/mingw64/bin/emacsclient.exe (27.2)
 funcall: c:/msys64/mingw64/bin/emacsclient.exe (27.2)
path:
  $PATH: "C:\\msys64\\mingw64\\bin;C:\\msys64\\usr\\local\\bin;C:\\msys64\\usr\\bin;C:\\msys64\\usr\\bin;C:\\Windows\\System32;C:\\Windows;C:\\Windows\\System32\\Wbem;C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\msys64\\usr\\bin\\site_perl;C:\\msys64\\usr\\bin\\vendor_perl;C:\\msys64\\usr\\bin\\core_perl"
  exec-path: (c:/msys64/mingw64/bin C:/msys64/usr/local/bin C:/msys64/usr/bin C:/msys64/usr/bin C:/Windows/System32 C:/Windows C:/Windows/System32/Wbem C:/Windows/System32/WindowsPowerShell/v1.0/ C:/msys64/usr/bin/site_perl C:/msys64/usr/bin/vendor_perl C:/msys64/usr/bin/core_perl c:/msys64/mingw64/libexec/emacs/27.2/x86_64-w64-mingw32)
  with-editor-emacsclient-path:
    c:/msys64/mingw64/bin (t)
      c:/msys64/mingw64/bin/emacsclient.exe (27.2)
    C:/msys64/usr/local/bin (nil)
    C:/msys64/usr/bin (t)
    C:/Windows/System32 (t)
    C:/Windows (t)
    C:/Windows/System32/Wbem (t)
    C:/Windows/System32/WindowsPowerShell/v1.0/ (t)
    C:/msys64/usr/bin/site_perl (t)
    C:/msys64/usr/bin/vendor_perl (t)
    C:/msys64/usr/bin/core_perl (t)
    c:/msys64/mingw64/libexec/emacs/27.2/x86_64-w64-mingw32 (t)

Emacs version: GNU Emacs 27.2 (build 1, x86_64-w64-mingw32) of 2021-08-27

I installed Emacs through MSYS2 using pacman (I specifically installed the MinGW version.)

For comparison, this is the output of M-x with-editor-debug when the git directory is on the system's Path variable (and hence in exec-path):

with-editor: c:/Users/joost.kremers/.emacs.d/elpa/with-editor-20220130.1942/with-editor.el
emacs: c:/msys64/mingw64/bin/emacs (27.2)
system:
  system-type: windows-nt
  system-configuration: x86_64-w64-mingw32
  system-configuration-options: --prefix=/mingw64 --build=x86_64-w64-mingw32 --with-modules --without-dbus --without-compress-install 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe' CPPFLAGS=-D__USE_MINGW_ANSI_STDIO=1 'LDFLAGS=-pipe -Wl,--dynamicbase,--high-entropy-va,--nxcompat,--default-image-base-high'
server:
  server-running-p: t
  server-process: #<process server>
  server-use-tcp: t
  server-name: server
  server-socket-dir: nil
    WARNING: not an accessible directory
  server-auth-dir: ~/.emacs.d/server/
    server
with-editor-emacsclient-executable:
 value:   c:/msys64/mingw64/bin/emacsclient.exe (27.2)
 default: c:/msys64/mingw64/bin/emacsclient.exe (27.2)
 funcall: c:/msys64/mingw64/bin/emacsclient.exe (27.2)
path:
  $PATH: "C:\\WINDOWS\\system32;C:\\WINDOWS;C:\\WINDOWS\\System32\\Wbem;C:\\WINDOWS\\System32\\WindowsPowerShell\\v1.0\\;C:\\WINDOWS\\System32\\OpenSSH\\;C:\\Program Files\\dotnet\\;C:\\Program Files\\nodejs\\;C:\\Program Files\\Git\\cmd;C:\\Users\\joost.kremers\\.pyenv\\pyenv-win\\bin;C:\\Users\\joost.kremers\\.pyenv\\pyenv-win\\shims;C:\\Users\\joost.kremers\\AppData\\Local\\Microsoft\\WindowsApps;C:\\Users\\joost.kremers\\AppData\\Local\\Pandoc\\;C:\\Users\\joost.kremers\\.dotnet\\tools;C:\\Users\\joost.kremers\\AppData\\Roaming\\Python\\Scripts;C:\\Users\\joost.kremers\\AppData\\Roaming\\npm;C:\\msys64\\mingw64\\bin;C:\\Program Files\\poppler-0.68.0_x86\\bin;C:\\Program Files\\Tesseract-OCR;"
  exec-path: (c:/WINDOWS/system32 C:/WINDOWS C:/WINDOWS/System32/Wbem C:/WINDOWS/System32/WindowsPowerShell/v1.0/ C:/WINDOWS/System32/OpenSSH/ C:/Program Files/dotnet/ C:/Program Files/nodejs/ C:/Program Files/Git/cmd C:/Users/joost.kremers/.pyenv/pyenv-win/bin C:/Users/joost.kremers/.pyenv/pyenv-win/shims C:/Users/joost.kremers/AppData/Local/Microsoft/WindowsApps C:/Users/joost.kremers/AppData/Local/Pandoc/ C:/Users/joost.kremers/.dotnet/tools C:/Users/joost.kremers/AppData/Roaming/Python/Scripts C:/Users/joost.kremers/AppData/Roaming/npm C:/msys64/mingw64/bin C:/Program Files/poppler-0.68.0_x86/bin C:/Program Files/Tesseract-OCR . c:/msys64/mingw64/libexec/emacs/27.2/x86_64-w64-mingw32)
  with-editor-emacsclient-path:
    c:/msys64/mingw64/bin (t)
      c:/msys64/mingw64/bin/emacsclient.exe (27.2)
    c:/WINDOWS/system32 (t)
    C:/WINDOWS (t)
    C:/WINDOWS/System32/Wbem (t)
    C:/WINDOWS/System32/WindowsPowerShell/v1.0/ (t)
    C:/WINDOWS/System32/OpenSSH/ (t)
    C:/Program Files/dotnet/ (t)
    C:/Program Files/nodejs/ (t)
    C:/Program Files/Git/cmd (t)
    C:/Users/joost.kremers/.pyenv/pyenv-win/bin (t)
    C:/Users/joost.kremers/.pyenv/pyenv-win/shims (t)
    C:/Users/joost.kremers/AppData/Local/Microsoft/WindowsApps (t)
    C:/Users/joost.kremers/AppData/Local/Pandoc/ (t)
    C:/Users/joost.kremers/.dotnet/tools (nil)
    C:/Users/joost.kremers/AppData/Roaming/Python/Scripts (t)
    C:/Users/joost.kremers/AppData/Roaming/npm (t)
    C:/msys64/mingw64/bin (t)
      C:/msys64/mingw64/bin/emacsclient.exe (27.2)
    C:/Program Files/poppler-0.68.0_x86/bin (t)
    C:/Program Files/Tesseract-OCR (t)
    . (t)
    c:/msys64/mingw64/libexec/emacs/27.2/x86_64-w64-mingw32 (t)

To be honest, I don't want to debug this for you. Not having to deal with stuff like this is one of the reasons I don't use Windows.

I was advised against that because Git for Windows is an MSYS2 binary, while Emacs is a MinGW binary. (I ran into different issues because I was mixing the two.)

But aren't you trying to do exactly that; using msys2 git with mingw emacs that is? And in addition to doing that you don't want to set PATH accordingly, because you were advised to do "that". I think your takeaway from that advice was to not set PATH but what they meant was "don't mix the two". If you mix anyway, then you have to set PATH, and the reason why people recommend against mixing is that you will have to do that and that many other things you would expect to just work may or may not work without additional settings and research.

To be honest, I don't want to debug this for you. Not having to deal with stuff like this is one of the reasons I don't use Windows.

Well, I wouldn't be using Windows either if I could, but it helps pay the bills, so...

But aren't you trying to do exactly that; using msys2 git with mingw emacs that is?

Yes, because there is no MingGW package for git. The suggestion was to minimise the use of MSYS2 binaries in a MinGW environment, and I had hoped setting magit-git-executable would be a way to do just that. Adding just git to the system's Path (or even just to exec-path) is the next best thing.

Anyway, the Windows stuff was just background, and not relevant to the problem itself, which is that even if magit-git-executable is set by the user, committing fails because git cannot be found.

You can, in fact, easily reproduce the problem on Linux: just install git to some non-standard location, say /opt/git, without putting /opt/git/bin in your PATH, and set magit-git-executable to /opt/git/bin/git. You'll see that magit-status works just fine, staging and unstaging work as well, and presumably everything that only requires calling out to git.

But committing fails with the exact same error message saying that git cannot be found.

I can understand if you'd prefer to use your time for other things. It's not going to be a very common use case and on Windows there's a fairly safe workaround. I just thought you might want to at least be aware of it.

You can, in fact, easily reproduce the problem on Linux

Well once I have switched to Guix it will be easy and safe to do that. So yes, I'll focus on other things for now and only look at this once the initial hurdle and risk is smaller.

Unless maybe if this produces something:

Not sure what you meant with the second toggle-debug-on-error.

toggle-debug-on-signal

I considered that unlikely before because I would expect the error to look different than *ERROR*: Searching for program: No such file or directory, git in that case, but are you sure no git hook is involved? Magit itself calls git using magit-git-executable but if some git hook calls git itself, then that might be the location where lookup fails.

I can trigger the same error message by dropping Git from $PATH:

$ guix shell --pure emacs grep gawk which -- which git
which: no git in (/gnu/store/mlngh0yx68i0gd4b6d4vrwyfalxnkc2c-profile/bin)

# vanilla config with magit-git-executable set
$ grep magit-git-exe ~/.emacs.d/test-configs/magit-config.el
(setq magit-git-executable "/home/kyle/src/c/git/bin-wrappers/git")

$ guix shell --pure emacs grep gawk which -- emacs -Q --load ~/.emacs.d/test-configs/shared.el --load ~/.emacs.d/test-configs/magit-config.el

Trying to make a commit gives this:

  1 git … commit --
hint: Waiting for your editor to close the file...
Waiting for Emacs...
*ERROR*: Searching for program: No such file or directory, git
error: There was a problem with the editor '/gnu/store/955rbw95zj8g2a3ail5zf9ml7jzi4zr0-emacs-27.2/bin/emacsclient --socket-name=/tmp/emacs1000/server'.
Please supply the message using either -m or -F option.

Based on looking for raw "git" strings in the Magit repo, git-commit.el looked like the likely culprit. And indeed the error goes away on my end with this change:

diff --git a/lisp/git-commit.el b/lisp/git-commit.el
index faca0157..b2b42ee3 100644
--- a/lisp/git-commit.el
+++ b/lisp/git-commit.el
@@ -1036,12 +1036,13 @@ (defun git-commit-setup-font-lock ()
     (modify-syntax-entry ?`  "." table)
     (set-syntax-table table))
   (setq-local comment-start
-              (or (with-temp-buffer
-                    (call-process "git" nil (current-buffer) nil
-                                  "config" "core.commentchar")
-                    (unless (bobp)
-                      (goto-char (point-min))
-                      (buffer-substring (point) (line-end-position))))
+              (or (ignore-errors
+                    (with-temp-buffer
+                      (call-process "git" nil (current-buffer) nil
+                                    "config" "core.commentchar")
+                      (unless (bobp)
+                        (goto-char (point-min))
+                        (buffer-substring (point) (line-end-position)))))
                   "#"))
   (setq-local comment-start-skip (format "^%s+[\s\t]*" comment-start))
   (setq-local comment-end-skip "\n")

@kyleam Thanks for looking into this.

Based on looking for raw "git" strings in the Magit repo, git-commit.el looked like the likely culprit.

So perhaps git-commit.el should be using the value of magit-git-executable if it's set?

So perhaps git-commit.el should be using the value of magit-git-executable if it's set?

I'd say that'd be a good change, yes, or more specifically the result of the magit-git-executable function. @tarsius, what do you think about something like below?

diff
diff --git a/lisp/git-commit.el b/lisp/git-commit.el
index faca0157..d4b50f55 100644
--- a/lisp/git-commit.el
+++ b/lisp/git-commit.el
@@ -795,6 +795,13 @@ (defun git-commit-buffer-message ()
         (setq str (replace-match "\n" t t str)))
       str)))
 
+;;; Utilities
+
+(defsubst git-commit-executable ()
+  (if (fboundp 'magit-git-executable)
+      (magit-git-executable)
+    "git"))
+
 ;;; Headers
 
 (transient-define-prefix git-commit-insert-pseudo-header ()
@@ -859,13 +866,17 @@ (defun git-commit-co-authored (name mail)
 (defun git-commit-self-ident ()
   (list (or (getenv "GIT_AUTHOR_NAME")
             (getenv "GIT_COMMITTER_NAME")
-            (ignore-errors (car (process-lines "git" "config" "user.name")))
+            (ignore-errors
+              (car (process-lines
+                    (git-commit-executable) "config" "user.name")))
             user-full-name
             (read-string "Name: "))
         (or (getenv "GIT_AUTHOR_EMAIL")
             (getenv "GIT_COMMITTER_EMAIL")
             (getenv "EMAIL")
-            (ignore-errors (car (process-lines "git" "config" "user.email")))
+            (ignore-errors
+              (car (process-lines
+                    (git-commit-executable) "config" "user.email")))
             (read-string "Email: "))))
 
 (defvar git-commit-read-ident-history nil)
@@ -1037,8 +1048,9 @@ (defun git-commit-setup-font-lock ()
     (set-syntax-table table))
   (setq-local comment-start
               (or (with-temp-buffer
-                    (call-process "git" nil (current-buffer) nil
-                                  "config" "core.commentchar")
+                    (call-process
+                     (git-commit-executable) nil (current-buffer) nil
+                     "config" "core.commentchar")
                     (unless (bobp)
                       (goto-char (point-min))
                       (buffer-substring (point) (line-end-position))))

I think we should have done it a long time ago. 😀

Why not go a step further and use (with-demoted-errors "git config user.name: %S" ...)? While we are at it we should also look for places where a Lisp hook could cause issues when committing, and treat them similarly. After all many type-errors were reported in the past that were caused by broken hook functions.

Why not go a step further and use (with-demoted-errors "git config user.name: %S" ...)? While [...]

Good point. Assuming I don't find free time tonight, I'll plan to look at that this weekend.

While we are at it we should also look for places where a Lisp hook could cause issues when committing, and treat them similarly.

It looks like git-commit-setup-hook is already wrapped with with-demoted-errors. I did the same for git-commit-setup-font-lock to hopefully avoid failures similar to the one reported here. There may be other places that should get similar treatment. I gave a half-hearted attempt at a fuller audit but was having trouble defining the set of things to be concerned about.

Thanks! That's certainly an improvement.