Fix issue 2751: Lost prop-mods on overlapping commits.
Approved by: +1 cmpilato, jszakmeister, dlr, dionisos
git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/1.1.x@864538 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/STATUS b/STATUS
index b65ed54..91a2fe9 100644
--- a/STATUS
+++ b/STATUS
@@ -82,14 +82,6 @@
commands without this patch.
Votes:
+1: jszakmeister
-
-Approved changes (further review is welcome, but not required):
- * r24073, r24126, r24128
- Branch: 1.1.x-issue-2751
- Fix rare FS corruption in both bdb and FSFS filesystem implementations
- Justification:
- Well, *corruption*...
- Votes:
- +1: dionisos, cmpilato, jszakmeister
+Approved changes (further review is welcome, but not required):
diff --git a/subversion/libsvn_fs_base/tree.c b/subversion/libsvn_fs_base/tree.c
index f264006..461becb 100644
--- a/subversion/libsvn_fs_base/tree.c
+++ b/subversion/libsvn_fs_base/tree.c
@@ -1,7 +1,7 @@
/* tree.c : tree-like filesystem, built on DAG filesystem
*
* ====================================================================
- * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ * Copyright (c) 2000-2007 CollabNet. All rights reserved.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -2034,14 +2034,20 @@
/* Possible early merge failure: if target and ancestor have
different property lists, then the merge should fail.
Propchanges can *only* be committed on an up-to-date directory.
+ ### TODO: see issue #418 about the inelegance of this.
- ### TODO: Please see issue #418 about the inelegance of this. */
+ Another possible, similar, early merge failure: if source and
+ ancestor have different property lists (meaning someone else
+ changed directory properties while our commit transaction was
+ happening), the merge should fail. See issue #2751.
+ */
{
- node_revision_t *tgt_nr, *anc_nr;
+ node_revision_t *tgt_nr, *anc_nr, *src_nr;
/* Get node revisions for our id's. */
SVN_ERR (svn_fs_bdb__get_node_revision (&tgt_nr, fs, target_id, trail));
SVN_ERR (svn_fs_bdb__get_node_revision (&anc_nr, fs, ancestor_id, trail));
+ SVN_ERR (svn_fs_bdb__get_node_revision (&src_nr, fs, source_id, trail));
/* Now compare the prop-keys of the skels. Note that just because
the keys are different -doesn't- mean the proplists have
@@ -2050,9 +2056,9 @@
it won't do that here either. Checking to see if the propkey
atoms are `equal' is enough. */
if (! svn_fs_base__same_keys (tgt_nr->prop_key, anc_nr->prop_key))
- {
- return conflict_err (conflict_p, target_path);
- }
+ return conflict_err (conflict_p, target_path);
+ if (! svn_fs_base__same_keys (src_nr->prop_key, anc_nr->prop_key))
+ return conflict_err (conflict_p, target_path);
}
/* ### todo: it would be more efficient to simply check for a NULL
diff --git a/subversion/libsvn_fs_fs/tree.c b/subversion/libsvn_fs_fs/tree.c
index 397b4f9..85b62ac 100644
--- a/subversion/libsvn_fs_fs/tree.c
+++ b/subversion/libsvn_fs_fs/tree.c
@@ -1,7 +1,7 @@
/* tree.c : tree-like filesystem, built on DAG filesystem
*
* ====================================================================
- * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ * Copyright (c) 2000-2007 CollabNet. All rights reserved.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -1347,15 +1347,20 @@
/* Possible early merge failure: if target and ancestor have
different property lists, then the merge should fail.
Propchanges can *only* be committed on an up-to-date directory.
+ ### TODO: see issue #418 about the inelegance of this.
- ### TODO: Please see issue #418 about the inelegance of this. */
+ Another possible, similar, early merge failure: if source and
+ ancestor have different property lists (meaning someone else
+ changed directory properties while our commit transaction was
+ happening), the merge should fail. See issue #2751.
+ */
{
- node_revision_t *tgt_nr, *anc_nr;
+ node_revision_t *tgt_nr, *anc_nr, *src_nr;
/* Get node revisions for our id's. */
-
SVN_ERR (svn_fs_fs__get_node_revision (&tgt_nr, fs, target_id, pool));
SVN_ERR (svn_fs_fs__get_node_revision (&anc_nr, fs, ancestor_id, pool));
+ SVN_ERR (svn_fs_fs__get_node_revision (&src_nr, fs, source_id, pool));
/* Now compare the prop-keys of the skels. Note that just because
the keys are different -doesn't- mean the proplists have
@@ -1364,9 +1369,9 @@
it won't do that here either. Checking to see if the propkey
atoms are `equal' is enough. */
if (! svn_fs_fs__noderev_same_rep_key (tgt_nr->prop_rep, anc_nr->prop_rep))
- {
- return conflict_err (conflict_p, target_path);
- }
+ return conflict_err (conflict_p, target_path);
+ if (! svn_fs_fs__noderev_same_rep_key (src_nr->prop_rep, anc_nr->prop_rep))
+ return conflict_err (conflict_p, target_path);
}
/* ### todo: it would be more efficient to simply check for a NULL
diff --git a/subversion/tests/libsvn_fs_base/fs-test.c b/subversion/tests/libsvn_fs_base/fs-test.c
index 955b305..963940c 100644
--- a/subversion/tests/libsvn_fs_base/fs-test.c
+++ b/subversion/tests/libsvn_fs_base/fs-test.c
@@ -1,7 +1,7 @@
/* fs-test.c --- tests for the filesystem
*
* ====================================================================
- * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ * Copyright (c) 2000-2007 CollabNet. All rights reserved.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -5193,6 +5193,54 @@
}
+static svn_error_t *
+unordered_txn_dirprops (const char **msg,
+ svn_boolean_t msg_only,
+ apr_pool_t *pool)
+{
+ svn_fs_t *fs;
+ svn_fs_txn_t *txn, *txn2;
+ svn_fs_root_t *txn_root, *txn_root2;
+ svn_string_t pval;
+ svn_revnum_t new_rev;
+
+ /* This is a regression test for issue #2751. */
+ *msg = "test dir prop preservation in unordered txns";
+
+ if (msg_only)
+ return SVN_NO_ERROR;
+
+ /* Prepare a filesystem. */
+ SVN_ERR (svn_test__create_fs (&fs, "test-repo-root-revisions", pool));
+
+ /* Create and commit the greek tree. */
+ SVN_ERR (svn_fs_begin_txn (&txn, fs, 0, pool));
+ SVN_ERR (svn_fs_txn_root (&txn_root, txn, pool));
+ SVN_ERR (svn_test__create_greek_tree (txn_root, pool));
+ SVN_ERR (test_commit_txn (&new_rev, txn, NULL, pool));
+
+ /* Open two transactions */
+ SVN_ERR (svn_fs_begin_txn (&txn, fs, new_rev, pool));
+ SVN_ERR (svn_fs_txn_root (&txn_root, txn, pool));
+ SVN_ERR (svn_fs_begin_txn (&txn2, fs, new_rev, pool));
+ SVN_ERR (svn_fs_txn_root (&txn_root2, txn2, pool));
+
+ /* Change a child file in one. */
+ SVN_ERR (svn_test__set_file_contents (txn_root, "/A/B/E/alpha",
+ "New contents", pool));
+
+ /* Change dir props in the other. */
+ SET_STR (&pval, "value");
+ SVN_ERR (svn_fs_change_node_prop (txn_root2, "/A/B", "name", &pval, pool));
+
+ /* Commit the second one first. */
+ SVN_ERR (test_commit_txn (&new_rev, txn2, NULL, pool));
+
+ /* Then commit the first -- but expect an conflict due to the
+ propchanges made by the other txn. */
+ return test_commit_txn (&new_rev, txn, "/A/B", pool);
+}
+
/* ------------------------------------------------------------------------ */
/* The test table. */
@@ -5236,5 +5284,6 @@
SVN_TEST_PASS (verify_checksum),
SVN_TEST_PASS (skip_deltas),
SVN_TEST_PASS (redundant_copy),
+ SVN_TEST_PASS (unordered_txn_dirprops),
SVN_TEST_NULL
};
