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

Sharing libsvn_fs_* internals [was: "Locked" messages useless]

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-11-02 14:33:45 CET

Peter N. Lundblad wrote:
> On Tue, 1 Nov 2005, Julian Foad wrote:
>
>>(While we're on the subject, shouldn't those two "err.c" files be unified,
>>given that only about 2 of the 25 functions differ at all between them?)

To recap, I'm talking about the two files subversion/libsvn_fs_{base,fs}/err.c
being almost identical. I've just committed cosmetic changes to remove
spurious differences so that they can be compared side-by-side more easily.
Likewise their header files err.h. The remaining differences are tiny.

It's not just those files. Presumably, if the two filesystems use the same set
of error messages, then there is much in common in their implementation. I
haven't looked further in detail.

> The question is *where* this file should go. libsvn_subr doesn't seem
> right. Do we want a libsvn_fs_util just for this? (I think this has been
> discussed before.)

Definitely not libsvn_subr. It's some private implementation that's common to
the two existing FS back-ends. So, yes, the logical place is a
"libsvn_fs_util" or "libsvn_fs_impl", and the question is whether that is
worthwhile "just for this".

I notice that the fs_* back-ends already have a private and circular dependency
on "../libsvn_fs/fs-loader.h", to share the implementation of "svn_fs_t" etc.
We have two options:

1) Stick with this private and circular dependency:

     libsvn_fs_base <==> libsvn_fs <==> libsvn_fs_fs

In this case, more shared stuff could also be placed in libsvn_fs.

2) Refactor so that the dependencies come in clean layers:

                 libsvn_fs
                ^ ^ ^
               / | \
              / | \
      libsvn_fs_base | libsvn_fs_fs
             ^ | ^
              \ | /
               \ | /
               libsvn_fs_impl

In this case, the definition of "svn_fs_t" and any shared stuff like the error
handling would go in "libsvn_fs_impl". This is definitely the "proper" way to
do it if there is likely to be any complexity.

This structure would give us the option of making the interfaces between them
public if we so wished.

At the moment, while I'd be happy to go the clean, layered, potentially public
route, I don't think the amount of sharing is large enough to require that, and
so I'd be happy to put more shared stuff in libsvn_fs which is simpler in the
short term, as long as we keep our eyes open for the point when it starts to
get unreasonably complex and do the refactoring then.

That said, I'm not going to do anything unless those who work in that part of
the code base want it.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 2 14:34:46 2005

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