[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Follow-up to r879452.

From: Noorul Islam K M <noorul_at_collab.net>
Date: Tue, 07 Dec 2010 21:57:11 +0530

Julian Foad <julian.foad_at_wandisco.com> writes:

> On Tue, 2010-12-07 at 16:06 +0530, Noorul Islam K M wrote:
>
>> Julian Foad <julian.foad_at_wandisco.com> writes:
>>
>> > Noorul Islam K M wrote:
>> >
>> >> Log
>> >> [[[
>> >> Follow-up to r879452.
>> >>
>> >> * subversion/libsvn_ra_local/ra_plugin.c
>> >> (svn_ra_local__obliterate_path_rev): Replace call to svn_path_join()
>> >> with svn_dirent_join() function.
>> >
>> > Hi Noorul.
>> >
>> > Why? Please explain.
>> >
>>
>> Because svn_path_join() is deprecated. I could see similar change done in
>> r879452. I thought it will be obvious from the log message because I
>> mentioned the revision.
>
> The problem is that svn_dirent_join() is not a simple drop-in
> replacement for svn_path_join(). The doc string of svn_path_join()
> says:
>
> * New code should use either svn_dirent_join() (for local paths) or
> * svn_uri_join() (for urls) or svn_relpath_join() (for relative paths).
> *
> * @deprecated Provided for backward compatibility with the 1.6 API.
>
> So you have to work out which kind of path is being joined. Have a look
> at where the arguments come from and how the result is used, and read up
> about the three kinds of path mentioned in the doc string, and work out
> which one.
>
> (There is also a fourth kind of path, "fspath" which means a relative
> path starting with "/", and the function svn_fspath__join(), which
> should also be mentioned. I'll update the doc string in a moment, to
> mention that option.)
>
> Also please say how you tested the change: did you run "make
> check" (which combination?)? Did you step through the code in a
> debugger and observe the values? Did you test it in another way?
>

I did not test it because I thought it is an obvious change. I was
confident that the change was correct. I was sure that it should be
svn_dirent_join() since I was making the change to function
svn_ra_local__obliterate_path_rev under subversion/libsvn_ra_local.

But after reading your mail, I felt my assumptions might have been
wrong. So I went and checked further. It looks like this function is not
implemented for ra_neon, ra_serf and ra_svn. There are two test cases
for obliterate command in test/cmdline and they are all marked as
SKIP. Later I realised that obliterate command is not yet part of svn
command line or am I missing something?

Thanks and Regards
Noorul
Received on 2010-12-07 17:28:12 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.