http://subversion.tigris.org/issues/show_bug.cgi?id=3641
Briefly, the issue is that when svnsync encounters the following history:
------------------------------------------------------------------------
r5 | pm | 2010-05-18 17:53:46 +0100 (Tue, 18 May 2010)
Changed paths:
A /H (from /A:4)
R /H/B (from /X:4)
------------------------------------------------------------------------
r3 | pm | 2010-05-18 17:53:46 +0100 (Tue, 18 May 2010)
Changed paths:
A /A/B/C
------------------------------------------------------------------------
it looks for /H/B/C in the sync source when trying to replay r5.
I've attached a patch (with log msg) that seems to have some positive
effects: it causes the sync to pass (and properly add children of /X as
children of /H/B).
I'm asking for review for two reasons:
* authz. The whole function is authz-sensitive --- its goal is to
represent a copy as an add. I think the patch is okay from this
angle, but a second pair of eyes wouldn't hurt.
* assertions. The patch asserts that svn_fs_path_change2_t->copyfrom_known
is TRUE. However, the FS API used --- svn_fs_paths_changed2() ---
does not guarantee that copyfrom_known will in fact be TRUE, and
I haven't found a different API that does promise to provide the
copyfrom information. (I think the patch only needs this information
for directory replaces.)
Testing, analysis, reviews are welcome.
Daniel[[[
Fix issue #3641: improve svn_repos_replay2()'s handling of dir replaces.
* subversion/libsvn_repos/replay.c
(add_subdir):
Set SOURCE_PATH and SOURCE_ROOT correctly when recursing.
* subversion/tests/cmdline/svnsync_tests.py
(test_bikeshed, test_list):
TODO: Add a test for this issue.
]]]
Index: subversion/libsvn_repos/replay.c
===================================================================
--- subversion/libsvn_repos/replay.c (revision 956596)
+++ subversion/libsvn_repos/replay.c (working copy)
@@ -199,6 +201,8 @@ add_subdir(svn_fs_root_t *source_root,
svn_fs_path_change2_t *change;
svn_boolean_t readable = TRUE;
svn_fs_dirent_t *dent;
+ const char *copyfrom_path = NULL;
+ svn_revnum_t copyfrom_rev = SVN_INVALID_REVNUM;
const char *new_path;
void *val;
@@ -221,6 +225,13 @@ add_subdir(svn_fs_root_t *source_root,
/* If it's a delete, skip this entry. */
if (change->change_kind == svn_fs_path_change_delete)
continue;
+ else if (change->change_kind == svn_fs_path_change_replace)
+ {
+ /* XXX: can this assert fail? */
+ SVN_ERR_ASSERT(change->copyfrom_known);
+ copyfrom_path = change->copyfrom_path;
+ copyfrom_rev = change->copyfrom_rev;
+ }
}
if (authz_read_func)
@@ -232,12 +243,32 @@ add_subdir(svn_fs_root_t *source_root,
if (dent->kind == svn_node_dir)
{
+ /* XXX: add a test to svnsync_tests.py */
+ svn_fs_root_t *new_source_root;
+ const char *new_source_path;
void *new_dir_baton;
- SVN_ERR(add_subdir(source_root, target_root, editor, edit_baton,
+ if (copyfrom_path)
+ {
+ svn_fs_t *fs = svn_fs_root_fs(source_root);
+ SVN_ERR(svn_fs_revision_root(&new_source_root, fs, copyfrom_rev,
pool));
+ new_source_path = copyfrom_path;
+ }
+ else
+ {
+ new_source_root = source_root;
+ new_source_path = svn_path_join(source_path, dent->name,
+ subpool);
+ }
+
+ /* XXX: authz considerations?
+ *
+ * I think not; when path_driver_cb_func() calls add_subdir(), it
+ * passes SOURCE_ROOT and SOURCE_PATH that are unreadable.
+ */
+ SVN_ERR(add_subdir(new_source_root, target_root, editor, edit_baton,
new_path, *dir_baton,
- svn_path_join(source_path, dent->name,
- subpool),
+ new_source_path,
authz_read_func, authz_read_baton,
changed_paths, subpool, &new_dir_baton));
