threads / patch / 22377

patchHandle UNC paths everywhere

Subject: [PATCH] Handle UNC paths everywhere

## tl;dr

21 messages between Jan 25, 2010 and Jan 26, 2010. Diffs are folded; open one to read it.

replies: 20people: 5as markdown or json

Robin Rosenberg· Jan 25, 2010, 00:55 UTC · lore
>From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Mon, 25 Jan 2010 01:41:03 +0100
Subject: [PATCH] Handle UNC paths everywhere

In Windows paths beginning with // are knows as UNC paths. They are absolute paths, usually referring to a shared resource on a server.

Examples of legal UNC paths
	\\hub\repos\repo
	\\?\unc\hub\repos
	\\?\d:\repo
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 cache.h           |    2 +-
 compat/basename.c |    2 +-
 compat/mingw.h    |    8 +++++++-
 connect.c         |    2 +-
 git-compat-util.h |    9 +++++++++
 path.c            |    2 +-
 setup.c           |    2 +-
 sha1_file.c       |   20 ++++++++++++++++++++
 transport.c       |    2 +-
 9 files changed, 42 insertions(+), 7 deletions(-)
Show changes to 9 files +42 −7

cache.h, compat/basename.c, compat/mingw.h, connect.c, git-compat-util.h, path.c, setup.c, sha1_file.c, transport.c

diff --git a/cache.h b/cache.h
index 767a50e..8f63640 100644
--- a/cache.h
+++ b/cache.h
@@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char 
*path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
-	return path[0] == '/' || has_dos_drive_prefix(path);
+	return path[0] == '/' || has_win32_abs_prefix(path);
 }
 int is_directory(const char *);
 const char *make_absolute_path(const char *path);
