You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2015/01/22 19:14:53 UTC
svn commit: r1653988 - in /subversion/trunk/subversion:
svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
Author: julianfoad
Date: Thu Jan 22 18:14:53 2015
New Revision: 1653988
URL: http://svn.apache.org/r1653988
Log:
Fix issue #4551 "svnrdump load commits wrong properties, or fails, on a
non-deltas dumpfile", by teaching the svnrdump loader to find the original
properties of a copied node at the copy source location.
* subversion/svnrdump/load_editor.c
(ARE_VALID_COPY_ARGS): New macro.
(directory_baton): Add copyfrom fields.
(node_baton): Add a copyfrom URL field.
(new_revision_record): Explicitly initialize 'rb->db', for clarity, now
that we rely on it starting out as null.
(push_directory): New function, factored out of new_node_record() and
extended to store the copyfrom source of the directory.
(new_node_record): Factor out the push_directory() functionality. Store
the copyfrom URL separately from the copyfrom path. Move the variable
'relpath_compose' into a tighter scope. Tweak comments.
(remove_node_props): Look up the original properties in the copy source
even if it is implicitly copied (in other words a child of a copy).
* subversion/tests/cmdline/svnrdump_tests.py
(load_non_deltas_copy_with_props): Remove XFail marker. Extend.
(load_non_deltas_replace_copy_with_props): Remove XFail marker. Extend
a little.
Modified:
subversion/trunk/subversion/svnrdump/load_editor.c
subversion/trunk/subversion/tests/cmdline/svnrdump_tests.py
Modified: subversion/trunk/subversion/svnrdump/load_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/load_editor.c?rev=1653988&r1=1653987&r2=1653988&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/load_editor.c (original)
+++ subversion/trunk/subversion/svnrdump/load_editor.c Thu Jan 22 18:14:53 2015
@@ -41,6 +41,8 @@
#define SVNRDUMP_PROP_LOCK SVN_PROP_PREFIX "rdump-lock"
+#define ARE_VALID_COPY_ARGS(p,r) ((p) && SVN_IS_VALID_REVNUM(r))
+
#if 0
#define LDR_DBG(x) SVN_DBG(x)
#else
@@ -102,6 +104,15 @@ struct directory_baton
{
void *baton;
const char *relpath;
+
+ /* The copy-from source of this directory, no matter whether it is
+ copied explicitly (the root node of a copy) or implicitly (being an
+ existing child of a copied directory). For a node that is newly
+ added (without history), even inside a copied parent, these are
+ NULL and SVN_INVALID_REVNUM. */
+ const char *copyfrom_path;
+ svn_revnum_t copyfrom_rev;
+
struct directory_baton *parent;
};
@@ -115,8 +126,13 @@ struct node_baton
svn_node_kind_t kind;
enum svn_node_action action;
+ /* Is this directory explicitly added? If not, then it already existed
+ or is a child of a copy. */
+ svn_boolean_t is_added;
+
svn_revnum_t copyfrom_rev;
const char *copyfrom_path;
+ const char *copyfrom_url;
void *file_baton;
const char *base_checksum;
@@ -387,6 +403,7 @@ new_revision_record(void **revision_bato
pb = parse_baton;
rb->pool = svn_pool_create(pool);
rb->pb = pb;
+ rb->db = NULL;
for (hi = apr_hash_first(pool, headers); hi; hi = apr_hash_next(hi))
{
@@ -435,6 +452,47 @@ uuid_record(const char *uuid,
return SVN_NO_ERROR;
}
+/* Push information about another directory onto the linked list RB->db.
+ *
+ * CHILD_BATON is the baton returned by the commit editor. RELPATH is the
+ * repository-relative path of this directory. IS_ADDED is true iff this
+ * directory is being added (with or without history). If added with
+ * history then COPYFROM_PATH/COPYFROM_REV are the copyfrom source, else
+ * are NULL/SVN_INVALID_REVNUM.
+ */
+static void
+push_directory(struct revision_baton *rb,
+ void *child_baton,
+ const char *relpath,
+ svn_boolean_t is_added,
+ const char *copyfrom_path,
+ svn_revnum_t copyfrom_rev)
+{
+ struct directory_baton *child_db = apr_pcalloc(rb->pool, sizeof (*child_db));
+
+ SVN_ERR_ASSERT_NO_RETURN(
+ is_added || (copyfrom_path == NULL && copyfrom_rev == SVN_INVALID_REVNUM));
+
+ /* If this node is an existing (not newly added) child of a copied node,
+ calculate where it was copied from. */
+ if (!is_added
+ && ARE_VALID_COPY_ARGS(rb->db->copyfrom_path, rb->db->copyfrom_rev))
+ {
+ const char *name = svn_relpath_basename(relpath, NULL);
+
+ copyfrom_path = svn_relpath_join(rb->db->copyfrom_path, name,
+ rb->pool);
+ copyfrom_rev = rb->db->copyfrom_rev;
+ }
+
+ child_db->baton = child_baton;
+ child_db->relpath = relpath;
+ child_db->copyfrom_path = copyfrom_path;
+ child_db->copyfrom_rev = copyfrom_rev;
+ child_db->parent = rb->db;
+ rb->db = child_db;
+}
+
static svn_error_t *
new_node_record(void **node_baton,
apr_hash_t *headers,
@@ -445,16 +503,15 @@ new_node_record(void **node_baton,
const struct svn_delta_editor_t *commit_editor = rb->pb->commit_editor;
void *commit_edit_baton = rb->pb->commit_edit_baton;
struct node_baton *nb;
- struct directory_baton *child_db;
apr_hash_index_t *hi;
void *child_baton;
- char *relpath_compose;
const char *nb_dirname;
nb = apr_pcalloc(rb->pool, sizeof(*nb));
nb->rb = rb;
-
+ nb->is_added = FALSE;
nb->copyfrom_path = NULL;
+ nb->copyfrom_url = NULL;
nb->copyfrom_rev = SVN_INVALID_REVNUM;
/* If the creation of commit_editor is pending, create it now and
@@ -488,12 +545,9 @@ new_node_record(void **node_baton,
LDR_DBG(("Opened root %p\n", child_baton));
- /* child_db corresponds to the root directory baton here */
- child_db = apr_pcalloc(rb->pool, sizeof(*child_db));
- child_db->baton = child_baton;
- child_db->relpath = "";
- child_db->parent = NULL;
- rb->db = child_db;
+ /* child_baton corresponds to the root directory baton here */
+ push_directory(rb, child_baton, "", TRUE /*is_added*/,
+ NULL, SVN_INVALID_REVNUM);
}
for (hi = apr_hash_first(rb->pool, headers); hi; hi = apr_hash_next(hi))
@@ -527,7 +581,8 @@ new_node_record(void **node_baton,
}
/* Before handling the new node, ensure depth-first editing order by
- traversing the directory hierarchy from the old to the new node. */
+ traversing the directory hierarchy from the old node's to the new
+ node's parent directory. */
nb_dirname = svn_relpath_dirname(nb->path, pool);
if (svn_path_compare_paths(nb_dirname,
rb->db->relpath) != 0)
@@ -562,7 +617,7 @@ new_node_record(void **node_baton,
for (i = 0; i < residual_open_path->nelts; i ++)
{
- relpath_compose =
+ char *relpath_compose =
svn_relpath_join(rb->db->relpath,
APR_ARRAY_IDX(residual_open_path, i, const char *),
rb->pool);
@@ -571,11 +626,8 @@ new_node_record(void **node_baton,
rb->rev - rb->rev_offset - 1,
rb->pool, &child_baton));
LDR_DBG(("Opened dir %p\n", child_baton));
- child_db = apr_pcalloc(rb->pool, sizeof(*child_db));
- child_db->baton = child_baton;
- child_db->relpath = relpath_compose;
- child_db->parent = rb->db;
- rb->db = child_db;
+ push_directory(rb, child_baton, relpath_compose, TRUE /*is_added*/,
+ NULL, SVN_INVALID_REVNUM);
}
}
@@ -604,7 +656,7 @@ new_node_record(void **node_baton,
nb->copyfrom_path = svn_relpath_join(rb->pb->parent_dir,
nb->copyfrom_path, rb->pool);
/* Convert to a URL, as the commit editor requires. */
- nb->copyfrom_path = svn_path_url_add_component2(rb->pb->root_url,
+ nb->copyfrom_url = svn_path_url_add_component2(rb->pb->root_url,
nb->copyfrom_path,
rb->pool);
}
@@ -622,11 +674,12 @@ new_node_record(void **node_baton,
else
/* FALL THROUGH */;
case svn_node_action_add:
+ nb->is_added = TRUE;
switch (nb->kind)
{
case svn_node_file:
SVN_ERR(commit_editor->add_file(nb->path, rb->db->baton,
- nb->copyfrom_path,
+ nb->copyfrom_url,
nb->copyfrom_rev,
rb->pool, &(nb->file_baton)));
LDR_DBG(("Added file %s to dir %p as %p\n",
@@ -634,16 +687,13 @@ new_node_record(void **node_baton,
break;
case svn_node_dir:
SVN_ERR(commit_editor->add_directory(nb->path, rb->db->baton,
- nb->copyfrom_path,
+ nb->copyfrom_url,
nb->copyfrom_rev,
rb->pool, &child_baton));
LDR_DBG(("Added dir %s to dir %p as %p\n",
nb->path, rb->db->baton, child_baton));
- child_db = apr_pcalloc(rb->pool, sizeof(*child_db));
- child_db->baton = child_baton;
- child_db->relpath = apr_pstrdup(rb->pool, nb->path);
- child_db->parent = rb->db;
- rb->db = child_db;
+ push_directory(rb, child_baton, nb->path, TRUE /*is_added*/,
+ nb->copyfrom_path, nb->copyfrom_rev);
break;
default:
break;
@@ -661,11 +711,8 @@ new_node_record(void **node_baton,
SVN_ERR(commit_editor->open_directory(nb->path, rb->db->baton,
rb->rev - rb->rev_offset - 1,
rb->pool, &child_baton));
- child_db = apr_pcalloc(rb->pool, sizeof(*child_db));
- child_db->baton = child_baton;
- child_db->relpath = apr_pstrdup(rb->pool, nb->path);
- child_db->parent = rb->db;
- rb->db = child_db;
+ push_directory(rb, child_baton, nb->path, FALSE /*is_added*/,
+ NULL, SVN_INVALID_REVNUM);
break;
}
break;
@@ -787,29 +834,71 @@ delete_node_property(void *baton,
return SVN_NO_ERROR;
}
+/* Delete all the properties of the node, if any.
+ *
+ * The commit editor doesn't have a method to delete a node's properties
+ * without knowing what they are, so we have to first find out what
+ * properties the node would have had. If it's copied (explicitly or
+ * implicitly), we look at the copy source. If it's only being changed,
+ * we look at the node's current path in the head revision.
+ */
static svn_error_t *
remove_node_props(void *baton)
{
struct node_baton *nb = baton;
+ struct revision_baton *rb = nb->rb;
apr_pool_t *pool = nb->rb->pool;
apr_hash_index_t *hi;
apr_hash_t *props;
+ const char *orig_path;
+ svn_revnum_t orig_rev;
+
+ /* Find the path and revision that has the node's original properties */
+ if (ARE_VALID_COPY_ARGS(nb->copyfrom_path, nb->copyfrom_rev))
+ {
+ SVN_DBG(("using nb->copyfrom %s@%ld", nb->copyfrom_path, nb->copyfrom_rev));
+ orig_path = nb->copyfrom_path;
+ orig_rev = nb->copyfrom_rev;
+ }
+ else if (!nb->is_added
+ && ARE_VALID_COPY_ARGS(rb->db->copyfrom_path, rb->db->copyfrom_rev))
+ {
+ /* If this is a dir, then it's described by rb->db;
+ if this is a file, then it's a child of the dir in rb->db. */
+ SVN_DBG(("using rb->db->copyfrom (k=%d) %s@%ld",
+ nb->kind, rb->db->copyfrom_path, rb->db->copyfrom_rev));
+ orig_path = (nb->kind == svn_node_dir)
+ ? rb->db->copyfrom_path
+ : svn_relpath_join(rb->db->copyfrom_path,
+ svn_relpath_basename(nb->path, NULL),
+ rb->pool);
+ orig_rev = rb->db->copyfrom_rev;
+ }
+ else
+ {
+ SVN_DBG(("using self.path@head %s@%ld", nb->path, SVN_INVALID_REVNUM));
+ /* ### Should we query at a known, fixed, "head" revision number
+ instead of passing SVN_INVALID_REVNUM and getting a moving target? */
+ orig_path = nb->path;
+ orig_rev = SVN_INVALID_REVNUM;
+ }
+ SVN_DBG(("Trying %s@%ld", orig_path, orig_rev));
if ((nb->action == svn_node_action_add
|| nb->action == svn_node_action_replace)
- && ! SVN_IS_VALID_REVNUM(nb->copyfrom_rev))
+ && ! ARE_VALID_COPY_ARGS(orig_path, orig_rev))
/* Add-without-history; no "old" properties to worry about. */
return SVN_NO_ERROR;
if (nb->kind == svn_node_file)
{
- SVN_ERR(svn_ra_get_file(nb->rb->pb->aux_session, nb->path,
- SVN_INVALID_REVNUM, NULL, NULL, &props, pool));
+ SVN_ERR(svn_ra_get_file(nb->rb->pb->aux_session,
+ orig_path, orig_rev, NULL, NULL, &props, pool));
}
else /* nb->kind == svn_node_dir */
{
SVN_ERR(svn_ra_get_dir2(nb->rb->pb->aux_session, NULL, NULL, &props,
- nb->path, SVN_INVALID_REVNUM, 0, pool));
+ orig_path, orig_rev, 0, pool));
}
for (hi = apr_hash_first(pool, props); hi; hi = apr_hash_next(hi))
Modified: subversion/trunk/subversion/tests/cmdline/svnrdump_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnrdump_tests.py?rev=1653988&r1=1653987&r2=1653988&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnrdump_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnrdump_tests.py Thu Jan 22 18:14:53 2015
@@ -825,59 +825,77 @@ def load_mergeinfo_contains_r0(sbox):
# Regression test for issue 4551 "svnrdump load commits wrong properties,
# or fails, on a non-deltas dumpfile". In this test, the copy source does
# not exist and the failure mode is to error out.
-@XFail()
@Issue(4551)
def load_non_deltas_copy_with_props(sbox):
"load non-deltas copy with props"
sbox.build()
- # Set props on a file and on a dir and on a child of the dir to be copied
- sbox.simple_propset('p', 'v', 'A/mu', 'A/B', 'A/B/E')
+ # Case (1): Copies that do not replace anything: the copy target path
+ # at (new rev - 1) does not exist
+
+ # Set properties on each node to be copied
+ sbox.simple_propset('-R', 'p', 'pval', 'A')
+ sbox.simple_propset('-R', 'q', 'qval', 'A')
sbox.simple_commit()
sbox.simple_update() # avoid mixed-rev
- # Case (1): Copy file/dir, not replacing anything; the copy target path
- # at (new rev - 1) does not exist
+ # Do the copies
sbox.simple_copy('A/mu@2', 'A/mu_COPY')
sbox.simple_copy('A/B@2', 'A/B_COPY')
- # On the copy, delete a prop
- sbox.simple_propdel('p', 'A/mu_COPY', 'A/B_COPY', 'A/B_COPY/E')
+ # Also add new nodes inside the copied dir, to test more code paths
+ sbox.simple_copy('A/B/E@2', 'A/B_COPY/copied')
+ sbox.simple_mkdir('A/B_COPY/added')
+ sbox.simple_copy('A/B/E@2', 'A/B_COPY/added/copied')
+ # On each copied node, delete a prop
+ sbox.simple_propdel('p', 'A/mu_COPY', 'A/B_COPY', 'A/B_COPY/E',
+ 'A/B_COPY/copied', 'A/B_COPY/added/copied')
sbox.simple_commit()
# Dump with 'svnadmin' (non-deltas mode)
dumpfile = svntest.actions.run_and_verify_dump(sbox.repo_dir, deltas=False)
- # Load with 'svnrdump'
+ # Load with 'svnrdump'. This used to throw an error:
+ # svnrdump: E160013: File not found: revision 2, path '/A/B_COPY'
new_repo_dir, new_repo_url = sbox.add_repo_path('new_repo')
svntest.main.create_repos(new_repo_dir)
svntest.actions.enable_revprop_changes(new_repo_dir)
svntest.actions.run_and_verify_svnrdump(dumpfile,
svntest.verify.AnyOutput,
[], 0, 'load', new_repo_url)
- # For regression test purposes, all we require is that the 'load'
- # doesn't throw an error
+
+ # Check that property 'p' really was deleted on each copied node
+ for tgt_path in ['A/mu_COPY', 'A/B_COPY', 'A/B_COPY/E',
+ 'A/B_COPY/copied', 'A/B_COPY/added/copied']:
+ tgt_url = new_repo_url + '/' + tgt_path
+ _, out, _ = svntest.main.run_svn(None, 'proplist', tgt_url)
+ expected = ["Properties on '%s':" % (tgt_url,),
+ 'q']
+ actual = map(str.strip, out)
+ svntest.verify.compare_and_display_lines(None, 'PROPS', expected, actual)
# Regression test for issue 4551 "svnrdump load commits wrong properties,
# or fails, on a non-deltas dumpfile". In this test, the copy source does
# exist and the failure mode is to fail to delete a property.
-@XFail()
@Issue(4551)
def load_non_deltas_replace_copy_with_props(sbox):
"load non-deltas replace© with props"
sbox.build()
- # Set props on a file and on a dir and on a child of the dir to be copied
+ # Case (2): Copies that replace something: the copy target path
+ # at (new rev - 1) exists and has no property named 'p'
+
+ # Set props on the copy source nodes (a file, a dir, a child of the dir)
sbox.simple_propset('p', 'v', 'A/mu', 'A/B', 'A/B/E')
+ sbox.simple_propset('q', 'v', 'A/mu', 'A/B', 'A/B/E')
sbox.simple_commit()
sbox.simple_update() # avoid mixed-rev
- # Case (2): Copy file/dir, replacing something; the copy target path
- # at (new rev - 1) exists and has no property named 'p'
+ # Do the copies, replacing something
sbox.simple_rm('A/D/gamma', 'A/C')
sbox.simple_copy('A/mu@2', 'A/D/gamma')
sbox.simple_copy('A/B@2', 'A/C')
- # On the copy, delete a prop that isn't present on the replaced node
+ # On the copy, delete a prop that wasn't present on the node that it replaced
sbox.simple_propdel('p', 'A/D/gamma', 'A/C', 'A/C/E')
sbox.simple_commit()
@@ -894,11 +912,13 @@ def load_non_deltas_replace_copy_with_pr
[], 0, 'load', new_repo_url)
# Check that property 'p' really was deleted on each copied node
+ # This used to fail, finding that property 'p' was still present
for tgt_path in ['A/D/gamma', 'A/C', 'A/C/E']:
- _, out, _ = svntest.main.run_svn(None, 'proplist',
- new_repo_url + '/' + tgt_path)
- expected = []
- actual = out[1:]
+ tgt_url = new_repo_url + '/' + tgt_path
+ _, out, _ = svntest.main.run_svn(None, 'proplist', tgt_url)
+ expected = ["Properties on '%s':" % (tgt_url,),
+ 'q']
+ actual = map(str.strip, out)
svntest.verify.compare_and_display_lines(None, 'PROPS', expected, actual)
# Regression test for issue #4552 "svnrdump writes duplicate headers for a
Re: svn commit: r1653988 - in /subversion/trunk/subversion: svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
Posted by Julian Foad <ju...@btopenworld.com>.
I (Julian Foad) wrote:
> I (Julian Foad) wrote:
>> Bert Huijben wrote:
>>>> URL: http://svn.apache.org/r1653988
>>>> Log:
>>>> Fix issue #4551 "svnrdump load commits wrong properties, or fails, on a
>>>> non-deltas dumpfile", [...]
>>>
>>> I also see new test failures on the two new tests for ra_serf.
>>
>> It's another real bug in svnrdump. It calls
>> commit_editor->change_dir_prop() twice for the same property name, [...]
> r1654186 adds such a regression test. [...]
> [Since http://svn.apache.org/r1654194 ] tests for this issue now pass,
> although svndumpfilter is still violating the editor rules.
r1654271 fixes svnrdump's bad editor driving.
[...]
> We should definitely backport the issue #4551 fixes when they're ready.
I have proposed the fixes for this issue for backport to 1.8.x and 1.7.x.
- Julian
Re: svn commit: r1653988 - in /subversion/trunk/subversion: svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
Posted by Julian Foad <ju...@btopenworld.com>.
I (Julian Foad) wrote:
> I also noticed that configuring with '--enable-ev2-shims' creates the
> same driving order violation, so that several tests would fail against RA-serf,
> but since r1654194 they no longer fail.
Correction: tests wouldn't fail on their own in that configuration. What I had noticed was test failures when using my additional dump-load verification, but that only means those tests produced prop changes that 'svnrdump load' doesn't handle right.
- Julian
Re: svn commit: r1653988 - in /subversion/trunk/subversion: svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
Posted by Julian Foad <ju...@btopenworld.com>.
I (Julian Foad) wrote:
> Bert Huijben wrote:
>> I also see new test failures on the two new tests for ra_serf.
>
> It's another real bug in svnrdump. It calls
> commit_editor->change_dir_prop() twice for the same property name, which is
> documented as not allowed. The other RA layers don't mind, but RA-serf does.
RA-serf accepts the calls but commits an unexpected result because it processes the non-deleting prop changes first and then the prop deletes.
For example (as in svnrdump_tests.py 52), to change properties from (p=v, q=v) to (q=v), svnrdump calls:
set_node_property("p", NULL)
set_node_property("q", NULL)
set_node_property("q", "v")
which results in libsvn_ra_serf/commit.c sending:
PROPPATCH: pname="q", pval="v"
PROPPATCH: pname="p", pval=NULL
PROPPATCH: pname="q", pval=NULL
resulting in no properties.
> I should be able to test for this bug independently of issue #4551.
r1654186 adds such a regression test. I'm keeping it within issue #4551, but it's independent of *copying* a node which was where that issue showed up until now.
Bert's commit http://svn.apache.org/r1654194 "Simplify the ra_serf proppatch code ..." changes RA-serf to work the same way as the other RA layers and thus avoids the problem here. The tests for this issue now pass, although svndumpfilter is still violating the editor rules.
Why haven't we found this before? Because we haven't tested it, is the easy answer. But we already had a test that exercises this kind of scenario (svnrdump_tests.py 49 -- but it's only looking to see if the commit fails, not checking the results). If we had generic editor call checking in place then maybe we would have found this driving order violation earlier.
Maybe we should introduce an editor validator, even at this late stage. Note that Ev2 attempts to provide one -- see ENABLE_ORDERING_CHECK in libsvn_delta/editor.c.
I also noticed that configuring with '--enable-ev2-shims' creates the same driving order violation, so that several tests would fail against RA-serf, but since r1654194 they no longer fail.
Wondering what to do next... I think we should still fix the driving order in svndumpfilter.
We should definitely backport the issue #4551 fixes when they're ready.
- Julian
Re: svn commit: r1653988 - in /subversion/trunk/subversion: svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
Posted by Julian Foad <ju...@btopenworld.com>.
Bert Huijben wrote:
>> Author: julianfoad
>> Date: Thu Jan 22 18:14:53 2015
>> New Revision: 1653988
>>
>> URL: http://svn.apache.org/r1653988
>> Log:
>> Fix issue #4551 "svnrdump load commits wrong properties, or fails, on a
>> non-deltas dumpfile", by teaching the svnrdump loader to find the original
>> properties of a copied node at the copy source location.
>
> This commit accidentally adds a few SVN_DBG() calls which I disabled in r1654039
> to unbreak compilation on a few bots.
Thank you!
> I also see new test failures on the two new tests for ra_serf.
> See
> http://ci.apache.org/builders/svn-windows-ra/builds/964/steps/Test%20fsfs%2Bserf/logs/faillog
It's another real bug in svnrdump. It calls commit_editor->change_dir_prop() twice for the same property name, which is documented as not allowed. The other RA layers don't mind, but RA-serf does.
DBG: load_editor.c: 584: new_node_record: A/B_COPY, k=2, a=1, copy=A/B@2
DBG: load_editor.c: 860: remove_node_props on A/B_COPY
DBG: load_editor.c: 865: rnp: using nb->copyfrom A/B@2
DBG: load_editor.c: 827: delete_node_property on A/B_COPY: p
DBG: commit.c:1739: serf.change_dir_prop(A/B_COPY): removed_props[p]=(null)
DBG: load_editor.c: 827: delete_node_property on A/B_COPY: q
DBG: commit.c:1739: serf.change_dir_prop(A/B_COPY): removed_props[q]=(null)
DBG: load_editor.c: 776: set_node_property on A/B_COPY: q=v
DBG: commit.c:1732: serf.change_dir_prop(A/B_COPY): changed_props[q]=v
DBG: load_editor.c: 968: close_node on A/B_COPY
I should be able to test for this bug independently of issue #4551.
- Julian
RE: svn commit: r1653988 - in /subversion/trunk/subversion: svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: julianfoad@apache.org [mailto:julianfoad@apache.org]
> Sent: donderdag 22 januari 2015 19:15
> To: commits@subversion.apache.org
> Subject: svn commit: r1653988 - in /subversion/trunk/subversion:
> svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
>
> Author: julianfoad
> Date: Thu Jan 22 18:14:53 2015
> New Revision: 1653988
>
> URL: http://svn.apache.org/r1653988
> Log:
> Fix issue #4551 "svnrdump load commits wrong properties, or fails, on a
> non-deltas dumpfile", by teaching the svnrdump loader to find the original
> properties of a copied node at the copy source location.
This commit accidentally adds a few SVN_DBG() calls which I disabled in r1654039 to unbreak compilation on a few bots.
I also see new test failures on the two new tests for ra_serf.
See http://ci.apache.org/builders/svn-windows-ra/builds/964/steps/Test%20fsfs%2Bserf/logs/faillog
Bert