Author: rhuijben
Date: Wed May 18 13:33:03 2011
New Revision: 1124259
URL: http://svn.apache.org/viewvc?rev=1124259&view=rev
Log:
Resolve issue #3533, "Entry-prop edits resulting in tree conflicts", by making
the update editor determine that a node (or one of its children) is really
edited before installing an edited tree conflict.
* subversion/libsvn_wc/update_editor.c
(dir_baton,
file_baton): Add two variables.
(mark_directory_edited,
mark_file_edited): New functions.
(delete_entry): Mark parent directory edited.
(add_directory): Mark directory edited.
(open_directory): Don't install tree conflict directly, but delay it until
the directory is really edited.
(change_dir_prop): If a not-entry prop is received mark the directory edited.
(close_directory): Only do an update notification if the directory is edited.
(Needed because we don't always notify from open_file on new tree
conflicts)
(absent_node): Mark the parent directory edited.
(add_file): Mark the file edited.
(open_file): Don't install tree conflict directly, but delay it until
the file is really edited.
(apply_textdelta): Mark file edited.
(change_file_prop): If a not-entry prop is received mark the file edited.
(close_file): Only do an update notification if the file is edited.
(Needed because we don't always notify from open_file on new tree
conflicts)
* subversion/tests/cmdline/basic_tests.py
(delete_child_parent_update): Don't expect a treeconflict in this specific
case where the only reason we get a change is that we were mixed revision.
* subversion/tests/cmdline/lock_tests.py
(update_locked_deleted): Extend coverage to include a deleted directory with
files inside and then try to steal the lock.
Modified:
subversion/trunk/subversion/libsvn_wc/update_editor.c
subversion/trunk/subversion/tests/cmdline/basic_tests.py
subversion/trunk/subversion/tests/cmdline/lock_tests.py
Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1124259&r1=1124258&r2=1124259&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Wed May 18 13:33:03
2011
@@ -330,6 +330,14 @@ struct dir_baton
changes to be applied to this directory. */
apr_array_header_t *propchanges;
+ /* A boolean indicating whether this node or one of its children has
+ received any 'real' changes. Used to avoid tree conflicts for simple
+ entryprop changes, like lock management */
+ svn_boolean_t edited;
+
+ /* The tree conflict to install once the node is really edited */
+ svn_wc_conflict_description2_t *edit_conflict;
+
/* The bump information for this directory. */
struct bump_dir_info *bump_info;
@@ -731,6 +739,14 @@ struct file_baton
/* Bump information for the directory this file lives in */
struct bump_dir_info *bump_info;
+
+ /* A boolean indicating whether this node or one of its children has
+ received any 'real' changes. Used to avoid tree conflicts for simple
+ entryprop changes, like lock management */
+ svn_boolean_t edited;
+
+ /* The tree conflict to install once the node is really edited */
+ svn_wc_conflict_description2_t *edit_conflict;
};
@@ -787,6 +803,63 @@ make_file_baton(struct file_baton **f_p,
return SVN_NO_ERROR;
}
+/* Called when a directory is really edited, to avoid marking a
+ tree conflict on a node for a no-change edit */
+static svn_error_t *
+mark_directory_edited(struct dir_baton *db, apr_pool_t *scratch_pool)
+{
+ if (db->edited)
+ return SVN_NO_ERROR;
+
+ if (db->parent_baton)
+ SVN_ERR(mark_directory_edited(db->parent_baton, scratch_pool));
+
+ db->edited = TRUE;
+
+ if (db->edit_conflict)
+ {
+ /* We have a (delayed) tree conflict to install */
+ SVN_ERR(svn_wc__db_op_set_tree_conflict(db->edit_baton->db,
+ db->local_abspath,
+ db->edit_conflict,
+ scratch_pool));
+
+ do_notification(db->edit_baton, db->local_abspath, svn_node_dir,
+ svn_wc_notify_tree_conflict, scratch_pool);
+ db->already_notified = TRUE;
+ }
+
+ return SVN_NO_ERROR;
+}
+
+/* Called when a file is really edited, to avoid marking a
+ tree conflict on a node for a no-change edit */
+static svn_error_t *
+mark_file_edited(struct file_baton *fb, apr_pool_t *scratch_pool)
+{
+ if (fb->edited)
+ return SVN_NO_ERROR;
+
+ SVN_ERR(mark_directory_edited(fb->dir_baton, scratch_pool));
+
+ fb->edited = TRUE;
+
+ if (fb->edit_conflict)
+ {
+ /* We have a (delayed) tree conflict to install */
+ SVN_ERR(svn_wc__db_op_set_tree_conflict(fb->edit_baton->db,
+ fb->local_abspath,
+ fb->edit_conflict,
+ scratch_pool));
+
+ do_notification(fb->edit_baton, fb->local_abspath, svn_node_file,
+ svn_wc_notify_tree_conflict, scratch_pool);
+ fb->already_notified = TRUE;
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* Handle the next delta window of the file described by BATON. If it is
* the end (WINDOW == NULL), then check the checksum, store the text in the
@@ -1594,6 +1667,8 @@ delete_entry(const char *path,
scratch_pool = svn_pool_create(pb->pool);
+ SVN_ERR(mark_directory_edited(pb, scratch_pool));
+
SVN_ERR(check_path_under_root(pb->local_abspath, base, scratch_pool));
local_abspath = svn_dirent_join(pb->local_abspath, base, scratch_pool);
@@ -1822,6 +1897,8 @@ add_directory(const char *path,
if (db->skip_this)
return SVN_NO_ERROR;
+ SVN_ERR(mark_directory_edited(db, pool));
+
SVN_ERR(check_path_under_root(pb->local_abspath, db->name, pool));
if (strcmp(eb->target_abspath, db->local_abspath) == 0)
@@ -2238,19 +2315,12 @@ open_directory(const char *path,
SVN_ERR(check_tree_conflict(&tree_conflict, eb, db->local_abspath,
status, wc_kind, TRUE,
svn_wc_conflict_action_edit, svn_node_dir,
- db->new_relpath, pool, pool));
+ db->new_relpath, db->pool, pool));
/* Remember the roots of any locally deleted trees. */
if (tree_conflict != NULL)
{
- /* Place a tree conflict into the parent work queue. */
- SVN_ERR(svn_wc__db_op_set_tree_conflict(eb->db, db->local_abspath,
- tree_conflict, pool));
-
- do_notification(eb, db->local_abspath, svn_node_dir,
- svn_wc_notify_tree_conflict, pool);
- db->already_notified = TRUE;
-
+ db->edit_conflict = tree_conflict;
/* Other modifications wouldn't be a tree conflict */
SVN_ERR_ASSERT(
tree_conflict->reason == svn_wc_conflict_reason_deleted ||
@@ -2287,6 +2357,9 @@ change_dir_prop(void *dir_baton,
propchange->name = apr_pstrdup(db->pool, name);
propchange->value = value ? svn_string_dup(value, db->pool) : NULL;
+ if (!db->edited && svn_property_kind(NULL, name) != svn_prop_entry_kind)
+ SVN_ERR(mark_directory_edited(db, pool));
+
return SVN_NO_ERROR;
}
@@ -2651,7 +2724,7 @@ close_directory(void *dir_baton,
happened in that case - unless the add was obstructed by a dir
scheduled for addition without history, in which case we handle
notification here). */
- if (!db->already_notified && eb->notify_func)
+ if (!db->already_notified && eb->notify_func && db->edited)
{
svn_wc_notify_t *notify;
svn_wc_notify_action_t action;
@@ -2699,6 +2772,8 @@ absent_node(const char *path,
if (pb->skip_this)
return SVN_NO_ERROR;
+ SVN_ERR(mark_directory_edited(pb, scratch_pool));
+
local_abspath = svn_dirent_join(pb->local_abspath, name, scratch_pool);
/* If an item by this name is scheduled for addition that's a
@@ -2830,6 +2905,8 @@ add_file(const char *path,
if (fb->skip_this)
return SVN_NO_ERROR;
+ SVN_ERR(mark_file_edited(fb, pool));
+
SVN_ERR(check_path_under_root(pb->local_abspath, fb->name, pool));
/* The file_pool can stick around for a *long* time, so we want to
@@ -3216,14 +3293,12 @@ open_file(const char *path,
SVN_ERR(check_tree_conflict(&tree_conflict, eb, fb->local_abspath,
status, wc_kind, TRUE,
svn_wc_conflict_action_edit, svn_node_file,
- fb->new_relpath, scratch_pool, scratch_pool));
+ fb->new_relpath, fb->pool, scratch_pool));
/* Is this path the victim of a newly-discovered tree conflict? */
if (tree_conflict != NULL)
{
- SVN_ERR(svn_wc__db_op_set_tree_conflict(eb->db,
- fb->local_abspath,
- tree_conflict, scratch_pool));
+ fb->edit_conflict = tree_conflict;
/* Other modifications wouldn't be a tree conflict */
SVN_ERR_ASSERT(
@@ -3232,10 +3307,6 @@ open_file(const char *path,
/* Continue updating BASE */
fb->shadowed = TRUE;
-
- fb->already_notified = TRUE;
- do_notification(eb, fb->local_abspath, svn_node_unknown,
- svn_wc_notify_tree_conflict, scratch_pool);
}
svn_pool_destroy(scratch_pool);
@@ -3268,6 +3339,8 @@ apply_textdelta(void *file_baton,
return SVN_NO_ERROR;
}
+ SVN_ERR(mark_file_edited(fb, pool));
+
/* Parse checksum or sets expected_base_checksum to NULL */
SVN_ERR(svn_checksum_parse_hex(&expected_base_checksum, svn_checksum_md5,
expected_checksum, pool));
@@ -3399,6 +3472,9 @@ change_file_prop(void *file_baton,
propchange->name = apr_pstrdup(fb->pool, name);
propchange->value = value ? svn_string_dup(value, fb->pool) : NULL;
+ if (!fb->edited && svn_property_kind(NULL, name) != svn_prop_entry_kind)
+ SVN_ERR(mark_file_edited(fb, scratch_pool));
+
return SVN_NO_ERROR;
}
@@ -4182,7 +4258,7 @@ close_file(void *file_baton,
/* Send a notification to the callback function. (Skip notifications
about files which were already notified for another reason.) */
- if (eb->notify_func && !fb->already_notified)
+ if (eb->notify_func && !fb->already_notified && fb->edited)
{
const svn_string_t *mime_type;
svn_wc_notify_t *notify;
Modified: subversion/trunk/subversion/tests/cmdline/basic_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/basic_tests.py?rev=1124259&r1=1124258&r2=1124259&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/basic_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/basic_tests.py Wed May 18
13:33:03 2011
@@ -2605,9 +2605,8 @@ def delete_child_parent_update(sbox):
expected_disk = svntest.main.greek_state.copy()
expected_disk.remove('A/B/E/alpha', 'A/B/E/beta', 'A/B/E')
- # This produces a tree-conflict
+ # This produced a tree-conflict until we fixed issue #3533
expected_status.tweak(wc_rev=2)
- expected_status.tweak('A/B/E', treeconflict='C')
svntest.actions.run_and_verify_update(wc_dir,
[],
expected_disk,
Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1124259&r1=1124258&r2=1124259&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Wed May 18 13:33:03
2011
@@ -1657,12 +1657,15 @@ def update_locked_deleted(sbox):
sbox.build()
wc_dir = sbox.wc_dir
- iota_path = os.path.join(wc_dir, 'iota')
- mu_path = os.path.join(wc_dir, 'A', 'mu')
+ iota_path = sbox.ospath('iota')
+ mu_path = sbox.ospath('A/mu')
+ alpha_path = sbox.ospath('A/B/E/alpha')
- svntest.main.run_svn(None, 'lock', '-m', 'locked', mu_path, iota_path)
+ svntest.main.run_svn(None, 'lock', '-m', 'locked', mu_path, iota_path,
+ alpha_path)
sbox.simple_rm('iota')
sbox.simple_rm('A/mu')
+ sbox.simple_rm('A/B/E')
# Create expected output tree for an update.
expected_output = svntest.wc.State(wc_dir, {
@@ -1670,17 +1673,26 @@ def update_locked_deleted(sbox):
# Create expected status tree for the update.
expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
- expected_status.tweak('iota', 'A/mu', status='D ', writelocked='K')
-
- svntest.actions.run_and_verify_update(wc_dir,
- expected_output,
- None,
- expected_status,
- None, None, None,
- None, None)
+ expected_status.tweak('A/B/E', status='D ')
+ expected_status.tweak('iota', 'A/mu', 'A/B/E/alpha',
+ status='D ', writelocked='K')
+ expected_status.tweak('A/B/E/beta', status='D ')
+
+ svntest.actions.run_and_verify_update(wc_dir, expected_output,
+ None, expected_status)
+
+ # Now we steal the lock of iota and A/mu via URL and retry
+ svntest.main.run_svn(None, 'lock', '-m', 'locked', sbox.repo_url + '/iota',
+ '--force', sbox.repo_url + '/A/mu',
+ sbox.repo_url + '/A/B/E/alpha')
+
+ expected_status.tweak('iota', 'A/mu', 'A/B/E/alpha',
+ status='D ', writelocked='O')
+
+ svntest.actions.run_and_verify_update(wc_dir, expected_output,
+ None, expected_status)
+
-
-
########################################################################
# Run the tests
