On Fri, 08 Sep 2006, Kamesh Jayachandran wrote:
> Malcolm Rowe wrote:
> >On Thu, Sep 07, 2006 at 08:11:33PM +0530, Kamesh Jayachandran wrote:
> >
> >>Move mergeinfo indexing code out of libsvn_fs_fs to libsvn_fs_util(new
> >>module)
> >>so that it can be reused by bdb repos.
> >
> >It occurs to me that if you're creating a new utility library for
> >the filesystem implementations to use then we would want to consider
> >whether it's intended to be private (solely used by the filesystem
> >implementations) or public (usable by anyone). For example, should
> >users be able to call functions in it directly?
>
> Currently it is meant to be consumed by libsvn_fs_fs and libsvn_fs_base.
> How can I make it private only to 'libsvn_fs_fs' and 'libsvn_fs_base'?
It doesn't look like anyone's come up with a portable way to do that
yet (at least, not using static linkage, or build magic to dump the
code directly into _fs and _base). What we discussed while Kamesh was
visiting in Brisbane is to use the new subversion/include/private/
area for the Subversion-private header files.
> >I'd suggest that we'd just be adding complexity if we made this a
> >publicly-accessible library: we'd have to support a whole new set of
> >interfaces in a binary-compatible fashion. On the other hand, if we
> >make it private, we can change the implementation at will.
> >
> >It looks like you're planning to make it private, since you aren't
> >installing a public include file. I'd just like to make sure that that's
> >a deliberate choice rather than an accident of the current design.
>
> It is an accident, I did not think about the BC issues and visiblity of
> this libs to other modules other than libsvn_fs_fs and libsvn_fs_base.
If the include/private/ area is used for the svn_fs_util.h, should we
bother worrying about BC issues? I tend to think no -- anyone
violating our encapsulation strategy to that extent rather deserves
any compat headaches that they receive as a result.
...
> >If we do keep it as a shared library, you'll need to add version checks
> >(i.e. svn_ver_check_list()) to it, and also add it to the existing checks
> >in libsvn_fs_{base,fs}. If we make it static, that won't be necessary.
> >
> >Finally, while I prefer the 'util' name, I wonder whether it'd be better
> >to call it something like 'libsvn_fs_subr' for parity with 'libsvn_subr'.
>
> Sounds good.
This exact discussion came up one morning between Kamesh and Mike and
myself. IIRC, while we were aware of the minor inconsistency it would
cause, Mike and I still preferred "util", on the grounds that it's
more obivous (no more "What's 'subr' stand for?" questions). I
maintain the same preference today.
> >[The really important question - whether this change actually makes sense
> >in the context of the merge-tracking work - I'll leave to someone with
> >more experience of that code.]
It doesn't matter one way or the other. Mike mentioned that there is
other FS code -- unrelated to merge tracking -- which should really
live in a utility library, too.
- application/pgp-signature attachment: stored
Received on Tue Sep 12 00:31:06 2006