On Sun, Dec 22, 2013 at 2:46 PM, Branko Čibej <br...@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 <br...@wandisco.com> wrote: > >> On 08.12.2013 11:52, Stefan Fuhrmann wrote: >> >> On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf >> <d...@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.
