magit can't find "git" in Tramp from local PC to remote nix-like #3345

ShuguangSun · Jan 26, 2018

(1) what behavior you expected
In Tramp mode (plink or ssh, local Windows 7 PC, remote linux/unix), I expect magit can find "git" remotely and works as normal after setting
(add-to-list 'tramp-remote-path 'tramp-own-remote-path)

(2) what behavior you observed
The magit can't find "git".

(3) and how we can reproduce the issue.
Step 1. (setq magit-git-executable "git") and (add-to-list 'tramp-remote-path 'tramp-own-remote-path)
Step 2. Goto git repo in the remote via tramp, and magit-status

I have looked in the code, and it seems due to my-magit-process-file where process-environment is let-assigned. In windows, there is a PATH env variables in process-environment, and if it is let-assigned here, it will bring the local window PC PATH to the remote linux/unix environment. Therefore, magit can't find "git" in the remote server.
I don't think it is proper to take local process-environment to the remote.
Maybe it is just benefit Cygwin's noglob option and during the call and ensure unix eol conversion as it is documented?

Currently I use an advice as a temporary solution as reference for those meet same issue:

    (defun my-magit-process-file (&rest args)
      "Don't set process-environment in tramp-like mode"
      (let ((default-process-coding-system (magit--process-coding-system)))
        (apply #'process-file args)))
    (advice-add 'magit-process-file :override #'my-magit-process-file)

I don't think it is proper to take local process-environment to the remote.

Agreed. Does something like the below fix it?

--- i/lisp/magit-process.el
+++ w/lisp/magit-process.el
@@ -336,7 +336,9 @@ (defun magit-process-file (&rest args)
 Identical to `process-file' but temporarily enable Cygwin's
 \"noglob\" option during the call and ensure unix eol
 conversion."
-  (let ((process-environment (append (magit-cygwin-env-vars)
+  (let ((process-environment (append ;; Drop w32 env hacks for remote calls.
+                                     (unless (file-remote-p default-directory)
+                                       (magit-cygwin-env-vars))
                                      process-environment))
         (default-process-coding-system (magit--process-coding-system)))
     (apply #'process-file args)))

npostavs · Jan 26, 2018

I don't think it is proper to take local process-environment to the remote.

Agreed. Does something like the below fix it?

--- i/lisp/magit-process.el
+++ w/lisp/magit-process.el
@@ -336,7 +336,9 @@ (defun magit-process-file (&rest args)
 Identical to `process-file' but temporarily enable Cygwin's
 \"noglob\" option during the call and ensure unix eol
 conversion."
-  (let ((process-environment (append (magit-cygwin-env-vars)
+  (let ((process-environment (append ;; Drop w32 env hacks for remote calls.
+                                     (unless (file-remote-p default-directory)
+                                       (magit-cygwin-env-vars))
                                      process-environment))
         (default-process-coding-system (magit--process-coding-system)))
     (apply #'process-file args)))

FYI, I get around this issue by: (setq magic-git-executable “git”) so that the executable can be found in the local path.

…

On Jan 26, 2018, at 6:59 AM, Noam Postavsky ***@***.***> wrote: I don't think it is proper to take local process-environment to the remote. Agreed. Does something like the below fix it? --- i/lisp/magit-process.el +++ w/lisp/magit-process.el @@ -336,7 +336,9 @@ (defun magit-process-file (&rest args) Identical to `process-file' but temporarily enable Cygwin's \"noglob\" option during the call and ensure unix eol conversion." - (let ((process-environment (append (magit-cygwin-env-vars) + (let ((process-environment (append ;; Drop w32 env hacks for remote calls. + (unless (file-remote-p default-directory) + (magit-cygwin-env-vars)) process-environment)) (default-process-coding-system (magit--process-coding-system))) (apply #'process-file args))) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

articuluxe · Jan 26, 2018

FYI, I get around this issue by: (setq magic-git-executable “git”) so that the executable can be found in the local path.

…

On Jan 26, 2018, at 6:59 AM, Noam Postavsky ***@***.***> wrote: I don't think it is proper to take local process-environment to the remote. Agreed. Does something like the below fix it? --- i/lisp/magit-process.el +++ w/lisp/magit-process.el @@ -336,7 +336,9 @@ (defun magit-process-file (&rest args) Identical to `process-file' but temporarily enable Cygwin's \"noglob\" option during the call and ensure unix eol conversion." - (let ((process-environment (append (magit-cygwin-env-vars) + (let ((process-environment (append ;; Drop w32 env hacks for remote calls. + (unless (file-remote-p default-directory) + (magit-cygwin-env-vars)) process-environment)) (default-process-coding-system (magit--process-coding-system))) (apply #'process-file args))) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

Does something like the below fix it?

Doesn't that just make sure some variables that are irrelevant on Linux are not set?

FYI, I get around this issue by: (setq magic-git-executable “git”)

So the problem is that the the full path to the executable that is appropriate on your local Windows machine isn't appropriate on your remote Linux machine, right? We use a full path on Windows because the executable would otherwise often not be found because many users do not setup PATH properly. Of course that causes the breakage you have observed, but that seems to be much less common.

Setting magit-git-exectuable as you have done is the correct fix as far as I can tell from a distance.

tarsius · Jan 26, 2018

Does something like the below fix it?

Doesn't that just make sure some variables that are irrelevant on Linux are not set?

FYI, I get around this issue by: (setq magic-git-executable “git”)

So the problem is that the the full path to the executable that is appropriate on your local Windows machine isn't appropriate on your remote Linux machine, right? We use a full path on Windows because the executable would otherwise often not be found because many users do not setup PATH properly. Of course that causes the breakage you have observed, but that seems to be much less common.

Setting magit-git-exectuable as you have done is the correct fix as far as I can tell from a distance.

Doesn't that just make sure some variables that are irrelevant on Linux are not set?

On non-windows-nt machines, magit-cygwin-env-vars will return nil. On Windows, it has overrides for PATH (perhaps the function name should be updated, it's not Cygwin-only anymore...). I'm not very familiar with tramp, so I don't know if/how it interacts with tramp-remote-path settings from the OP. Seems simplest to just disable it for remote calls, as it cannot be useful there.

npostavs · Jan 26, 2018

Doesn't that just make sure some variables that are irrelevant on Linux are not set?

On non-windows-nt machines, magit-cygwin-env-vars will return nil. On Windows, it has overrides for PATH (perhaps the function name should be updated, it's not Cygwin-only anymore...). I'm not very familiar with tramp, so I don't know if/how it interacts with tramp-remote-path settings from the OP. Seems simplest to just disable it for remote calls, as it cannot be useful there.

On non-windows-nt machines, magit-cygwin-env-vars will return nil.

Or wait, there is (list (format "INSIDE_EMACS=%s,magit" emacs-version)) from magit-git-environment. Really seems like that shouldn't be hidden away in magit-cygwin-env-vars.

npostavs · Jan 26, 2018

On non-windows-nt machines, magit-cygwin-env-vars will return nil.

Or wait, there is (list (format "INSIDE_EMACS=%s,magit" emacs-version)) from magit-git-environment. Really seems like that shouldn't be hidden away in magit-cygwin-env-vars.

The patch makes magit-status works, although the process-environment still contains local windoes PC path.
In windows PC (win 7), the (magit-cygwin-env-vars) will return "INSIDE_EMACS=27.0.50,magit" and the env variable "PATH" which has the path to the lcoal "git".
In my global env variable "PATH", it has "PortableGit\cmd" which help to find "git", and from (magit-cygwin-env-vars) the PATH has "PortableGit\mingw64\libexec\git-core; PortableGit\mingw64\bin;PortableGit\usr\bin;" at the begining (it is for the efficient as the magit manual said).
I have set (setq magit-git-executable "git").
So I don't understand why the patch works.

The following code print the process-environment which is total local PC setting, and it show in tramp, the (magit-cygwin-env-vars) is not appended.

      (let ((process-environment (append ;; Drop w32 env hacks for remote calls.
                                  (unless (file-remote-p default-directory)
                                    (magit-cygwin-env-vars))
                                  process-environment))
            (default-process-coding-system (magit--process-coding-system)))
        (print process-environment)
        (apply #'process-file args)))

ShuguangSun · Jan 26, 2018

The patch makes magit-status works, although the process-environment still contains local windoes PC path.
In windows PC (win 7), the (magit-cygwin-env-vars) will return "INSIDE_EMACS=27.0.50,magit" and the env variable "PATH" which has the path to the lcoal "git".
In my global env variable "PATH", it has "PortableGit\cmd" which help to find "git", and from (magit-cygwin-env-vars) the PATH has "PortableGit\mingw64\libexec\git-core; PortableGit\mingw64\bin;PortableGit\usr\bin;" at the begining (it is for the efficient as the magit manual said).
I have set (setq magit-git-executable "git").
So I don't understand why the patch works.

The following code print the process-environment which is total local PC setting, and it show in tramp, the (magit-cygwin-env-vars) is not appended.

      (let ((process-environment (append ;; Drop w32 env hacks for remote calls.
                                  (unless (file-remote-p default-directory)
                                    (magit-cygwin-env-vars))
                                  process-environment))
            (default-process-coding-system (magit--process-coding-system)))
        (print process-environment)
        (apply #'process-file args)))

The patch makes magit-status works, although the process-environment still contains local windoes PC path.

You mean let-binding process-environment to itself is somehow different from not let-binding it all, from tramp's point of view? Seems odd, I can't see how it should make a difference.

npostavs · Jan 26, 2018

The patch makes magit-status works, although the process-environment still contains local windoes PC path.

You mean let-binding process-environment to itself is somehow different from not let-binding it all, from tramp's point of view? Seems odd, I can't see how it should make a difference.

OK. I remove the path to git from the local PC PATH.
Then ... "apply: Searching for program: No such file or directory, git" (it is because magit can't find local git and the can't be loaded)

~~It maybe the issue of process-file, not only of the magit. In my prevous test, the local git cheats process-file and my-magit-process-file.~~

ShuguangSun · Jan 26, 2018

OK. I remove the path to git from the local PC PATH.
Then ... "apply: Searching for program: No such file or directory, git" (it is because magit can't find local git and the can't be loaded)

~~It maybe the issue of process-file, not only of the magit. In my prevous test, the local git cheats process-file and my-magit-process-file.~~

After a step by step evaluation:

remove local path to "git" from environment PATH which means magit can't find git and can't be loaded
magit-process-file with your patch above will output: git version 2.14.3, and 0 (#o0, #x0, ?\C-@) in message buffer

Sorry for the confusing of my last comments.

ShuguangSun · Jan 27, 2018

After a step by step evaluation:

remove local path to "git" from environment PATH which means magit can't find git and can't be loaded
magit-process-file with your patch above will output: git version 2.14.3, and 0 (#o0, #x0, ?\C-@) in message buffer

Sorry for the confusing of my last comments.

I'm still confused.

remove local path to "git" from environment PATH which means magit can't find git and can't be loaded

If you can't load magit, how do you get to step 2?

npostavs · Jan 27, 2018

I'm still confused.

remove local path to "git" from environment PATH which means magit can't find git and can't be loaded

If you can't load magit, how do you get to step 2?

If you can't load magit, how do you get to step 2?

In the local pc, I tried to eval each define of the involed functions and variables and tried to get them work... In the remote server, there is git and therefore I got the outputs.
The magit can't be loaded most likely si because there are some defcustom involes git to get evironment setting. For example magit-git-executable and some cygwin related variables, in windows-nt. I understand it is intend to find out the bin/git.exe and to PATH.

ShuguangSun · Jan 27, 2018

If you can't load magit, how do you get to step 2?

In the local pc, I tried to eval each define of the involed functions and variables and tried to get them work... In the remote server, there is git and therefore I got the outputs.
The magit can't be loaded most likely si because there are some defcustom involes git to get evironment setting. For example magit-git-executable and some cygwin related variables, in windows-nt. I understand it is intend to find out the bin/git.exe and to PATH.

If I understand this correctly the situation is as follows.

Without any Magit or Tramp configuration and after removing all patches to Magit again,

You can use Magit on your local Windows machine.
When you try to use Magit over Tramp, connecting to a Linux machine you get an error about /c/path/to/magit cannot be found. (The error you get is different, I used a made up path and I am also not sure where the error is displayed. The important part is to notice that the path cannot possibly be correct for Linux.)

You then fix that using (setq magit-git-executable "git"). Now,

Magit works on the remote Linux machine.
Magit no longer works on the local Windows machine. It complains about not being able to find git.

It this is so, then keep the (setq magit-git-executable "git") and additionally set the PATH variable, follow the instructions at https://github.com/magit/magit/wiki/Pushing-with-Magit-from-Windows#before-starting-set-home.

tarsius · Jan 29, 2018

If I understand this correctly the situation is as follows.

Without any Magit or Tramp configuration and after removing all patches to Magit again,

You can use Magit on your local Windows machine.
When you try to use Magit over Tramp, connecting to a Linux machine you get an error about /c/path/to/magit cannot be found. (The error you get is different, I used a made up path and I am also not sure where the error is displayed. The important part is to notice that the path cannot possibly be correct for Linux.)

You then fix that using (setq magit-git-executable "git"). Now,

Magit works on the remote Linux machine.
Magit no longer works on the local Windows machine. It complains about not being able to find git.

It this is so, then keep the (setq magit-git-executable "git") and additionally set the PATH variable, follow the instructions at https://github.com/magit/magit/wiki/Pushing-with-Magit-from-Windows#before-starting-set-home.

You then fix that using (setq magit-git-executable "git"). Now,
Magit works on the remote Linux machine.

Magit doesn't work on the remote Linux Machine without the patch. It raises the message "Git repository: default path", and after enter, it asks "Create repository in: default path".
If applied the patch, magit-status will work.

If I run (M-:) (magit-status-internal default-directory) directly, I got the warning "Error (magit): Magit cannot find Git on ~~name of the remote server~~ ....."

Not intend to make it more confusing, However, if I put '(magit-git-executable "git") in (custom-set-variables...), the magit-status will work even without the patch.

Magit no longer works on the local Windows machine. It complains about not being able to find git.

Magit works on local Windows as I have set PATH point to git.
Just for note, actually I set the PATH point to PortableGit/cmd, and magit will add PortableGitmingw64\libexec\git-core etc. to the PATH. From the manual, I understand it is for the performance.

ShuguangSun · Jan 29, 2018

You then fix that using (setq magit-git-executable "git"). Now,
Magit works on the remote Linux machine.

Magit doesn't work on the remote Linux Machine without the patch. It raises the message "Git repository: default path", and after enter, it asks "Create repository in: default path".
If applied the patch, magit-status will work.

If I run (M-:) (magit-status-internal default-directory) directly, I got the warning "Error (magit): Magit cannot find Git on ~~name of the remote server~~ ....."

Not intend to make it more confusing, However, if I put '(magit-git-executable "git") in (custom-set-variables...), the magit-status will work even without the patch.

Magit no longer works on the local Windows machine. It complains about not being able to find git.

Magit works on local Windows as I have set PATH point to git.
Just for note, actually I set the PATH point to PortableGit/cmd, and magit will add PortableGitmingw64\libexec\git-core etc. to the PATH. From the manual, I understand it is for the performance.

The unexpected behaviour is likely due to this code in tramp-sh-handle-start-file-process:

	   ;; We use as environment the difference to toplevel
	   ;; `process-environment'.
	   env uenv
	   (env (dolist (elt (cons prompt process-environment) env)
                  (or (member elt (default-toplevel-value 'process-environment))
                      (if (string-match "=" elt)
                          (setq env (append env `(,elt)))
                        (if (tramp-get-env-with-u-option v)
                            (setq env (append `("-u" ,elt) env))
                          (setq uenv (cons elt uenv)))))))

Not intend to make it more confusing, However, if I put '(magit-git-executable "git") in (custom-set-variables...), the magit-status will work even without the patch.

The environment manipulation occurs inside the default value expression of magit-git-executable, so that quirk isn't too surprising. I'm realizing now that it's not a good place to put it though. The expression can be re-evaluated a lot (e.g., every time you do <f1> v magit-git-executable).

npostavs · Jan 29, 2018

The unexpected behaviour is likely due to this code in tramp-sh-handle-start-file-process:

	   ;; We use as environment the difference to toplevel
	   ;; `process-environment'.
	   env uenv
	   (env (dolist (elt (cons prompt process-environment) env)
                  (or (member elt (default-toplevel-value 'process-environment))
                      (if (string-match "=" elt)
                          (setq env (append env `(,elt)))
                        (if (tramp-get-env-with-u-option v)
                            (setq env (append `("-u" ,elt) env))
                          (setq uenv (cons elt uenv)))))))

Not intend to make it more confusing, However, if I put '(magit-git-executable "git") in (custom-set-variables...), the magit-status will work even without the patch.

The environment manipulation occurs inside the default value expression of magit-git-executable, so that quirk isn't too surprising. I'm realizing now that it's not a good place to put it though. The expression can be re-evaluated a lot (e.g., every time you do <f1> v magit-git-executable).

The unexpected behaviour is likely due to this code in tramp-sh-handle-start-file-process:

This indeed looks like it could lead to conflicts with our code, but I still don't understand the description of this issue and won't be able to reproduce. (Not using Windows obviously also doesn't help here.) So I cannot help here.

tarsius · Feb 2, 2018

The unexpected behaviour is likely due to this code in tramp-sh-handle-start-file-process:

This indeed looks like it could lead to conflicts with our code, but I still don't understand the description of this issue and won't be able to reproduce. (Not using Windows obviously also doesn't help here.) So I cannot help here.

If I understand the problem correctly, if you do something like mkdir ~/special-bin/; ln -s $(which git) ~/special-bin and then (push (substitute-env-vars "PATH=$HOME/special-bin/") magit-git-environment) you should be able to reproduce it on a GNU/Linux box accessing a repo with tramp (I don't have a tramp-accessible repo handy to verify this).

npostavs · Feb 2, 2018

If I understand the problem correctly, if you do something like mkdir ~/special-bin/; ln -s $(which git) ~/special-bin and then (push (substitute-env-vars "PATH=$HOME/special-bin/") magit-git-environment) you should be able to reproduce it on a GNU/Linux box accessing a repo with tramp (I don't have a tramp-accessible repo handy to verify this).

@tarsius

Hi @tarsius, I know what your think, and you are in a wrong way.

The right solution will be:

try to find git via (executable-find "git").
Note: executable-find already searching in %PATH%, why we need set magit-git-executable full path?
if git had been found, just (setq mgit-git-executable "git").
if git not be found, try to search default other paths in Windows, then set magit-git-executable to full path.

code in magit 2.13.0

(defcustom magit-git-executable
  ;; Git might be installed in a different location on a remote, so
  ;; it is better not to use the full path to the executable, except
  ;; on Window were we would otherwise end up using one one of the
  ;; wrappers "cmd/git.exe" or "cmd/git.cmd", which are much slower
  ;; than using "bin/git.exe" directly.
  (or (and (eq system-type 'windows-nt)
           (--when-let (executable-find "git")
....

junjiemars · Jul 4, 2018

Hi @tarsius, I know what your think, and you are in a wrong way.

The right solution will be:

try to find git via (executable-find "git").
Note: executable-find already searching in %PATH%, why we need set magit-git-executable full path?
if git had been found, just (setq mgit-git-executable "git").
if git not be found, try to search default other paths in Windows, then set magit-git-executable to full path.

code in magit 2.13.0

(defcustom magit-git-executable
  ;; Git might be installed in a different location on a remote, so
  ;; it is better not to use the full path to the executable, except
  ;; on Window were we would otherwise end up using one one of the
  ;; wrappers "cmd/git.exe" or "cmd/git.cmd", which are much slower
  ;; than using "bin/git.exe" directly.
  (or (and (eq system-type 'windows-nt)
           (--when-let (executable-find "git")
....

Note: executable-find already searching in %PATH%, why we need set magit-git-executable full path?

As explained in the comment, the reason we set magit-git-executable to the full path is because using plain "git" is slower.

if git not be found, try to search default other paths in Windows, then set magit-git-executable to full path.

What "default other paths" are there?

npostavs · Jul 4, 2018

Note: executable-find already searching in %PATH%, why we need set magit-git-executable full path?

As explained in the comment, the reason we set magit-git-executable to the full path is because using plain "git" is slower.

if git not be found, try to search default other paths in Windows, then set magit-git-executable to full path.

What "default other paths" are there?

Actually I think there might a better way, the basic idea being:

(let ((exec-suffixes (remove ".cmd" exec-suffixes)))
  (apply #'magit-call-process magit-git-executable ...))

tarsius · Jul 4, 2018

Actually I think there might a better way, the basic idea being:

(let ((exec-suffixes (remove ".cmd" exec-suffixes)))
  (apply #'magit-call-process magit-git-executable ...))

The git.cmd is no longer installed in current versions of Git for Windows. But the cmd/git.exe is still some kind of wrapper that is slower than the "real" one. See #1289 for some benchmarks.

npostavs · Jul 4, 2018

The git.cmd is no longer installed in current versions of Git for Windows. But the cmd/git.exe is still some kind of wrapper that is slower than the "real" one. See #1289 for some benchmarks.

Ah, then that won't work.

Is %PATH% like $PATH and we could just remove /path/to/cmd/ from it?

tarsius · Jul 4, 2018

Ah, then that won't work.

Is %PATH% like $PATH and we could just remove /path/to/cmd/ from it?

Is %PATH% like $PATH and we could just remove /path/to/cmd/ from it?

Yes, although /path/to/bin/ is not guaranteed to be there (the installer gives options of (1) don't change PATH, (2) add only cmd/ to PATH, or (3) add bin/ to PATH).

Hmm, also, rerunning my old benchmarks on a newer git, I'm not getting quite the same results:

git = c:/Git/2.14.0/cmd/git.exe
Elapsed time: 6.816000s
git = c:/Git/2.14.0/bin/git.exe
Elapsed time: 6.436000s
git = c:/Git/2.14.0/mingw64/libexec/git-core/git.exe
Elapsed time: 5.462000s

(I also notice the "Antimalware Service Executable" taking a lot of CPU while this is running, so changes in that could be influencing things too)

;; bench.el
(require 'benchmark)

(dolist (git-exe '("cmd/git.exe" "bin/git.exe" "mingw64/libexec/git-core/git.exe"))
  (let* ((git-dir "C:/Git/2.14.0/")
         (the-git (expand-file-name git-exe git-dir)))
    (message "git = %s" the-git)
    (benchmark 100 '(call-process the-git nil nil nil "status"))))

npostavs · Jul 4, 2018

Is %PATH% like $PATH and we could just remove /path/to/cmd/ from it?

Yes, although /path/to/bin/ is not guaranteed to be there (the installer gives options of (1) don't change PATH, (2) add only cmd/ to PATH, or (3) add bin/ to PATH).

Hmm, also, rerunning my old benchmarks on a newer git, I'm not getting quite the same results:

git = c:/Git/2.14.0/cmd/git.exe
Elapsed time: 6.816000s
git = c:/Git/2.14.0/bin/git.exe
Elapsed time: 6.436000s
git = c:/Git/2.14.0/mingw64/libexec/git-core/git.exe
Elapsed time: 5.462000s

(I also notice the "Antimalware Service Executable" taking a lot of CPU while this is running, so changes in that could be influencing things too)

;; bench.el
(require 'benchmark)

(dolist (git-exe '("cmd/git.exe" "bin/git.exe" "mingw64/libexec/git-core/git.exe"))
  (let* ((git-dir "C:/Git/2.14.0/")
         (the-git (expand-file-name git-exe git-dir)))
    (message "git = %s" the-git)
    (benchmark 100 '(call-process the-git nil nil nil "status"))))

the installer gives options

At least it doesn't give as many ways to misconfigure things as cygwin does. 😜

tarsius · Jul 4, 2018

the installer gives options

At least it doesn't give as many ways to misconfigure things as cygwin does. 😜

@npostavs

@npostavs I mean WE CANNOT DO that: guess user's settings.

Let's clear the case:

On Windows, if we using (executable-find "git") then we can't git remote repo via tramp.
We can not modify user's settings.
Do the right thing first, then think about performance issue.

I wrote a variant macro of executable-find which can find executable in %PATH% or $PATH.
basic.el

(defmacro executable-find% (command &optional prefer)
  "Search for COMMAND in `exec-path' and return the absolute file name 
at compile-time when PREFER is nil, same as `executable-find'.

Search for COMMAND in %PATH% or $PATH and return the absolute file name 
at compile-time when PREFER is non nil.

Return nil if no COMMAND found or no PREFER command found.
Return the first matched one, if multiple COMMANDs had been found
and `funcall' PREFER returns t.
"
  (if prefer
      (let ((ss (shell-command-to-string
		 (platform-supported-if windows-nt
		     (concat "where " command)
		   (concat "command -v " command)))))
	(when ss
	  (let* ((path (split-string* ss "\n" t))
		 (p (cond ((and (consp path) (functionp prefer))
			   (catch 'prefer
			     (dolist (x path)
			       (when (funcall prefer
					      (shell-quote-argument
					       (platform-supported-if windows-nt
						   (windows-nt-posix-path x)
						 x)))
				 (throw 'prefer x)))
			     nil))
			  ((consp path) (car path))
			  (t path))))
	    `,(when p (shell-quote-argument (platform-supported-if windows-nt
						(windows-nt-posix-path p)
					      p))))))
    (let ((path (executable-find command)))
      (ignore* prefer)
      `,path)))

And call it with use-magit-autoload.el

(platform-supported-when windows-nt
		
		(when (executable-find% "git" t)
			;; On Windows try to open remote git repo via sshx
			;; will trigger `magit' error: No such file or directory.
			;; GitHub issue: https://github.com/magit/magit/issues/3345
			(setq% magit-git-executable "git" magit)))

junjiemars · Jul 24, 2018

@npostavs I mean WE CANNOT DO that: guess user's settings.

Let's clear the case:

On Windows, if we using (executable-find "git") then we can't git remote repo via tramp.
We can not modify user's settings.
Do the right thing first, then think about performance issue.

I wrote a variant macro of executable-find which can find executable in %PATH% or $PATH.
basic.el

(defmacro executable-find% (command &optional prefer)
  "Search for COMMAND in `exec-path' and return the absolute file name 
at compile-time when PREFER is nil, same as `executable-find'.

Search for COMMAND in %PATH% or $PATH and return the absolute file name 
at compile-time when PREFER is non nil.

Return nil if no COMMAND found or no PREFER command found.
Return the first matched one, if multiple COMMANDs had been found
and `funcall' PREFER returns t.
"
  (if prefer
      (let ((ss (shell-command-to-string
		 (platform-supported-if windows-nt
		     (concat "where " command)
		   (concat "command -v " command)))))
	(when ss
	  (let* ((path (split-string* ss "\n" t))
		 (p (cond ((and (consp path) (functionp prefer))
			   (catch 'prefer
			     (dolist (x path)
			       (when (funcall prefer
					      (shell-quote-argument
					       (platform-supported-if windows-nt
						   (windows-nt-posix-path x)
						 x)))
				 (throw 'prefer x)))
			     nil))
			  ((consp path) (car path))
			  (t path))))
	    `,(when p (shell-quote-argument (platform-supported-if windows-nt
						(windows-nt-posix-path p)
					      p))))))
    (let ((path (executable-find command)))
      (ignore* prefer)
      `,path)))

And call it with use-magit-autoload.el

(platform-supported-when windows-nt
		
		(when (executable-find% "git" t)
			;; On Windows try to open remote git repo via sshx
			;; will trigger `magit' error: No such file or directory.
			;; GitHub issue: https://github.com/magit/magit/issues/3345
			(setq% magit-git-executable "git" magit)))

tarsius added the unconfirmed label Feb 2, 2018

npostavs referenced this issue Feb 5, 2018
Merged
Don't apply w32 env hacks for remote repositories #3365

npostavs closed this in #3365 Feb 11, 2018

nithyanandan referenced this issue May 30, 2018
Closed
Git repository not recognized over Tramp #3463

tarsius added the . label Jul 4, 2018

magit/magit

Join GitHub today

magit can't find "git" in Tramp from local PC to remote nix-like #3345

Comments

ShuguangSun commented Jan 26, 2018

This comment has been minimized.

npostavs commented Jan 26, 2018

This comment has been minimized.

articuluxe commented Jan 26, 2018

This comment has been minimized.

tarsius commented Jan 26, 2018

This comment has been minimized.

npostavs commented Jan 26, 2018

This comment has been minimized.

npostavs commented Jan 26, 2018

This comment has been minimized.

ShuguangSun commented Jan 26, 2018

This comment has been minimized.

npostavs commented Jan 26, 2018

This comment has been minimized.

ShuguangSun commented Jan 26, 2018 • edited

This comment has been minimized.

ShuguangSun commented Jan 27, 2018

This comment has been minimized.

npostavs commented Jan 27, 2018

This comment has been minimized.

ShuguangSun commented Jan 27, 2018 • edited

This comment has been minimized.

tarsius commented Jan 29, 2018 • edited

This comment has been minimized.

ShuguangSun commented Jan 29, 2018

This comment has been minimized.

npostavs commented Jan 29, 2018

This comment has been minimized.

tarsius commented Feb 2, 2018

tarsius added the unconfirmed label Feb 2, 2018

This comment has been minimized.

npostavs commented Feb 2, 2018

npostavs referenced this issue Feb 5, 2018

npostavs closed this in #3365 Feb 11, 2018

nithyanandan referenced this issue May 30, 2018

This comment has been minimized.

junjiemars commented Jul 4, 2018 • edited

This comment has been minimized.

npostavs commented Jul 4, 2018

This comment has been minimized.

tarsius commented Jul 4, 2018

tarsius added the . label Jul 4, 2018

This comment has been minimized.

npostavs commented Jul 4, 2018

This comment has been minimized.

tarsius commented Jul 4, 2018

This comment has been minimized.

npostavs commented Jul 4, 2018

This comment has been minimized.

tarsius commented Jul 4, 2018

This comment has been minimized.

junjiemars commented Jul 24, 2018

ShuguangSun commented Jan 26, 2018 •

edited

ShuguangSun commented Jan 27, 2018 •

edited

tarsius commented Jan 29, 2018 •

edited

junjiemars commented Jul 4, 2018 •

edited