* Git commit path vs rebase path @ 2012-05-06 4:24 Steven Penny 2012-05-07 17:27 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Steven Penny @ 2012-05-06 4:24 UTC (permalink / raw) To: git I have noticed git commit uses this path .git/COMMIT_EDITMSG git rebase uses this path /home/Steven/jquery/.git/rebase-merge/git-rebase-todo So git commit is using a relative path while git rebase is using absolute path. This causes problem in Windows if your editor does not understand linux paths, e.g. notepad, Notepad2, Notepad++, etc. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-06 4:24 Git commit path vs rebase path Steven Penny @ 2012-05-07 17:27 ` Junio C Hamano 2012-05-08 6:22 ` Johannes Sixt 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-07 17:27 UTC (permalink / raw) To: Steven Penny; +Cc: git Steven Penny <svnpenn@gmail.com> writes: > I have noticed > > git commit uses this path > > .git/COMMIT_EDITMSG > > git rebase uses this path > > /home/Steven/jquery/.git/rebase-merge/git-rebase-todo > > So git commit is using a relative path while git rebase is using absolute path. > > This causes problem in Windows if your editor does not understand linux paths, > e.g. notepad, Notepad2, Notepad++, etc. Hrm, this is not limited to rebase, though. All shell scripted Porcelain command use git-sh-setup that gives GIT_DIR as the full path, primarily so that the implementation of the Porcelain can safely chdir around without having to worry about relative paths in GIT_DIR. Most of the time, the commands that use git-sh-setup do cd_to_toplevel very early. For these commands, unless you are using GIT_DIR from your own environment (i.e. where people set the environment point to a place totally unrelated to the working tree and/or the current directory), it might look nicer if GIT_DIR given were .git/rebase-merge/git-rebase-todo, but that will not be the real solution, as sometimes your editor _must_ deal with the full path anyway. So the issue is _not_ that the path is absolute, it is that the path is given as a wrong kind of absolute path. Which suggests that "$(cd "$GIT_DIR" && pwd)" must give a full path that is suitable for the platform, and your platform wants it to be something like "c:\home\steven\jquery\..."? I do not have any Windows environment to further my speculation, so I'll leave the rest to Windows experts who may be lurking on this list. The relevant code snippet in git-sh-setup.sh is this part. # Make sure we are in a valid repository of a vintage we understand, # if we require to be in a git repository. if test -z "$NONGIT_OK" then GIT_DIR=$(git rev-parse --git-dir) || exit if [ -z "$SUBDIRECTORY_OK" ] then test -z "$(git rev-parse --show-cdup)" || { exit=$? echo >&2 "You need to run this command from the toplevel of the working tree." exit $exit } fi test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { echo >&2 "Unable to determine absolute path of git directory" exit 1 } : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} fi ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-07 17:27 ` Junio C Hamano @ 2012-05-08 6:22 ` Johannes Sixt 2012-05-08 6:44 ` Steven Penny 0 siblings, 1 reply; 33+ messages in thread From: Johannes Sixt @ 2012-05-08 6:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: Steven Penny, git Am 5/7/2012 19:27, schrieb Junio C Hamano: > Steven Penny <svnpenn@gmail.com> writes: > >> I have noticed >> >> git commit uses this path >> >> .git/COMMIT_EDITMSG >> >> git rebase uses this path >> >> /home/Steven/jquery/.git/rebase-merge/git-rebase-todo >> >> So git commit is using a relative path while git rebase is using absolute path. >> >> This causes problem in Windows if your editor does not understand linux paths, >> e.g. notepad, Notepad2, Notepad++, etc. > > ... the issue is _not_ that the path is > absolute, it is that the path is given as a wrong kind of absolute path. > > Which suggests that "$(cd "$GIT_DIR" && pwd)" must give a full path that > is suitable for the platform, and your platform wants it to be something > like "c:\home\steven\jquery\..."? The problem should be mitigated by be39048a7 (git-sh-setup.sh: Add an pwd() function for MinGW), where the above now returns a Windows-style absolute path, albeit with forward slashes instead of the backslashes. I don't know what "/home/..." is in Steven's case, because it should look more like "/c/home/..." unless it is an MSYS mount point, but even then it should be reported as Windows-style path with the new pwd function. IOW, the problem should be fixed in the next release. -- Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 6:22 ` Johannes Sixt @ 2012-05-08 6:44 ` Steven Penny 2012-05-08 7:06 ` Johannes Sixt 0 siblings, 1 reply; 33+ messages in thread From: Steven Penny @ 2012-05-08 6:44 UTC (permalink / raw) To: Johannes Sixt; +Cc: Junio C Hamano, git Johannes Sixt wrote: > The problem should be mitigated by be39048a7 (git-sh-setup.sh: Add an > pwd() function for MinGW), where the above now returns a Windows-style > absolute path, albeit with forward slashes instead of the backslashes. http://github.com/git/git/commit/be390 This looks like a good solution for MinGW, but cygwin will not support that $ pwd -W bash: pwd: -W: invalid option pwd: usage: pwd [-LP] Cygwin uses cygpath $ cygpath -m /c/Windows/System32 C:/Windows/System32 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 6:44 ` Steven Penny @ 2012-05-08 7:06 ` Johannes Sixt 2012-05-08 7:11 ` Steven Penny 0 siblings, 1 reply; 33+ messages in thread From: Johannes Sixt @ 2012-05-08 7:06 UTC (permalink / raw) To: Steven Penny; +Cc: Junio C Hamano, git Am 5/8/2012 8:44, schrieb Steven Penny: > Johannes Sixt wrote: >> The problem should be mitigated by be39048a7 (git-sh-setup.sh: Add an >> pwd() function for MinGW), where the above now returns a Windows-style >> absolute path, albeit with forward slashes instead of the backslashes. > > http://github.com/git/git/commit/be390 > > This looks like a good solution for MinGW, but cygwin will not support that > > $ pwd -W > bash: pwd: -W: invalid option > pwd: usage: pwd [-LP] Are you saying that the new pwd function will also be used on Cygwin? That would be a bug. -- Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 7:06 ` Johannes Sixt @ 2012-05-08 7:11 ` Steven Penny 2012-05-08 17:02 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Steven Penny @ 2012-05-08 7:11 UTC (permalink / raw) To: Johannes Sixt; +Cc: Junio C Hamano, git Johannes Sixt wrote: > Are you saying that the new pwd function will also be used on Cygwin? That > would be a bug. The linked patch should fix the problem for _MinGW_ users. The problem will persist with _Cygwin_ users. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 7:11 ` Steven Penny @ 2012-05-08 17:02 ` Junio C Hamano 2012-05-08 17:25 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-08 17:02 UTC (permalink / raw) To: Steven Penny; +Cc: Johannes Sixt, git Steven Penny <svnpenn@gmail.com> writes: > Johannes Sixt wrote: >> Are you saying that the new pwd function will also be used on Cygwin? That >> would be a bug. > > The linked patch should fix the problem for _MinGW_ users. > > The problem will persist with _Cygwin_ users. What does a full pathname, fed as a parameter when invoking Windows native binaries like notepad, look like in a Cygwin environment? That is, if you are writing a bash script that is meant to run in a Cygwin environment, and if the script takes the name of a file in the current directory, but it needs to chdir around for its own reasons before spawning notepad on the file, i.e. #!/bin/bash file="$(pwd)/$1" ... cd ..some..where..else..you..have..no..control..over notepad "$file" what is the right incantation to replace `pwd` in 'file="$(pwd)/$1"' above? Whatever that is, using that instead of `pwd` in git-sh-setup.sh here: test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { would be the solution. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 17:02 ` Junio C Hamano @ 2012-05-08 17:25 ` Junio C Hamano 2012-05-08 22:47 ` Steven Penny 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-08 17:25 UTC (permalink / raw) To: Steven Penny; +Cc: Johannes Sixt, git Junio C Hamano <gitster@pobox.com> writes: > Steven Penny <svnpenn@gmail.com> writes: > >> Johannes Sixt wrote: >>> Are you saying that the new pwd function will also be used on Cygwin? That >>> would be a bug. >> >> The linked patch should fix the problem for _MinGW_ users. >> >> The problem will persist with _Cygwin_ users. > > What does a full pathname, fed as a parameter when invoking Windows native > binaries like notepad, look like in a Cygwin environment? That is, if you > are writing a bash script that is meant to run in a Cygwin environment, > and if the script takes the name of a file in the current directory, but > it needs to chdir around for its own reasons before spawning notepad on > the file, i.e. > > #!/bin/bash > file="$(pwd)/$1" > ... > cd ..some..where..else..you..have..no..control..over > notepad "$file" > > what is the right incantation to replace `pwd` in 'file="$(pwd)/$1"' > above? > > Whatever that is, using that instead of `pwd` in git-sh-setup.sh here: > > test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { > > would be the solution. Actually, the above is stated rather poorly. The path that ends up in $file must be usable by both Windows native and Cygwin programs, as the user may be using "vi" from Cygwin, or "notepad" like your example. If there is no way to formulate $file to be acceptable for both, I do not see any sane solution other than one side compromising for the rest, perhaps by having $ cat >notepad.sh <<\EOF file=$(cygpath -m "$1") && exec notepad.exe "$file" EOF on the end-user side, or something like that. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 17:25 ` Junio C Hamano @ 2012-05-08 22:47 ` Steven Penny 2012-05-09 21:54 ` Junio C Hamano 2012-05-10 18:10 ` Ramsay Jones 0 siblings, 2 replies; 33+ messages in thread From: Steven Penny @ 2012-05-08 22:47 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Sixt, git Junio C Hamano wrote: > Actually, the above is stated rather poorly. The path that ends up in > $file must be usable by both Windows native and Cygwin programs, as the > user may be using "vi" from Cygwin, or "notepad" like your example. Excellent point. I ran some test and this is what I came up with # VI /cygdrive/c/test/hello.sh # works C:\test\hello.sh # works C:/test/hello/sh # works # NOTEPAD /cygdrive/c/test/hello.sh # does not work C:\test\hello.sh # works C:/test/hello.sh # works so the best compromise would be "C:/test/hello.sh" which can be created with diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 7b3ae75..ba198d2 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -260,6 +260,11 @@ case $(uname -s) in return 1 } ;; +*CYGWIN*) + pwd () { + builtin cygpath -m + } + ;; *) is_absolute_path () { case "$1" in http://github.com/svnpenn/git/commit/692bc ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 22:47 ` Steven Penny @ 2012-05-09 21:54 ` Junio C Hamano 2012-05-09 23:14 ` Steven Penny 2012-05-10 18:10 ` Ramsay Jones 1 sibling, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-09 21:54 UTC (permalink / raw) To: Steven Penny; +Cc: Johannes Sixt, git Steven Penny <svnpenn@gmail.com> writes: > so the best compromise would be "C:/test/hello.sh" which can be created with > > diff --git a/git-sh-setup.sh b/git-sh-setup.sh > index 7b3ae75..ba198d2 100644 > --- a/git-sh-setup.sh > +++ b/git-sh-setup.sh > @@ -260,6 +260,11 @@ case $(uname -s) in > return 1 > } > ;; > +*CYGWIN*) > + pwd () { > + builtin cygpath -m > + } > + ;; What does "uname -s" say on cygwin exactly? Our existing system detection code I found was in the Makefile and it looks out "uname -o" output and compares it with "Cygwin". I am not suggesting to change this part of the code to use "uname -o" at all; I just want to double check that the above *CYGWIN* matches it. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-09 21:54 ` Junio C Hamano @ 2012-05-09 23:14 ` Steven Penny 0 siblings, 0 replies; 33+ messages in thread From: Steven Penny @ 2012-05-09 23:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Sixt, git Junio C Hamano wrote: > What does "uname -s" say on cygwin exactly? Our existing system detection > code I found was in the Makefile and it looks out "uname -o" output and > compares it with "Cygwin". This is what I get $ uname -s CYGWIN_NT-6.1-WOW64 $ uname -o Cygwin ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-08 22:47 ` Steven Penny 2012-05-09 21:54 ` Junio C Hamano @ 2012-05-10 18:10 ` Ramsay Jones 2012-05-11 4:35 ` Steven Penny 1 sibling, 1 reply; 33+ messages in thread From: Ramsay Jones @ 2012-05-10 18:10 UTC (permalink / raw) To: Steven Penny; +Cc: Junio C Hamano, Johannes Sixt, git Steven Penny wrote: > Junio C Hamano wrote: >> Actually, the above is stated rather poorly. The path that ends up in >> $file must be usable by both Windows native and Cygwin programs, as the >> user may be using "vi" from Cygwin, or "notepad" like your example. > > Excellent point. > > I ran some test and this is what I came up with > > # VI > /cygdrive/c/test/hello.sh # works > C:\test\hello.sh # works > C:/test/hello/sh # works > > # NOTEPAD > /cygdrive/c/test/hello.sh # does not work > C:\test\hello.sh # works > C:/test/hello.sh # works > > so the best compromise would be "C:/test/hello.sh" which can be created with > > diff --git a/git-sh-setup.sh b/git-sh-setup.sh > index 7b3ae75..ba198d2 100644 > --- a/git-sh-setup.sh > +++ b/git-sh-setup.sh > @@ -260,6 +260,11 @@ case $(uname -s) in > return 1 > } > ;; > +*CYGWIN*) > + pwd () { > + builtin cygpath -m > + } > + ;; > *) > is_absolute_path () { > case "$1" in > > http://github.com/svnpenn/git/commit/692bc I would rather define a script; it can then be used independently of git. Personally, I don't have this specific problem because I use (the cygwin version of) vim. (does anybody actually use notepad?) I mostly, but not exclusively, use cygwin tools on cygwin. For example I use win32 versions of doxygen, ghostscript, tex (MikTex 2.7), graphviz etc. However, the makefiles which drive those tools use relative paths ... In fact the only problem I have encountered is with firefox, which I resolved by hacking up a script. Indeed I have already posted to the list on this issue before (back in January 2009) and, to save you the effort of trawling the archives, I append that mail below. HTH ATB, Ramsay Jones -- >8 -- Date: Sat, 24 Jan 2009 18:51:23 +0000 From: Ramsay Jones <ramsay@ramsay1.demon.co.uk> To: =?ISO-8859-1?Q?Bj=F6rn_Steinbrink?= <B.Steinbrink@gmx.de> CC: Junio C Hamano <gitster@pobox.com>, jaeckel@stzedn.de, git@vger.kernel.org Subject: Re: [PATCH] cygwin: Convert paths for html help from posix to windows Björn Steinbrink wrote: > When using "git help --web" with cygwin, we used to pass the posix path > to the browser, but a native windows browser will expect a windows path > and is unable to make use of the given path. > > So the cygwin port gets its own open_html implementation that handles > the path conversion. > > Reported-by: Steffen Jaeckel <jaeckel@stzedn.de> > Tested-by: Steffen Jaeckel <jaeckel@stzedn.de> > > Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de> > --- > OK, I don't really know if this is the right way to do it. Maybe when > the browser was built for cygwin this breaks? I have no clue, it's > admittedly just the result of a quick glance at the code and some > googling to find the "right" cygwin function... :-/ > Hi Björn, I had the same problem. However, rather than modifying git, I created a firefox wrapper script (in ~/bin) which used the cygpath command line tool to do the path conversion. Also, if you use "git instaweb", you also need to filter out http URLs and pass them through un-molested by cygpath (it turns http://localhost into http:\localhost). My script is clearly a "quick hack" just to get something working for me, but you may find it useful as a starting point for your own ("proper" ;-) script, so I've included it below. HTH, Ramsay Jones -->8-- #!/bin/sh # while test $# != 0 do case "$1" in -version) echo "firefox 1.5.0.2" exit 0 ;; -new-tab) echo "-new-tab not supported" exit 1 ;; --) shift break ;; -*) echo "option '$1' not supported" exit 1 ;; *) break ;; esac shift done if test "$1" = "" then p= else case "$1" in http*) p="$1" ;; *) p="$(cygpath -w "$1")" ;; esac fi "/cygdrive/c/Program Files/Mozilla Firefox/firefox.exe" "$p" exit 0 -- 8< -- ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-10 18:10 ` Ramsay Jones @ 2012-05-11 4:35 ` Steven Penny 2012-05-13 22:58 ` Ramsay Jones 0 siblings, 1 reply; 33+ messages in thread From: Steven Penny @ 2012-05-11 4:35 UTC (permalink / raw) To: Ramsay Jones; +Cc: Junio C Hamano, Johannes Sixt, git Ramsay Jones wrote: > I would rather define a script; it can then be used independently of git. So your suggestion is to have git-sh-setup.sh account for MinGW, which is its current state, but not account for Cygwin? > Personally, I don't have this specific problem because I use (the cygwin > version of) vim. (does anybody actually use notepad?) If you had read carefully, you would have noticed that I mentioned more than notepad. As well Notepad2, and Notepad++, etc. > I mostly, but not exclusively, use cygwin tools on cygwin. For example I > use win32 versions of doxygen, ghostscript, tex (MikTex 2.7), graphviz etc. > However, the makefiles which drive those tools use relative paths ... This convo is not about what tools _you_ use, but about the current incompatibility with several native windows text editors. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-11 4:35 ` Steven Penny @ 2012-05-13 22:58 ` Ramsay Jones 2012-05-13 23:42 ` Steven Penny 2012-05-14 6:02 ` Johannes Sixt 0 siblings, 2 replies; 33+ messages in thread From: Ramsay Jones @ 2012-05-13 22:58 UTC (permalink / raw) To: Steven Penny; +Cc: Junio C Hamano, Johannes Sixt, git Steven Penny wrote: > Ramsay Jones wrote: >> I would rather define a script; it can then be used independently of git. > > So your suggestion is to have git-sh-setup.sh account for MinGW, which is its > current state, but not account for Cygwin? I wasn't specifically suggesting that no; I was suggesting that I prefer to fix the problem with a script on cygwin. (Again, the script can be used independently of git) BTW, Johannes, earlier you said commit be39048 ("git-sh-setup.sh: Add an pwd() function for MinGW", 17-04-2012) would fix the problem on MinGW; I'm not so sure it will. I haven't actually tested it, so don't take my word for it. ;-P (See below for an explanation of my doubts) >> Personally, I don't have this specific problem because I use (the cygwin >> version of) vim. (does anybody actually use notepad?) > > If you had read carefully, you would have noticed that I mentioned more than > notepad. As well Notepad2, and Notepad++, etc. Yes, I did notice that you mentioned more than notepad ... >> I mostly, but not exclusively, use cygwin tools on cygwin. For example I >> use win32 versions of doxygen, ghostscript, tex (MikTex 2.7), graphviz etc. >> However, the makefiles which drive those tools use relative paths ... > > This convo is not about what tools _you_ use, but about the current > incompatibility with several native windows text editors. OK. So, yes, I didn't give your patch a look; sorry about that. Let's take a look now (quoting from earlier email): > diff --git a/git-sh-setup.sh b/git-sh-setup.sh > > index 7b3ae75..ba198d2 100644 > > --- a/git-sh-setup.sh > > +++ b/git-sh-setup.sh > > @@ -260,6 +260,11 @@ case $(uname -s) in > > return 1 > > } > > ;; > > +*CYGWIN*) > > + pwd () { > > + builtin cygpath -m > > + } > > + ;; > > *) > > is_absolute_path () { > > case "$1" in > > > > http://github.com/svnpenn/git/commit/692bc I haven't actually tried to apply or test your patch, so take the following with a pinch of salt ... I don't think this will work because: - cygpath is not a bash builtin, so bash *should* simply issue an error something along the lines of "not a shell builtin". - cygpath requires an input path So, I would have expected the body of the pwd function to be something like: cygpath -m "$PWD" or maybe cygpath -m "$(builtin pwd)" (Again, I'm just typing into my mail client, so not tested ...) Also note that the MinGW pwd() uses a shell builtin and so, unlike the above, does not suffer any fork+exec overhead. If we fix the above, then another problem (*which MinGW shares*) is that the pwd() function is defined *after* the code that sets $GIT_DIR from which the rebase state directory name is derived (see git-sh-setup.sh lines 223-239). Note that cygwin git will create the various inputs (commit template say) with lf only line endings; so the windows text editor you use must be able to cope with such an input. (I think the PSEdit editor will cope just fine). Similar comments apply to all other external programs launched by git (for example, external diff/merge tools, clean/smudge filters ...). HTH ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-13 22:58 ` Ramsay Jones @ 2012-05-13 23:42 ` Steven Penny 2012-05-14 6:02 ` Johannes Sixt 1 sibling, 0 replies; 33+ messages in thread From: Steven Penny @ 2012-05-13 23:42 UTC (permalink / raw) To: Ramsay Jones; +Cc: Junio C Hamano, Johannes Sixt, git Ramsay Jones wrote: > I haven't actually tried to apply or test your patch, so take the > following with a pinch of salt ... > > I don't think this will work because: > > - cygpath is not a bash builtin, so bash *should* simply issue > an error something along the lines of "not a shell builtin". > > - cygpath requires an input path > > So, I would have expected the body of the pwd function to be something like: > > cygpath -m "$PWD" If you would actually look at the file, you would see that pretty much everything you just wrote is wrong SNIPPET *MINGW*) pwd () { builtin pwd -W } http://github.com/git/git/blob/master/git-sh-setup.sh ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-13 22:58 ` Ramsay Jones 2012-05-13 23:42 ` Steven Penny @ 2012-05-14 6:02 ` Johannes Sixt 2012-05-15 17:32 ` Ramsay Jones 2012-05-16 18:00 ` [PATCH 0/2] " Junio C Hamano 1 sibling, 2 replies; 33+ messages in thread From: Johannes Sixt @ 2012-05-14 6:02 UTC (permalink / raw) To: Ramsay Jones; +Cc: Steven Penny, Junio C Hamano, git Am 5/14/2012 0:58, schrieb Ramsay Jones: > BTW, Johannes, earlier you said commit be39048 ("git-sh-setup.sh: Add an pwd() > function for MinGW", 17-04-2012) would fix the problem on MinGW; I'm not so > sure it will. > [...] problem (*which MinGW shares*) is that the > pwd() function is defined *after* the code that sets $GIT_DIR from which the > rebase state directory name is derived (see git-sh-setup.sh lines 223-239). Good catch! Nevertheless, if I set GIT_EDITOR to "notepad", 'git rebase -i' works to some degree. Leaving aside that it does not understand LF line endings, it opens the git-rebase-todo file; I can edit and save it. That is, it looks like a usable Windows style path was passed to Notepad. (This is with MinGW git, of course.) But I can't use "Save As": Notepad just responds with "Error #3002". I would attribute this to the forward slashes in the absolute path name, and that the file dialog can't cope with them. Personally, I don't think this is a big deal: We have heard of people who set GIT_EDITOR to something Windows-y, but there were no complaints of the kind I mention in this paragraph. (Why should there be any? There is no reason to use "Save As" when git invokes an editor.) -- Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-14 6:02 ` Johannes Sixt @ 2012-05-15 17:32 ` Ramsay Jones 2012-05-16 5:52 ` Johannes Sixt 2012-05-16 18:00 ` [PATCH 0/2] " Junio C Hamano 1 sibling, 1 reply; 33+ messages in thread From: Ramsay Jones @ 2012-05-15 17:32 UTC (permalink / raw) To: Johannes Sixt; +Cc: Steven Penny, Junio C Hamano, git Johannes Sixt wrote: > Am 5/14/2012 0:58, schrieb Ramsay Jones: >> BTW, Johannes, earlier you said commit be39048 ("git-sh-setup.sh: Add an pwd() >> function for MinGW", 17-04-2012) would fix the problem on MinGW; I'm not so >> sure it will. > >> [...] problem (*which MinGW shares*) is that the >> pwd() function is defined *after* the code that sets $GIT_DIR from which the >> rebase state directory name is derived (see git-sh-setup.sh lines 223-239). > > Good catch! > > Nevertheless, if I set GIT_EDITOR to "notepad", 'git rebase -i' works to > some degree. Leaving aside that it does not understand LF line endings, it > opens the git-rebase-todo file; I can edit and save it. That is, it looks > like a usable Windows style path was passed to Notepad. (This is with > MinGW git, of course.) > I was about to, confidently, proclaim that it should fail when you run that rebase from a subdirectory ... Unfortunately, it seems to work! *ahem* The reason I was confident, is simply that I have seen this pattern: p= ...some path... p=$(cd "$p" && pwd) many times while debugging scripts on MinGW, so I 'know' that if p is initially a relative path, then the result is an absolute windows path. For example, while cwd is /home/ramsay/git then: .git => C:/msysgit/msysgit/home/ramsay/git/.git Whereas if p is initially an absolute windows path, then the result is a POSIX-like path, viz: C:/msysgit/msysgit/home/ramsay/git/.git => /usr/home/ramsay/git/.git If you combine that with the behaviour of 'git rev-parse', thus: $ cd /home/ramsay/git $ git rev-parse --git-dir .git $ cd xdiff $ git rev-parse --git-dir C:/msysgit/msysgit/home/ramsay/git/.git $ you should see "git rebase -i" invoking the editor with a POSIX path. [BTW, I have noticed that I sometimes get different behaviour when I type this stuff into an interactive shell. :(] Anyway, a bit of debugging shows that I was right, kinda! :-D I placed a "set -x" in git-sh-setup (line 222) just before the code to set up GIT_DIR and ran git-rebase like so: $ cd /home/ramsay/git $ GIT_EDITOR=false ./bin-wrappers/git rebase -i master uname >ttt 2>&1 $ cd xdiff $ GIT_EDITOR=false ../bin-wrappers/git rebase -i master uname >sss 2>&1 $ diff ../ttt sss | more which showed that in the subdirectory case, git was indeed invoking the editor with a POSIX-like path. (The output files, ttt and sss, show some other interesting/odd behaviour). ['uname' is an old branch of mine that, currently, is based on v1.7.10] The reason it works, despite the above, is clear if you now run: $ GIT_EDITOR=args ../bin-wrappers/git rebase -i master uname ... +++ git var GIT_EDITOR ++ GIT_SEQUENCE_EDITOR=args ++ eval args '"$@"' +++ args /usr/home/ramsay/git/.git/rebase-merge/git-rebase-todo argv[0] = 'C:\msysgit\msysgit\home\ramsay\bin\args.exe' argv[1] = 'C:/msysgit/msysgit/home/ramsay/git/.git/rebase-merge/git-rebase-todo' ++ die_abort 'Could not execute editor' ++ rm -rf /usr/home/ramsay/git/.git/rebase-merge ++ die 'Could not execute editor' ++ die_with_status 1 'Could not execute editor' ++ status=1 ++ shift ++ echo 'Could not execute editor' Could not execute editor ++ exit 1 [I'm sure you can guess what the args program looks like!] So, the msys "path munging" of program arguments saves the day! HTH ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-15 17:32 ` Ramsay Jones @ 2012-05-16 5:52 ` Johannes Sixt 2012-05-17 18:30 ` Ramsay Jones 0 siblings, 1 reply; 33+ messages in thread From: Johannes Sixt @ 2012-05-16 5:52 UTC (permalink / raw) To: Ramsay Jones; +Cc: Steven Penny, Junio C Hamano, git Am 5/15/2012 19:32, schrieb Ramsay Jones: > ++ GIT_SEQUENCE_EDITOR=args > ++ eval args '"$@"' > +++ args /usr/home/ramsay/git/.git/rebase-merge/git-rebase-todo > argv[0] = 'C:\msysgit\msysgit\home\ramsay\bin\args.exe' > argv[1] = 'C:/msysgit/msysgit/home/ramsay/git/.git/rebase-merge/git-rebase-todo' ... > So, the msys "path munging" of program arguments saves the day! Absolutely. This path munging is an essential detail in the process. I don't know whether Cygwin has a similar feature, but I suppose not, otherwise we wouldn't have received this issue report. -- Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-16 5:52 ` Johannes Sixt @ 2012-05-17 18:30 ` Ramsay Jones 2012-05-17 19:19 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Ramsay Jones @ 2012-05-17 18:30 UTC (permalink / raw) To: Johannes Sixt; +Cc: Steven Penny, Junio C Hamano, git Johannes Sixt wrote: > Am 5/15/2012 19:32, schrieb Ramsay Jones: >> ++ GIT_SEQUENCE_EDITOR=args >> ++ eval args '"$@"' >> +++ args /usr/home/ramsay/git/.git/rebase-merge/git-rebase-todo >> argv[0] = 'C:\msysgit\msysgit\home\ramsay\bin\args.exe' >> argv[1] = 'C:/msysgit/msysgit/home/ramsay/git/.git/rebase-merge/git-rebase-todo' > ... >> So, the msys "path munging" of program arguments saves the day! > > Absolutely. This path munging is an essential detail in the process. > > I don't know whether Cygwin has a similar feature, but I suppose not, > otherwise we wouldn't have received this issue report. Yes, but I keep forgetting about this msys feature (I don't know why!). I've had this "slap forehead" moment 3 or 4 times already (why do you think I have the args program close to hand? :-D ). As you surmised, cygwin does not have this feature. On cygwin you are encouraged to use the cygpath utility in scripts. (Also, cygwin does provide an API to convert to/from POSIX/win32 paths from your own programs. eg. cygwin_conv_to_[full]_win32_path() and cygwin_conv_to_\ [full]_posix_path().) >From the cygwin user guide, in a section titled "Using Cygwin effectively with Windows", we find this: "Windows programs do not understand POSIX pathnames, so any arguments that reference the filesystem must be in Windows (or DOS) format or translated. Cygwin provides the cygpath utility for converting between Windows and POSIX paths. A complete description of its options and examples of its usage are in Section 3.7.2, including a shell script for starting Windows Explorer in any directory. The same format works for most Windows programs, for example notepad.exe "$(cygpath -aw "Desktop/Phone Numbers.txt")" A few programs require a Windows-style, semicolon-delimited path list, which cygpath can translate from a POSIX path with the -p option. For example, a Java compilation from bash might look like this: javac -cp "$(cygpath -pw "$CLASSPATH")" hello.java Since using quoting and subshells is somewhat awkward, it is often preferable to use cygpath in shell scripts." Just as an exercise, I created a script to use the windows PSPad editor (included below), using it to create a commit and also run this rebase: GIT_EDITOR=pspad git rebase -i master uname Both git commands launched the editor (and completed their task) just fine. Note that the script can be improved greatly, but it only took ten minutes to create and is sufficient to this task. (PSPad supports more than one file on the command line, despite what it's help file says, although all options apply to all files. You should be able to "bundle" the options ...). ATB, Ramsay Jones -- >8 -- #!/bin/sh file= opts= while test $# != 0 do case "$1" in -[hH]) # open file in hex editor opts="$opts /H" ;; -[rR]) # open file in read-only mode opts="$opts /R" ;; -[0-9]*) # open file and goto line n opts="$opts /${1:1}" ;; -*) echo "option '$1' not supported" exit 1 ;; *) if test -n "$file"; then echo "only one filename allowed" exit 1 fi file="$1" ;; esac shift done if test -n "$file"; then file="$(cygpath -aw "$file")" fi "C:/Program Files/PSPad editor/PSPad.exe" "$file" $opts -- 8< -- ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Git commit path vs rebase path 2012-05-17 18:30 ` Ramsay Jones @ 2012-05-17 19:19 ` Junio C Hamano 0 siblings, 0 replies; 33+ messages in thread From: Junio C Hamano @ 2012-05-17 19:19 UTC (permalink / raw) To: Ramsay Jones; +Cc: Johannes Sixt, Steven Penny, Junio C Hamano, git Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > As you surmised, cygwin does not have this feature. On cygwin you are > encouraged to use the cygpath utility in scripts. (Also, cygwin does > provide an API to convert to/from POSIX/win32 paths from your own > programs. eg. cygwin_conv_to_[full]_win32_path() and cygwin_conv_to_\ > [full]_posix_path().) > ... > Just as an exercise, I created a script to use the windows PSPad editor > (included below), using it to create a commit and also run this rebase: > > GIT_EDITOR=pspad git rebase -i master uname > > Both git commands launched the editor (and completed their task) just fine. So where does the above leave us? Your message showed a way to "solve" it without changing git, but the presense of workaround alone is not a good reason not to change git, if the change makes things better without breaking other things. I'd like to know if Steven's patch breaks Cygwin users in other ways. Steven's message showed that spawning Cygwin programs and feeding paths in $GIT_DIR to them would not be broken if we start giving GIT_DIR as a Windows style path inside shell scripted Porcelains. But will there be other things that get broken by the change? An example of what would worry me is that we define has_drive_prefix() only for compat/mingw.h, so if somebody exports GIT_DIR back to call the binaries from scripted Porcelains, it is likely that it would break git running on Cygwin, where it expects only posix-style paths. Upon seeing C:/foo/bar.txt, it will start saying "Ah, that is a relative path" and may end up adding the $(pwd) in front when it needs to know its absolute path. ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 0/2] Re: Git commit path vs rebase path 2012-05-14 6:02 ` Johannes Sixt 2012-05-15 17:32 ` Ramsay Jones @ 2012-05-16 18:00 ` Junio C Hamano 2012-05-16 18:00 ` [PATCH 1/2] git-sh-setup: define workaround wrappers before they are used Junio C Hamano 2012-05-16 18:00 ` [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas Junio C Hamano 1 sibling, 2 replies; 33+ messages in thread From: Junio C Hamano @ 2012-05-16 18:00 UTC (permalink / raw) To: git; +Cc: Johannes Sixt, Ramsay Jones, Steven Penny I do not want to see a discussion hanging around waiting for somebody to bring it to conclusion, especially when the topic is not something I can give a tested definitive solution. It is a sure way to lose the effort so far that brought us almost there but not quite by not following through and forgetting about it. Here are two patches I *think* represent the conclusion of the discussion, but it needs eyeballing from stakeholders. I may have misunderstood the issue entirely. Also Steven's patch needs a sign-off. Junio C Hamano (1): git-sh-setup: define workaround wrappers before they are used Steven Penny (1): git-sh-setup: work around Cygwin path handling gotchas git-sh-setup.sh | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) -- 1.7.10.2.537.g0ac6509 ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/2] git-sh-setup: define workaround wrappers before they are used 2012-05-16 18:00 ` [PATCH 0/2] " Junio C Hamano @ 2012-05-16 18:00 ` Junio C Hamano 2012-05-17 22:36 ` Ramsay Jones 2012-05-16 18:00 ` [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas Junio C Hamano 1 sibling, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-16 18:00 UTC (permalink / raw) To: git; +Cc: Johannes Sixt, Ramsay Jones, Steven Penny Recently we tweaked this scriptlet to let mingw port redefine "pwd" to always return Windows-style path, but the code to do so came after the first use of "pwd" to set up $GIT_DIR shell variable. Move the block to define these workaround wrappers, so that everything everything that executes when the scriptlet is dot-sourced uses the replacements. Noticed-by: Ramsay Jones Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-sh-setup.sh | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 7b3ae75..770a86e 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -218,27 +218,8 @@ clear_local_git_env() { unset $(git rev-parse --local-env-vars) } -# Make sure we are in a valid repository of a vintage we understand, -# if we require to be in a git repository. -if test -z "$NONGIT_OK" -then - GIT_DIR=$(git rev-parse --git-dir) || exit - if [ -z "$SUBDIRECTORY_OK" ] - then - test -z "$(git rev-parse --show-cdup)" || { - exit=$? - echo >&2 "You need to run this command from the toplevel of the working tree." - exit $exit - } - fi - test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { - echo >&2 "Unable to determine absolute path of git directory" - exit 1 - } - : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} -fi -# Fix some commands on Windows +# Platform specific tweaks to work around some commands case $(uname -s) in *MINGW*) # Windows has its own (incompatible) sort and find @@ -269,3 +250,23 @@ case $(uname -s) in return 1 } esac + +# Make sure we are in a valid repository of a vintage we understand, +# if we require to be in a git repository. +if test -z "$NONGIT_OK" +then + GIT_DIR=$(git rev-parse --git-dir) || exit + if [ -z "$SUBDIRECTORY_OK" ] + then + test -z "$(git rev-parse --show-cdup)" || { + exit=$? + echo >&2 "You need to run this command from the toplevel of the working tree." + exit $exit + } + fi + test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { + echo >&2 "Unable to determine absolute path of git directory" + exit 1 + } + : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} +fi -- 1.7.10.2.537.g0ac6509 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/2] git-sh-setup: define workaround wrappers before they are used 2012-05-16 18:00 ` [PATCH 1/2] git-sh-setup: define workaround wrappers before they are used Junio C Hamano @ 2012-05-17 22:36 ` Ramsay Jones 0 siblings, 0 replies; 33+ messages in thread From: Ramsay Jones @ 2012-05-17 22:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Johannes Sixt, Steven Penny Junio C Hamano wrote: > Recently we tweaked this scriptlet to let mingw port redefine "pwd" to > always return Windows-style path, but the code to do so came after the > first use of "pwd" to set up $GIT_DIR shell variable. > > Move the block to define these workaround wrappers, so that everything > everything that executes when the scriptlet is dot-sourced uses the > replacements. > > Noticed-by: Ramsay Jones > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > git-sh-setup.sh | 41 +++++++++++++++++++++-------------------- > 1 file changed, 21 insertions(+), 20 deletions(-) > > diff --git a/git-sh-setup.sh b/git-sh-setup.sh > index 7b3ae75..770a86e 100644 > --- a/git-sh-setup.sh > +++ b/git-sh-setup.sh > @@ -218,27 +218,8 @@ clear_local_git_env() { > unset $(git rev-parse --local-env-vars) > } > > -# Make sure we are in a valid repository of a vintage we understand, > -# if we require to be in a git repository. > -if test -z "$NONGIT_OK" > -then > - GIT_DIR=$(git rev-parse --git-dir) || exit > - if [ -z "$SUBDIRECTORY_OK" ] > - then > - test -z "$(git rev-parse --show-cdup)" || { > - exit=$? > - echo >&2 "You need to run this command from the toplevel of the working tree." > - exit $exit > - } > - fi > - test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { > - echo >&2 "Unable to determine absolute path of git directory" > - exit 1 > - } > - : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} > -fi > > -# Fix some commands on Windows > +# Platform specific tweaks to work around some commands > case $(uname -s) in > *MINGW*) > # Windows has its own (incompatible) sort and find > @@ -269,3 +250,23 @@ case $(uname -s) in > return 1 > } > esac > + > +# Make sure we are in a valid repository of a vintage we understand, > +# if we require to be in a git repository. > +if test -z "$NONGIT_OK" > +then > + GIT_DIR=$(git rev-parse --git-dir) || exit > + if [ -z "$SUBDIRECTORY_OK" ] > + then > + test -z "$(git rev-parse --show-cdup)" || { > + exit=$? > + echo >&2 "You need to run this command from the toplevel of the working tree." > + exit $exit > + } > + fi > + test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { > + echo >&2 "Unable to determine absolute path of git directory" > + exit 1 > + } > + : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} > +fi Thanks for doing this. (I would have got around to it, honest! However, it does solve a minor problem for me, since I kinda promised not to post anymore MinGW specific patches. :-D ). I have not done a full test on this patch; I have only run the following tests on MinGW and cygwin: $ git grep -l -e 'git *submodule' -- t t/t5526-fetch-submodules.sh t/t6008-rev-list-submodule.sh t/t7003-filter-branch.sh t/t7400-submodule-basic.sh t/t7401-submodule-summary.sh t/t7403-submodule-sync.sh t/t7405-submodule-merge.sh t/t7406-submodule-update.sh t/t7407-submodule-foreach.sh t/t7408-submodule-reference.sh t/t7506-status-submodule.sh t/t7610-mergetool.sh t/t9300-fast-import.sh t/t9350-fast-export.sh (On MinGW i have to use a "GIT_TEST_CMP='diff -ub'" prefix, otherwise there are some failures caused by "lf/crlf line ending" problems). All of these tests pass. I don't anticipate any problems (especially on cygwin, since it is essentially a noop there), but I have not done a complete test. I probably won't get to it soon, so I'm hoping somebody can beat me to it! ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-16 18:00 ` [PATCH 0/2] " Junio C Hamano 2012-05-16 18:00 ` [PATCH 1/2] git-sh-setup: define workaround wrappers before they are used Junio C Hamano @ 2012-05-16 18:00 ` Junio C Hamano 2012-05-16 18:51 ` Steven Penny 1 sibling, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-16 18:00 UTC (permalink / raw) To: git; +Cc: Steven Penny, Johannes Sixt, Ramsay Jones From: Steven Penny <svnpenn@gmail.com> From: Steven Penny <svnpenn@gmail.com> On Cygwin, tools built for Cygwin can take both Windows-style paths (e.g. C:/dir/file.txt or C:\dir\file.txt) and Cygwin-style paths (e.g. /cygdrive/c/dir/file.txt), but Windows-native tools can only take Windows-style paths. Because the paths that are relative to $GIT_DIR, e.g. the name of the insn sheet file of the "rebase -i" command, are given to the programs with $GIT_DIR prefixed, and $GIT_DIR in turn is computed by calling "pwd", wrap "pwd" to call "cygpath -m" to give a Windows-style path, in a way similar to how mingw does this. --- git-sh-setup.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 770a86e..f0f941d 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -241,6 +241,11 @@ case $(uname -s) in return 1 } ;; +*CYGWIN*) + pwd () { + builtin cygpath -m + } + ;; *) is_absolute_path () { case "$1" in -- 1.7.10.2.537.g0ac6509 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-16 18:00 ` [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas Junio C Hamano @ 2012-05-16 18:51 ` Steven Penny 2012-05-16 19:02 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Steven Penny @ 2012-05-16 18:51 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Johannes Sixt, Ramsay Jones Junio C Hamano wrote: > +*CYGWIN*) > + pwd () { > + builtin cygpath -m > + } > + ;; Ok I got it! The problem is twofold 1. Ramsay Jones was right, it needs to be called like cygpath -m "$PWD" 2. The Cygwin "pwd" (and quite possibly MinGW "pwd") needs to be defined **before** it is called diff -u a/git-sh-setup.sh b/git-sh-setup.sh --- a/git-sh-setup.sh 2012-05-16 13:48:43.160399300 -0500 +++ b/git-sh-setup.sh 2012-05-16 13:50:29.465479600 -0500 @@ -218,26 +218,6 @@ unset $(git rev-parse --local-env-vars) } -# Make sure we are in a valid repository of a vintage we understand, -# if we require to be in a git repository. -if test -z "$NONGIT_OK" -then - GIT_DIR=$(git rev-parse --git-dir) || exit - if [ -z "$SUBDIRECTORY_OK" ] - then - test -z "$(git rev-parse --show-cdup)" || { - exit=$? - echo >&2 "You need to run this command from the toplevel of the working tree." - exit $exit - } - fi - test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { - echo >&2 "Unable to determine absolute path of git directory" - exit 1 - } - : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} -fi - # Fix some commands on Windows case $(uname -s) in *MINGW*) @@ -260,6 +240,11 @@ return 1 } ;; +*CYGWIN*) + pwd () { + cygpath -m "$PWD" + } + ;; *) is_absolute_path () { case "$1" in @@ -269,3 +254,23 @@ return 1 } esac + +# Make sure we are in a valid repository of a vintage we understand, +# if we require to be in a git repository. +if test -z "$NONGIT_OK" +then + GIT_DIR=$(git rev-parse --git-dir) || exit + if [ -z "$SUBDIRECTORY_OK" ] + then + test -z "$(git rev-parse --show-cdup)" || { + exit=$? + echo >&2 "You need to run this command from the toplevel of the working tree." + exit $exit + } + fi + test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || { + echo >&2 "Unable to determine absolute path of git directory" + exit 1 + } + : ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"} +fi ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-16 18:51 ` Steven Penny @ 2012-05-16 19:02 ` Junio C Hamano 2012-05-17 23:15 ` Ramsay Jones 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-16 19:02 UTC (permalink / raw) To: Steven Penny; +Cc: git, Johannes Sixt, Ramsay Jones Steven Penny <svnpenn@gmail.com> writes: > Junio C Hamano wrote: >> +*CYGWIN*) >> + pwd () { >> + builtin cygpath -m >> + } >> + ;; > > Ok I got it! > > The problem is twofold > > 1. Ramsay Jones was right, it needs to be called like > > cygpath -m "$PWD" > > 2. The Cygwin "pwd" (and quite possibly MinGW "pwd") needs to be defined > **before** it is called OK, I missed the first point, it seems. But you seem to have missed that these two problems are more or less independent---that is why I sent two patches, not a single ball of wax like the one I am responding to. So the replacement for [PATCH 2/2] would now look like this? In addition to "applies fine, tested and works" reports from Windows stakeholders, I still prefer to have a sign off from you (see Documentation/SubmittingPatches). Thanks. -- >8 -- From: Steven Penny <svnpenn@gmail.com> Date: Wed, 16 May 2012 10:44:49 -0700 Subject: [PATCH] git-sh-setup: work around Cygwin path handling gotchas On Cygwin, tools built for Cygwin can take both Windows-style paths (e.g. C:/dir/file.txt or C:\dir\file.txt) and Cygwin-style paths (e.g. /cygdrive/c/dir/file.txt), but Windows-native tools can only take Windows-style paths. Because the paths that are relative to $GIT_DIR, e.g. the name of the insn sheet file of the "rebase -i" command, are given to the programs with $GIT_DIR prefixed, and $GIT_DIR in turn is computed by calling "pwd", wrap "pwd" to call "cygpath -m" to give a Windows-style path, in a way similar to how mingw does this. --- git-sh-setup.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 770a86e..b8e6327 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -241,6 +241,11 @@ case $(uname -s) in return 1 } ;; +*CYGWIN*) + pwd () { + cygpath -m "$PWD" + } + ;; *) is_absolute_path () { case "$1" in -- 1.7.10.2.537.g0ac6509 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-16 19:02 ` Junio C Hamano @ 2012-05-17 23:15 ` Ramsay Jones 2012-05-18 2:34 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Ramsay Jones @ 2012-05-17 23:15 UTC (permalink / raw) To: Junio C Hamano; +Cc: Steven Penny, git, Johannes Sixt Junio C Hamano wrote: > Steven Penny <svnpenn@gmail.com> writes: > >> Junio C Hamano wrote: >>> +*CYGWIN*) >>> + pwd () { >>> + builtin cygpath -m >>> + } >>> + ;; >> Ok I got it! >> >> The problem is twofold >> >> 1. Ramsay Jones was right, it needs to be called like >> >> cygpath -m "$PWD" >> >> 2. The Cygwin "pwd" (and quite possibly MinGW "pwd") needs to be defined >> **before** it is called > > OK, I missed the first point, it seems. But you seem to have missed that > these two problems are more or less independent---that is why I sent two > patches, not a single ball of wax like the one I am responding to. > > So the replacement for [PATCH 2/2] would now look like this? > > In addition to "applies fine, tested and works" reports from Windows > stakeholders, I still prefer to have a sign off from you (see > Documentation/SubmittingPatches). > > Thanks. > > -- >8 -- > From: Steven Penny <svnpenn@gmail.com> > Date: Wed, 16 May 2012 10:44:49 -0700 > Subject: [PATCH] git-sh-setup: work around Cygwin path handling gotchas > > On Cygwin, tools built for Cygwin can take both Windows-style paths > (e.g. C:/dir/file.txt or C:\dir\file.txt) and Cygwin-style paths > (e.g. /cygdrive/c/dir/file.txt), but Windows-native tools can only take > Windows-style paths. Because the paths that are relative to $GIT_DIR, > e.g. the name of the insn sheet file of the "rebase -i" command, are given > to the programs with $GIT_DIR prefixed, and $GIT_DIR in turn is computed > by calling "pwd", wrap "pwd" to call "cygpath -m" to give a Windows-style > path, in a way similar to how mingw does this. > --- > git-sh-setup.sh | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/git-sh-setup.sh b/git-sh-setup.sh > index 770a86e..b8e6327 100644 > --- a/git-sh-setup.sh > +++ b/git-sh-setup.sh > @@ -241,6 +241,11 @@ case $(uname -s) in > return 1 > } > ;; > +*CYGWIN*) > + pwd () { > + cygpath -m "$PWD" > + } > + ;; > *) > is_absolute_path () { > case "$1" in I guess you won't be shocked to hear that I don't think this patch is necessary. :-P However, I can appreciate that some people would rather not have to create a shell script to wrap their text editor, just to use git. So I'm certainly not opposed to finding a solution to this problem that doesn't require the user to do so. My concerns about this patch include: - the additional fork+exec overhead associated with calling cygpath. I'm not actually claiming there is any substantial increase; I haven't tried it, so I don't know how "hot" the pwd() function is. - this is a "big hammer" which will affect much more code that is required to fix this problem. The latter is my main concern. I would rather have the call to cygpath at the point in the code where the editor is launched. This would reduce the scope of the change and any side-effects that go with it. Anyway, I applied this patch tonight to give it a go. The very first test I tried failed. I've attached the log of the failing test below. Note that it is attempting to use "ssh" to a "host" that ends in ".../C:". I haven't investigated yet (I've got to get some sleep). HTH ATB, Ramsay Jones expecting success: ( cd addtest && git submodule add ../repo relative && test "$(git config -f .gitmodules submodule.relative.url)" = ../ repo && git submodule sync relative && test "$(git config submodule.relative.url)" = "$submodurl/repo" ) Cloning into 'relative'... ssh: /home/ramsay/git/t/trash directory.t7400-submodule-basic/addtest/C: no addr ess associated with name fatal: The remote end hung up unexpectedly Clone of 'C:/cygwin/home/ramsay/git/t/trash directory.t7400-submodule-basic/repo ' into submodule path 'relative' failed not ok - 41 use superproject as upstream when path is relative and no url is set there # # ( # cd addtest && # git submodule add ../repo relative && # test "$(git config -f .gitmodules submodule.relative.url )" = ../repo && # git submodule sync relative && # test "$(git config submodule.relative.url)" = "$submodur l/repo" # ) # $ ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-17 23:15 ` Ramsay Jones @ 2012-05-18 2:34 ` Junio C Hamano 2012-05-19 0:43 ` Steven Penny 2012-05-21 18:43 ` Ramsay Jones 0 siblings, 2 replies; 33+ messages in thread From: Junio C Hamano @ 2012-05-18 2:34 UTC (permalink / raw) To: Ramsay Jones; +Cc: Steven Penny, git, Johannes Sixt Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > I guess you won't be shocked to hear that I don't think this patch is > necessary. :-P That is more or less irrelevant, not in the sense that what you say is irrelevant, but in the sense that something can be worked around in a different way alone is not a good reason to reject a patch, if its benefit outweigh its costs. The more important message for you to respond to is this one: Subject: Re: Git commit path vs rebase path To: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Cc: Johannes Sixt <j.sixt@viscovery.net>, Steven Penny <svnpenn@gmail.com>, Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org Date: Thu, 17 May 2012 12:19:33 -0700 Message-ID: <xmqqfwayobsq.fsf@junio.mtv.corp.google.com> > My concerns about this patch include: > > - the additional fork+exec overhead associated with calling cygpath. > I'm not actually claiming there is any substantial increase; I > haven't tried it, so I don't know how "hot" the pwd() function is. > > - this is a "big hammer" which will affect much more code that is > required to fix this problem. If I speculated in the other message is correct (in short, "In Cygwin world, Git is compiled to use POSIX paths and would not work with Windows paths."), I think this "problem" is fundamentally un"fix"able. And from Cygwin Git, your programs (like $EDITOR and hooks) will get POSIX paths. It is your programs' responsibility to turn them into Windows paths if/as necessary. > Anyway, I applied this patch tonight to give it a go. The very first test > I tried failed. I've attached the log of the failing test below. > Note that it is attempting to use "ssh" to a "host" that ends in ".../C:". Of course. That is one typical symptom that suggests my speculation was correct. So "I don't think this patch is necessary" is irrelevant, but "This patch is harmful; Git on Cygwin is never supposed to use Windows paths" is very relevant ;-) ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-18 2:34 ` Junio C Hamano @ 2012-05-19 0:43 ` Steven Penny 2012-05-21 18:43 ` Ramsay Jones 1 sibling, 0 replies; 33+ messages in thread From: Steven Penny @ 2012-05-19 0:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramsay Jones, git, Johannes Sixt On Thu, May 17, 2012 at 9:34 PM, Junio C Hamano wrote: > And from Cygwin Git, your programs (like $EDITOR and hooks) will get POSIX > paths. It is your programs' responsibility to turn them into Windows > paths if/as necessary. I think this is correct. These program should be accepting Cygwin paths and are not for obvious reason, they exist outside the the Cygwin world. For now I will do something like this editor(){ cygpath -m "$1" | xargs notepad } export -f editor git config core.editor editor ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-18 2:34 ` Junio C Hamano 2012-05-19 0:43 ` Steven Penny @ 2012-05-21 18:43 ` Ramsay Jones 2012-05-21 22:24 ` Junio C Hamano 1 sibling, 1 reply; 33+ messages in thread From: Ramsay Jones @ 2012-05-21 18:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Steven Penny, git, Johannes Sixt Junio C Hamano wrote: > Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > >> I guess you won't be shocked to hear that I don't think this patch is >> necessary. :-P > > That is more or less irrelevant, not in the sense that what you say is > irrelevant, but in the sense that something can be worked around in a > different way alone is not a good reason to reject a patch, if its benefit > outweigh its costs. I completely agree. In case it was not clear, I was not suggesting that the patch be rejected on the basis that the problem could be worked around in a different way. > If I speculated in the other message is correct (in short, "In Cygwin > world, Git is compiled to use POSIX paths and would not work with Windows > paths."), I think this "problem" is fundamentally un"fix"able. Yes, Cygwin is essentially just another POSIX target as far as Git is concerned. Cygwin tries hard to provide a POSIX-like environment, but it is not possible for it to completely hide the fact that the base OS is not actually a POSIX system. > And from Cygwin Git, your programs (like $EDITOR and hooks) will get POSIX > paths. It is your programs' responsibility to turn them into Windows > paths if/as necessary. I would say that this is the only sensible way to proceed. However, you could imagine adding code to accommodate external windows programs. If we limit ourselves to the text editor, for example, I could imagine something like the diff attached below to fix up the C based git programs. (You would need to make similar changes to the shell and perl scripts which launch the text editor). I would not like to see a patch based on this (or any others like it) applied, since it is going in the wrong direction. (Why do people use Cygwin git rather than MinGW git and vice versa). Of course, it is not my decision to make ... :-P >> Anyway, I applied this patch tonight to give it a go. The very first test >> I tried failed. I've attached the log of the failing test below. >> Note that it is attempting to use "ssh" to a "host" that ends in ".../C:". > > Of course. That is one typical symptom that suggests my speculation was > correct. Heh, I was surprised that it did as well as it did; after all, it passed 44 out of the 45 tests run (from t7400-submodule-basic.sh). ;-) > So "I don't think this patch is necessary" is irrelevant, but "This patch > is harmful; Git on Cygwin is never supposed to use Windows paths" is very > relevant ;-) I agree. ATB, Ramsay Jones -- >8 -- diff --git a/editor.c b/editor.c index d834003..cf36e62 100644 --- a/editor.c +++ b/editor.c @@ -1,6 +1,9 @@ #include "cache.h" #include "strbuf.h" #include "run-command.h" +#ifdef __CYGWIN__ +# include <sys/cygwin.h> +#endif #ifndef DEFAULT_EDITOR #define DEFAULT_EDITOR "vi" @@ -37,6 +40,12 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; +#ifdef __CYGWIN__ + char win32_path[1024]; + + cygwin_conv_to_full_win32_path(path, win32_path); + args[1] = win32_path; +#endif if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env)) return error("There was a problem with the editor '%s'.", -- 8< -- ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-21 18:43 ` Ramsay Jones @ 2012-05-21 22:24 ` Junio C Hamano 2012-05-24 18:27 ` Ramsay Jones 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2012-05-21 22:24 UTC (permalink / raw) To: Ramsay Jones; +Cc: Steven Penny, git, Johannes Sixt Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > However, you could imagine adding code to accommodate external windows > programs. If we limit ourselves to the text editor, for example, I could > imagine something like the diff attached below to fix up the C based git > programs. (You would need to make similar changes to the shell and perl > scripts which launch the text editor). If you _only_ allow editors that understands windows style paths, your patch may make sense, but doesn't it break editors that wants only POSIX style paths? > > ATB, > Ramsay Jones > > -- >8 -- > diff --git a/editor.c b/editor.c > index d834003..cf36e62 100644 > --- a/editor.c > +++ b/editor.c > @@ -1,6 +1,9 @@ > #include "cache.h" > #include "strbuf.h" > #include "run-command.h" > +#ifdef __CYGWIN__ > +# include <sys/cygwin.h> > +#endif > > #ifndef DEFAULT_EDITOR > #define DEFAULT_EDITOR "vi" > @@ -37,6 +40,12 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en > > if (strcmp(editor, ":")) { > const char *args[] = { editor, path, NULL }; > +#ifdef __CYGWIN__ > + char win32_path[1024]; > + > + cygwin_conv_to_full_win32_path(path, win32_path); > + args[1] = win32_path; > +#endif > > if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env)) > return error("There was a problem with the editor '%s'.", > -- 8< -- ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas 2012-05-21 22:24 ` Junio C Hamano @ 2012-05-24 18:27 ` Ramsay Jones 0 siblings, 0 replies; 33+ messages in thread From: Ramsay Jones @ 2012-05-24 18:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Steven Penny, git, Johannes Sixt Junio C Hamano wrote: > Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes: > >> However, you could imagine adding code to accommodate external windows >> programs. If we limit ourselves to the text editor, for example, I could >> imagine something like the diff attached below to fix up the C based git >> programs. (You would need to make similar changes to the shell and perl >> scripts which launch the text editor). > > If you _only_ allow editors that understands windows style paths, your > patch may make sense, but doesn't it break editors that wants only POSIX > style paths? Yes. (If it wasn't clear, I included the code to show what I *didn't* want to see!). Cygwin built text editors will, of course, work fine with POSIX paths, but may also work with win32 paths as well. For example, vim supports win32 paths just fine (it's the *only* editor I tested). I would not be surprised if vim is not alone in that; I think it mainly depends on whether the editor attempts to "interpret" the parameter, or simply treats it as an opaque token to be passed straight to [f]open() [1]. Equally, I suspect some cygwin built editors will fail miserably (maybe they try to interpret the parameter as an scp-like url, say). I don't know and I'm not about to test every cygwin text editor to find out! So, in my opinion, adding code to explicitly support win32 paths, while possible, is likely to open up a can-o-worms (even if this support were opt-in via config). I would prefer that we don't go there. :-D ATB, Ramsay Jones [1] Which is why git already supports win32 paths to a certain degree. For example, the following will work as expected: $ git config --global core.excludesfile 'C:/cygwin/home/ramsay/.gitignore' (ie git will read and use the given .gitignore file without problem) ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas @ 2012-05-21 23:51 Matt Seitz (matseitz) 0 siblings, 0 replies; 33+ messages in thread From: Matt Seitz (matseitz) @ 2012-05-21 23:51 UTC (permalink / raw) To: git "Junio C Hamano" <gitster@pobox.com> wrote in message news:<7vaa116ulx.fsf@alter.siamese.dyndns.org>... > > If you _only_ allow editors that understands windows style paths, your > patch may make sense, but doesn't it break editors that wants only POSIX > style paths? The Cygwin Users' Guide says "Using native Win32 paths in Cygwin, while possible, is generally inadvisable." http://cygwin.com/cygwin-ug-net/using.html#pathnames-win32 Is this similar to the problem of line ending (POSIX lf vs. Windows cr-lf)? Windows Notepad won't correctly display files with lf line endings, either. So maybe this should be a configuration option, as with line endings? ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2012-05-24 18:30 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-05-06 4:24 Git commit path vs rebase path Steven Penny 2012-05-07 17:27 ` Junio C Hamano 2012-05-08 6:22 ` Johannes Sixt 2012-05-08 6:44 ` Steven Penny 2012-05-08 7:06 ` Johannes Sixt 2012-05-08 7:11 ` Steven Penny 2012-05-08 17:02 ` Junio C Hamano 2012-05-08 17:25 ` Junio C Hamano 2012-05-08 22:47 ` Steven Penny 2012-05-09 21:54 ` Junio C Hamano 2012-05-09 23:14 ` Steven Penny 2012-05-10 18:10 ` Ramsay Jones 2012-05-11 4:35 ` Steven Penny 2012-05-13 22:58 ` Ramsay Jones 2012-05-13 23:42 ` Steven Penny 2012-05-14 6:02 ` Johannes Sixt 2012-05-15 17:32 ` Ramsay Jones 2012-05-16 5:52 ` Johannes Sixt 2012-05-17 18:30 ` Ramsay Jones 2012-05-17 19:19 ` Junio C Hamano 2012-05-16 18:00 ` [PATCH 0/2] " Junio C Hamano 2012-05-16 18:00 ` [PATCH 1/2] git-sh-setup: define workaround wrappers before they are used Junio C Hamano 2012-05-17 22:36 ` Ramsay Jones 2012-05-16 18:00 ` [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas Junio C Hamano 2012-05-16 18:51 ` Steven Penny 2012-05-16 19:02 ` Junio C Hamano 2012-05-17 23:15 ` Ramsay Jones 2012-05-18 2:34 ` Junio C Hamano 2012-05-19 0:43 ` Steven Penny 2012-05-21 18:43 ` Ramsay Jones 2012-05-21 22:24 ` Junio C Hamano 2012-05-24 18:27 ` Ramsay Jones -- strict thread matches above, loose matches on Subject: below -- 2012-05-21 23:51 Matt Seitz (matseitz)
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).