Greg Stein <gstein@lyra.org> writes:
> On Wed, Mar 05, 2003 at 12:38:35PM -0600, cmpilato@tigris.org wrote:
> >...
> > * subversion/libsvn_fs/bdb/bdb-fs.c
> > New.
> > The following functions have been re-named and moved from fs.c:
> > (bdb_check_already_open): was (check_already_open)
> > (cleanup_fs_bdb): was (cleanup_fs_db)
> > (bdb_cleanup_fs): was (cleanup_fs)
> > (bdb_allocate_env): was (allocate_env)
> > (bdb_print_fs_stats): was (print_fs_stats)
> > (bdb_cleanup_fs_apr): was (cleanup_fs_apr)
> > (bdb_set_berkeley_errcall): was (svn_fs_set_berkeley_errcall)
> > (bdb_close_fs): was (svn_fs_close_fs)
> > (svn_fs_create_berkeley): was (svn_fs_create_fs)
> > (bdb_open_fs): was (svn_fs_open_berkeley)
> > (bdb_recover_fs): was (svn_fs_berkeley_recover)
> > (bdb_delete_fs): was (svn_fs_delete_berkeley)
>
> Woah. That's not good. Those functions are visible outside of the link unit.
> They should all be prefixed with svn_fs__
Yeah, yeah, we knew that. The state is intermediate, but I'm sure
Glenn wouldn't mind using the svn_fs__ prefix in the meantime.
>
> > * subversion/libsvn_fs/bdb/bdb-fs.h
> > New. This file contains table "open function" declarations removed
> > from their respective xxx-table.h include files. It also has
>
> Why just the open functions? That seems like a strange division. The other
> functions in the API are quite specific to the BDB implementation, so why
> didn't they move, too? Why not just put all of the prototypes into bdb-fs.h?
Hmm, yeah, I have to admit that I'm sure why the division happened
here, either.
> Call me -0.5 on this commit :-(
I can certainly revert the commit. Here's what I'd rather do though:
1. Branch from trunk@HEAD right now (/branches/fs-db-abstract)
2. Revert 5212 from trunk
3. Give Glenn commit privs to the new branch with the stipulation
that his work *will not* get merged into the trunk unless he
continues to work incrementally, poking us (me?) at his
breakpoints where merging with trunk would be appropriate.
I *want* this DB abstraction to happen. It is a Good Thing for
Subversion (and for CollabNet, IMO).
> It would seem to be better to define the vtable, complete the discussion
> around *that*, add it to the code, and then start migrating stuff
> into it.
That is a good step, but this change (at least, most of it) was a
necessary precursor to such a move. This change was long overdue, and
was simply the completion of his previous one (where much of the
BDB-table stuff moved into the /bdb subdir).
> With the current approach, a lot of stuff is shifting around, but the
> reasoning isn't clear at all. And what that *really* means is that we have
> no way to evaluate the *design* of this vtable stuff.
Glenn's design has been publically available on his website for
... gosh, many many months (has it been a year yet?), and he's been
very good at advertising that location in his correspondences to this
list. If you want to evaluate the design for yourself, go read it.
> I'm highly suspect of the design right now. The FS is very dependent upon
> the BDB-based design.
The only aspect of our filesystem that is strongly tied to Berkeley
(that I can think of right now) is the trail notion. All of the
tree/dag stuff has long since been converted to use internal
structures to represent the data being passed around (i.e., nobody
except the actual BDB-calling functions knows anything about skels) --
a new database backend need only know how to populate and store
representations of those structures, and to answer the generic
questions we ask of the database layer.
> While I'm fine with a vtable for the database portions (the
> "backend"), I'm really not a fan of a vtable for the FS API
> itself. I don't really see a need to change things at that level;
> the probability for changing the semantics is just too great,
> leading to problems down the line. Just how much rope do we want to
> provide?
For what it's worth, I also would prefer static FS APIs, and don't see
the need for an FS vtable. Glenn, how much of your design is
absolutely dependent on that part of the change. If memory serves, it
was more about flexibility for implementers of new schemas than for
correctness. How wrong am I?
> > (svn_fs_set_berkeley_errcall, svn_fs_close_fs, svn_fs_recover_fs,
> > svn_fs_create_berkeley, svn_fs_open_fs, svn_fs_delete_fs): These
> > are now just wrappers around new functions in bdb-fs.c which contain
> > the old "guts" from these functions.
>
> Hunh? There is no svn_fs_close_fs().
Oops. Out of date log message.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 6 17:10:45 2003