Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Magit on windows no longer works with Cygwin Git #1318

Closed
rneatherway opened this issue Apr 2, 2014 · 41 comments
Closed

Magit on windows no longer works with Cygwin Git #1318

rneatherway opened this issue Apr 2, 2014 · 41 comments
Labels
bug windows Something isn't working on Windows
Milestone

Comments

@rneatherway
Copy link

I've used it quite successfully in this configuration for a year or so. Perhaps I was lucky and this support wasn't intended, but it would be nice if it continued to work. MSysGit doesn't understand my file permissions properly among other issues.

The problem seems to be in magit-get-top-dir when it shells out to git rev-parse --show-toplevel: https://github.com/magit/magit/blob/master/magit.el#L2058

On my machine the default-directory variable in that function (which doesn't seem to be used anymore?) is correct, something like d:/cygwin/home/user/repo, but git rev-parse --show-toplevel then returns /home/user/repo which gets expanded back out to d:/home/user/repo. This later triggers the "%s isn't an existing directory" error, so I can't use magit-status.

@tarsius tarsius added this to the support/later/needinfo/... milestone Apr 2, 2014
@tarsius
Copy link
Member

tarsius commented Apr 2, 2014

Could someone with Windows experience give me a hand here, please? To my inexperienced ears it sounds like this is a Git, not a Magit problem; and that there is some confusion about Cygwin vs. MSysGit going on (and it could very well be me who is confused :-).

@rneatherway
Copy link
Author

Just to clarify, magit no longer works with Cygwin Git, but it does work with MSysGit. I previously used magit successfully with Cygwin Git for a long time.

I believe the problem is that magit is using the output of git rev-parse --show-toplevel, which from Cygwin Git will output a path starting with "/".

@Silex
Copy link
Contributor

Silex commented Apr 2, 2014

IIRC you have to use msysgit because cygwin assumes you use it inside the cygwin shell, and thus cygwin's git returns cygwin paths which of course non-cygwin emacs cannot understand.

Running emacs inside cygwin should work tho.

[EDIT] just reading about your update, I think you are right but I'm not sure it's really a problem for magit to solve. Magit should be able to rely on files returned by git's output IMHO.

@tarsius
Copy link
Member

tarsius commented Apr 2, 2014

I agree with @Silex, I am afraid we cannot do anything about this issue.

The reason this used to work is that we previously did not use rev-parse --some-path-flag to get the paths to various places.

@Silex
Copy link
Contributor

Silex commented Apr 2, 2014

@rneatherway: can we know why you want to use cygwin's git outside of cygwin? almost all of cygwin's tool behave badly when used outside of cygwin because of these paths issues. Also, as far as I remember msysgit was much better.

@rneatherway
Copy link
Author

I think that other paths returned by git are typically relative, which is another reason why this didn't turn up before.

I agree magit should be able to rely on git's output. It's just a real shame because MSysGit is inferior in not understanding file permissions. Magit is the best git interface I've seen, so I'll continue using it in any case.

I'm using cygwin bash and ntemacs because I'm not interested in running an X server just to run emacs. I'm currently trying msysgit, but I can't see any advantages.

@rneatherway
Copy link
Author

It seems that there is a version of emacs in cygwin, emacs-w32, that doesn't require an X server. I wasn't aware of this before -- I will give it a try. Thanks for the responses.

@tarsius tarsius modified the milestones: 2.1.0, support/later/needinfo/... Apr 2, 2014
@hdhoang
Copy link

hdhoang commented Aug 5, 2014

You can replace --show-toplevel with --show-cdup as it was in magit 1.2, and cygwin/msys2 git works

@tarsius
Copy link
Member

tarsius commented Sep 4, 2014

@hdhoang What are you trying to say here? We use --show-toplevel instead of --show-cdup for a reason - they do different things. So if "you" refers to the maintainers then; no we won't do that. If it refers to users then; please don't do that, there will be bugs.

@kensanata
Copy link

This is what I am using:

(defadvice magit-expand-git-file-name
  (before magit-expand-git-file-name-cygwin activate)
  "Handle Cygwin directory names such as /cygdrive/c/*
by changing them to C:/*"
  (when (string-match "^/cygdrive/\\([a-z]\\)/\\(.*\\)" filename)
(setq filename (concat (match-string 1 filename) ":/"
               (match-string 2 filename)))))

@poldy
Copy link

poldy commented Jan 23, 2015

I can confirm the workaround from @kensanata. I modified it slightly to work with msys git and use the new nadvice.el package in Emacs 24.4 if anyone's interested:

(defun magit-expand-git-file-name--msys (args)
  "Handle Msys directory names such as /c/* by changing them to C:/*"
  (let ((filename (car args)))
        (when (string-match "^/\\([a-z]\\)/\\(.*\\)" filename)
          (setq filename (concat (match-string 1 filename) ":/"
                                 (match-string 2 filename))))
        (list filename)))
(advice-add 'magit-expand-git-file-name :filter-args #'magit-expand-git-file-name--msys)

@PeterMosmans
Copy link

Thanks @poldy and @kensanata for the workaround. Confirmed working with msys2 and the latest (master branch, 25.0.50.1) build of Emacs on Windows.

@tarsius
Copy link
Member

tarsius commented Apr 29, 2015

See http://unix.stackexchange.com/questions/44677/how-do-i-get-rid-of-cygwins-cygdrive-prefix-in-all-paths. I think that question sums up the problem pretty well :-)

Then check whether the accepted answer fixes the issue.

@tarsius
Copy link
Member

tarsius commented Apr 29, 2015

Also please test whether the version from the next branch works without using an advice or editing fstab.

@tarsius
Copy link
Member

tarsius commented Apr 29, 2015

Reopening. And closing the recent duplicate.

@tarsius tarsius reopened this Apr 29, 2015
@tarsius
Copy link
Member

tarsius commented Apr 29, 2015

Eli wrote (on g.e.help): "I believe cygwin-mount does it thing via expand-file-name, so I'm
guessing some call to expand-file-name is missing somehere in magit."

So please also try

diff --git a/magit.el b/magit.el
index b71948e..ad8a848 100644
--- a/magit.el
+++ b/magit.el
@@ -2092,10 +2092,11 @@ (defun magit-get-top-dir (&optional directory)

 (defun magit-expand-git-file-name (filename)
   (when (tramp-tramp-file-p default-directory)
-    (setq filename (file-relative-name filename
-                                       (with-parsed-tramp-file-name
-                                           default-directory nil
-                                         localname))))
+    (setq filename (file-relative-name
+                    (expand-file-name filename)
+                    (with-parsed-tramp-file-name
+                        (expand-file-name default-directory) nil
+                      localname))))
   (expand-file-name filename))

 (defun magit-file-relative-name (file)

Edit: This only affects tramp - so probably not.

@tarsius
Copy link
Member

tarsius commented Apr 29, 2015

But please try sprinkling expand-file-name in some place that seem relevant. I cannot perform these experiments myself because I don't use Windows. And if I did I would stay the hell away from cygwin - it seems to be a constant source of frustration.

@tarsius
Copy link
Member

tarsius commented May 5, 2015

I'm gonna assume the solution I linked to actually works. I have added a faq entry at https://github.com/magit/magit/wiki/FAQ#windows-there-is-no-git-repository-in-pathtolocalrepo. Please try it and then report back here.

@tarsius tarsius closed this as completed May 5, 2015
@PeterMosmans
Copy link

@tarsius , please reopen this issue, as the cygwin trick does not work (for me, at least).

tarsius added a commit that referenced this issue May 6, 2015
hopefully.

In `magit-expand-git-file-name' turn paths such as
- /cygdrive/c/the/repo
- /c/the/repo
into paths like
- /c:/the/repo

If it turns out that there are places where an absolute path is
requested from git, then we need to use this function there too.
But I think this function is currently used when appropriate.

This issue only exists, or so I assume, when using Cygwin Git with a
non-cygwin Emacs.  It this were the case, then it would be appropriate
to only use this kludge when the `system-type' is `windows-nt'.  But I
am not sure, so for now it is also used when it is `cygwin'.  In the
latter case it could be that Git returns `/c/the/repo' while Emacs
expects `/c:/the/repo'.

Re #1318.
@tarsius
Copy link
Member

tarsius commented May 6, 2015

I have just installed a work around based on the advices above and some assumption of my own. Does that do the trick for everyone?

@ilohmar
Copy link
Contributor

ilohmar commented May 7, 2015

Awesome! With that change, my favored combination "NTEmacs + cygwin tools (incl git)" seems to work flawlessly again (and fast, and OOTB), without any personal patches (that had accumulated for the master branch).

Just wanted to thank you: I know first-hand how hard it is to troubleshoot the win/cygwin/ntemacs idiosyncrasies sitting in front of a windows machine (not my preference..). Amazing job fixing this stuff relying on 2nd-hand comments! Magit just keeps getting better.

@pete-lee
Copy link

pete-lee commented May 8, 2015

It seems code was added to address this issue:

Magit on windows no longer works with Cygwin Git #1318

(defun magit-expand-git-file-name (filename &optional localname)
  (setq filename
        (if (file-name-absolute-p filename)
            (if localname
                filename
              (concat (file-remote-p default-directory) filename))
          (expand-file-name
           filename
           (and localname (file-remote-p default-directory 'localname)))))
  (if (and (memq system-type '(cygwin windows-nt)) ; #1318
           (string-match "^/\\(cygdrive/\\)?\\([a-z]\\)/\\(.*\\)" filename))
      (concat (match-string 2 filename) ":/"
              (match-string 3 filename))
    filename)
  )

I run console emacs within cygwin and the conversion from /cygdrive/ to :/ breaks magit-status.

Commenting out that if statement works for me.

If system-type is 'cygwin... then it shouldn't be converting. I believe it should be constrained to windows-nt. Conversion makes sense to do if running non-cygwin emacs + cygwin git... it doesn't make sense for cygwin emacs (at least the console version) + cygwin git

@tarsius
Copy link
Member

tarsius commented May 8, 2015

Makes sense. I have changed it as you suggested.

@benquike
Copy link

benquike commented Jun 6, 2015

I have solved this problem.

  1. downlow cygwin-mount.el(http://emacswiki.org/emacs/cygwin-mount.el) to somewhere
  2. download setup-cygwin.el(http://www.emacswiki.org/emacs/setup-cygwin.el) to somewhere
  3. in your init.el(or .emacs),add the following
(add-to-list 'load-path "where you placed cygwin-mount.el and setup-cygwin.el")
(require 'setup-cygwin)

See my setting

@tarsius
Copy link
Member

tarsius commented Jul 30, 2015

Maybe someone in here could help out with #2127?

@amitahire
Copy link

Just to reiterate - Solved the issue #2127 with @benquike's suggested setup.

@thorrr
Copy link

thorrr commented Sep 23, 2015

I solved the issue without using cygwin-mount.el which was somewhat buggy for me. Had to use kensanata / poldy's approach with the following additional hook for git-commit-mode:

(defun un-cygwin-buffer-file-name ()
  (when (string-match "^\\([a-z]\\):/cygdrive/\\([a-z]\\)/\\(.*\\)" buffer-file-name)
    ;; assertion:  filename should look like "c:/cygwin/c/Users..." i.e. the drive is repeated
    (if (equal (match-string 1 buffer-file-name) (match-string 2 buffer-file-name)) (progn
      (set-visited-file-name
            (concat (match-string 1 buffer-file-name) ":/"
                    (match-string 3 buffer-file-name)) 't)))))
(add-hook 'git-commit-mode-hook 'un-cygwin-buffer-file-name)

@nickstares

This comment has been minimized.

@PeterMosmans

This comment has been minimized.

@nickstares

This comment has been minimized.

@npostavs

This comment has been minimized.

@nickstares

This comment has been minimized.

@npostavs

This comment has been minimized.

@nickstares

This comment has been minimized.

@npostavs

This comment has been minimized.

@nickstares

This comment has been minimized.

@npostavs

This comment has been minimized.

@nickstares

This comment has been minimized.

@npostavs

This comment has been minimized.

@nickstares
Copy link

I'm running emacs-nox 25.3.1

@npostavs
Copy link
Contributor

Oh, you're using cygwin Emacs with w32 git. This is the opposite configuration from what this issue is about (w32 Emacs with cygwin git), please open a new issue. (Possibly some of the troubleshooting snippets from #2284 and #2714 could be relevant)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug windows Something isn't working on Windows
Projects
None yet
Development

No branches or pull requests