magit use hard-coded mount prefix

[PierreTechoueyres]

if an user customised the mount prefix in his cygwin / MINGW install, he is unable to use last magit version.

In function magit-expand-git-file-name (lisp/magit-git.el) the moun prefix is hard coded with cygdrive.

here is an attempt to fix this (this work for me with /mnt prefix) :
Sorry I was unable to attach it with a file.

[PATCH] * lisp/magit-git.el magit-cygwin-prefix
 magit-expand-git-file-name

Use `magit-cygwin-prefix' in `magit-expand-git-file-name' instead of
hard-coded cygdrive prefix.

---
 lisp/magit-git.el | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/magit-git.el b/lisp/magit-git.el
index 22327f6..add2739 100644
--- a/lisp/magit-git.el
+++ b/lisp/magit-git.el
@@ -509,11 +509,20 @@ range.  Otherwise, it can be any revision or range accepted by
         (setq pos (point)))
       status)))

+(defconst magit-cygwin-prefix
+  (with-temp-buffer
+    (call-process-shell-command "mount -p | tail -1 | cut -d ' ' -f1" nil t)
+    (forward-line -1)
+    (setq cygwin-prefix (buffer-substring-no-properties
+                         (+ 1 (point)) (line-end-position)))))
+
 (defun magit-expand-git-file-name (filename)
   (unless (file-name-absolute-p filename)
     (setq filename (expand-file-name filename)))
   (if (and (eq system-type 'windows-nt) ; together with cygwin git, see #1318
-           (string-match "^/\\(cygdrive/\\)?\\([a-z]\\)/\\(.*\\)" filename))
+           (string-match
+            (concat "^/\\(" magit-cygwin-prefix "/\\)?\\([a-z]\\)/\\(.*\\)")
+            filename))
       (concat (match-string 2 filename) ":/"
               (match-string 3 filename))
     filename))
-- 
2.5.2

[tarsius]

Please look in https://cygwin.com/cygwin-ug-net.html for a command which can more reliably return the prefix.

[PierreTechoueyres]

Sorry I don't understand what you expect.
The -p switch of mount return the prefix. This switch work with cygwin, MINGW, MSYS2.
May be add a test to check if system-type eq windows-nt before the call for building magit-cygwin-prefix ?

[tarsius]

Sorry I don't understand what you expect.

Something like cygwin-query-command --show-the-prefix.

Show me the output of mount. Can we really be sure that | tail -1 | cut -d ' ' -f1 extracts the right information for everyone?

[PierreTechoueyres]

I guess yes

 mount --help                                                                                                                 ~
Usage: mount [OPTION] [<win32path> <posixpath>]
       mount -a
       mount <posixpath>

Display information about mounted filesystems, or mount a filesystem

  -a, --all                     mount all filesystems mentioned in fstab
  -c, --change-cygdrive-prefix  change the cygdrive path prefix to <posixpath>
  -f, --force                   force mount, don't warn about missing mount
                                point directories
  -h, --help                    output usage information and exit
  -m, --mount-entries           write fstab entries to replicate mount points
                                and cygdrive prefixes
  -o, --options X[,X...]        specify mount options
  -p, --show-cygdrive-prefix    show user and/or system cygdrive path prefix
  -V, --version                 output version information and exit

Valid options are: acl,auto,binary,bind,cygexec,dos,exec,ihash,noacl,nosuid,notexec,nouser,override,posix=0,posix=1,sparse,text,user

and

mount -p                                                                                                                     ~
Prefix              Type         Flags
/mnt                user         binmode

so

 mount -p | tail -1 | cut -d ' ' -f 1                                                                                         ~
/mnt

obviously the tail and cut can be achieved in elisp with something like

(forward-line -1)
(buffer-substring-no-properties (+ 1 (point)) (line-end-position))

[tarsius]

Or just mount --show-cygdrive-prefix (again, show me the output).

[PierreTechoueyres]

Same as with -p (short switch vs long switch)

