[PATCH] Fix some deprecation warnings

View: New views
11 Messages — Rating Filter:   Alert me  

[PATCH] Fix some deprecation warnings

by Kannan Rengarajan :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Log:
Resolve some deprecation warnings.

* subversion/libsvn_client/list.c
 (get_dir_contents): Use `svn_relpath_join()' and `svn_uri_join()'.

Patch by: Kannan R <kannanr@...>

- --
Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvFK9HlTqcY7ytmIAQIz/wgAkyg4zugdxvIP27chmgND7X9Y2fbwj/7n
b0nNT96VG4IQXjy3u+5068O9jTaKsRYestmMOTj+vA9e6EP4LzQIpTVyAS/bfORD
ARD4fdEsYkm7aKSkFM03/Uhv53UroeHSrLf/uWSseZsjL7DMDDpHfVs0eDYgHsaW
beL3v1nkWUaHpFD7hsKNcuWLwPaptRuSylPqjcflW1OOdXxOpo2LoB0sctflR+73
b2kC7BHF1cacj3v02yvoMpjE/q5eSrdAy6imkgOCPk6fEpNX1hP1tjwfwzIhrdss
1kC/WUkcltLWVi0CM1Yv2cR3+dREmF29GTuqMBAP1Cbl2lsnTyLd5g==
=HIsd
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414347
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


smime.p7s (4K) Download Attachment

Re: [PATCH] Fix some deprecation warnings

by Daniel Rall :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Wed, Nov 4, 2009 at 1:35 AM, Kannan <kannanr@...> wrote:
...
> Resolve some deprecation warnings.
>
> * subversion/libsvn_client/list.c
>  (get_dir_contents): Use `svn_relpath_join()' and `svn_uri_join()'.

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

get_dir_contents() documents "fs_path" as "the absolute filesystem
path of the RA session". The "locks" input parameter's value appears
to always come from svn_ra_get_locks(), which also indicates that its
output hash contains keys which are "absolute fs paths".

1) Is one of these doc strings wrong (meaning that one of the paths is
relative)?

2) Why use svn_uri_join() rather than svn_dirent_join()?

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415197

Re: [PATCH] Fix some deprecation warnings

by Branko Cibej :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Daniel Rall wrote:
> 2) Why use svn_uri_join() rather than svn_dirent_join()?
>  

uri_join is for paths within the versionable filesystem, and URLs to
entities in a repository. dirent_join is for paths in the WC. As I
learned the hard way.

-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415198

Re: [PATCH] Fix some deprecation warnings

by Daniel Rall :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, Nov 6, 2009 at 11:08 AM, Branko Čibej <brane@...> wrote:
> Daniel Rall wrote:
>> 2) Why use svn_uri_join() rather than svn_dirent_join()?
>
> uri_join is for paths within the versionable filesystem, and URLs to
> entities in a repository. dirent_join is for paths in the WC. As I
> learned the hard way.

Yep yep.

The documentation that I cited, plus the fact that we're in
libsvn_client here, makes me think that we're referencing local file
system paths; doesn't that mean we want svn_dirent_join()? (And maybe
that's what you're getting at.)

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415225

Re: [PATCH] Fix some deprecation warnings

by Kannan Rengarajan :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Daniel Rall wrote:

> On Fri, Nov 6, 2009 at 11:08 AM, Branko Čibej <brane@...> wrote:
>> Daniel Rall wrote:
>>> 2) Why use svn_uri_join() rather than svn_dirent_join()?
>> uri_join is for paths within the versionable filesystem, and URLs to
>> entities in a repository. dirent_join is for paths in the WC. As I
>> learned the hard way.
>
> Yep yep.
>
> The documentation that I cited, plus the fact that we're in
> libsvn_client here, makes me think that we're referencing local file
> system paths; doesn't that mean we want svn_dirent_join()? (And maybe
> that's what you're getting at.)
Thank you Branko/Daniel, for your comments. DIR entries are relative to
root of ra session, doesn't that mean we use svn_relpath_join()? 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?

- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvesXHlTqcY7ytmIAQLQHQf/dgz7E5hN//U52S7ZnT13frD43mCThXg+
1MVTUOsJmTKWSc4tDj7eiJRdP7faXxJFEJhp1fIsLQQMsZOBT/8LGBNLuL7lUp8r
rkc4Y2HQyfZ9j+Ohk6p5gH4qsTdeC0ZVugKN2ZPyVARh6wl42hYKeT9rD1Sq6jY+
0nNtnnci3WNsG7YEqsNN7d0uN+GErWO6ZlLyFgajB1CVAQhaqXdMgZ8ZiEnO3yi+
YXRmHbxs7+P1LXnuVF2wR+xk6K0y+iBWe15dzbK/BDb4PKlt5Hn90jZPE07ITgAA
Ld1Y8WLqESMC72z8q2CIyg/g4vP9fSirNppa3hWN2a6vws7UJ/7qVA==
=K9dl
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415680

