Stefan Sperling wrote on Tue, May 08, 2012 at 12:56:46 +0200: > On Thu, Apr 26, 2012 at 10:04:35PM -0000, danie...@apache.org wrote: > > Author: danielsh > > Date: Thu Apr 26 22:04:34 2012 > > New Revision: 1331125 > > > > URL: http://svn.apache.org/viewvc?rev=1331125&view=rev > > Log: > > Follow-up to r1331050: try to unbreak the build. > > > > * subversion/libsvn_fs_fs/fs.c > > (fs_hotcopy): Properly open SRC_FS. Call svn_fs__check_fs(dst_fs). > > The log message lacks some substance. > What exactly is the problem being solved by opening the source FS > here instead of letting fs_fs.c open it? > > I understand the rush involved in fixing the build, but it would > be nice to revise this log message to make the semantic change > that actually happened more clear. >
Edied the log message in light of these comments. > One more remark below. > > > > > Modified: > > subversion/trunk/subversion/libsvn_fs_fs/fs.c > > > > Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.c > > URL: > > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.c?rev=1331125&r1=1331124&r2=1331125&view=diff > > ============================================================================== > > --- subversion/trunk/subversion/libsvn_fs_fs/fs.c (original) > > +++ subversion/trunk/subversion/libsvn_fs_fs/fs.c Thu Apr 26 22:04:34 2012 > > @@ -293,10 +293,34 @@ fs_hotcopy(svn_fs_t *src_fs, > > void *cancel_baton, > > apr_pool_t *pool) > > { > > - SVN_ERR(initialize_fs_struct(src_fs)); > > - SVN_ERR(fs_serialized_init(src_fs, pool, pool)); > > - SVN_ERR(initialize_fs_struct(dst_fs)); > > - SVN_ERR(fs_serialized_init(dst_fs, pool, pool)); > > + { > > + svn_fs_t *fs = src_fs; > > + const char *path = src_path; > > + > > + SVN_ERR(svn_fs__check_fs(fs, FALSE)); > > + SVN_ERR(initialize_fs_struct(fs)); > > + SVN_ERR(svn_fs_fs__open(fs, path, pool)); > > + SVN_ERR(svn_fs_fs__initialize_caches(fs, pool)); > > + SVN_ERR(fs_serialized_init(fs, pool, pool)); > > + } > > + > > + { > > + svn_fs_t *fs = dst_fs; > > + const char *path = dst_path; > > + > > + SVN_ERR(svn_fs__check_fs(fs, FALSE)); > > + SVN_ERR(initialize_fs_struct(fs)); > > +#if 0 > > Did you intend to clean up this #if 0 marker or leave it in here forever? > > I find it is somewhat confusing. It makes it look as if we were undecided > about what to do here. > The code is #if 0'd because I wanted it when incremental=TRUE but it breaks the incremental=FALSE case. We should remove that temp code and ensure we open DST_FS and initialize its caches properly in all case.s > > + /* In INCREMENTAL mode, svn_fs_fs__hotcopy() will open DST_FS. > > + Otherwise, it's not an FS yet --- possibly just an empty dir --- > > so > > + can't be opened. > > + */ > > + SVN_ERR(svn_fs_fs__open(fs, path, pool)); > > + SVN_ERR(svn_fs_fs__initialize_caches(fs, pool)); > > +#endif > > + SVN_ERR(fs_serialized_init(fs, pool, pool)); > > + } > > + > > return svn_fs_fs__hotcopy(src_fs, dst_fs, src_path, dst_path, > > incremental, cancel_func, cancel_baton, pool); > > } > >
