[Second attempt to attach the patch.]
I (Julian Foad) wrote:
>> I'll come to 'svnsync' later, but basically my current thought is it
>> should 'correct' the mergeinfo by removing the r0 reference.
>
> I have written a test for this, and hacked up code to do this by textual
> substitution. It isn't quite right -- for example it would go wrong if a
> path contained the character sequence ":0", among other issues. I
> ought to rewrite it in the form of a proper mergeinfo parser but one that is
> more lenient than the regular parser.
Here's the patch in progress...
- Julian
Fix the 'svnsync' part of issue #4476 "Mergeinfo containing r0 makes
svnsync and svnadmin dump fail".
Make 'svnsync' adjust mergeinfo containing r0 to remove the r0 before trying
to commit it to the target repository.
### TODO: Give a warning or notification?
### TODO:
Revert svnsync_tests.py r1230676 "re-dump", at it is no longer necessary
since r1292507 "Use a simple dump file parser to compare dumps ...", and
as re-loading-dumping hides errors introduced by 'load'.
### TODO: Implement fix_mergeinfo() as a more 'proper' parser. The current
hack has at least two flaws: goes wrong if a path contains ':0', and
doesn't elide the whole source path reference if '0' was the only
revision mentioned.
* subversion/svnsync/sync.c
(fix_mergeinfo): New function -- a bit of a hack at the moment.
(change_file_prop,
change_dir_prop): Use it.
* subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump
subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump
New files.
* subversion/tests/cmdline/svnsync_tests.py
(setup_and_sync,
verify_mirror): Tweak to avoid passing the expected results through
'svnadmin load' before comparison, because that can alter mergeinfo.
(mergeinfo_contains_r0): New test.
(test_list): Run it.
--This line, and those below, will be ignored--
Index: subversion/svnsync/sync.c
===================================================================
--- subversion/svnsync/sync.c (revision 1643101)
+++ subversion/svnsync/sync.c (working copy)
@@ -30,12 +30,13 @@
#include "svn_auth.h"
#include "svn_opt.h"
#include "svn_ra.h"
#include "svn_utf.h"
#include "svn_subst.h"
#include "svn_string.h"
+#include "svn_ctype.h"
#include "sync.h"
#include "svn_private_config.h"
#include <apr_network_io.h>
@@ -372,12 +373,58 @@ absent_directory(const char *path,
{
node_baton_t *db = dir_baton;
edit_baton_t *eb = db->edit_baton;
return eb->wrapped_editor->absent_directory(path, db->wrapped_node_baton,
pool);
}
+/* Adjust the mergeinfo VALUE to remove any references to r0. */
+static svn_error_t *
+fix_mergeinfo(const svn_string_t **value_p,
+ const svn_string_t *old_value,
+ apr_pool_t *result_pool)
+{
+ svn_string_t *value = svn_string_dup(old_value, result_pool);
+ char *colon_pos;
+
+ /* remove r0 references by textual substitution */
+ /* ### Goes wrong if a path contains ':0'; should instead search for
+ the *last* colon on each line. */
+ while ((colon_pos = strstr(value->data, ":0")))
+ {
+ char *r0_pos = colon_pos + 1;
+
+ SVN_DBG(("fix mergeinfo r0: was ...%.30s", colon_pos));
+
+ if (r0_pos[1] == '*' && r0_pos[2] == ',')
+ {
+ value->len -= 3;
+ memmove(r0_pos, r0_pos + 3, value->len + 1);
+ }
+ else if (r0_pos[1] == ',' || r0_pos[1] == '*' /* at end of line */
+ || (r0_pos[1] == '-' && r0_pos[2] == '1' && ! svn_ctype_isdigit(r0_pos[3])))
+ {
+ value->len -= 2;
+ memmove(r0_pos, r0_pos + 2, value->len + 1);
+ }
+ else if (r0_pos[1] == '\n' || r0_pos[1] == '\r' || r0_pos[1] == '\0')
+ {
+ value->len -= 1;
+ memmove(r0_pos, r0_pos + 1, value->len + 1);
+ }
+ else if (r0_pos[1] == '-')
+ {
+ /* '0-5' should become '1-5'.
+ '0-1' should become '1' but '1-1' is probably good enough. */
+ r0_pos[0] = '1';
+ }
+ SVN_DBG(("fix mergeinfo r0: now ...%.30s", colon_pos));
+ }
+
+ *value_p = value;
+ return SVN_NO_ERROR;
+}
static svn_error_t *
change_file_prop(void *file_baton,
const char *name,
const svn_string_t *value,
apr_pool_t *pool)
@@ -417,12 +464,18 @@ change_file_prop(void *file_baton,
SVN_ERR(normalize_string(&value, &was_normalized,
eb->source_prop_encoding, pool, pool));
if (was_normalized)
(*(eb->normalized_node_props_counter))++;
}
+ /* */
+ if (strcmp(name, SVN_PROP_MERGEINFO) == 0)
+ {
+ SVN_ERR(fix_mergeinfo(&value, value, pool));
+ }
+
return eb->wrapped_editor->change_file_prop(fb->wrapped_node_baton,
name, value, pool);
}
static svn_error_t *
change_dir_prop(void *dir_baton,
@@ -516,12 +569,18 @@ change_dir_prop(void *dir_baton,
SVN_ERR(normalize_string(&value, &was_normalized, eb->source_prop_encoding,
pool, pool));
if (was_normalized)
(*(eb->normalized_node_props_counter))++;
}
+ /* */
+ if (strcmp(name, SVN_PROP_MERGEINFO) == 0)
+ {
+ SVN_ERR(fix_mergeinfo(&value, value, pool));
+ }
+
return eb->wrapped_editor->change_dir_prop(db->wrapped_node_baton,
name, value, pool);
}
static svn_error_t *
close_edit(void *edit_baton,
Index: subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump
===================================================================
--- subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump (revision 0)
+++ subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump (working copy)
@@ -0,0 +1,85 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 6ad9f820-0205-0410-94a2-c8cf366bb2b3
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2005-11-07T23:36:48.095832Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 101
+Content-length: 101
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2000-01-01T00:00:00.000000Z
+K 7
+svn:log
+V 0
+
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 22
+Content-length: 22
+
+K 1
+p
+V 1
+v
+PROPS-END
+
+
+Revision-number: 2
+Prop-content-length: 113
+Content-length: 113
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2005-11-07T23:37:17.705159Z
+K 7
+svn:log
+V 11
+add foo.txt
+PROPS-END
+
+Node-path: foo.txt
+Node-kind: file
+Node-action: add
+Prop-content-length: 86
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 86
+
+K 13
+svn:mergeinfo
+V 51
+/a:0,1
+/b:0*,1
+/c:0,2
+/d:0-1
+/e:0-1,3
+/f:0-2
+/g:0-3
+PROPS-END
+
+
Index: subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump
===================================================================
--- subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump (revision 0)
+++ subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump (working copy)
@@ -0,0 +1,85 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 6ad9f820-0205-0410-94a2-c8cf366bb2b3
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2005-11-07T23:36:48.095832Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 101
+Content-length: 101
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2000-01-01T00:00:00.000000Z
+K 7
+svn:log
+V 0
+
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 22
+Content-length: 22
+
+K 1
+p
+V 1
+v
+PROPS-END
+
+
+Revision-number: 2
+Prop-content-length: 113
+Content-length: 113
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2005-11-07T23:37:17.705159Z
+K 7
+svn:log
+V 11
+add foo.txt
+PROPS-END
+
+Node-path: foo.txt
+Node-kind: file
+Node-action: add
+Prop-content-length: 75
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 75
+
+K 13
+svn:mergeinfo
+V 40
+/a:1
+/b:1
+/c:2
+/d:1
+/e:1,3
+/f:1-2
+/g:1-3
+PROPS-END
+
+
* subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump
* subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump
* subversion/tests/cmdline/svnsync_tests.py
(setup_and_sync):
(verify_mirror):
(fd_leak_sync_from_serf_to_local):
--This line, and those below, will be ignored--
Index: subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump
===================================================================
--- subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump (revision 0)
+++ subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump (working copy)
@@ -0,0 +1,85 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 6ad9f820-0205-0410-94a2-c8cf366bb2b3
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2005-11-07T23:36:48.095832Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 101
+Content-length: 101
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2000-01-01T00:00:00.000000Z
+K 7
+svn:log
+V 0
+
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 22
+Content-length: 22
+
+K 1
+p
+V 1
+v
+PROPS-END
+
+
+Revision-number: 2
+Prop-content-length: 113
+Content-length: 113
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2005-11-07T23:37:17.705159Z
+K 7
+svn:log
+V 11
+add foo.txt
+PROPS-END
+
+Node-path: foo.txt
+Node-kind: file
+Node-action: add
+Prop-content-length: 86
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 86
+
+K 13
+svn:mergeinfo
+V 51
+/a:0,1
+/b:0*,1
+/c:0,2
+/d:0-1
+/e:0-1,3
+/f:0-2
+/g:0-3
+PROPS-END
+
+
Index: subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump
===================================================================
--- subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump (revision 0)
+++ subversion/tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump (working copy)
@@ -0,0 +1,85 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 6ad9f820-0205-0410-94a2-c8cf366bb2b3
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2005-11-07T23:36:48.095832Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 101
+Content-length: 101
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2000-01-01T00:00:00.000000Z
+K 7
+svn:log
+V 0
+
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 22
+Content-length: 22
+
+K 1
+p
+V 1
+v
+PROPS-END
+
+
+Revision-number: 2
+Prop-content-length: 113
+Content-length: 113
+
+K 10
+svn:author
+V 7
+jrandom
+K 8
+svn:date
+V 27
+2005-11-07T23:37:17.705159Z
+K 7
+svn:log
+V 11
+add foo.txt
+PROPS-END
+
+Node-path: foo.txt
+Node-kind: file
+Node-action: add
+Prop-content-length: 75
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 75
+
+K 13
+svn:mergeinfo
+V 40
+/a:1
+/b:1
+/c:2
+/d:1
+/e:1,3
+/f:1-2
+/g:1-3
+PROPS-END
+
+
Index: subversion/tests/cmdline/svnsync_tests.py
===================================================================
--- subversion/tests/cmdline/svnsync_tests.py (revision 1643074)
+++ subversion/tests/cmdline/svnsync_tests.py (working copy)
@@ -206,26 +206,33 @@ def setup_and_sync(sbox, dump_file_conte
source_prop_encoding=source_prop_encoding)
run_copy_revprops(dest_repo_url, repo_url,
source_prop_encoding=source_prop_encoding)
return dest_sbox
-def verify_mirror(dest_sbox, src_sbox):
- """Compare the contents of the DEST_SBOX repository with EXP_DUMP_FILE_CONTENTS."""
+def verify_mirror(dest_sbox, src_sbox=None, src_dump=None):
+ """Compare the contents of the mirror repository in DEST_SBOX with the
+ reference repository in SRC_SBOX or SRC_DUMP,
+ by comparing the parsed dump stream content.
+ First remove svnsync rev-props from the DEST_SBOX repository.
+ """
# Remove some SVNSync-specific housekeeping properties from the
# mirror repository in preparation for the comparison dump.
for prop_name in ("svn:sync-from-url", "svn:sync-from-uuid",
"svn:sync-last-merged-rev"):
svntest.actions.run_and_verify_svn(
None, None, [], "propdel", "--revprop", "-r", "0",
prop_name, dest_sbox.repo_url)
# Create a dump file from the mirror repository.
dest_dump = svntest.actions.run_and_verify_dump(dest_sbox.repo_dir)
- src_dump = svntest.actions.run_and_verify_dump(src_sbox.repo_dir)
+ # Create a dump file from the reference repository if needed.
+ if src_sbox:
+ assert src_dump is None
+ src_dump = svntest.actions.run_and_verify_dump(src_sbox.repo_dir)
svntest.verify.compare_dump_files(
"Dump files", "DUMP", src_dump, dest_dump)
def run_test(sbox, dump_file_name, subdir=None, exp_dump_file_name=None,
bypass_prop_validation=False, source_prop_encoding=None,
@@ -248,22 +255,17 @@ or another dump file."""
is_src_ra_local, is_dest_ra_local)
# Compare the dump produced by the mirror repository with either the original
# dump file (used to create the master repository) or another specified dump
# file.
if exp_dump_file_name:
- build_repos(sbox)
- svntest.actions.run_and_verify_load(sbox.repo_dir,
- open(os.path.join(svnsync_tests_dir,
- exp_dump_file_name),
- 'rb').readlines())
- src_sbox = sbox
+ src_dump_path = os.path.join(svnsync_tests_dir, exp_dump_file_name)
+ src_dump = open(src_dump_path, 'rb').readlines()
+ verify_mirror(dest_sbox, src_dump=src_dump)
else:
- src_sbox = sbox
-
- verify_mirror(dest_sbox, sbox)
+ verify_mirror(dest_sbox, sbox)
######################################################################
# Tests
@@ -573,12 +575,20 @@ def delete_revprops(sbox):
def fd_leak_sync_from_serf_to_local(sbox):
"fd leak during sync from serf to local"
import resource
resource.setrlimit(resource.RLIMIT_NOFILE, (128, 128))
run_test(sbox, "largemods.dump", is_src_ra_local=None, is_dest_ra_local=True)
+#----------------------------------------------------------------------
+
+def mergeinfo_contains_r0(sbox):
+ "mergeinfo contains r0"
+ run_test(sbox, "mergeinfo-contains-r0.dump",
+ exp_dump_file_name="mergeinfo-contains-r0.expected.dump",
+ bypass_prop_validation=True)
+
########################################################################
# Run the tests
# list all tests here, starting with None:
@@ -609,12 +619,13 @@ test_list = [ None,
copy_bad_encoding,
delete_svn_props,
commit_a_copy_of_root,
descend_into_replace,
delete_revprops,
fd_leak_sync_from_serf_to_local, # calls setrlimit
+ mergeinfo_contains_r0,
]
if __name__ == '__main__':
svntest.main.run_tests(test_list)
# NOTREACHED
