{"thread":{"id":"22377","subject":"[PATCH] Handle UNC paths everywhere","startedAt":"2010-01-25T00:55:47Z","lastAt":"2010-01-26T11:01:38Z","messageCount":21,"participants":["Robin Rosenberg","Sverre Rabbelier","Erik Faye-Lund","Johannes Schindelin","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"132571","messageId":"201001250155.47664.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":null,"subject":"[PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T00:55:47Z","receivedAt":"2010-01-25T00:55:47Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":">From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\nFrom: Robin Rosenberg <robin.rosenberg@dewire.com>\nDate: Mon, 25 Jan 2010 01:41:03 +0100\nSubject: [PATCH] Handle UNC paths everywhere\n\nIn Windows paths beginning with // are knows as UNC paths. They are\nabsolute paths, usually referring to a shared resource on a server.\n\nExamples of legal UNC paths\n\n\t\\\\hub\\repos\\repo\n\t\\\\?\\unc\\hub\\repos\n\t\\\\?\\d:\\repo\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n cache.h           |    2 +-\n compat/basename.c |    2 +-\n compat/mingw.h    |    8 +++++++-\n connect.c         |    2 +-\n git-compat-util.h |    9 +++++++++\n path.c            |    2 +-\n setup.c           |    2 +-\n sha1_file.c       |   20 ++++++++++++++++++++\n transport.c       |    2 +-\n 9 files changed, 42 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 767a50e..8f63640 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char \n*path);\n char *enter_repo(char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\n-\treturn path[0] == '/' || has_dos_drive_prefix(path);\n+\treturn path[0] == '/' || has_win32_abs_prefix(path);\n }\n int is_directory(const char *);\n const char *make_absolute_path(const char *path);\ndiff --git a/compat/basename.c b/compat/basename.c\nindex d8f8a3c..c1d81f6 100644\n--- a/compat/basename.c\n+++ b/compat/basename.c\n@@ -5,7 +5,7 @@ char *gitbasename (char *path)\n {\n \tconst char *base;\n \t/* Skip over the disk name in MSDOS pathnames. */\n-\tif (has_dos_drive_prefix(path))\n+\tif (has_win32_abs_prefix(path))\n \t\tpath += 2;\n \tfor (base = path; *path; path++) {\n \t\tif (is_dir_sep(*path))\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 1b528da..d1aa8be 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char *format, \n...) __attribute__((format\n  * git specific compatibility\n  */\n \n-#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n+#define has_dos_drive_prefix(path) \\\n+\t(isalpha(*(path)) && (path)[1] == ':')\n+#define has_unc_prefix(path) \\\n+\t(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))\n+#define has_win32_abs_prefix(path) \\\n+\t(has_dos_drive_prefix(path) || has_unc_prefix(path))\n+\n #define is_dir_sep(c) ((c) == '/' || (c) == '\\\\')\n #define PATH_SEP ';'\n #define PRIuMAX \"I64u\"\ndiff --git a/connect.c b/connect.c\nindex 7945e38..9d4556c 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char \n*url_orig,\n \t\tend = host;\n \n \tpath = strchr(end, c);\n-\tif (path && !has_dos_drive_prefix(end)) {\n+\tif (path && !has_win32_abs_prefix(end)) {\n \t\tif (c == ':') {\n \t\t\tprotocol = PROTO_SSH;\n \t\t\t*path++ = '\\0';\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ef60803..0de9dac 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -170,6 +170,15 @@ extern char *gitbasename(char *);\n #define has_dos_drive_prefix(path) 0\n #endif\n \n+#ifndef has_unc_prefix\n+#define has_unc_prefix(path) 0\n+#endif\n+\n+#ifndef has_win32_abs_prefix\n+#error no abs\n+#define has_win32_abs_prefix(path) 0\n+#endif\n+\n #ifndef is_dir_sep\n #define is_dir_sep(c) ((c) == '/')\n #endif\ndiff --git a/path.c b/path.c\nindex 047fdb0..79451a2 100644\n--- a/path.c\n+++ b/path.c\n@@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)\n {\n \tchar *dst0;\n \n-\tif (has_dos_drive_prefix(src)) {\n+\tif (has_win32_abs_prefix(src)) {\n \t\t*dst++ = *src++;\n \t\t*dst++ = *src++;\n \t}\ndiff --git a/setup.c b/setup.c\nindex 029371e..4f72817 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\tdie_errno(\"Unable to read current working directory\");\n \n \tceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);\n-\tif (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n+\tif (ceil_offset < 0 && has_win32_abs_prefix(cwd))\n \t\tceil_offset = 1;\n \n \t/*\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4cc8939..f1ad3f5 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -87,6 +87,26 @@ static inline int offset_1st_component(const char *path)\n {\n \tif (has_dos_drive_prefix(path))\n \t\treturn 2 + (path[2] == '/');\n+\tif (has_unc_prefix(path)) {\n+\t\tint p = 2;\n+\t\tint skip;\n+\t\tif (path[p] == '?') {\n+\t\t\tif (path[p+1] && !has_dos_drive_prefix(path+p+2)) {\n+\t\t\t\tskip = 3;\n+\t\t\t} else {\n+\t\t\t\tskip = 2;\n+\t\t\t}\n+\t\t} else\n+\t\t\tskip = 2;\n+\n+\t\twhile (skip && path[p]) {\n+\t\t\tif (is_dir_sep(path[p]))\n+\t\t\t\t--skip;\n+\t\t\t++p;\n+\t\t}\n+\t\tprintf(\"Left with %s\\n\", path+p);\n+\t\treturn p;\n+\t}\n \treturn *path == '/';\n }\n \ndiff --git a/transport.c b/transport.c\nindex 644a30a..9f5b24e 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -797,7 +797,7 @@ static int is_local(const char *url)\n \tconst char *colon = strchr(url, ':');\n \tconst char *slash = strchr(url, '/');\n \treturn !colon || (slash && slash < colon) ||\n-\t\thas_dos_drive_prefix(url);\n+\t\thas_win32_abs_prefix(url);\n }\n \n static int is_file(const char *url)\n-- \n1.6.4.msysgit.0.598.g37a74\n"},{"id":"132583","messageId":"fabb9a1e1001250136n2fb0043av7348db9177f4d096@mail.gmail.com","threadId":"22377","inReplyTo":"201001250155.47664.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-01-25T09:36:59Z","receivedAt":"2010-01-25T09:36:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jan 25, 2010 at 01:55, Robin Rosenberg\n<robin.rosenberg@dewire.com> wrote:\n> In Windows paths beginning with // are knows as UNC paths. They are\n> absolute paths, usually referring to a shared resource on a server.\n\nCute, but will it actually work? I've tried to use them // paths on\nwindows before with MSysGit, and it's never worked, probably due to\nthe same reason why it doesn't work in the cmd prompt (whatever reason\nthat may be).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"132584","messageId":"201001251058.22714.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"fabb9a1e1001250136n2fb0043av7348db9177f4d096@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T09:58:22Z","receivedAt":"2010-01-25T09:58:22Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 10.36.59 skrev  Sverre Rabbelier:\n> Heya,\n> \n> On Mon, Jan 25, 2010 at 01:55, Robin Rosenberg\n> \n> <robin.rosenberg@dewire.com> wrote:\n> > In Windows paths beginning with // are knows as UNC paths. They are\n> > absolute paths, usually referring to a shared resource on a server.\n> \n> Cute, but will it actually work? I've tried to use them // paths on\n> windows before with MSysGit, and it's never worked, probably due to\nWorks here (tm). Latest msygit + rebuilt git binaries on Windows 2003.\n\nThe only program I know in the msysgit installation that didn't accept UNC \npaths prior to this patch was in fact git. \n\n> the same reason why it doesn't work in the cmd prompt (whatever reason\n> that may be).\n\nAny code of the this form will of course not support UNC paths:\n\nif (!has_drive_letter(path))\n\tboom();\n\n-- robin\n"},{"id":"132585","messageId":"40aa078e1001250211w2dcc5e97vf89f64f136bd2f0@mail.gmail.com","threadId":"22377","inReplyTo":"fabb9a1e1001250136n2fb0043av7348db9177f4d096@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-01-25T10:11:26Z","receivedAt":"2010-01-25T10:11:26Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Jan 25, 2010 at 10:36 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> Heya,\n>\n> On Mon, Jan 25, 2010 at 01:55, Robin Rosenberg\n> <robin.rosenberg@dewire.com> wrote:\n>> In Windows paths beginning with // are knows as UNC paths. They are\n>> absolute paths, usually referring to a shared resource on a server.\n>\n> Cute, but will it actually work? I've tried to use them // paths on\n> windows before with MSysGit, and it's never worked, probably due to\n> the same reason why it doesn't work in the cmd prompt (whatever reason\n> that may be).\n>\n\nThis works for me, Vista 64-bit:\n\nC:\\Users\\kusma>dir \\\\mongo\\code\nThe request is not supported.\n\nC:\\Users\\kusma>explorer \\\\mongo\\code\n<login on the gui>\n\nC:\\Users\\kusma>dir \\\\mongo\\code\n Volume in drive \\\\mongo\\code is Code\n Volume Serial Number is 04C3-0225\n\n Directory of \\\\mongo\\code\n\n10.01.2010  21:39    <DIR>          .\n04.01.2010  01:28    <DIR>          ..\n04.01.2010  00:51    <DIR>          qt-rocket\n10.01.2010  21:38    <DIR>          very_last_64k_ever\n11.01.2010  19:57    <DIR>          scratch\n               0 File(s)              0 bytes\n               5 Dir(s)  958 499 627 008 bytes free\n\nC:\\Users\\kusma>\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"132586","messageId":"fabb9a1e1001250222n6912905fqfd2e76f8d4496bb7@mail.gmail.com","threadId":"22377","inReplyTo":"40aa078e1001250211w2dcc5e97vf89f64f136bd2f0@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-01-25T10:22:19Z","receivedAt":"2010-01-25T10:22:19Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jan 25, 2010 at 11:11, Erik Faye-Lund <kusmabite@googlemail.com> wrote:\n> C:\\Users\\kusma>dir \\\\mongo\\code\n> The request is not supported.\n>\n> C:\\Users\\kusma>explorer \\\\mongo\\code\n> <login on the gui>\n>\n> C:\\Users\\kusma>dir \\\\mongo\\code\n>  Volume in drive \\\\mongo\\code is Code\n>  Volume Serial Number is 04C3-0225\n\nAh, that's very interesting. Not sure that will help MSysGit a lot though.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"132590","messageId":"201001251201.23064.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"fabb9a1e1001250222n6912905fqfd2e76f8d4496bb7@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T11:01:22Z","receivedAt":"2010-01-25T11:01:22Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 11.22.19 skrev  Sverre Rabbelier:\n> Heya,\n> \n> On Mon, Jan 25, 2010 at 11:11, Erik Faye-Lund <kusmabite@googlemail.com> \nwrote:\n> > C:\\Users\\kusma>dir \\\\mongo\\code\n> > The request is not supported.\n> >\n> > C:\\Users\\kusma>explorer \\\\mongo\\code\n> > <login on the gui>\n> >\n> > C:\\Users\\kusma>dir \\\\mongo\\code\n> >  Volume in drive \\\\mongo\\code is Code\n> >  Volume Serial Number is 04C3-0225\n> \n> Ah, that's very interesting. Not sure that will help MSysGit a lot though.\n> \n\nCould you perhaps *try* it before claiming it won't work? I suggest you\nuse forward slashes to avoid quoting problems.\n\n-- robin\n"},{"id":"132592","messageId":"fabb9a1e1001250306l2b9aba53s6a884b618a80063b@mail.gmail.com","threadId":"22377","inReplyTo":"201001251201.23064.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-01-25T11:06:21Z","receivedAt":"2010-01-25T11:06:21Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jan 25, 2010 at 12:01, Robin Rosenberg\n<robin.rosenberg@dewire.com> wrote:\n>> Ah, that's very interesting. Not sure that will help MSysGit a lot though.\n>>\n>\n> Could you perhaps *try* it before claiming it won't work? I suggest you\n> use forward slashes to avoid quoting problems.\n\nActually, I can't, cos I don't have a MSysGit build environment, I'm\njust saying that in the environment I tested it in, I didn't have to\nlog in, so I suspect that it won't work, I'm not claiming anything.\n\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"132594","messageId":"40aa078e1001250317r77334917le88c5adb74d1939b@mail.gmail.com","threadId":"22377","inReplyTo":"fabb9a1e1001250306l2b9aba53s6a884b618a80063b@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-01-25T11:17:53Z","receivedAt":"2010-01-25T11:17:53Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Jan 25, 2010 at 12:06 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> Heya,\n>\n> On Mon, Jan 25, 2010 at 12:01, Robin Rosenberg\n> <robin.rosenberg@dewire.com> wrote:\n>>> Ah, that's very interesting. Not sure that will help MSysGit a lot though.\n>>>\n>>\n>> Could you perhaps *try* it before claiming it won't work? I suggest you\n>> use forward slashes to avoid quoting problems.\n>\n> Actually, I can't, cos I don't have a MSysGit build environment, I'm\n> just saying that in the environment I tested it in, I didn't have to\n> log in, so I suspect that it won't work, I'm not claiming anything.\n>\n\nFor shares that doesn't require login, I can list files without doing\nanything. Remember that Windows might try some saved password in the\nbackground when you open the folder in explorer, so not seeing a\npassword-prompt might not be the same thing as the share not needing\nlogin.\n\nPerhaps \"net use \\\\mongo\\code\" would have been a better example than\nusing explorer? I don't have the setup where I am right now to test\nthat it does what I expect it to, though.\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"132611","messageId":"alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302","threadId":"22377","inReplyTo":"201001250155.47664.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-25T17:34:01Z","receivedAt":"2010-01-25T17:34:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jan 2010, Robin Rosenberg wrote:\n\n> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n> From: Robin Rosenberg <robin.rosenberg@dewire.com>\n> Date: Mon, 25 Jan 2010 01:41:03 +0100\n> Subject: [PATCH] Handle UNC paths everywhere\n> \n> In Windows paths beginning with // are knows as UNC paths. They are\n> absolute paths, usually referring to a shared resource on a server.\n\nAnd even a simple \"cd\" with them does not work.\n\n> Examples of legal UNC paths\n> \n> \t\\\\hub\\repos\\repo\n> \t\\\\?\\unc\\hub\\repos\n> \t\\\\?\\d:\\repo\n> \n> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n> ---\n>  cache.h           |    2 +-\n>  compat/basename.c |    2 +-\n>  compat/mingw.h    |    8 +++++++-\n>  connect.c         |    2 +-\n>  git-compat-util.h |    9 +++++++++\n>  path.c            |    2 +-\n>  setup.c           |    2 +-\n>  sha1_file.c       |   20 ++++++++++++++++++++\n>  transport.c       |    2 +-\n>  9 files changed, 42 insertions(+), 7 deletions(-)\n\nOuch.  You should know better than to clutter non-Windows-specific parts \nwith that ugly kludge.\n\n> diff --git a/cache.h b/cache.h\n> index 767a50e..8f63640 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char \n> *path);\n>  char *enter_repo(char *path, int strict);\n>  static inline int is_absolute_path(const char *path)\n>  {\n> -\treturn path[0] == '/' || has_dos_drive_prefix(path);\n> +\treturn path[0] == '/' || has_win32_abs_prefix(path);\n\nWhy?  We can still keep the name.  Well, maybe not, see below.\n\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 1b528da..d1aa8be 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char *format, \n> ...) __attribute__((format\n>   * git specific compatibility\n>   */\n>  \n> -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')\n> +#define has_dos_drive_prefix(path) \\\n> +\t(isalpha(*(path)) && (path)[1] == ':')\n\nWhy?\n\n> +#define has_unc_prefix(path) \\\n> +\t(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))\n> +#define has_win32_abs_prefix(path) \\\n> +\t(has_dos_drive_prefix(path) || has_unc_prefix(path))\n\n\"c:hello.txt\" is not an absolute path.\n\n> diff --git a/connect.c b/connect.c\n> index 7945e38..9d4556c 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char \n> *url_orig,\n>  \t\tend = host;\n>  \n>  \tpath = strchr(end, c);\n> -\tif (path && !has_dos_drive_prefix(end)) {\n> +\tif (path && !has_win32_abs_prefix(end)) {\n>  \t\tif (c == ':') {\n\nWhy?  Do we really have to exclude UNC paths from that \":\" handling?\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index ef60803..0de9dac 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -170,6 +170,15 @@ extern char *gitbasename(char *);\n>  #define has_dos_drive_prefix(path) 0\n>  #endif\n>  \n> +#ifndef has_unc_prefix\n> +#define has_unc_prefix(path) 0\n> +#endif\n> +\n> +#ifndef has_win32_abs_prefix\n> +#error no abs\n\nYeah, sure.  I do have abs, thank you very much.\n\nIn general, I am _very_ worried about your patch.  It does not acknowledge \nthat there is a fundamental difference between DOS drive prefixes and UNC \npaths, and not being able to \"cd\" to the latter is just a symptom.\n\nI am also not quite sure if you can get away with having the same offset \nfor both: if I have \"C:\\blah\" and strip off \"C:\", I always have a \ndirectory separator to bounce against, whereas I do not have that if I \nstrip off the two \"\\\\\" of a UNC path.  Besides, I maintain that the host \nname, and maybe even the share name, should not ever be stripped off!\n\nAt least a discussion is missing from the commit message.\n\nCiao,\nDscho\n"},{"id":"132613","messageId":"40aa078e1001250957h292f8b01me8f7dec4ba2b425b@mail.gmail.com","threadId":"22377","inReplyTo":"alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-01-25T17:57:06Z","receivedAt":"2010-01-25T17:57:06Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Mon, 25 Jan 2010, Robin Rosenberg wrote:\n>\n>> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n>> From: Robin Rosenberg <robin.rosenberg@dewire.com>\n>> Date: Mon, 25 Jan 2010 01:41:03 +0100\n>> Subject: [PATCH] Handle UNC paths everywhere\n>>\n>> In Windows paths beginning with // are knows as UNC paths. They are\n>> absolute paths, usually referring to a shared resource on a server.\n>\n> And even a simple \"cd\" with them does not work.\n>\n\nBut it does, at least for me - both in bash and cmd.exe. I just need\nto log on to the server first, unless it's a public share.\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"132617","messageId":"alpine.DEB.1.00.1001251916030.8733@intel-tinevez-2-302","threadId":"22377","inReplyTo":"40aa078e1001250957h292f8b01me8f7dec4ba2b425b@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-25T18:19:12Z","receivedAt":"2010-01-25T18:19:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jan 2010, Erik Faye-Lund wrote:\n\n> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:\n> >\n> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>\n> >> Date: Mon, 25 Jan 2010 01:41:03 +0100\n> >> Subject: [PATCH] Handle UNC paths everywhere\n> >>\n> >> In Windows paths beginning with // are knows as UNC paths. They are\n> >> absolute paths, usually referring to a shared resource on a server.\n> >\n> > And even a simple \"cd\" with them does not work.\n> >\n> \n> But it does, at least for me - both in bash and cmd.exe. I just need\n> to log on to the server first, unless it's a public share.\n\nI love it when people say \"it works for me, so let's do it\".\n\n_My_ _only_ instance of Windows cmd says this:\n\n\tC:\\Blah> cd \\\\localhost\n\t'\\\\localhost'\n\tCMD does not support UNC paths as current directories.\n\t\n\tC:\\Blah>\n\nSo.\n\nBesides, the patch was not in a form where I can say that it was obviously \nfixing the issue. It was rather in a form where I would have to have set \naside a substantial amount of time to verify that nothing undesired was \nintroduced as a side effect.\n\nCiao,\nDscho\n"},{"id":"132622","messageId":"201001252037.41497.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"40aa078e1001250957h292f8b01me8f7dec4ba2b425b@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T19:37:41Z","receivedAt":"2010-01-25T19:37:41Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 18.57.06 skrev  Erik Faye-Lund:\n> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin\n> \n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:\n> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n> >>\n> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>\n> >> Date: Mon, 25 Jan 2010 01:41:03 +0100\n> >> Subject: [PATCH] Handle UNC paths everywhere\n> >>\n> >> In Windows paths beginning with // are knows as UNC paths. They are\n> >> absolute paths, usually referring to a shared resource on a server.\n> >\n> > And even a simple \"cd\" with them does not work.\n> \n> But it does, at least for me - both in bash and cmd.exe. I just needt.\n\nIn cmd,exe surprises me a bit. pushd \\\\server\\share is not the same\nas it maps a drive and then uses it to cd.\n\n-- robin\n"},{"id":"132624","messageId":"201001252045.28778.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T19:45:28Z","receivedAt":"2010-01-25T19:45:28Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:\n> Hi,\n> \n> On Mon, 25 Jan 2010, Robin Rosenberg wrote:\n> > >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n> >\n> > From: Robin Rosenberg <robin.rosenberg@dewire.com>\n> > Date: Mon, 25 Jan 2010 01:41:03 +0100\n> > Subject: [PATCH] Handle UNC paths everywhere\n> >\n> > In Windows paths beginning with // are knows as UNC paths. They are\n> > absolute paths, usually referring to a shared resource on a server.\n> \n> And even a simple \"cd\" with them does not work.\n> \n> > Examples of legal UNC paths\n> >\n> > \t\\\\hub\\repos\\repo\n> > \t\\\\?\\unc\\hub\\repos\n> > \t\\\\?\\d:\\repo\n> >\n> > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n> > ---\n> >  cache.h           |    2 +-\n> >  compat/basename.c |    2 +-\n> >  compat/mingw.h    |    8 +++++++-\n> >  connect.c         |    2 +-\n> >  git-compat-util.h |    9 +++++++++\n> >  path.c            |    2 +-\n> >  setup.c           |    2 +-\n> >  sha1_file.c       |   20 ++++++++++++++++++++\n> >  transport.c       |    2 +-\n> >  9 files changed, 42 insertions(+), 7 deletions(-)\n> \n> Ouch.  You should know better than to clutter non-Windows-specific parts\n> with that ugly kludge.\n> \n> > diff --git a/cache.h b/cache.h\n> > index 767a50e..8f63640 100644\n> > --- a/cache.h\n> > +++ b/cache.h\n> > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char\n> > *path);\n> >  char *enter_repo(char *path, int strict);\n> >  static inline int is_absolute_path(const char *path)\n> >  {\n> > -\treturn path[0] == '/' || has_dos_drive_prefix(path);\n> > +\treturn path[0] == '/' || has_win32_abs_prefix(path);\n> \n> Why?  We can still keep the name.  Well, maybe not, see below.\n\nI do think function names should imply something about their behaviour.\n\n> \n> > diff --git a/compat/mingw.h b/compat/mingw.h\n> > index 1b528da..d1aa8be 100644\n> > --- a/compat/mingw.h\n> > +++ b/compat/mingw.h\n> > @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char\n> > *format, ...) __attribute__((format\n> >   * git specific compatibility\n> >   */\n> >\n> > -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] ==\n> > ':') +#define has_dos_drive_prefix(path) \\\n> > +\t(isalpha(*(path)) && (path)[1] == ':')\n> \n> Why?\n\nTo avoid very long lines and format this (now) set of related macros \nuniformely.\n\n> > +#define has_unc_prefix(path) \\\n> > +\t(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))\n> > +#define has_win32_abs_prefix(path) \\\n> > +\t(has_dos_drive_prefix(path) || has_unc_prefix(path))\n> \n> \"c:hello.txt\" is not an absolute path.\nOk. Nevertheless that was how it was treated before, It's not relative,\neither, but some quasirelative thing. has_win32_quasi_abs_prefix?\n\n> > diff --git a/connect.c b/connect.c\n> > index 7945e38..9d4556c 100644\n> > --- a/connect.c\n> > +++ b/connect.c\n> > @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const\n> > char *url_orig,\n> >  \t\tend = host;\n> >\n> >  \tpath = strchr(end, c);\n> > -\tif (path && !has_dos_drive_prefix(end)) {\n> > +\tif (path && !has_win32_abs_prefix(end)) {\n> >  \t\tif (c == ':') {\n> \n> Why?  Do we really have to exclude UNC paths from that \":\" handling?\n\nThat colon is about URL-ish things... Right.\n\n> > diff --git a/git-compat-util.h b/git-compat-util.h\n> > index ef60803..0de9dac 100644\n> > --- a/git-compat-util.h\n> > +++ b/git-compat-util.h\n> > @@ -170,6 +170,15 @@ extern char *gitbasename(char *);\n> >  #define has_dos_drive_prefix(path) 0\n> >  #endif\n> >\n> > +#ifndef has_unc_prefix\n> > +#define has_unc_prefix(path) 0\n> > +#endif\n> > +\n> > +#ifndef has_win32_abs_prefix\n> > +#error no abs\nouch, a leftover from trying to figure out a complation message. \n\n> \n> Yeah, sure.  I do have abs, thank you very much.\n> \n> In general, I am _very_ worried about your patch.  It does not acknowledge\n> that there is a fundamental difference between DOS drive prefixes and UNC\n> paths, and not being able to \"cd\" to the latter is just a symptom.\n\nAs I said. Most programs including bash, but excluding cmd.exe can set the\nworking directory to an UNC path. I cannot fix cmd.exe and rarely use it\nwith git, but the patch helps even if you cannot cd from a UNC challenged\nshell.\n\n> I am also not quite sure if you can get away with having the same offset\n> for both: if I have \"C:\\blah\" and strip off \"C:\", I always have a\n> directory separator to bounce against, whereas I do not have that if I\n> strip off the two \"\\\\\" of a UNC path.  Besides, I maintain that the host\n> name, and maybe even the share name, should not ever be stripped off!\n\nWhen creating directoties you only strip them off for the purpose of finding\npaths to mkdir. The server and share part you cannot mkdir anyway, they\nmust exist before attempting to create a directory, hence I skip past those  \nportions. As for the \\-less path beginning with a drive I'll reconsider. I did\nnot test that one.\n\n-- robin\n"},{"id":"132625","messageId":"40aa078e1001251145o13545328o7d46086ff09e6d33@mail.gmail.com","threadId":"22377","inReplyTo":"alpine.DEB.1.00.1001251916030.8733@intel-tinevez-2-302","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-01-25T19:45:48Z","receivedAt":"2010-01-25T19:45:48Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Jan 25, 2010 at 7:19 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Mon, 25 Jan 2010, Erik Faye-Lund wrote:\n>\n>> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>> > Hi,\n>> >\n>> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:\n>> >\n>> >> >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n>> >> From: Robin Rosenberg <robin.rosenberg@dewire.com>\n>> >> Date: Mon, 25 Jan 2010 01:41:03 +0100\n>> >> Subject: [PATCH] Handle UNC paths everywhere\n>> >>\n>> >> In Windows paths beginning with // are knows as UNC paths. They are\n>> >> absolute paths, usually referring to a shared resource on a server.\n>> >\n>> > And even a simple \"cd\" with them does not work.\n>> >\n>>\n>> But it does, at least for me - both in bash and cmd.exe. I just need\n>> to log on to the server first, unless it's a public share.\n>\n> I love it when people say \"it works for me, so let's do it\".\n>\n> _My_ _only_ instance of Windows cmd says this:\n>\n>        C:\\Blah> cd \\\\localhost\n>        '\\\\localhost'\n>        CMD does not support UNC paths as current directories.\n>\n>        C:\\Blah>\n>\n> So.\n\nActually, you're right about cmd.exe - I somehow mixed that up.\nHowever, it works fine in bash (and simply by doing chdir() from a\nnormal C-program), as long as I've logged on in advance.\n\nThis applies to my Vista 64 and XP installations.\n\n>\n> Besides, the patch was not in a form where I can say that it was obviously\n> fixing the issue. It was rather in a form where I would have to have set\n> aside a substantial amount of time to verify that nothing undesired was\n> introduced as a side effect.\n>\n\nThis I can agree on. I just wanted to clear up the situation about\ncd'ing. But I failed - hopefully that's corrected now.\n\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"132626","messageId":"40aa078e1001251148p6263feeevc85a3223f85873@mail.gmail.com","threadId":"22377","inReplyTo":"201001252037.41497.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-01-25T19:48:22Z","receivedAt":"2010-01-25T19:48:22Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Jan 25, 2010 at 8:37 PM, Robin Rosenberg\n<robin.rosenberg@dewire.com> wrote:\n> måndagen den 25 januari 2010 18.57.06 skrev  Erik Faye-Lund:\n>> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin\n>>\n>> <Johannes.Schindelin@gmx.de> wrote:\n>> >\n>> > And even a simple \"cd\" with them does not work.\n>>\n>> But it does, at least for me - both in bash and cmd.exe. I just needt.\n>\n> In cmd,exe surprises me a bit. pushd \\\\server\\share is not the same\n> as it maps a drive and then uses it to cd.\n>\n> -- robin\n>\n\nMy guess would be that Dscho mentioned this because git internally\ndoes a chdir() to the path that is cloned, so currently chdir'ing must\nbe supported for clone to work with a \"local repo\".\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"132627","messageId":"201001252104.10328.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"alpine.DEB.1.00.1001251553150.8733@intel-tinevez-2-302","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T20:04:10Z","receivedAt":"2010-01-25T20:04:10Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:\n> I am also not quite sure if you can get away with having the same offset\n> for both: if I have \"C:\\blah\" and strip off \"C:\", I always have a\n> directory separator to bounce against, whereas I do not have that if I\n> strip off the two \"\\\\\" of a UNC path.  Besides, I maintain that the host\n> name, and maybe even the share name, should not ever be stripped off!\n\nAdvices needed:\n\nd:somedir (when cwd=d:\\msysgit, == /) may be tricky to fix.\nMsysgit seems confused by the syntax and treats it as d:\\ \n\nroro@SIENA / (master)\n$ cmd\nMicrosoft Windows [Version 5.2.3790]\n(C) Copyright 1985-2003 Microsoft Corp.\n\nD:\\msysgit>exit\n\nroro@SIENA / (master)\n$ mkdir d:x\nmkdir: cannot create directory `d:x': File exist\n\nroro@SIENA / (master)\n$ cd d:x\n\nroro@SIENA /d\n$ ls -l x\nls: x: No such file or directory\n\nroro@SIENA /d\n$\n\nFrom that I think that even if we try to make git handle d:path, msys\nwill break regardless. We can fix truly absolute and normal relative paths.\n\n-- robin\n"},{"id":"132628","messageId":"201001252107.45745.j6t@kdbg.org","threadId":"22377","inReplyTo":"201001250155.47664.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-01-25T20:07:44Z","receivedAt":"2010-01-25T20:07:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Montag, 25. Januar 2010, Robin Rosenberg wrote:\n> In Windows paths beginning with // are knows as UNC paths. They are\n> absolute paths, usually referring to a shared resource on a server.\n>\n> Examples of legal UNC paths\n>\n> \t\\\\hub\\repos\\repo\n> \t\\\\?\\unc\\hub\\repos\n> \t\\\\?\\d:\\repo\n\nI agree that that the problem that you are addressing needs a solution.\n\nHowever, the solution is not a whole-sale replacement of \nhave_dos_drive_prefix() by a function that is only a tiny bit fancier. \nAccompanying changes are needed, and perhaps more code locations need change.\n\n> @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char\n> *path);\n>  char *enter_repo(char *path, int strict);\n>  static inline int is_absolute_path(const char *path)\n>  {\n> -\treturn path[0] == '/' || has_dos_drive_prefix(path);\n> +\treturn path[0] == '/' || has_win32_abs_prefix(path);\n\nPerhaps we need is_dir_sep(path[0]) here? But since I have not observed any \nbreakage in connection with this code, I think that all callers feed only \nnormalized paths (i.e. with forward slash). (Note that our getcwd() \nimplementation converts backslashes to forward slashes.) This means that a \nfull-fledged check is not needed.\n\n> @@ -5,7 +5,7 @@ char *gitbasename (char *path)\n>  {\n>  \tconst char *base;\n>  \t/* Skip over the disk name in MSDOS pathnames. */\n> -\tif (has_dos_drive_prefix(path))\n> +\tif (has_win32_abs_prefix(path))\n>  \t\tpath += 2;\n\nThis change is unnecessary; it really is only to skip an initial driver \nprefix. If you want to support \\\\?\\X: style paths, more work is needed here  \nso that you do not return X: or ? as the basename.\n\n> +#define has_win32_abs_prefix(path) \\\n\nDo we really have to name everything \"win32\" when it is about Windows?\n\n> @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const char\n> *url_orig,\n>  \t\tend = host;\n>\n>  \tpath = strchr(end, c);\n> -\tif (path && !has_dos_drive_prefix(end)) {\n> +\tif (path && !has_win32_abs_prefix(end)) {\n\nThis change is wrong because the check is really only about the drive prefix: \nIt checks that we do not mistake c:/foo as a ssh connection to host c, \npath /foo. Yes, it does mean that on Windows we cannot have remotes to hosts \nwhose name consists only of a single letter using the rcp notation (you must \nsay ssh://c/foo if you mean it).\n\n> @@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)\n>  {\n>  \tchar *dst0;\n>\n> -\tif (has_dos_drive_prefix(src)) {\n> +\tif (has_win32_abs_prefix(src)) {\n>  \t\t*dst++ = *src++;\n>  \t\t*dst++ = *src++;\n>  \t}\n\nIs skipping just two characters for \\\\ or \\\\?\\whatever paths the right thing?\n\n> @@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>  \t\tdie_errno(\"Unable to read current working directory\");\n>\n>  \tceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);\n> -\tif (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n> +\tif (ceil_offset < 0 && has_win32_abs_prefix(cwd))\n>  \t\tceil_offset = 1;\n\nI doubt that this is correct. The purpose of this check is that \"c:/\" is the \nlast directory that is checked (on Unix it would be \"/\") when path components \nare stripped from cwd. For UNC paths this must be adjusted depending on how \nyou want to support \\\\server\\share and \\\\?\\c:\\paths: You do not want to check \nwhether \\\\server\\.git or \\\\.git or \\\\?\\.git are git directories.\n\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -797,7 +797,7 @@ static int is_local(const char *url)\n>  \tconst char *colon = strchr(url, ':');\n>  \tconst char *slash = strchr(url, '/');\n>  \treturn !colon || (slash && slash < colon) ||\n> -\t\thas_dos_drive_prefix(url);\n> +\t\thas_win32_abs_prefix(url);\n\nThis check is again to not mistake c:/foo as rcp style connection. No change \nneeded.\n\nAs I said, changes to other parts are perhaps also needed, most prominently, \nmake_relative_path() that prompted this patch. What about \nmake_absolute_path() and make_non_relative_path()?\n\n-- Hannes\n"},{"id":"132629","messageId":"201001252115.52081.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"40aa078e1001251148p6263feeevc85a3223f85873@mail.gmail.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T20:15:51Z","receivedAt":"2010-01-25T20:15:51Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 20.48.22 skrev  Erik Faye-Lund:\n> On Mon, Jan 25, 2010 at 8:37 PM, Robin Rosenberg\n> \n> <robin.rosenberg@dewire.com> wrote:\n> > måndagen den 25 januari 2010 18.57.06 skrev  Erik Faye-Lund:\n> >> On Mon, Jan 25, 2010 at 6:34 PM, Johannes Schindelin\n> >>\n> >> <Johannes.Schindelin@gmx.de> wrote:\n> >> > And even a simple \"cd\" with them does not work.\n> >>\n> >> But it does, at least for me - both in bash and cmd.exe. I just needt.\n> >\n> > In cmd,exe surprises me a bit. pushd \\\\server\\share is not the same\n> > as it maps a drive and then uses it to cd.\n> >\n> > -- robin\n> \n> My guess would be that Dscho mentioned this because git internally\n> does a chdir() to the path that is cloned, so currently chdir'ing must\n\nchdir doesn't call cmd.exe and does not suffer from cmd's limitations.\n\n-- robin\n"},{"id":"132637","messageId":"201001252242.40117.robin.rosenberg@dewire.com","threadId":"22377","inReplyTo":"201001252107.45745.j6t@kdbg.org","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2010-01-25T21:42:39Z","receivedAt":"2010-01-25T21:42:39Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 25 januari 2010 21.07.44 skrev  Johannes Sixt:\n> On Montag, 25. Januar 2010, Robin Rosenberg wrote:\n> > In Windows paths beginning with // are knows as UNC paths. They are\n> > absolute paths, usually referring to a shared resource on a server.\n> >\n> > Examples of legal UNC paths\n> >\n> > \t\\\\hub\\repos\\repo\n> > \t\\\\?\\unc\\hub\\repos\n> > \t\\\\?\\d:\\repo\n> \n> I agree that that the problem that you are addressing needs a solution.\n> \n> However, the solution is not a whole-sale replacement of\n> have_dos_drive_prefix() by a function that is only a tiny bit fancier.\n> Accompanying changes are needed, and perhaps more code locations need\n>  change.\n\nI was hoping to get help in identifying these and perhaps more cases to test\nthan then ones I thought of at first.\n\n> > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char\n> > *path);\n> >  char *enter_repo(char *path, int strict);\n> >  static inline int is_absolute_path(const char *path)\n> >  {\n> > -\treturn path[0] == '/' || has_dos_drive_prefix(path);\n> > +\treturn path[0] == '/' || has_win32_abs_prefix(path);\n> \n> Perhaps we need is_dir_sep(path[0]) here? But since I have not observed any\n> breakage in connection with this code, I think that all callers feed only\n> normalized paths (i.e. with forward slash). (Note that our getcwd()\nprobably true.\n\n> implementation converts backslashes to forward slashes.) This means that a\n> full-fledged check is not needed.\nack.\n\n> > @@ -5,7 +5,7 @@ char *gitbasename (char *path)\n> >  {\n> >  \tconst char *base;\n> >  \t/* Skip over the disk name in MSDOS pathnames. */\n> > -\tif (has_dos_drive_prefix(path))\n> > +\tif (has_win32_abs_prefix(path))\n> >  \t\tpath += 2;\n> \n> This change is unnecessary; it really is only to skip an initial driver\n> prefix. If you want to support \\\\?\\X: style paths, more work is needed here\n> so that you do not return X: or ? as the basename.\n\nlate night hacks aren't always good.\n\n> > +#define has_win32_abs_prefix(path) \\\n> \n> Do we really have to name everything \"win32\" when it is about Windows?\nhmm\n\n> > @@ -535,7 +535,7 @@ struct child_process *git_connect(int fd[2], const\n> > char *url_orig,\n> >  \t\tend = host;\n> >\n> >  \tpath = strchr(end, c);\n> > -\tif (path && !has_dos_drive_prefix(end)) {\n> > +\tif (path && !has_win32_abs_prefix(end)) {\n> \n> This change is wrong because the check is really only about the drive\n>  prefix: It checks that we do not mistake c:/foo as a ssh connection to\n>  host c, path /foo. Yes, it does mean that on Windows we cannot have\n>  remotes to hosts whose name consists only of a single letter using the rcp\n>  notation (you must say ssh://c/foo if you mean it).\nright.\n\n> > @@ -409,7 +409,7 @@ int normalize_path_copy(char *dst, const char *src)\n> >  {\n> >  \tchar *dst0;\n> >\n> > -\tif (has_dos_drive_prefix(src)) {\n> > +\tif (has_win32_abs_prefix(src)) {\n> >  \t\t*dst++ = *src++;\n> >  \t\t*dst++ = *src++;\n> >  \t}\n> \n> Is skipping just two characters for \\\\ or \\\\?\\whatever paths the right\n>  thing?\nI shouldn't skip anything. I wasn't converting the first two \\'s to //.\n\n> > @@ -342,7 +342,7 @@ const char *setup_git_directory_gently(int\n> > *nongit_ok) die_errno(\"Unable to read current working directory\");\n> >\n> >  \tceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);\n> > -\tif (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n> > +\tif (ceil_offset < 0 && has_win32_abs_prefix(cwd))\n> >  \t\tceil_offset = 1;\n> \n> I doubt that this is correct. The purpose of this check is that \"c:/\" is\n>  the last directory that is checked (on Unix it would be \"/\") when path\n>  components are stripped from cwd. For UNC paths this must be adjusted\n>  depending on how you want to support \\\\server\\share and \\\\?\\c:\\paths: You\n>  do not want to check whether \\\\server\\.git or \\\\.git or \\\\?\\.git are git\n>  directories.\n\n\\\\server\\.git seems valid. Probably not a good idea, but who am I to judge?\n\n> \n> > --- a/transport.c\n> > +++ b/transport.c\n> > @@ -797,7 +797,7 @@ static int is_local(const char *url)\n> >  \tconst char *colon = strchr(url, ':');\n> >  \tconst char *slash = strchr(url, '/');\n> >  \treturn !colon || (slash && slash < colon) ||\n> > -\t\thas_dos_drive_prefix(url);\n> > +\t\thas_win32_abs_prefix(url);\n> \n> This check is again to not mistake c:/foo as rcp style connection. No\n>  change needed.\n> \n> As I said, changes to other parts are perhaps also needed, most\n>  prominently, make_relative_path() that prompted this patch. What about\n> make_absolute_path() and make_non_relative_path()?\n\nThanks for the feedback. \n\n-- robin\n"},{"id":"132667","messageId":"alpine.DEB.1.00.1001261146040.4641@intel-tinevez-2-302","threadId":"22377","inReplyTo":"201001252045.28778.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-26T10:59:56Z","receivedAt":"2010-01-26T10:59:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nthanks for Cc:ing me this time.\n\nOn Mon, 25 Jan 2010, Robin Rosenberg wrote:\n\n> måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:\n> \n> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:\n> > > >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001\n> > >\n> > > From: Robin Rosenberg <robin.rosenberg@dewire.com>\n> > > Date: Mon, 25 Jan 2010 01:41:03 +0100\n> > > Subject: [PATCH] Handle UNC paths everywhere\n> > >\n> > > In Windows paths beginning with // are knows as UNC paths. They are\n> > > absolute paths, usually referring to a shared resource on a server.\n> > \n> > And even a simple \"cd\" with them does not work.\n> > \n> > > Examples of legal UNC paths\n> > >\n> > > \t\\\\hub\\repos\\repo\n> > > \t\\\\?\\unc\\hub\\repos\n> > > \t\\\\?\\d:\\repo\n> > >\n> > > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n> > > ---\n> > >  cache.h           |    2 +-\n> > >  compat/basename.c |    2 +-\n> > >  compat/mingw.h    |    8 +++++++-\n> > >  connect.c         |    2 +-\n> > >  git-compat-util.h |    9 +++++++++\n> > >  path.c            |    2 +-\n> > >  setup.c           |    2 +-\n> > >  sha1_file.c       |   20 ++++++++++++++++++++\n> > >  transport.c       |    2 +-\n> > >  9 files changed, 42 insertions(+), 7 deletions(-)\n> > \n> > Ouch.  You should know better than to clutter non-Windows-specific parts\n> > with that ugly kludge.\n\nI did suspect that it would be better to handle unc paths differently than \nthe dos prefix, but I did not have the time to form arguments like Hannes \ndid.\n\nSo I think the first order of business is to add new code paths, rather \nthan modify existing ones.  And then you can '#define \nnetwork_path_prefix_length(name) 0' in git-compat-util.h so that on \nnon-Windows platforms (i.e. our 1st class citizens), the code path can be \noptimized out.\n\n> > > diff --git a/cache.h b/cache.h\n> > > index 767a50e..8f63640 100644\n> > > --- a/cache.h\n> > > +++ b/cache.h\n> > > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char\n> > > *path);\n> > >  char *enter_repo(char *path, int strict);\n> > >  static inline int is_absolute_path(const char *path)\n> > >  {\n> > > -\treturn path[0] == '/' || has_dos_drive_prefix(path);\n> > > +\treturn path[0] == '/' || has_win32_abs_prefix(path);\n> > \n> > Why?  We can still keep the name.  Well, maybe not, see below.\n> \n> I do think function names should imply something about their behaviour.\n\nActually, in this case, you do not even need to change anything, as Hannes \npointed out.\n\n> > > diff --git a/compat/mingw.h b/compat/mingw.h\n> > > index 1b528da..d1aa8be 100644\n> > > --- a/compat/mingw.h\n> > > +++ b/compat/mingw.h\n> > > @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char\n> > > *format, ...) __attribute__((format\n> > >   * git specific compatibility\n> > >   */\n> > >\n> > > -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] ==\n> > > ':') +#define has_dos_drive_prefix(path) \\\n> > > +\t(isalpha(*(path)) && (path)[1] == ':')\n> > \n> > Why?\n> \n> To avoid very long lines and format this (now) set of related macros \n> uniformely.\n\nIf you want your patch to go in (which you probably did not, you just \nforgot to prefix the subject with RFC or RFH), you need it to be reviewed.  \nIt is not a good idea to distract reviewers.  Such a change does so.\n\n> > > +#define has_unc_prefix(path) \\\n> > > +\t(is_dir_sep((path)[0]) && is_dir_sep((path)[1]))\n> > > +#define has_win32_abs_prefix(path) \\\n> > > +\t(has_dos_drive_prefix(path) || has_unc_prefix(path))\n> > \n> > \"c:hello.txt\" is not an absolute path.\n> Ok. Nevertheless that was how it was treated before, It's not relative,\n> either, but some quasirelative thing. has_win32_quasi_abs_prefix?\n\nNo, none of this is good.  You should not even pretend that the unc prefix \nand the DOS drive prefix are the same.  Just leave the old code paths \nalone.\n\n> > > diff --git a/git-compat-util.h b/git-compat-util.h\n> > > index ef60803..0de9dac 100644\n> > > --- a/git-compat-util.h\n> > > +++ b/git-compat-util.h\n> > > @@ -170,6 +170,15 @@ extern char *gitbasename(char *);\n> > >  #define has_dos_drive_prefix(path) 0\n> > >  #endif\n> > >\n> > > +#ifndef has_unc_prefix\n> > > +#define has_unc_prefix(path) 0\n> > > +#endif\n> > > +\n> > > +#ifndef has_win32_abs_prefix\n> > > +#error no abs\n> ouch, a leftover from trying to figure out a complation message. \n\nThought so.\n\n> > In general, I am _very_ worried about your patch.  It does not \n> > acknowledge that there is a fundamental difference between DOS drive \n> > prefixes and UNC paths, and not being able to \"cd\" to the latter is \n> > just a symptom.\n> \n> As I said. Most programs including bash, but excluding cmd.exe can set \n> the working directory to an UNC path. I cannot fix cmd.exe and rarely \n> use it with git, but the patch helps even if you cannot cd from a UNC \n> challenged shell.\n\nThe point I tried to make is that you should treat UNC paths differently, \nand you can even leave the door open for non-Windows stuff.  Just call the \nfunction network_path_prefix_length() returning the length of the prefix.  \nIf it becomes standard, say, on Linux, to have support for smb:// style \npaths, we can always add support for that, too, and do not have to change \nthe name yet again.\n\nPlus, by having separate code paths, you can be sure that you do not break \nthe existing ones.\n\nAnd by defining network_path_prefix_length(path) to 0, the new code paths \ncan be optimized out on platforms which do not support network paths.\n\n> > I am also not quite sure if you can get away with having the same \n> > offset for both: if I have \"C:\\blah\" and strip off \"C:\", I always have \n> > a directory separator to bounce against, whereas I do not have that if \n> > I strip off the two \"\\\\\" of a UNC path.  Besides, I maintain that the \n> > host name, and maybe even the share name, should not ever be stripped \n> > off!\n> \n> When creating directoties you only strip them off for the purpose of \n> finding paths to mkdir. The server and share part you cannot mkdir \n> anyway, they must exist before attempting to create a directory, hence I \n> skip past those portions.\n\nI must have missed that.  From what I saw, you treat the offset to be the \nsame as for DOS drive paths: 2 characters, which is definitely not enough.\n\nUnfortunately, this patch needs more revisions, so it will probably not \nmake it into the upcoming Git for Windows, I am afraid.  But then, we do \nnot need to let 3 months happen without a Git for Windows release.\n\nCiao,\nDscho\n"},{"id":"132668","messageId":"alpine.DEB.1.00.1001261200010.4641@intel-tinevez-2-302","threadId":"22377","inReplyTo":"201001252104.10328.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Handle UNC paths everywhere","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-26T11:01:38Z","receivedAt":"2010-01-26T11:01:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jan 2010, Robin Rosenberg wrote:\n\n> måndagen den 25 januari 2010 18.34.01 skrev  Johannes Schindelin:\n> > I am also not quite sure if you can get away with having the same offset\n> > for both: if I have \"C:\\blah\" and strip off \"C:\", I always have a\n> > directory separator to bounce against, whereas I do not have that if I\n> > strip off the two \"\\\\\" of a UNC path.  Besides, I maintain that the host\n> > name, and maybe even the share name, should not ever be stripped off!\n> \n> Advices needed:\n> \n> d:somedir (when cwd=d:\\msysgit, == /) may be tricky to fix.\n\nThis is a totally different issue from the UNC issue you brought up.  May \nI suggest to fix the UNC thing first, and if you feel inclined, take care \nof the relative DOS drive path afterwards?\n\nThe relative DOS drive path is also not something I find overly pressing, \nas I work from the Git bash, as you know, which does not translate \nPOSIX-style paths into such relative DOS drive paths.\n\nCiao,\nDscho"}]}