You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hugo Bastos Weber <hu...@pgr.mpf.gov.br> on 2009/10/13 15:45:48 UTC
[PATCH] fix to use repository in root drive letter under win32
[[[
* subversion/libsvn_ra_local/split_url.c
(svn_ra_local__split_URL): Allow file:///X:/ in addition to file:///X:/path. This fix allows one to use repository in the root drive lettler.
]]]
Re: [PATCH] fix to use repository in root drive letter under win32
Posted by Julian Foad <ju...@wandisco.com>.
Gavin Baumanis wrote:
> I have logged the patch submission in the tracker: #3535;
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=3535
It looks like the submitter, Hugo, tried some tests manually, as quoted
in
<http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2407695>. It sounds like the patch is considered good, but as Brane and I can't test it, it would be especially great if Hugo or someone could add those tests to one of the regression test files. Then we could commit it and rely on the build-bots to test it.
- Julian
> On 14/10/2009, at 02:45 , Hugo Bastos Weber wrote:
>
> > [[[
> >
> > * subversion/libsvn_ra_local/split_url.c
> > (svn_ra_local__split_URL): Allow file:///X:/ in addition to file:///X:/path. This fix allows one to use repository in the root drive lettler.
> >
> > ]]]
> >
> > <svn_root_win.patch>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2420512
Re: [PATCH] fix to use repository in root drive letter under win32
Posted by Gavin Baumanis <ga...@thespidernet.com>.
I have logged the patch submission in the tracker: #3535;
http://subversion.tigris.org/issues/show_bug.cgi?id=3535
Gavin.
On 14/10/2009, at 02:45 , Hugo Bastos Weber wrote:
> [[[
>
> * subversion/libsvn_ra_local/split_url.c
> (svn_ra_local__split_URL): Allow file:///X:/ in addition to file:///X:/path. This fix allows one to use repository in the root drive lettler.
>
> ]]]
>
> <svn_root_win.patch>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2420498
Re: [PATCH] fix to use repository in root drive letter under win32
Posted by Branko Cibej <br...@xbc.nu>.
[Please keep the thread on the list]
Hugo Bastos Weber wrote:
> Banko, thanks for the comments. My tests didn't show the result you
> wrote, please read on:
>
Oh duh sorry, I totally misread your patch. Shame on me, I forgot that
we canonicalizing paths strips trailing slashes, so you'll never get a
file:///X:/ in that function if it's called properly.
So your patch appears to be correct, but I can't test it as I currently
don't have a Windows development box. I'd personally avoid the strlen,
though, it's wasteful and not necessary;
(!dup_path[3] || dup_path[3] == '/')
works just as well.
-- Brane
> the following comment was taken from
> subversion/libsvn_ra_local/split_url.c, line 79:
>
> /* On Windows, we'll typically have to skip the leading / if the
> path starts with a drive letter. Like most Web browsers, We
> support two variants of this scheme:
> ...
>
> I tried file:///X:/ on Mozilla Firefox and it works perfectlly.
>
> subversion/libsvn_ra_local/split_url.c, line 102, skips the leading
> slash when it enters the following:
>
> if (!hostname && dup_path[1] && strchr(valid_drive_letters,
> dup_path[1])
> && (dup_path[2] == ':' || dup_path[2] == '|')
> && dup_path[3] == '/')
> {
>
> My tests:
> if called file:///x:/ then dup_path is "/x:", then dup_path[3] is not
> == '/'
> if called file:///x:. then dup_path is "/x:.", then dup_path[3] is not
> == '/'
> if called file:///x:/. then dup_path is "/x:", then dup_path[3] is not
> == '/'
> if called file://x:/ then an error show "contains only a hostname, no
> path"
>
> The solution in my patch is to change the if to:
>
> if (!hostname && dup_path[1] && strchr(valid_drive_letters,
> dup_path[1])
> && (dup_path[2] == ':' || dup_path[2] == '|')
> && (dup_path[3] == '/' || strlen(dup_path) == 3)
> {
>
> now:
>
> if called file:///x:/ then dup_path is "/x:", then dup_path[3] is not
> == '/' however dup_path[2] == ':' and strlen(dup_path) == 3 <-- OK, it
> works, it's canonical
> if called file:///x:. then dup_path is "/x:.", then dup_path[3] is not
> == '/' and strlen(dup_path) != 3 <-- Don't work
> if called file:///x:/. then dup_path is "/x:", then dup_path[3] is not
> == '/' just like the first case, it works too.
> if called file://x:/ then an error show "contains only a hostname, no
> path" <-- still the same behavior
>
>
> Branko, if you try to run file:///X:/, file:///X:. or file:///X:/.
> you'll see none of them work. Also, the patch is not acception
> file:///X:. because it will have a lenght == 4 no 3.
>
>
> Waiting for more feedbacks,
> Hugo.
>
>
>
>
>
>
>
>>>> Branko Čibej <br...@xbc.nu> 10/14/09 1:55 am >>>
>>>>
> Hugo Bastos Weber wrote:
>
>> [[[
>>
>> * subversion/libsvn_ra_local/split_url.c
>> (svn_ra_local__split_URL): Allow file:///X:/ in addition to
>>
> file:///X:/path. This fix allows one to use repository in the root drive
> lettler.
>
>> ]]]
>>
>>
>
> Isn't the log message wrong? You're not allowing file:///X:/ which is
> already supported, but file:///X:. But that's not a canonical URL (we
> "know" that /^[A-Za-z]:/ is a directory on Windows), and
> libsvn_ra_local
> has no business accepting non-canonical paths. The bug is IMHO in the
> code that calls, e.g., svn_ra_open with a non-canonical URL.
>
> -- Brane
>
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2407695
Re: [PATCH] fix to use repository in root drive letter under win32
Posted by Branko Cibej <br...@xbc.nu>.
Hugo Bastos Weber wrote:
> [[[
>
> * subversion/libsvn_ra_local/split_url.c
> (svn_ra_local__split_URL): Allow file:///X:/ in addition to file:///X:/path. This fix allows one to use repository in the root drive lettler.
>
> ]]]
>
Isn't the log message wrong? You're not allowing file:///X:/ which is
already supported, but file:///X:. But that's not a canonical URL (we
"know" that /^[A-Za-z]:/ is a directory on Windows), and libsvn_ra_local
has no business accepting non-canonical paths. The bug is IMHO in the
code that calls, e.g., svn_ra_open with a non-canonical URL.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2407405