$> mount --show-cygdrive-prefix
Prefix              Type         Flags
/mnt                user         binmode

[tarsius]

Ups sorry, though that would just print "the prefix". But actually there might be no "one and only prefix", and that's what I was getting at all along.

[npostavs]

But actually there might be no "one and only prefix"

Indeed, cygwin-mount.el stores a list of prefixes and tries to use the longest match. EDIT: this is a list mount points which isn't the same.

As I said in #2319, I think we should try to use cygwin-mount.el instead of reimplementing it.

[tarsius]

If we do that, then we should also try to locate the latest version and get someone to maintain it in a git repository.

[PierreTechoueyres]

I'm not convinced it's possible to have multiple prefix at the same time.
As stated in https://cygwin.com/cygwin-ug-net/using.html#cygdrive it look you can have only one prefix line in fstab file.
And even in http://emacswiki.org/emacs/cygwin-mount.el the function cygwin-mount-get-cygdrive-prefix return a string not a list.

here is the doc of the function (line 209) :

Return prefix used for the "/cygdrive/X/" style of cygwin.
This is done by calling "mount --show-cygdrive-prefixes".
The result is either "/" or "/<string>/".

I think it"s possible to just get the prefix without using the full machinery of cygwin-mount.
But that's remain your decision.

I agree the constant must be created more carefully by adding a check on system-type and maybe on the presence of cygwin / msys etc.

[tarsius]

I think it"s possible to just get the prefix without using the full machinery of cygwin-mount.

I would prefer that too.

[PierreTechoueyres]

I reviewed my patch and I should really add checks for building magit-cygwin-prefix.
Actually this work by accident ... sorry
I'll try to post something soon.

[npostavs]

As stated in https://cygwin.com/cygwin-ug-net/using.html#cygdrive it look you can have only one prefix line in fstab file.

I think I got confused between mount points (cygwin-mount-build-table-internal) and cygdrive-prefix.

[PierreTechoueyres]

What do you think of this version ?

