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 > > > >
