You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2012/05/08 12:56:46 UTC
Re: svn commit: r1331125 -
/subversion/trunk/subversion/libsvn_fs_fs/fs.c
On Thu, Apr 26, 2012 at 10:04:35PM -0000, danielsh@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.
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.
> + /* 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);
> }
>
Re: svn commit: r1331125 -
/subversion/trunk/subversion/libsvn_fs_fs/fs.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Tue, May 08, 2012 at 13:46:23 +0200:
> On Tue, May 08, 2012 at 02:34:32PM +0300, Daniel Shahaf wrote:
> > 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
>
> OK, I'll try to take care of that. Thanks.
Thanks for takin gcare of that :)
Re: svn commit: r1331125 -
/subversion/trunk/subversion/libsvn_fs_fs/fs.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, May 08, 2012 at 02:34:32PM +0300, Daniel Shahaf wrote:
> Edied the log message in light of these comments.
Cheers!
> 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
OK, I'll try to take care of that. Thanks.
Re: svn commit: r1331125 -
/subversion/trunk/subversion/libsvn_fs_fs/fs.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Tue, May 08, 2012 at 12:56:46 +0200:
> On Thu, Apr 26, 2012 at 10:04:35PM -0000, danielsh@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);
> > }
> >