(defconst magit-cygwin-prefix
  (let ((magit-cygwin-prefix "/cygdrive"))
    ;; then try to retrive the real one.
    (when (eq system-type 'windows-nt)
      (with-temp-buffer
    (call-process-shell-command "mount --show-cygdrive-prefixe | tail -1 | cut -d ' ' -f1" nil '(t nil))
    (forward-line -1)
    (setq magit-cygwin-prefix
          (buffer-substring-no-properties (point) (line-end-position)))
    (if (string= magit-cygwin-prefix "")
        (setq magit-cygwin-prefix nil)
      magit-cygwin-prefix)))
    magit-cygwin-prefix)
  "Store prefix used for the \"/cygdrive/X/\" style of cygwin.
This is done by calling \"mount --show-cygdrive-prefixes\".
The result is either \"/\", \"/<string>\" or nil if not in `windows-nt' system.")

[johnmastro]

Here's a slightly cleaned up version (IMHO). Most importantly, it should be --show-cygdrive-prefix (no "e" at the end).

It does correctly detect the prefix of / on my system.

(defconst magit-cygwin-prefix
  (when (eq system-type 'windows-nt)
    (with-temp-buffer
      (call-process-shell-command
       "mount --show-cygdrive-prefix | tail -1 | cut -d ' ' -f1"
       nil '(t nil))
      (forward-line -1)
      (let ((str (buffer-substring-no-properties (point) (line-end-position))))
        (unless (string= str "")
          str))))
  "Prefix used for \"/cygdrive/X/\" style Cygwin paths.
The result is either \"/\", \"/<string>\" or nil if not on a
`windows-nt' system.")

[tarsius]

I think this should be an option and use process-lines.

[johnmastro]

Indeed, it comes out better with process-lines. How about something like this? (@PierreTechoueyres, I hope I'm not stepping on your toes here).

Edit: I like this version better

(defcustom magit-cygwin-prefix
  (when (eq system-type 'windows-nt)
    (ignore-errors
      (-> (process-lines "mount" "--show-cygdrive-prefix")
          cadr
          split-string
          car)))
  "Prefix used for \"/cygdrive/X/\" style Cygwin paths.
The result is either \"/\", \"/<string>\" or nil if not on a
`windows-nt' system."
  :package-version '(magit . "2.3.0")
  :group 'magit-process
  :type '(choice (const :tag "None" nil)
                 string))

Original:

(defcustom magit-cygwin-prefix
  (when (eq system-type 'windows-nt)
    (ignore-errors
      (--when-let (process-lines "mount" "--show-cygdrive-prefix")
        (car (split-string (cadr it))))))
  "Prefix used for \"/cygdrive/X/\" style Cygwin paths.
The result is either \"/\", \"/<string>\" or nil if not on a
`windows-nt' system."
  :package-version '(magit . "2.3.0")
  :group 'magit-process
  :type '(choice (const :tag "None" nil)
                 string))

[PierreTechoueyres]

No, I'm at home now and haven't access to an Windows machine.
I will try that tomorrow.

[PierreTechoueyres]

@johnmastro: I prefer the second. It has less dashims. I don't mind.
@tarsius: should I make a patch ? If so which version of the defcustom should I use ?

[tarsius]

I also prefer "the original". Yes, please open a pr. But I'll leave it to @npostavs to decide whether it should be merged.

[tarsius]

Or rather just update the pr you already opened.

[npostavs]

But I'll leave it to @npostavs to decide whether it should be merged.

To clarify, I am inclined to merge this; after looking at cygwin-mount.el some more I'm less eager to recommend using it (it's rather invasive) and the code to calculate the cygdrive prefix looks simple enough.

---

Only snag is, I think we need the cygdrive prefix in git-commit-setup as well. Otherwise committing still won't work. (@PierreTechoueyres have you tested committing yet?)

[PierreTechoueyres]

@npostavs Not tested yet. But if both git-commit and magit need to be changed, where should I put the definition of magit-cygdrive-prefix ?
Should I duplicate the prefix var with two different names (git-commit-cygwin-prefix and mangit-cygwin-prefx) ?

[PierreTechoueyres]

@npostavs I've used commit from from magit to correct my repo with the use of the defcustom and everything has worked without any modifications to git-commit-setup.
Doesn't magit use git-commit ?

[npostavs]

I've used commit from from magit to correct my repo with the use of the defcustom and everything has worked without any modifications to git-commit-setup.

Huh. I don't quite understand, but good news I guess.

Doesn't magit use git-commit ?

Essentially magit tells git to call emacsclient on the COMMIT_EDITMSG file at which point git-commit-mode is triggered. I thought with cygwin git would send emacsclient a cygwin path using your custom prefix, but apparently not...

[PierreTechoueyres]

Sorry, I'm not really fluent in English.
I've commited the defcustom declaration with magit-status buffer.
Commit happened without problem and I have not modified git-commit-setup.

[npostavs]

Commit happened without problem and I have not modified git-commit-setup.

Okay, please update #2319 (or open new pull request if you can't, I see it is saying unknown repository so perhaps it is broken)

---

Also, please post the output when running GIT_EDITOR='x(){ printf "[%s]\n" "$@"; false; }; x' git commit 2>/dev/null from the cygwin command line. Here is mine (not using a custom mount prefix):

$ GIT_EDITOR='x(){ printf "[%s]\n" "$@"; false; }; x' git commit 2>/dev/null
[/cygdrive/c/home/npostavs/src/magit/.git/COMMIT_EDITMSG]

[PierreTechoueyres]

I think I'll have to open another pull request. I've destroyed the previous repository.
Unless you could give me an advice on how to add the new patch to the previous PR.

[npostavs]

Okay, just open a new one then.

[PierreTechoueyres]

Here is the output of git commit

➜ ❯ GIT_EDITOR='x(){ printf "[%s]\n" "$@"; false; }; x' git commit -a 2>/dev/null
[/home/frup59827/projets/emacs-magit-pte/.git/COMMIT_EDITMSG]

And it's work because my /home is mounted on c:\home

I've created PR #2324

[npostavs]

And it's work because my /home is mounted on c:\home

Hmm, then it won't work if someone chooses a trickier mapping. Well, I guess we can wait until someone complains about it.

[tarsius]

Well, I guess we can wait until someone complains about it.

Please address the issue proactively.

[npostavs]

Hmm problem is the cygdrive prefix is actually not sufficient, it's only a fallback.

https://cygwin.com/cygwin-ug-net/using.html#mount-table

Whenever Cygwin generates a Win32 path from a POSIX one, it uses the longest matching prefix in the mount table. Thus, if C: is mounted as /c and also as /, then Cygwin would translate C:/foo/bar to /c/foo/bar. This translation is normally only used when trying to derive the POSIX equivalent current directory. Otherwise, the handling of MS-DOS filenames bypasses the mount table.

http://www.cygwin.com/cygwin-ug-net/mount.html

Whenever Cygwin cannot use any of the existing mounts to convert from a particular Win32 path to a POSIX one, Cygwin will, instead, convert to a POSIX path using a default mount point: /cygdrive. For example, if Cygwin accesses z:\foo and the z drive is not currently in the mount table, then z:\ will be accessible as /cygdrive/z

Example: I map C:\Users to /home, now I get

~/src/magit$ grep -Ev '(^#)|(^ *$)' /etc/fstab
none /cygdrive cygdrive binary,posix=0,user 0 0
C:/Users/ /home ntfs binary,posix=0,user 0 0
~/src/magit$ mount --show-cygdrive-prefix
Prefix              Type         Flags
/cygdrive           user         binmode
~/src/magit$ git rev-parse --show-toplevel
/home/npostavs/src/magit
~/src/magit$ GIT_EDITOR='x(){ printf "[%s]\n" "$@"; false; }; x' git commit --allow-empty 2>/dev/null
[/home/npostavs/src/magit/.git/COMMIT_EDITMSG]

To translate paths like /home/npostavs/src/magit to C:/Users/npostavs/src/magit we need to parse all the mount points:

~/src/magit$ mount
C:/cygwin/bin on /usr/bin type ntfs (binary,auto)
C:/cygwin/lib on /usr/lib type ntfs (binary,auto)
C:/cygwin on / type ntfs (binary,auto)
C:/Users on /home type ntfs (binary,posix=0,user)
C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto)
D: on /cygdrive/d type ntfs (binary,posix=0,user,noumount,auto)

[PierreTechoueyres]

Did you think we could use df instead of mount to retrieve informations ?

➜ ❯ df --output=target,source                                                                                                    ~
Monté sur Sys. de fichiers
/home     D:/home
/         C:/Programmes/msys64
/mnt/e    E:
/mnt/i    I:
/mnt/l    L:
/mnt/m    M:
/mnt/n    N:
/mnt/s    S:
/mnt/t    T:
/mnt/x    X:
/mnt/z    Z:

mount's output seems more difficult to parse

➜ ❯ mount                                                                                                                        ~
D:/home on /home type ntfs (binary,noacl,user)
D:/Travail on /travail type ntfs (binary,noacl,user)
C:/Programmes/msys64 on / type ntfs (binary,noacl,auto)
C:/Programmes/msys64/usr/bin on /bin type ntfs (binary,noacl,auto)
C: on /mnt/c type ntfs (binary,noacl,posix=0,user,noumount,auto)
D: on /mnt/d type ntfs (binary,noacl,posix=0,user,noumount,auto)
E: on /mnt/e type ntfs (binary,noacl,posix=0,user,noumount,auto)
I: on /mnt/i type vfat (binary,noacl,posix=0,user,noumount,auto)
L: on /mnt/l type smbfs (binary,noacl,posix=0,user,noumount,auto)
M: on /mnt/m type smbfs (binary,noacl,posix=0,user,noumount,auto)
N: on /mnt/n type smbfs (binary,noacl,posix=0,user,noumount,auto)
S: on /mnt/s type smbfs (binary,noacl,posix=0,user,noumount,auto)
T: on /mnt/t type smbfs (binary,noacl,posix=0,user,noumount,auto)
X: on /mnt/x type smbfs (binary,noacl,posix=0,user,noumount,auto)
Z: on /mnt/z type smbfs (binary,noacl,posix=0,user,noumount,auto)

[npostavs]

Did you think we could use df instead of mount to retrieve informations ?

Unfortunately no: df only lists drives, so it doesn't cover most of the mappings. Still with the same C:\Users to /home mapping I get only

~$ df --output=target,source
Mounted on  Filesystem
/           C:/cygwin
/cygdrive/d D:

mount's output seems more difficult to parse

I think matching against "^\\(.*\\) on \\(.*\\) type" should work, it even handles the nasty C:/Documents\040and\040Settings /docs ext3 binary 0 0 example from the user guide (the spaces have to be backslash-encoded in /etc/fstab, but they are not escaped in mount's output):

(let ((line "C:/Documents and Settings on /docs type ntfs (binary)"))
  (if (string-match "^\\(.*\\) on \\(.*\\) type" line)
      (cons (match-string 1 line) (match-string 2 line))
    (display-warning '(magit) (format "Failed to parse Cygwin mount: %S" line))))
    ;; => ("C:/Documents and Settings" . "/docs")

[PierreTechoueyres]

Isn't the output of mount -m more easy to parse ?

➜ ❯ mount -m                                                                                                                     ~
D:/home /home ntfs binary,noacl,user 0 0
D:/Travail /travail ntfs binary,noacl,user 0 0
C:/Users/myUser/Documents/Bluetooth\040Folder /documents\040bidons ntfs binary,noacl,user 0 0
C:/Users/myUser/Documents/Bluetooth\040Folder /documents ntfs binary,noacl,user 0 0
none /mnt cygdrive binary,noacl,posix=0,user 0 0

my fstab is

➜ ❯ cat /etc/fstab                                                                                                               ~
# For a description of the file format, see the Users Guide
# http://cygwin.com/cygwin-ug-net/using.html#mount-table

# DO NOT REMOVE NEXT LINE. It remove cygdrive prefix from path
# none                       /cygdrive cygdrive binary,noacl,posix=0,user 0 0
none                         /mnt               cygdrive binary,noacl,posix=0,user 0 0
# C:/Programmes/msys64/usr/lib /lib     ntfs    binary,noacl,user 0 0
D:/home                      /home      ntfs    binary,noacl,user 0 0
D:/Travail                   /travail   ntfs    binary,noacl,user 0 0
C:/Users/myUser/Documents/Bluetooth\040Folder /documents ntfs        binary,noacl,user 0 0
C:/Users/myUser/Documents/Bluetooth\040Folder /documents/documents\040bidons ntfs    binary,noacl,user 0 0

Another bonus of mount -m is that the output is ordered with the longest mount point first.
So with the following elisp code I got an list of cons

(let ((mount-points (ignore-errors (process-lines "mount" "-m"))))
  (mapcar #'(lambda (l)
          (let ((entry (split-string l)))
        (cons (replace-regexp-in-string "\\\\040" " " (cadr entry))
              (replace-regexp-in-string "none" ""
               (replace-regexp-in-string "\\\\040" " " (car entry))))))
      mount-points))
;;=>
(("/home" . "D:/home") ("/travail" . "D:/Travail")
 ("/documents/documents bidons" . "C:/Users/myUser/Documents/Bluetooth Folder")
 ("/documents" . "C:/Users/myUser/Documents/Bluetooth Folder")
 ("/mnt" . ""))

What did you think ?

[npostavs]

Isn't the output of mount -m more easy to parse ?

Ah, didn't notice that option.

The drawback of the mount -m output is you will need to treat the cygdrive prefix specially. The plain mount output lists the all the drives under the cygdrive prefix as normal mount points. I think that will make using the plain mount output easier, but go with whichever seems better to you.