Ping.
Is there anything left to do with this submission?
I haven't noticed that it has been committed and Julians comments seem to be unanswered.
Gavin "Beau" Baumanis
On 09/12/2010, at 2:43 AM, Julian Foad wrote:
> On Tue, 2010-12-07 at 21:57 +0530, Noorul Islam K M wrote:
>> 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.
>
> This code is indeed in RA-local, but that doesn't mean the paths that it
> is processing are local disk paths - they are not.
>
>> 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?
>
> You are correct. The obliterate stuff is not implemented for the other
> RA layers. In addition, I might remove it all from the code base before
> we branch for 1.7, because it is not developed far enough to be useful.
> If you want to stop working on this patch, that's fine with me, or if
> you want to continue, that's fine too.
>
> - Julian
>
>
Received on 2010-12-31 22:34:38 CET