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

Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h temp_serializer.c temp_serializer.h

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Tue, 31 Dec 2013 17:28:37 +0100

On Sun, Dec 22, 2013 at 2:46 PM, Branko Čibej <brane_at_wandisco.com> wrote:

> On 22.12.2013 13:54, Stefan Fuhrmann wrote:
>
> [Back from enjoying the plague]
>
> On Sun, Dec 8, 2013 at 4:53 PM, Branko Čibej <brane_at_wandisco.com> wrote:
>
>> On 08.12.2013 11:52, Stefan Fuhrmann wrote:
>>
>> On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name>wrote:
>>
>>> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:> >
>>> >
>>> > Duly noted for Subversion 2.0
>>> >
>>> > For now, however, we have to implement id_vtable_t.
>>>
>>> Huh? id_vtable_t is not a public API. I assume the id vtable falls
>>> under the same rules as the fs_library vtable --- see
>>> fs_library_vtable_t.get_version's docstring.
>>>
>>
>> True, but the following API simply don't provide any more information:
>>
>> svn_fs_check_related(const svn_fs_id_t *a, const svn_fs_id_t *b)
>> svn_fs_compare_ids(const svn_fs_id_t *a, const svn_fs_id_t *b)
>>
>> So, our private vtable depends on the data provided by public API.
>> Adding the pool member to the internal struct is exactly the way to
>> decouple the implementation from the existing public interface.
>>
>>
>> There's nothing wrong with revising the public API to require a scratch
>> pool. Sure, the deprecated, pool-less version will be less efficient. But
>> I've yet to hear why "lean and smart", i.e., unstructured IDs are so much
>> better. than what we have currently. After all, their structure matches the
>> semantics of our repository model, regardless of implementation.
>>
>
> This is not about structured vs. unstructured IDs. It's about needlessly
> exposing internal representation at the API level. For instance, it
> requires
> us to never change the internal node_id assignment rules (copy vs.
> branch):
> svn_fs_compare_ids does not provide a pool argument; hence the ID struct
> needs to always carry all information.
>
>
> I'll just point out that the API does not dictate what kind of IDs you use
> internally in the implementation. The fact that svn_fs_id_t exists does not
> mean you have to use its fixed-size, binary representation in directory
> reps, or anywhere else in the storage layer.
>

I fully agree. That is the whole point of the initial commit.
I suppose you support that one now?

> So I wonder who's confusing API with implementation now.
>

Well, adding a pool member to the ID *implementation* struct
has become necessary due to the limitations of the ID *API*.

-- Stefan^2.
Received on 2013-12-31 17:29:17 CET

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