On Tue, 2009-11-10 at 20:53 +0530, Kannan wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Julian Foad wrote:
> > On Tue, 2009-11-10 at 19:34 +0530, Kannan wrote:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >> Julian Foad wrote:
> >>> The value of "fs_path" there comes from
> >>> svn_client__path_relative_to_root(..., include_leading_slash=TRUE) which
> >>> says, "Return the path of ABSPATH_OR_URL relative to the repository root
> >>> (REPOS_ROOT) in REL_PATH (URI-decoded)".
> >>>
> >>> The dirent_uri API defines "relpath" as never having a leading slash, so
> >>> we can't use svn_relpath_join(). The API does not specify whether a
> >>> "uri" is URI-encoded, so we suppose we can use svn_uri_join().
> >>>
> >>> I think that way of using the APIs is not ideal, but it will do for now.
> >> Thank you for the clarification, Julian.
> >>>> And for
> >>>> the other case(from your comments) use svn_dirent_join() as it points
> >>>> only to local absolute paths, or I'm still not getting you?. But the
> >>>> 'ls' tests seem to pass for me for the patch I sent, which is odd?
> >>> Not sure what "other case" you are referring to.
> >> Sorry for the brevity. Referred to the change to svn_uri_join() in the
> >> "if" block of the same patch.
> >
> > Index: subversion/libsvn_client/list.c
> > ===================================================================
> > --- subversion/libsvn_client/list.c (revision 40358)
> > +++ subversion/libsvn_client/list.c (working copy)
> > @@ -86,11 +86,11 @@
> >
> > svn_pool_clear(iterpool);
> >
> > - path = svn_path_join(dir, item->key, iterpool);
> > + path = svn_relpath_join(dir, item->key, iterpool);
> >
> > if (locks)
> > {
> > - const char *abs_path = svn_path_join(fs_path, path,
> > iterpool);
> > + const char *abs_path = svn_uri_join(fs_path, path, iterpool);
> > lock = apr_hash_get(locks, abs_path, APR_HASH_KEY_STRING);
> > }
> > else
> >
> > That "if" block? But that's the one we have been talking about, since
> > Dan Rall asked "Why use svn_uri_join() rather than svn_dirent_join()?"
> > in this message:
> > <http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415197>.
>
> Oops, thought you answered for the svn_relpath_join()! So, the
> svn_relpath_join() is fine then? (Bit confused)
In the message
<http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416141>, I was talking about that "if" block. (You can follow the quoted context back to this earlier message: <http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415197>.)
I said "we can't use svn_relpath_join() [...] we suppose we can use
svn_uri_join()". I meant that svn_uri_join() does the right thing. I
wrote "we suppose" because I don't like the design of svn_uri_join()
being used for this purpose, but it is better than using
svn_path_join(). So this change is correct:
if (locks)
{
- const char *abs_path = svn_path_join(fs_path, path,
iterpool);
+ const char *abs_path = svn_uri_join(fs_path, path, iterpool);
lock = apr_hash_get(locks, abs_path, APR_HASH_KEY_STRING);
}
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416183
Received on 2009-11-10 16:45:32 CET