smime.p7s (4K) Download Attachment

Re: [PATCH] Fix some deprecation warnings

by Julian Foad :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Mon, 2009-11-09 at 11:15 +0530, Kannan wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Daniel Rall wrote:
> > On Fri, Nov 6, 2009 at 11:08 AM, Branko Čibej <brane@...> wrote:
> >> Daniel Rall wrote:
> >>> 2) Why use svn_uri_join() rather than svn_dirent_join()?
> >> uri_join is for paths within the versionable filesystem, and URLs to
> >> entities in a repository. dirent_join is for paths in the WC. As I
> >> learned the hard way.
> >
> > Yep yep.
> >
> > The documentation that I cited, plus the fact that we're in
> > libsvn_client here, makes me think that we're referencing local file
> > system paths; doesn't that mean we want svn_dirent_join()? (And maybe
> > that's what you're getting at.)
>
> Thank you Branko/Daniel, for your comments. DIR entries are relative to
> root of ra session, doesn't that mean we use svn_relpath_join()?

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.

>  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.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416141

Re: [PATCH] Fix some deprecation warnings

by Kannan Rengarajan :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----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.


- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvly/3lTqcY7ytmIAQJZXgf7BMnN24SDHZizZWe8nmbzbtAuBbASuXAk
TCyQ+hEXiH/KjPidTv4oXN9uAJID5YsLYTznNzU7dUl5a4dDuTWH1FnoLXlOp43c
O7LynGcA430/+Zeqx3HUTCmcOvMEFE1Aaqb73kYWMB790sYJ5GUVxGqZ3F6F69/N
7EvuMmDN+QPI+GdEI7N5/D3tBaDmoA+JxtFxX8U/ERSd1a86Iz7332POps0v9n/t
M82b4X5ZdAmNdjCAbxg1J7I36Mc8io+xXql0ad4r2G+cTBZm4XukR467Sqw0J7RC
BqtO15MMoYc/cMP8kheTfKtUZ7RPmYpJBdUYT33wZNkhWZVw7QTq+A==
=hjD7
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416149

smime.p7s (4K) Download Attachment

Re: [PATCH] Fix some deprecation warnings

by Julian Foad :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

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>.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416166

Re: [PATCH] Fix some deprecation warnings

by Kannan Rengarajan :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----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)


- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvmFinlTqcY7ytmIAQKzgAf/bEU/eqmUAiKnCC9tBGxAE8N2pVboSavE
GplpxkK3maM47QAucZi+K/zlO4Yqk1BrpzGCcbiamb6GpjRqQNJKgSGm6o/aFP/b
XJoWU7MKucSlTavXCmEQ6hLLQqFLIk23Mxuma4suzlQrn9sYtrMZBv5M8EdC60Lr
hbM+o+ZDKvN0TFjYQCJWUd0Q2wP19lEQqF/AVwz8Q4SFRLCiSVMWbq25N9pLaImo
WWo++ppPTY54pICXik8lvDzqoowKGWCFkxW4XxpieT6rM32H9PRdmt8rQ2c4ZFPh
9qgtIDeJUUExy8hbECn8GkIFF6Kz9fB6bDvZTgWtWSzqzO/LZ54zxg==
=kjy2
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416178

smime.p7s (4K) Download Attachment

Re: [PATCH] Fix some deprecation warnings

by Julian Foad :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

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

Re: [PATCH] Fix some deprecation warnings

by Julian Foad :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Julian Foad wrote:

> 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);
>          }

Oops, you were asking about the svn_relpath_join() outside the "if"
block. In that case, (copying from IRC so everyone can see ...)

The base is the "dir" parameter, which is "relative to the root of the
RA session". Therefore svn_dirent_join() is definitely wrong. You should
use svn_relpath_join() if it doesn't start with a slash, or
svn_uri_join() if it might start with a slash. From what I can see, it
doesn't start with a slash so your patch is correct in using
svn_relpath_join().

Looking back, I see that those are the only two changes in the patch, so
that means the patch is correct! I will commit it.

Committed revision 40443. Thanks for the patch!

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416202