Git commit path vs rebase path

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.

Steven Penny <svnpenn@gmail.com> writes:

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

Am 5/7/2012 19:27, schrieb Junio C Hamano:

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

Johannes Sixt wrote:

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

Am 5/8/2012 8:44, schrieb Steven Penny:

Are you saying that the new pwd function will also be used on Cygwin? That would be a bug.

-- Hannes

Johannes Sixt wrote:

The linked patch should fix the problem for _MinGW_ users.

The problem will persist with _Cygwin_ users.

Steven Penny <svnpenn@gmail.com> writes:

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.

Junio C Hamano <gitster@pobox.com> writes:

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.

Junio C Hamano wrote:

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

Steven Penny <svnpenn@gmail.com> writes:

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.

Junio C Hamano wrote:

This is what I get

$ uname -s CYGWIN_NT-6.1-WOW64

$ uname -o Cygwin

Steven Penny wrote:

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:

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< --

Ramsay Jones wrote:

So your suggestion is to have git-sh-setup.sh account for MinGW, which is its current state, but not account for Cygwin?

If you had read carefully, you would have noticed that I mentioned more than notepad. As well Notepad2, and Notepad++, etc.

This convo is not about what tools _you_ use, but about the current incompatibility with several native windows text editors.

Steven Penny wrote:

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)

Yes, I did notice that you mentioned more than notepad ...

OK.

So, yes, I didn't give your patch a look; sorry about that. Let's take a look now (quoting from earlier email):

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

Ramsay Jones wrote:

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

Am 5/14/2012 0:58, schrieb Ramsay Jones:

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

Johannes Sixt wrote:

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

Am 5/15/2012 19:32, schrieb Ramsay Jones:

...

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

Johannes Sixt wrote:

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().)

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< --

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

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.

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(-)

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(-)

Junio C Hamano wrote:

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

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(+)

Junio C Hamano wrote:

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

Steven Penny <svnpenn@gmail.com> writes:

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(+)

Junio C Hamano wrote:

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"
#               )
#
$

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

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>

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.

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 ;-)

On Thu, May 17, 2012 at 9:34 PM, Junio C Hamano wrote:

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

Junio C Hamano wrote:

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.

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.
 

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

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). ;-)

I agree.

ATB, Ramsay Jones

-- >8 --

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

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?

Junio C Hamano wrote:

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)