Re: [PATCH] Handle UNC paths everywhere
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:48:06
Hi, On Mon, 25 Jan 2010, Robin Rosenberg wrote:
quoted
From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001From: 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.
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 hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
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