Daniel Shahaf wrote:
> One alternative solution is having svn_fs_fs__verify() stat() the DB
> file for existence before attempting to operate on it.
>
> I don't think it's the most elegant thing in the world, but it avoids
> spuriously creating the DB file without complicating the common case
> logic.
That sounds Good Enough.
- Julian
> I'm also open to alternative suggestions if anyone has them.
>
>
> Julian Foad wrote on Tue, Jul 19, 2011 at 19:12:33 +0100:
> > On Thu, 2011-07-14, Daniel Shahaf wrote:
> > > I've looked again at the code. Right now it opens the db, with
> > > svn_sqlite__mode_rwcreate, within an svn_atomic_init_once() block.
> > >
> > > Adding the special case "For <this> caller use svn_sqlite__mode_readwrite"
> > > means we can't use svn_atomic_init_once() any more. (since it's no
> > > longer "once", maybe the first call failed due to being non-rwcreate)
> > > And have to add some error handling logic around the open() call in that
> > > function.
> > >
> > > How about going for the less elegant but more readable solution,
> > >
> > > [[[
> > > Index: subversion/libsvn_fs_fs/rep-cache.c
> > > ===================================================================
> > > @@ -182,6 +187,11 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *fs,
> > > }
> > > +
> > > + if (iterations == 0)
> > > + SVN_ERR(svn_io_remove_file2(path_rep_cache_db(fs->path, iterpool),
> > > + TRUE /* ignore_enoent */,
> > > + iterpool));
> > >
> > > SVN_ERR(svn_sqlite__reset(stmt));
> > >
> > > ]]]
> > >
> > > ?
> >
> > I've just looked at the code to see what you're suggesting here. You're
> > suggesting to make svn_fs_fs__walk_rep_reference() delete the rep cache
> > DB file if it's empty. Relying on the fact that svn_fs_fs__verify() is
> > currently the only caller of that function. That seems too ugly.
> >
> > - Julian
> >
> >
> > > > > We could make the code pass readwrite (rather than rwcreate) for that
> > > > > one caller, and then mask the error when the DB doesn't exist.
> > > >
> > > > +1.
> > > >
> > > > > (We do
> > > > > want to verify the DB if it's exists but fsfs.conf says "Don't use it",
> > > > > since fsfs.conf may get changed.)
> > > >
> > > > +1.
> > > >
> > > > > I'm not sure that non-creating an
> > > > > unused db justifies this additional code.
> > > >
> > > > I'm with Greg - it would be good for the users.
> > > >
> > > > - Julian
> >
> >
Received on 2011-07-20 16:31:25 CEST