[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1331125 - /subversion/trunk/subversion/libsvn_fs_fs/fs.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 8 May 2012 12:56:46 +0200

On Thu, Apr 26, 2012 at 10:04:35PM -0000, danielsh_at_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);
> }
>
Received on 2012-05-08 12:57:24 CEST

This is an archived mail posted to the Subversion Dev mailing list.