git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.37k stars 2.54k forks source link

git commit -a sends wrong network drive path to editor... #4207

Closed martinfabian closed 1 year ago

martinfabian commented 1 year ago

Setup

$ git --version --build-options

git version 2.39.0.windows.2
cpu: x86_64
built from commit: e7d4c504802f93f5914ed48944ced4e593bcaf6c
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon

Using both portable and non-portable install. 
The problem occurs also with earlier versions of git
Windows 7, Windows 10, 64-bit

Default install options on all machines, tried both portable and non-portable versions of git.

Small network of W7 and W10 computers that share repo on network drive. The network drive is connected to a Netgear Nighthawk R8000 router, with ReadySHARE.

git commit -am "commit message" works as expected irrespective of whether the repo is on internal drive or network drive.

Details

git-cmd.exe

git commit -a

That Git opens the text editor with Repo\.git\COMMIT_EDITMSG, and waits until the file is edited, saved and the editor is closed. This works when the repo is on an internal drive, but not when the repo is on a (mapped) network drive.

git commit -am "commit message" works as expected whether the repo is on an internal or network drive (without opening any text editor, of course)

For a repo on the networked drive, Git sends to the text editor the path

\\readyshare\USB_Storage\Repo\readyshare\USB_Storage\Repo\.git\COMMIT_EDITMSG

which is wrong. The correct path would be \\readyshare\USB_Storage\Repo\.git\COMMIT_EDITMSG or equivalently using the drive letter mapped Windows path Z:\Repo\.git\COMMIT_EDITMSG

The issue occurs with all repos on the network drive. Tested on brand new repo after git init. Repos on internal drives work fine.

rimrul commented 1 year ago

How do you connect the network drive?

martinfabian commented 1 year ago

The network drive is connected to a Netgear Nighthawk R8000 router, with ReadySHARE. It is really a USB stick plugged into one of the router's USB3 ports.

rimrul commented 1 year ago

Yes, that part is clear, but how do you mount it in Windows? NET USE? SUBST? Group policy? The ribbon menu in Windows Explorer?

tboegi commented 1 year ago

What is the current directory, when you enter the git command ? What does pwd give you ?

martinfabian commented 1 year ago

Aha, sorry. I just do "Map network drive" from Win Explorer. I right-click the drive under NETWORK > READYSHARE, and choose Map network drive. Nothing fancier than that, just default settings.

MapNetworkDrive

Current directory when I issue the git commit -a command is Z:\Repo

tboegi commented 1 year ago

Z:\Repo is a DOS path. Are you you using "git bash", or how is git invoked ?

rimrul commented 1 year ago

Z:\Repo is a DOS path. Are you you using "git bash", or how is git invoked ?

Git CMD

PhilipOakley commented 1 year ago

Also, how do you invoke the core editor? Probably in your .gitconfig global file.

[core]
    editor = ?????
martinfabian commented 1 year ago

Also, how do you invoke the core editor? Probably in your .gitconfig global file.

[core]
  editor = ?????

core.editor='C:\Program Files (x86)\TextPad 6\TextPad.exe' -m

I am not sure that this is related to the issue, though, since git commit -a invokes the editor just fine when the repo is on an internal drive. The editor then gets, for instance, D:\Repo\.git\COMMIT_EDITMSG to edit and it all works as expected.

PhilipOakley commented 1 year ago

git commit -a invokes the editor just fine when the repo is on an internal drive.

Thanks. It was just clarifying if there was potentially something occurring via that route.

The other thing to check is your HOMEDRIVE, HOMEPATH, and HOME environment variables which are combined to create paths at various stages and have been the source of difficulties in the past.

The other issue is often the MSYS shim that has the difficult task of converting Git's posix paths to the Windows paths, and vice versa. The repeated readyshare\USB_Storage\Repo string in your bug report suggests that it may be a HOMEDRIVE\HOMEPATH concatenation problem.

martinfabian commented 1 year ago
HOME=C:\Users\fabian
HOMEDRIVE=C:
HOMEPATH=\Users\fabian

I can find no mention whatsoever of "readyshare" in any of the environment variables.

PhilipOakley commented 1 year ago
HOME=C:\Users\fabian
HOMEDRIVE=C:
HOMEPATH=\Users\fabian

I can find no mention whatsoever of "readyshare" in any of the environment variables.

Thanks. At least that closes of that route.

Aside: Have you tried alternate core.editors to see if the error reports are any more informative (Notepad++ being a common alternative). It should help work out which side of the interface the path construction error is happening on.

There's also the pwd -W in (git for windows) bash to see what it thinks is the windows path.

martinfabian commented 1 year ago

Oops! Setting the editor to notepad++ works! Notepad++ opens the correct file

\\READYSHARE\USB_Storage\Repo\.git\COMMIT_EDITMSG

And setting the editor back to TextPad again fails, so the issue is not git, but TextPad. Sorry about all of this. Thanks for helping out, and I apologize for wasting your time. I'm closing this. // MVHMF

tboegi commented 1 year ago

@martinfabian : I don't think that you wasted somebodies time here. And it is good that you found a workaround.

A probably somewhat incomplete analyzes: TextPad does not handle an UNC path for a file name (The one starting with \, e.g. \READYSHARE\USB_Storage\Repo.git\COMMIT_EDITMSG

Git, on the other hand, converts the DOS path Z:\Repo.git\COMMIT_EDITMSG into the UNC version (and this feature was new to me). There seems to be a relation to the commit below, which was introduced to fix another problem. @dscho Should we avoid translating network paths ?

`commit 0b4077419eaa7a89e094fa9044cba800a83e6c68 Author: Johannes Schindelin Johannes.Schindelin@gmx.de Date: Fri Jan 31 11:49:04 2020 +0100

mingw: implement a platform-specific strbuf_realpath()

There is a Win32 API function to resolve symbolic links, and we can use that instead of resolving them manually. Even better, this function also resolves NTFS junction points (which are somewhat similar to bind mounts).

This fixes https://github.com/git-for-windows/git/issues/2481 `

dscho commented 1 year ago

@tboegi if you have an idea how to do that without reintroducing the bug that was fixed by said commit, I'd like to hear it.

tboegi commented 1 year ago

@dscho For the basic idea, please see below. Fully untested, just an idea. The questions to be answered is if we need code to support the "last_component" case as well ? If this sounds like a way forward, I may do a patch of it.

index af260f67d4..622b6a434f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1437,7 +1437,17 @@ char *mingw_strbuf_realpath(struct strbuf *resolved, const char *path)
                strbuf_addch(resolved, '/');
                strbuf_addstr(resolved, last_component);
        }
-
+       if (has_dos_drive_prefix(path) &&
+           is_dir_sep(resolved->buf[0]) && is_dir_sep(resolved->buf[1])) {
+               /*
+                  A path with a drive letter was converted into UNC
+                  "Z:\myrepo" becomes "\\mynas\myshare\myrepo"
+                  Revert this change since not all programs are prepared
+                  to handle UNC path
+               */
+               strbuf_reset(resolved);
+               strbuf_addstr(resolved, path);
+       }
        return resolved->buf;