[PATCH] Allow using UNC path for git repository
Cezary Zawadka
compat/mingw.h | 1 +
git-compat-util.h | 4 ++++
sha1_file.c | 13 +++++++++++++
3 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 4650d8a..1369406 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -259,6 +259,7 @@ int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format
*/
#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
+#define has_unc_path_prefix(path) (is_dir_sep(*(path)) && is_dir_sep(*((path)+1)))
#define is_dir_sep(c) ((c) == '/' || (c) == '\\')
#define PATH_SEP ';'
#define PRIuMAX "I64u"
diff --git a/git-compat-util.h b/git-compat-util.h
index ef55e1a..6c306d3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -171,6 +171,10 @@ extern char *gitbasename(char *);
#define has_dos_drive_prefix(path) 0
#endif
+#ifndef has_unc_path_prefix
+#define has_unc_path_prefix(path) 0
+#endif
+
#ifndef is_dir_sep
#define is_dir_sep(c) ((c) == '/')
#endif
diff --git a/sha1_file.c b/sha1_file.c
index 006321e..880e1af 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -37,8 +37,21 @@ const unsigned char null_sha1[20];
static inline int offset_1st_component(const char *path)
{
+ char *pos;
+
if (has_dos_drive_prefix(path))
return 2 + (path[2] == '/');
+ if (has_unc_path_prefix(path)) {
+ /* skip server name */
+ pos = strchr(path+2,'/');
+ if (!pos)
+ return 0;
+ /* skip share name */
+ do {
+ pos++;
+ } while(*pos && *pos!='/');
+ return pos-path;
+ }
return *path == '/';
}
--
1.7.0.2.msysgit.0.3.g31725
Johannes Schindelin
On Thu, 18 Mar 2010, Cezary Zawadka wrote:
> ---
Thanks. Maybe a Sign-off next time?
> compat/mingw.h | 1 +
> git-compat-util.h | 4 ++++
> sha1_file.c | 13 +++++++++++++
The problem I see here is that it clutters the core of Git, just for the
sake of a single platform.
It might be wiser to move the Windows-specific implementation to a
compat/mingw.c and #ifndef HAVE_OFFSET_1ST_COMPONENT the inline (and
slender-again) implementation in sha1_file.c.
This paragraph does not contain _all_ the necessary steps, I am afraid,
but I am out of time to describe all the necessary steps.
Sorry,
Dscho
Cezary Zawadka
> Thanks. Maybe a Sign-off next time?
of course
> The problem I see here is that it clutters the core of Git, just for the
> sake of a single platform.
>
> It might be wiser to move the Windows-specific implementation to a
> compat/mingw.c and #ifndef HAVE_OFFSET_1ST_COMPONENT the inline (and
> slender-again) implementation in sha1_file.c.
>
> This paragraph does not contain _all_ the necessary steps, I am afraid,
> but I am out of time to describe all the necessary steps.
As I understand the same problem is regarding has_dos_drive_prefix not
only has_unc_path_prefix. So using HAVE_OFFSET_1ST_COMPONENT you wanna
move has_dos_drive_prefix&has_unc_path_prefix part of
offset_1st_component() inline function to compat/mingw.c?
czarek
Johannes Schindelin
Yes, I would like the implementation in sha1_file.c be an inline static
function wrapping a single statement, guarded by #ifndef.
Ciao,
Dscho
Johannes Sixt
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -37,8 +37,21 @@ const unsigned char null_sha1[20];
>
> static inline int offset_1st_component(const char *path)
> {
> + char *pos;
> +
> if (has_dos_drive_prefix(path))
> return 2 + (path[2] == '/');
> + if (has_unc_path_prefix(path)) {
> + /* skip server name */
> + pos = strchr(path+2,'/');
> + if (!pos)
> + return 0;
> + /* skip share name */
> + do {
> + pos++;
> + } while(*pos && *pos!='/');
> + return pos-path;
> + }
> return *path == '/';
> }
Two things:
1. In the current master of git.git this functions was moved to path.c. I can
take care of this when the patch is sent upstream.
2. Why is it necessary to skip past the server or share names? What happens if
you do not skip them?
-- Hannes
Cezary Zawadka
> 2. Why is it necessary to skip past the server or share names? What happens if
> you do not skip them?
"error: unable to create directory for //server/share/.git/HEAD"
function safe_create_leading_directories() fails when it tries to
create directory for \\server and \\server\share paths. Windows treats
'\\server\share' the same way it does with 'c:', they are not
managable in meaning of removing or creating directories. so we have
to skip them in safe_create_leading_directories().
czarek
Johannes Sixt
But while safe_create_leading_directories() traverses the path, it stat()s the
components. Are saying that these stat() calls fail for the server and/or
share part, and then s_c_l_d tries to mkdir them, which naturally fails,
resulting is the error mentioned above? In this case, why exactly would
stat() fail, and isn't it perhaps possible to fix our stat() implementation?
-- Hannes
Cezary Zawadka
> But while safe_create_leading_directories() traverses the path, it stat()s the
> components. Are saying that these stat() calls fail for the server and/or
> share part, and then s_c_l_d tries to mkdir them, which naturally fails,
> resulting is the error mentioned above? In this case, why exactly would
> stat() fail, and isn't it perhaps possible to fix our stat() implementation?
To be honest I haven't known there is custom stat() implementation :)
In my judgement paths 'c:' and '\\server\share' (no trailing
backslash) should be handled the same way. I think Windows treat of
them that way, because functions stat(), mkdir(), scandir() etc. work
without problem for paths c:\* or \\server\share\* but not for c:,
\\server . So I've found what the problem is, found how and where 'c:'
paths are handled and added appropriate handling of unc paths.
Of course the problem could be solved using "stat()'s way" as you are
suggesting. But there are unanswered questions: how should paths
\\server (no share) or \\invalidserver or \\server\invalidhsare be
handled? If we support stat() for previously mentioned paths then we
should support mkdir(), scandir(), etc. IMHO "stat()'s way" is more
complicated then treating \\server\share the same way as c: And I'm
not sure if "stat()'s way" has a strong advantage over "\\server\share
as c:".
czarek
Johannes Sixt
Assume stat("//server", &st) succeeds and S_ISDIR(st.st_mode). Then
mkdir("//server") will not be tried. Same for "//server/share".
stat("//invalidserver") must fail; mkdir("//invalidserver") will (and should)
fail. Same for "//server/invalidshare".
IOW, if we make sure that stat("//server") and stat("//server/share") succeed,
it should be sufficient to skip only past the leading slashes in
offset_1st_component.
OTOH, if it turns out that this complicates the stat() implementation
considerably, I would actually prefer something like your implementation of
offset_1st_component, but I would really see the stat() option explored[*]
before making a decision.
[*] A proof of concept or some exhibition what would be necessary is
sufficient.
-- Hannes