public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Ramsay Jones <ramsay@ramsay1•demon.co.uk>
To: Junio C Hamano <gitster@pobox•com>
Cc: Steven Penny <svnpenn@gmail•com>,
	git@vger•kernel.org, Johannes Sixt <j.sixt@viscovery•net>
Subject: Re: [PATCH 2/2] git-sh-setup: work around Cygwin path handling gotchas
Date: Fri, 18 May 2012 00:15:04 +0100	[thread overview]
Message-ID: <4FB58678.1050009@ramsay1.demon.co.uk> (raw)
In-Reply-To: <7vd364c5kt.fsf@alter.siamese.dyndns.org>

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

  reply	other threads:[~2012-05-17 23:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FB58678.1050009@ramsay1.demon.co.uk \
    --to=ramsay@ramsay1$(echo .)demon.co.uk \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=j.sixt@viscovery$(echo .)net \
    --cc=svnpenn@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox