Re: [PATCH] Handle UNC paths everywhere

måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:

Hi,

On Mon, 25 Jan 2010, Robin Rosenberg wrote:

quoted

quoted

From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001

From: Robin Rosenberg <redacted>
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.

quoted

Examples of legal UNC paths

	\\hub\repos\repo
	\\?\unc\hub\repos
	\\?\d:\repo

Signed-off-by: Robin Rosenberg <redacted>
---
 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.

quoted

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.

quoted

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.

quoted

+#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?

quoted

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.

quoted

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. 

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.

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