Re: Git commit path vs rebase path

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: May 13, 2012, 22:58 UTC
Message-ID: <4FB03CA1.4030703@ramsay1.demon.co.uk>
In-Reply-To: <CAAXzdLXbYp5YW9cZXxmRJk0MP=6PU897f4nuTe4ipLqk+EH9PQ@mail.gmail.com>

Steven Penny wrote:

Show 5 quoted lines

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

Show 5 quoted lines

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

Show 6 quoted lines

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

Show 18 quoted lines

> 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