diff --git a/compat/basename.c b/compat/basename.c
index d8f8a3c..c1d81f6 100644
--- a/compat/basename.c
+++ b/compat/basename.c
@@ -5,7 +5,7 @@ char *gitbasename (char *path)
 {
 	const char *base;
 	/* Skip over the disk name in MSDOS pathnames. */
-	if (has_dos_drive_prefix(path))
+	if (has_win32_abs_prefix(path))
 		path += 2;
 	for (base = path; *path; path++) {
 		if (is_dir_sep(*path))
diff --git a/compat/mingw.h b/compat/mingw.h
index 1b528da..d1aa8be 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char *format, 
...) __attribute__((format
  * git specific compatibility
  */
 
-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
+#define has_dos_drive_prefix(path) \
+	(isalpha(*(path)) && (path)[1] == ':')
+#define has_unc_prefix(path) \
+	(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
+#define has_win32_abs_prefix(path) \
+	(has_dos_drive_prefix(path) || has_unc_prefix(path))
+
 #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
 #define PATH_SEP ';'
 #define PRIuMAX "I64u"
diff --git a/connect.c b/connect.c
index 7945e38..9d4556c 100644
--- a/connect.c
+++ b/connect.c
@@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char 
*url_orig,
 		end = host;
 
 	path = strchr(end, c);
-	if (path && !has_dos_drive_prefix(end)) {
+	if (path && !has_win32_abs_prefix(end)) {
 		if (c == ':') {
 			protocol = PROTO_SSH;
 			*path++ = '\0';
diff --git a/git-compat-util.h b/git-compat-util.h
index ef60803..0de9dac 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -170,6 +170,15 @@ extern char *gitbasename(char *);
 #define has_dos_drive_prefix(path) 0
 #endif
 
+#ifndef has_unc_prefix
+#define has_unc_prefix(path) 0
+#endif
+
+#ifndef has_win32_abs_prefix
+#error no abs
+#define has_win32_abs_prefix(path) 0
+#endif
+
 #ifndef is_dir_sep
 #define is_dir_sep(c) ((c) == '/')
 #endif
diff --git a/path.c b/path.c
index 047fdb0..79451a2 100644
--- a/path.c
+++ b/path.c
@@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)
 {
 	char *dst0;
 
-	if (has_dos_drive_prefix(src)) {
+	if (has_win32_abs_prefix(src)) {
 		*dst++ = *src++;
 		*dst++ = *src++;
 	}
diff --git a/setup.c b/setup.c
index 029371e..4f72817 100644
--- a/setup.c
+++ b/setup.c
@@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		die_errno("Unable to read current working directory");
 
 	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
-	if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
+	if (ceil_offset < 0 && has_win32_abs_prefix(cwd))
 		ceil_offset = 1;
 
 	/*
diff --git a/sha1_file.c b/sha1_file.c
index 4cc8939..f1ad3f5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -87,6 +87,26 @@ static inline int offset_1st_component(const char *path)
 {
 	if (has_dos_drive_prefix(path))
 		return 2 + (path[2] == '/');
+	if (has_unc_prefix(path)) {
+		int p = 2;
+		int skip;
+		if (path[p] == '?') {
+			if (path[p+1] && !has_dos_drive_prefix(path+p+2)) {
+				skip = 3;
+			} else {
+				skip = 2;
+			}
+		} else
+			skip = 2;
+
+		while (skip && path[p]) {
+			if (is_dir_sep(path[p]))
+				--skip;
+			++p;
+		}
+		printf("Left with %s\n", path+p);
+		return p;
+	}
 	return *path == '/';
 }
 
diff --git a/transport.c b/transport.c
index 644a30a..9f5b24e 100644
--- a/transport.c
+++ b/transport.c
@@ -797,7 +797,7 @@ static int is_local(const char *url)
 	const char *colon = strchr(url, ':');
 	const char *slash = strchr(url, '/');
 	return !colon || (slash && slash < colon) ||
-		has_dos_drive_prefix(url);
+		has_win32_abs_prefix(url);
 }
 
 static int is_file(const char *url)
-- 
1.6.4.msysgit.0.598.g37a74
Sverre Rabbelier· Jan 25, 2010, 09:36 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

Heya,

On Mon, Jan 25, 2010 at 01:55, Robin Rosenberg <robin.rosenberg@dewire.com> wrote:

> In Windows paths beginning with // are knows as UNC paths. They are
> absolute paths, usually referring to a shared resource on a server.

Cute, but will it actually work? I've tried to use them // paths on windows before with MSysGit, and it's never worked, probably due to the same reason why it doesn't work in the cmd prompt (whatever reason that may be).

-- 
Cheers,

Sverre Rabbelier
Robin Rosenberg· Jan 25, 2010, 09:58 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 10.36.59 skrev  Sverre Rabbelier:
Show 10 quoted lines
> Heya,
> 
> On Mon, Jan 25, 2010 at 01:55, Robin Rosenberg
> 
> <robin.rosenberg@dewire.com> wrote:
> > In Windows paths beginning with // are knows as UNC paths. They are
> > absolute paths, usually referring to a shared resource on a server.
> 
> Cute, but will it actually work? I've tried to use them // paths on
> windows before with MSysGit, and it's never worked, probably due to
Works here (tm). Latest msygit + rebuilt git binaries on Windows 2003.

The only program I know in the msysgit installation that didn't accept UNC paths prior to this patch was in fact git.

> the same reason why it doesn't work in the cmd prompt (whatever reason
> that may be).
Any code of the this form will of course not support UNC paths:
if (!has_drive_letter(path))
	boom();
-- robin
Erik Faye-Lund· Jan 25, 2010, 10:11 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] Handle UNC paths everywhere

On Mon, Jan 25, 2010 at 10:36 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
Show 12 quoted lines
> Heya,
>
> On Mon, Jan 25, 2010 at 01:55, Robin Rosenberg
> <robin.rosenberg@dewire.com> wrote:
>> In Windows paths beginning with // are knows as UNC paths. They are
>> absolute paths, usually referring to a shared resource on a server.
>
> Cute, but will it actually work? I've tried to use them // paths on
> windows before with MSysGit, and it's never worked, probably due to
> the same reason why it doesn't work in the cmd prompt (whatever reason
> that may be).
>
This works for me, Vista 64-bit:

C:\Users\kusma>dir \\mongo\code The request is not supported.

C:\Users\kusma>explorer \\mongo\code <login on the gui>

C:\Users\kusma>dir \\mongo\code
 Volume in drive \\mongo\code is Code
 Volume Serial Number is 04C3-0225
 Directory of \\mongo\code
10.01.2010  21:39    <DIR>          .
04.01.2010  01:28    <DIR>          ..
04.01.2010  00:51    <DIR>          qt-rocket
10.01.2010  21:38    <DIR>          very_last_64k_ever
11.01.2010  19:57    <DIR>          scratch
               0 File(s)              0 bytes
               5 Dir(s)  958 499 627 008 bytes free
C:\Users\kusma>
-- 
Erik "kusma" Faye-Lund
Sverre Rabbelier· Jan 25, 2010, 10:22 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] Handle UNC paths everywhere

Heya,
On Mon, Jan 25, 2010 at 11:11, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
Show 9 quoted lines
> C:\Users\kusma>dir \\mongo\code
> The request is not supported.
>
> C:\Users\kusma>explorer \\mongo\code
> <login on the gui>
>
> C:\Users\kusma>dir \\mongo\code
>  Volume in drive \\mongo\code is Code
>  Volume Serial Number is 04C3-0225
Ah, that's very interesting. Not sure that will help MSysGit a lot though.
-- 
Cheers,

Sverre Rabbelier
Robin Rosenberg· Jan 25, 2010, 11:01 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 11.22.19 skrev  Sverre Rabbelier:
> Heya,
> 
> On Mon, Jan 25, 2010 at 11:11, Erik Faye-Lund <kusmabite@googlemail.com> 
wrote:
Show 12 quoted lines
> > C:\Users\kusma>dir \\mongo\code
> > The request is not supported.
> >
> > C:\Users\kusma>explorer \\mongo\code
> > <login on the gui>
> >
> > C:\Users\kusma>dir \\mongo\code
> >  Volume in drive \\mongo\code is Code
> >  Volume Serial Number is 04C3-0225
> 
> Ah, that's very interesting. Not sure that will help MSysGit a lot though.
> 

Could you perhaps *try* it before claiming it won't work? I suggest you use forward slashes to avoid quoting problems.

-- robin
Sverre Rabbelier· Jan 25, 2010, 11:06 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

Heya,

On Mon, Jan 25, 2010 at 12:01, Robin Rosenberg <robin.rosenberg@dewire.com> wrote:

Show 5 quoted lines
>> Ah, that's very interesting. Not sure that will help MSysGit a lot though.
>>
>
> Could you perhaps *try* it before claiming it won't work? I suggest you
> use forward slashes to avoid quoting problems.

Actually, I can't, cos I don't have a MSysGit build environment, I'm just saying that in the environment I tested it in, I didn't have to log in, so I suspect that it won't work, I'm not claiming anything.

-- 
Cheers,

Sverre Rabbelier
Erik Faye-Lund· Jan 25, 2010, 11:17 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] Handle UNC paths everywhere

On Mon, Jan 25, 2010 at 12:06 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
Show 14 quoted lines
> Heya,
>
> On Mon, Jan 25, 2010 at 12:01, Robin Rosenberg
> <robin.rosenberg@dewire.com> wrote:
>>> Ah, that's very interesting. Not sure that will help MSysGit a lot though.
>>>
>>
>> Could you perhaps *try* it before claiming it won't work? I suggest you
>> use forward slashes to avoid quoting problems.
>
> Actually, I can't, cos I don't have a MSysGit build environment, I'm
> just saying that in the environment I tested it in, I didn't have to
> log in, so I suspect that it won't work, I'm not claiming anything.
>

For shares that doesn't require login, I can list files without doing anything. Remember that Windows might try some saved password in the background when you open the folder in explorer, so not seeing a password-prompt might not be the same thing as the share not needing login.

Perhaps "net use \\mongo\code" would have been a better example than using explorer? I don't have the setup where I am right now to test that it does what I expect it to, though.

-- 
Erik "kusma" Faye-Lund
Johannes Schindelin· Jan 25, 2010, 17:34 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

Hi,
On Mon, 25 Jan 2010, Robin Rosenberg wrote:
Show 7 quoted lines
> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> Date: Mon, 25 Jan 2010 01:41:03 +0100
> Subject: [PATCH] Handle UNC paths everywhere
> 
> In Windows paths beginning with // are knows as UNC paths. They are
> absolute paths, usually referring to a shared resource on a server.
And even a simple "cd" with them does not work.
Show 18 quoted lines
> Examples of legal UNC paths
> 
> 	\\hub\repos\repo
> 	\\?\unc\hub\repos
> 	\\?\d:\repo
> 
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  cache.h           |    2 +-
>  compat/basename.c |    2 +-
>  compat/mingw.h    |    8 +++++++-
>  connect.c         |    2 +-
>  git-compat-util.h |    9 +++++++++
>  path.c            |    2 +-
>  setup.c           |    2 +-
>  sha1_file.c       |   20 ++++++++++++++++++++
>  transport.c       |    2 +-
>  9 files changed, 42 insertions(+), 7 deletions(-)

Ouch. You should know better than to clutter non-Windows-specific parts with that ugly kludge.

Show 11 quoted lines
> diff --git a/cache.h b/cache.h
> index 767a50e..8f63640 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char 
> *path);
>  char *enter_repo(char *path, int strict);
>  static inline int is_absolute_path(const char *path)
>  {
> -	return path[0] == '/' || has_dos_drive_prefix(path);
> +	return path[0] == '/' || has_win32_abs_prefix(path);
Why?  We can still keep the name.  Well, maybe not, see below.
Show 12 quoted lines
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 1b528da..d1aa8be 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char *format, 
> ...) __attribute__((format
>   * git specific compatibility
>   */
>  
> -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
> +#define has_dos_drive_prefix(path) \
> +	(isalpha(*(path)) && (path)[1] == ':')
Why?
> +#define has_unc_prefix(path) \
> +	(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
> +#define has_win32_abs_prefix(path) \
> +	(has_dos_drive_prefix(path) || has_unc_prefix(path))
"c:hello.txt" is not an absolute path.
Show 12 quoted lines
> diff --git a/connect.c b/connect.c
> index 7945e38..9d4556c 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char 
> *url_orig,
>  		end = host;
>  
>  	path = strchr(end, c);
> -	if (path && !has_dos_drive_prefix(end)) {
> +	if (path && !has_win32_abs_prefix(end)) {
>  		if (c == ':') {
Why?  Do we really have to exclude UNC paths from that ":" handling?
Show 14 quoted lines
> diff --git a/git-compat-util.h b/git-compat-util.h
> index ef60803..0de9dac 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -170,6 +170,15 @@ extern char *gitbasename(char *);
>  #define has_dos_drive_prefix(path) 0
>  #endif
>  
> +#ifndef has_unc_prefix
> +#define has_unc_prefix(path) 0
> +#endif
> +
> +#ifndef has_win32_abs_prefix
> +#error no abs
Yeah, sure.  I do have abs, thank you very much.

In general, I am _very_ worried about your patch. It does not acknowledge that there is a fundamental difference between DOS drive prefixes and UNC paths, and not being able to "cd" to the latter is just a symptom.

I am also not quite sure if you can get away with having the same offset for both: if I have "C:\blah" and strip off "C:", I always have a directory separator to bounce against, whereas I do not have that if I strip off the two "\\" of a UNC path. Besides, I maintain that the host name, and maybe even the share name, should not ever be stripped off!

At least a discussion is missing from the commit message.

Ciao, Dscho

Erik Faye-Lund· Jan 25, 2010, 17:57 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Handle UNC paths everywhere

On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 14 quoted lines
> Hi,
>
> On Mon, 25 Jan 2010, Robin Rosenberg wrote:
>
>> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
>> From: Robin Rosenberg <robin.rosenberg@dewire.com>
>> Date: Mon, 25 Jan 2010 01:41:03 +0100
>> Subject: [PATCH] Handle UNC paths everywhere
>>
>> In Windows paths beginning with // are knows as UNC paths. They are
>> absolute paths, usually referring to a shared resource on a server.
>
> And even a simple "cd" with them does not work.
>

But it does, at least for me - both in bash and cmd.exe. I just need to log on to the server first, unless it's a public share.

-- 
Erik "kusma" Faye-Lund
Johannes Schindelin· Jan 25, 2010, 18:19 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] Handle UNC paths everywhere

Hi,
On Mon, 25 Jan 2010, Erik Faye-Lund wrote:
Show 19 quoted lines
> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> >
> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> >> Date: Mon, 25 Jan 2010 01:41:03 +0100
> >> Subject: [PATCH] Handle UNC paths everywhere
> >>
> >> In Windows paths beginning with // are knows as UNC paths. They are
> >> absolute paths, usually referring to a shared resource on a server.
> >
> > And even a simple "cd" with them does not work.
> >
> 
> But it does, at least for me - both in bash and cmd.exe. I just need
> to log on to the server first, unless it's a public share.
I love it when people say "it works for me, so let's do it".
_My_ _only_ instance of Windows cmd says this:
	C:\Blah> cd \\localhost
	'\\localhost'
	CMD does not support UNC paths as current directories.
	
	C:\Blah>
So.

Besides, the patch was not in a form where I can say that it was obviously fixing the issue. It was rather in a form where I would have to have set aside a substantial amount of time to verify that nothing undesired was introduced as a side effect.

Ciao, Dscho

Erik Faye-Lund· Jan 25, 2010, 19:45 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Handle UNC paths everywhere

On Mon, Jan 25, 2010 at 7:19 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 35 quoted lines
> Hi,
>
> On Mon, 25 Jan 2010, Erik Faye-Lund wrote:
>
>> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi,
>> >
>> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
>> >
>> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
>> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>
>> >> Date: Mon, 25 Jan 2010 01:41:03 +0100
>> >> Subject: [PATCH] Handle UNC paths everywhere
>> >>
>> >> In Windows paths beginning with // are knows as UNC paths. They are
>> >> absolute paths, usually referring to a shared resource on a server.
>> >
>> > And even a simple "cd" with them does not work.
>> >
>>
>> But it does, at least for me - both in bash and cmd.exe. I just need
>> to log on to the server first, unless it's a public share.
>
> I love it when people say "it works for me, so let's do it".
>
> _My_ _only_ instance of Windows cmd says this:
>
>        C:\Blah> cd \\localhost
>        '\\localhost'
>        CMD does not support UNC paths as current directories.
>
>        C:\Blah>
>
> So.

Actually, you're right about cmd.exe - I somehow mixed that up. However, it works fine in bash (and simply by doing chdir() from a normal C-program), as long as I've logged on in advance.

This applies to my Vista 64 and XP installations.
Show 6 quoted lines
>
> Besides, the patch was not in a form where I can say that it was obviously
> fixing the issue. It was rather in a form where I would have to have set
> aside a substantial amount of time to verify that nothing undesired was
> introduced as a side effect.
>

This I can agree on. I just wanted to clear up the situation about cd'ing. But I failed - hopefully that's corrected now.

-- 
Erik "kusma" Faye-Lund
Robin Rosenberg· Jan 25, 2010, 19:37 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 18.57.06 skrev  Erik Faye-Lund:
Show 18 quoted lines
> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
> 
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> >>
> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> >> Date: Mon, 25 Jan 2010 01:41:03 +0100
> >> Subject: [PATCH] Handle UNC paths everywhere
> >>
> >> In Windows paths beginning with // are knows as UNC paths. They are
> >> absolute paths, usually referring to a shared resource on a server.
> >
> > And even a simple "cd" with them does not work.
> 
> But it does, at least for me - both in bash and cmd.exe. I just needt.

In cmd,exe surprises me a bit. pushd \\server\share is not the same as it maps a drive and then uses it to cd.

-- robin
Erik Faye-Lund· Jan 25, 2010, 19:48 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

On Mon, Jan 25, 2010 at 8:37 PM, Robin Rosenberg <robin.rosenberg@dewire.com> wrote:

Show 14 quoted lines
> måndagen den 25 januari 2010 18.57.06 skrev  Erik Faye-Lund:
>> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
>>
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > And even a simple "cd" with them does not work.
>>
>> But it does, at least for me - both in bash and cmd.exe. I just needt.
>
> In cmd,exe surprises me a bit. pushd \\server\share is not the same
> as it maps a drive and then uses it to cd.
>
> -- robin
>

My guess would be that Dscho mentioned this because git internally does a chdir() to the path that is cloned, so currently chdir'ing must be supported for clone to work with a "local repo".

-- 
Erik "kusma" Faye-Lund
Robin Rosenberg· Jan 25, 2010, 20:15 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 20.48.22 skrev  Erik Faye-Lund:
Show 18 quoted lines
> On Mon, Jan 25, 2010 at 8:37 PM, Robin Rosenberg
> 
> <robin.rosenberg@dewire.com> wrote:
> > måndagen den 25 januari 2010 18.57.06 skrev  Erik Faye-Lund:
> >> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin
> >>
> >> <Johannes.Schindelin@gmx.de> wrote:
> >> > And even a simple "cd" with them does not work.
> >>
> >> But it does, at least for me - both in bash and cmd.exe. I just needt.
> >
> > In cmd,exe surprises me a bit. pushd \\server\share is not the same
> > as it maps a drive and then uses it to cd.
> >
> > -- robin
> 
> My guess would be that Dscho mentioned this because git internally
> does a chdir() to the path that is cloned, so currently chdir'ing must
chdir doesn't call cmd.exe and does not suffer from cmd's limitations.
-- robin
Robin Rosenberg· Jan 25, 2010, 19:45 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:
Show 49 quoted lines
> Hi,
> 
> On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> > >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> >
> > From: Robin Rosenberg <robin.rosenberg@dewire.com>
> > Date: Mon, 25 Jan 2010 01:41:03 +0100
> > Subject: [PATCH] Handle UNC paths everywhere
> >
> > In Windows paths beginning with // are knows as UNC paths. They are
> > absolute paths, usually referring to a shared resource on a server.
> 
> And even a simple "cd" with them does not work.
> 
> > Examples of legal UNC paths
> >
> > 	\\hub\repos\repo
> > 	\\?\unc\hub\repos
> > 	\\?\d:\repo
> >
> > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> > ---
> >  cache.h           |    2 +-
> >  compat/basename.c |    2 +-
> >  compat/mingw.h    |    8 +++++++-
> >  connect.c         |    2 +-
> >  git-compat-util.h |    9 +++++++++
> >  path.c            |    2 +-
> >  setup.c           |    2 +-
> >  sha1_file.c       |   20 ++++++++++++++++++++
> >  transport.c       |    2 +-
> >  9 files changed, 42 insertions(+), 7 deletions(-)
> 
> Ouch.  You should know better than to clutter non-Windows-specific parts
> with that ugly kludge.
> 
> > diff --git a/cache.h b/cache.h
> > index 767a50e..8f63640 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> > *path);
> >  char *enter_repo(char *path, int strict);
> >  static inline int is_absolute_path(const char *path)
> >  {
> > -	return path[0] == '/' || has_dos_drive_prefix(path);
> > +	return path[0] == '/' || has_win32_abs_prefix(path);
> 
> Why?  We can still keep the name.  Well, maybe not, see below.
I do think function names should imply something about their behaviour.
Show 15 quoted lines
> 
> > diff --git a/compat/mingw.h b/compat/mingw.h
> > index 1b528da..d1aa8be 100644
> > --- a/compat/mingw.h
> > +++ b/compat/mingw.h
> > @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char
> > *format, ...) __attribute__((format
> >   * git specific compatibility
> >   */
> >
> > -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] ==
> > ':') +#define has_dos_drive_prefix(path) \
> > +	(isalpha(*(path)) && (path)[1] == ':')
> 
> Why?

To avoid very long lines and format this (now) set of related macros uniformely.

Show 6 quoted lines
> > +#define has_unc_prefix(path) \
> > +	(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
> > +#define has_win32_abs_prefix(path) \
> > +	(has_dos_drive_prefix(path) || has_unc_prefix(path))
> 
> "c:hello.txt" is not an absolute path.

Ok. Nevertheless that was how it was treated before, It's not relative, either, but some quasirelative thing. has_win32_quasi_abs_prefix?

Show 14 quoted lines
> > diff --git a/connect.c b/connect.c
> > index 7945e38..9d4556c 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const
> > char *url_orig,
> >  		end = host;
> >
> >  	path = strchr(end, c);
> > -	if (path && !has_dos_drive_prefix(end)) {
> > +	if (path && !has_win32_abs_prefix(end)) {
> >  		if (c == ':') {
> 
> Why?  Do we really have to exclude UNC paths from that ":" handling?
That colon is about URL-ish things... Right.
Show 14 quoted lines
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index ef60803..0de9dac 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -170,6 +170,15 @@ extern char *gitbasename(char *);
> >  #define has_dos_drive_prefix(path) 0
> >  #endif
> >
> > +#ifndef has_unc_prefix
> > +#define has_unc_prefix(path) 0
> > +#endif
> > +
> > +#ifndef has_win32_abs_prefix
> > +#error no abs
ouch, a leftover from trying to figure out a complation message. 
Show 6 quoted lines
> 
> Yeah, sure.  I do have abs, thank you very much.
> 
> In general, I am _very_ worried about your patch.  It does not acknowledge
> that there is a fundamental difference between DOS drive prefixes and UNC
> paths, and not being able to "cd" to the latter is just a symptom.

As I said. Most programs including bash, but excluding cmd.exe can set the working directory to an UNC path. I cannot fix cmd.exe and rarely use it with git, but the patch helps even if you cannot cd from a UNC challenged shell.

Show 5 quoted lines
> I am also not quite sure if you can get away with having the same offset
> for both: if I have "C:\blah" and strip off "C:", I always have a
> directory separator to bounce against, whereas I do not have that if I
> strip off the two "\\" of a UNC path.  Besides, I maintain that the host
> name, and maybe even the share name, should not ever be stripped off!

When creating directoties you only strip them off for the purpose of finding paths to mkdir. The server and share part you cannot mkdir anyway, they must exist before attempting to create a directory, hence I skip past those portions. As for the \-less path beginning with a drive I'll reconsider. I did not test that one.

-- robin
Johannes Schindelin· Jan 26, 2010, 10:59 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

Hi,
thanks for Cc:ing me this time.
On Mon, 25 Jan 2010, Robin Rosenberg wrote:
Show 35 quoted lines
> måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:
> 
> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> > > >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> > >
> > > From: Robin Rosenberg <robin.rosenberg@dewire.com>
> > > Date: Mon, 25 Jan 2010 01:41:03 +0100
> > > Subject: [PATCH] Handle UNC paths everywhere
> > >
> > > In Windows paths beginning with // are knows as UNC paths. They are
> > > absolute paths, usually referring to a shared resource on a server.
> > 
> > And even a simple "cd" with them does not work.
> > 
> > > Examples of legal UNC paths
> > >
> > > 	\\hub\repos\repo
> > > 	\\?\unc\hub\repos
> > > 	\\?\d:\repo
> > >
> > > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> > > ---
> > >  cache.h           |    2 +-
> > >  compat/basename.c |    2 +-
> > >  compat/mingw.h    |    8 +++++++-
> > >  connect.c         |    2 +-
> > >  git-compat-util.h |    9 +++++++++
> > >  path.c            |    2 +-
> > >  setup.c           |    2 +-
> > >  sha1_file.c       |   20 ++++++++++++++++++++
> > >  transport.c       |    2 +-
> > >  9 files changed, 42 insertions(+), 7 deletions(-)
> > 
> > Ouch.  You should know better than to clutter non-Windows-specific parts
> > with that ugly kludge.

I did suspect that it would be better to handle unc paths differently than the dos prefix, but I did not have the time to form arguments like Hannes did.

So I think the first order of business is to add new code paths, rather than modify existing ones. And then you can '#define network_path_prefix_length(name) 0' in git-compat-util.h so that on non-Windows platforms (i.e. our 1st class citizens), the code path can be optimized out.

Show 15 quoted lines
> > > diff --git a/cache.h b/cache.h
> > > index 767a50e..8f63640 100644
> > > --- a/cache.h
> > > +++ b/cache.h
> > > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> > > *path);
> > >  char *enter_repo(char *path, int strict);
> > >  static inline int is_absolute_path(const char *path)
> > >  {
> > > -	return path[0] == '/' || has_dos_drive_prefix(path);
> > > +	return path[0] == '/' || has_win32_abs_prefix(path);
> > 
> > Why?  We can still keep the name.  Well, maybe not, see below.
> 
> I do think function names should imply something about their behaviour.

Actually, in this case, you do not even need to change anything, as Hannes pointed out.

Show 17 quoted lines
> > > diff --git a/compat/mingw.h b/compat/mingw.h
> > > index 1b528da..d1aa8be 100644
> > > --- a/compat/mingw.h
> > > +++ b/compat/mingw.h
> > > @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char
> > > *format, ...) __attribute__((format
> > >   * git specific compatibility
> > >   */
> > >
> > > -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] ==
> > > ':') +#define has_dos_drive_prefix(path) \
> > > +	(isalpha(*(path)) && (path)[1] == ':')
> > 
> > Why?
> 
> To avoid very long lines and format this (now) set of related macros 
> uniformely.

If you want your patch to go in (which you probably did not, you just forgot to prefix the subject with RFC or RFH), you need it to be reviewed. It is not a good idea to distract reviewers. Such a change does so.

Show 8 quoted lines
> > > +#define has_unc_prefix(path) \
> > > +	(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
> > > +#define has_win32_abs_prefix(path) \
> > > +	(has_dos_drive_prefix(path) || has_unc_prefix(path))
> > 
> > "c:hello.txt" is not an absolute path.
> Ok. Nevertheless that was how it was treated before, It's not relative,
> either, but some quasirelative thing. has_win32_quasi_abs_prefix?

No, none of this is good. You should not even pretend that the unc prefix and the DOS drive prefix are the same. Just leave the old code paths alone.

Show 15 quoted lines
> > > diff --git a/git-compat-util.h b/git-compat-util.h
> > > index ef60803..0de9dac 100644
> > > --- a/git-compat-util.h
> > > +++ b/git-compat-util.h
> > > @@ -170,6 +170,15 @@ extern char *gitbasename(char *);
> > >  #define has_dos_drive_prefix(path) 0
> > >  #endif
> > >
> > > +#ifndef has_unc_prefix
> > > +#define has_unc_prefix(path) 0
> > > +#endif
> > > +
> > > +#ifndef has_win32_abs_prefix
> > > +#error no abs
> ouch, a leftover from trying to figure out a complation message. 
Thought so.
Show 9 quoted lines
> > In general, I am _very_ worried about your patch.  It does not 
> > acknowledge that there is a fundamental difference between DOS drive 
> > prefixes and UNC paths, and not being able to "cd" to the latter is 
> > just a symptom.
> 
> As I said. Most programs including bash, but excluding cmd.exe can set 
> the working directory to an UNC path. I cannot fix cmd.exe and rarely 
> use it with git, but the patch helps even if you cannot cd from a UNC 
> challenged shell.

The point I tried to make is that you should treat UNC paths differently, and you can even leave the door open for non-Windows stuff. Just call the function network_path_prefix_length() returning the length of the prefix. If it becomes standard, say, on Linux, to have support for smb:// style paths, we can always add support for that, too, and do not have to change the name yet again.

Plus, by having separate code paths, you can be sure that you do not break the existing ones.

And by defining network_path_prefix_length(path) to 0, the new code paths can be optimized out on platforms which do not support network paths.

Show 11 quoted lines
> > I am also not quite sure if you can get away with having the same 
> > offset for both: if I have "C:\blah" and strip off "C:", I always have 
> > a directory separator to bounce against, whereas I do not have that if 
> > I strip off the two "\\" of a UNC path.  Besides, I maintain that the 
> > host name, and maybe even the share name, should not ever be stripped 
> > off!
> 
> When creating directoties you only strip them off for the purpose of 
> finding paths to mkdir. The server and share part you cannot mkdir 
> anyway, they must exist before attempting to create a directory, hence I 
> skip past those portions.

I must have missed that. From what I saw, you treat the offset to be the same as for DOS drive paths: 2 characters, which is definitely not enough.

Unfortunately, this patch needs more revisions, so it will probably not make it into the upcoming Git for Windows, I am afraid. But then, we do not need to let 3 months happen without a Git for Windows release.

Ciao, Dscho

Robin Rosenberg· Jan 25, 2010, 20:04 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:
Show 5 quoted lines
> I am also not quite sure if you can get away with having the same offset
> for both: if I have "C:\blah" and strip off "C:", I always have a
> directory separator to bounce against, whereas I do not have that if I
> strip off the two "\\" of a UNC path.  Besides, I maintain that the host
> name, and maybe even the share name, should not ever be stripped off!
Advices needed:

d:somedir (when cwd=d:\msysgit, == /) may be tricky to fix. Msysgit seems confused by the syntax and treats it as d:\

roro@SIENA / (master) $ cmd Microsoft Windows [Version 5.2.3790] (C) Copyright 1985-2003 Microsoft Corp.

D:\msysgit>exit

roro@SIENA / (master) $ mkdir d:x mkdir: cannot create directory `d:x': File exist

roro@SIENA / (master) $ cd d:x

roro@SIENA /d $ ls -l x ls: x: No such file or directory

roro@SIENA /d $

From that I think that even if we try to make git handle d:path, msys will break regardless. We can fix truly absolute and normal relative paths.

-- robin
Johannes Schindelin· Jan 26, 2010, 11:01 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

Hi,
On Mon, 25 Jan 2010, Robin Rosenberg wrote:
Show 10 quoted lines
> måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:
> > I am also not quite sure if you can get away with having the same offset
> > for both: if I have "C:\blah" and strip off "C:", I always have a
> > directory separator to bounce against, whereas I do not have that if I
> > strip off the two "\\" of a UNC path.  Besides, I maintain that the host
> > name, and maybe even the share name, should not ever be stripped off!
> 
> Advices needed:
> 
> d:somedir (when cwd=d:\msysgit, == /) may be tricky to fix.

This is a totally different issue from the UNC issue you brought up. May I suggest to fix the UNC thing first, and if you feel inclined, take care of the relative DOS drive path afterwards?

The relative DOS drive path is also not something I find overly pressing, as I work from the Git bash, as you know, which does not translate POSIX-style paths into such relative DOS drive paths.

Ciao, Dscho

Johannes Sixt· Jan 25, 2010, 20:07 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Handle UNC paths everywhere

On Montag, 25. Januar 2010, Robin Rosenberg wrote:
Show 8 quoted lines
> In Windows paths beginning with // are knows as UNC paths. They are
> absolute paths, usually referring to a shared resource on a server.
>
> Examples of legal UNC paths
>
> 	\\hub\repos\repo
> 	\\?\unc\hub\repos
> 	\\?\d:\repo
I agree that that the problem that you are addressing needs a solution.

However, the solution is not a whole-sale replacement of have_dos_drive_prefix() by a function that is only a tiny bit fancier. Accompanying changes are needed, and perhaps more code locations need change.

Show 7 quoted lines
> @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> *path);
>  char *enter_repo(char *path, int strict);
>  static inline int is_absolute_path(const char *path)
>  {
> -	return path[0] == '/' || has_dos_drive_prefix(path);
> +	return path[0] == '/' || has_win32_abs_prefix(path);

Perhaps we need is_dir_sep(path[0]) here? But since I have not observed any breakage in connection with this code, I think that all callers feed only normalized paths (i.e. with forward slash). (Note that our getcwd() implementation converts backslashes to forward slashes.) This means that a full-fledged check is not needed.

Show 7 quoted lines
> @@ -5,7 +5,7 @@ char *gitbasename (char *path)
>  {
>  	const char *base;
>  	/* Skip over the disk name in MSDOS pathnames. */
> -	if (has_dos_drive_prefix(path))
> +	if (has_win32_abs_prefix(path))
>  		path += 2;

This change is unnecessary; it really is only to skip an initial driver prefix. If you want to support \\?\X: style paths, more work is needed here so that you do not return X: or ? as the basename.

> +#define has_win32_abs_prefix(path) \
Do we really have to name everything "win32" when it is about Windows?
Show 7 quoted lines
> @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char
> *url_orig,
>  		end = host;
>
>  	path = strchr(end, c);
> -	if (path && !has_dos_drive_prefix(end)) {
> +	if (path && !has_win32_abs_prefix(end)) {

This change is wrong because the check is really only about the drive prefix: It checks that we do not mistake c:/foo as a ssh connection to host c, path /foo. Yes, it does mean that on Windows we cannot have remotes to hosts whose name consists only of a single letter using the rcp notation (you must say ssh://c/foo if you mean it).

Show 9 quoted lines
> @@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)
>  {
>  	char *dst0;
>
> -	if (has_dos_drive_prefix(src)) {
> +	if (has_win32_abs_prefix(src)) {
>  		*dst++ = *src++;
>  		*dst++ = *src++;
>  	}
Is skipping just two characters for \\ or \\?\whatever paths the right thing?
Show 7 quoted lines
> @@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  		die_errno("Unable to read current working directory");
>
>  	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
> -	if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
> +	if (ceil_offset < 0 && has_win32_abs_prefix(cwd))
>  		ceil_offset = 1;

I doubt that this is correct. The purpose of this check is that "c:/" is the last directory that is checked (on Unix it would be "/") when path components are stripped from cwd. For UNC paths this must be adjusted depending on how you want to support \\server\share and \\?\c:\paths: You do not want to check whether \\server\.git or \\.git or \\?\.git are git directories.

Show 8 quoted lines
> --- a/transport.c
> +++ b/transport.c
> @@ -797,7 +797,7 @@ static int is_local(const char *url)
>  	const char *colon = strchr(url, ':');
>  	const char *slash = strchr(url, '/');
>  	return !colon || (slash && slash < colon) ||
> -		has_dos_drive_prefix(url);
> +		has_win32_abs_prefix(url);

This check is again to not mistake c:/foo as rcp style connection. No change needed.

As I said, changes to other parts are perhaps also needed, most prominently, make_relative_path() that prompted this patch. What about make_absolute_path() and make_non_relative_path()?

-- Hannes
Robin Rosenberg· Jan 25, 2010, 21:42 UTC · re: Johannes Sixt · lore

Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 21.07.44 skrev  Johannes Sixt:
Show 16 quoted lines
> On Montag, 25. Januar 2010, Robin Rosenberg wrote:
> > In Windows paths beginning with // are knows as UNC paths. They are
> > absolute paths, usually referring to a shared resource on a server.
> >
> > Examples of legal UNC paths
> >
> > 	\\hub\repos\repo
> > 	\\?\unc\hub\repos
> > 	\\?\d:\repo
> 
> I agree that that the problem that you are addressing needs a solution.
> 
> However, the solution is not a whole-sale replacement of
> have_dos_drive_prefix() by a function that is only a tiny bit fancier.
> Accompanying changes are needed, and perhaps more code locations need
>  change.

I was hoping to get help in identifying these and perhaps more cases to test than then ones I thought of at first.

Show 11 quoted lines
> > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> > *path);
> >  char *enter_repo(char *path, int strict);
> >  static inline int is_absolute_path(const char *path)
> >  {
> > -	return path[0] == '/' || has_dos_drive_prefix(path);
> > +	return path[0] == '/' || has_win32_abs_prefix(path);
> 
> Perhaps we need is_dir_sep(path[0]) here? But since I have not observed any
> breakage in connection with this code, I think that all callers feed only
> normalized paths (i.e. with forward slash). (Note that our getcwd()
probably true.
> implementation converts backslashes to forward slashes.) This means that a
> full-fledged check is not needed.
ack.
Show 11 quoted lines
> > @@ -5,7 +5,7 @@ char *gitbasename (char *path)
> >  {
> >  	const char *base;
> >  	/* Skip over the disk name in MSDOS pathnames. */
> > -	if (has_dos_drive_prefix(path))
> > +	if (has_win32_abs_prefix(path))
> >  		path += 2;
> 
> This change is unnecessary; it really is only to skip an initial driver
> prefix. If you want to support \\?\X: style paths, more work is needed here
> so that you do not return X: or ? as the basename.
late night hacks aren't always good.
> > +#define has_win32_abs_prefix(path) \
> 
> Do we really have to name everything "win32" when it is about Windows?
hmm
Show 13 quoted lines
> > @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const
> > char *url_orig,
> >  		end = host;
> >
> >  	path = strchr(end, c);
> > -	if (path && !has_dos_drive_prefix(end)) {
> > +	if (path && !has_win32_abs_prefix(end)) {
> 
> This change is wrong because the check is really only about the drive
>  prefix: It checks that we do not mistake c:/foo as a ssh connection to
>  host c, path /foo. Yes, it does mean that on Windows we cannot have
>  remotes to hosts whose name consists only of a single letter using the rcp
>  notation (you must say ssh://c/foo if you mean it).
right.
Show 12 quoted lines
> > @@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)
> >  {
> >  	char *dst0;
> >
> > -	if (has_dos_drive_prefix(src)) {
> > +	if (has_win32_abs_prefix(src)) {
> >  		*dst++ = *src++;
> >  		*dst++ = *src++;
> >  	}
> 
> Is skipping just two characters for \\ or \\?\whatever paths the right
>  thing?
I shouldn't skip anything. I wasn't converting the first two \'s to //.
Show 14 quoted lines
> > @@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int
> > *nongit_ok) die_errno("Unable to read current working directory");
> >
> >  	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
> > -	if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
> > +	if (ceil_offset < 0 && has_win32_abs_prefix(cwd))
> >  		ceil_offset = 1;
> 
> I doubt that this is correct. The purpose of this check is that "c:/" is
>  the last directory that is checked (on Unix it would be "/") when path
>  components are stripped from cwd. For UNC paths this must be adjusted
>  depending on how you want to support \\server\share and \\?\c:\paths: You
>  do not want to check whether \\server\.git or \\.git or \\?\.git are git
>  directories.
\\server\.git seems valid. Probably not a good idea, but who am I to judge?
Show 16 quoted lines
> 
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -797,7 +797,7 @@ static int is_local(const char *url)
> >  	const char *colon = strchr(url, ':');
> >  	const char *slash = strchr(url, '/');
> >  	return !colon || (slash && slash < colon) ||
> > -		has_dos_drive_prefix(url);
> > +		has_win32_abs_prefix(url);
> 
> This check is again to not mistake c:/foo as rcp style connection. No
>  change needed.
> 
> As I said, changes to other parts are perhaps also needed, most
>  prominently, make_relative_path() that prompted this patch. What about
> make_absolute_path() and make_non_relative_path()?
Thanks for the feedback. 
-- robin

← back to recent threads