On Tue, Oct 16, 2012 at 12:40 PM, Stefan Sperling <s...@elego.de> wrote:
> On Tue, Oct 16, 2012 at 01:12:10AM -0000, stef...@apache.org wrote: > > Author: stefan2 > > Date: Tue Oct 16 01:12:09 2012 > > New Revision: 1398598 > > > > URL: http://svn.apache.org/viewvc?rev=1398598&view=rev > > Log: > > Fix svnadmin hotcopy for packed format 6 repos. Packed revprops were > > not copied properly. Fixes issue #4246. > > > > * subversion/libsvn_fs_fs/fs_fs.c > > (hotcopy_copy_packed_shard): use "packed" copying for packed revprops > > > > Modified: > > subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c > > > > Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c > > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1398598&r1=1398597&r2=1398598&view=diff > > > ============================================================================== > > --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original) > > +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Tue Oct 16 01:12:09 > 2012 > > @@ -10316,6 +10316,7 @@ hotcopy_copy_packed_shard(svn_revnum_t * > > const char *src_subdir_packed_shard; > > svn_revnum_t revprop_rev; > > apr_pool_t *iterpool; > > + fs_fs_data_t *src_ffd = src_fs->fsap_data; > > > > /* Copy the packed shard. */ > > src_subdir = svn_dirent_join(src_fs->path, PATH_REVS_DIR, > scratch_pool); > > @@ -10333,18 +10334,43 @@ hotcopy_copy_packed_shard(svn_revnum_t * > > /* Copy revprops belonging to revisions in this pack. */ > > src_subdir = svn_dirent_join(src_fs->path, PATH_REVPROPS_DIR, > scratch_pool); > > dst_subdir = svn_dirent_join(dst_fs->path, PATH_REVPROPS_DIR, > scratch_pool); > > - iterpool = svn_pool_create(scratch_pool); > > - for (revprop_rev = rev; > > - revprop_rev < rev + max_files_per_dir; > > - revprop_rev++) > > + > > + if ( src_ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT > > + || src_ffd->min_unpacked_rev < rev + max_files_per_dir) > > The spacing here is a bit... unconventional. > I completely agree but I've grown to like it for the following reasons: > I'd expect this code to be formatted like this: > > if (src_ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT > || src_ffd->min_unpacked_rev < rev + max_files_per_dir) > Operand "nesting" level is obscured (just slightly). Remember, how you learned in school to do a tabular / place-aligned addition or subtraction. > or like this: > > if (src_ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT || > src_ffd->min_unpacked_rev < rev + max_files_per_dir) > That moves the root of the operation tree, i.e. the || operation, out of sight. > Other than that, this commit is looking good to me. > I don't cling to my tabular formatting but I use it for the above reasons rather than at a whim. -- Stefan^2. -- * Join us this October at Subversion Live 2012<http://www.wandisco.com/svn-live-2012> for two days of best practice SVN training, networking, live demos, committer meet and greet, and more! Space is limited, so get signed up today<http://www.wandisco.com/svn-live-2012> ! *
