You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2011/04/26 23:38:35 UTC
svn commit: r1096921 - in /subversion/trunk/subversion:
libsvn_client/merge.c libsvn_client/repos_diff.c
tests/cmdline/merge_tree_conflict_tests.py
tests/cmdline/tree_conflict_tests.py
Author: rhuijben
Date: Tue Apr 26 21:38:34 2011
New Revision: 1096921
URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
Log:
Remove the last bit of working copy obstruction detection from the repository
diff editor.
This patch makes the merge editor aware of the list of added directories to
allow dry run property merges to directories.
* subversion/libsvn_client/merge.c
(merge_cmd_baton_t): Add hashtable of merge added paths.
(dry_run_added_p): New function.
(perform_obstruction_check): Specialize handling of merge-added paths.
(merge_dir_props_changed): Don't perform actual prop merging for added
directories when doing a dry-run merge.
(merge_file_added,
merge_dir_added,
merge_dir_deleted,
merge_dir_closed): Don't set optional output arguments to their default.
* subversion/libsvn_client/repos_diff.c
(close_directory): When an obstruction is noticed, update the content state
and use updated notifications.
* subversion/tests/cmdline/merge_tree_conflict_tests.py
(tree_conflicts_merge_edit_onto_missing,
tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
* subversion/tests/cmdline/tree_conflict_tests.py
(at_directory_external): Mark as XFail again. This change moves the failure
to where we step into the external for the first time. The merge code
detects this properly now, but wants to apply mergeinfo on the external
to notice that it wasn't merged. But it can't and shouldn't store this
information.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/libsvn_client/repos_diff.c
subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1096921&r1=1096920&r2=1096921&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue Apr 26 21:38:34 2011
@@ -276,6 +276,10 @@ typedef struct merge_cmd_baton_t {
dry_run mode. */
apr_hash_t *dry_run_deletions;
+ /* The list of paths for entries we've added, used only when in
+ dry_run mode. */
+ apr_hash_t *dry_run_added;
+
/* The list of any paths which remained in conflict after a
resolution attempt was made. We track this in-memory, rather
than just using WC entry state, since the latter doesn't help us
@@ -353,6 +357,20 @@ dry_run_deleted_p(const merge_cmd_baton_
APR_HASH_KEY_STRING) != NULL);
}
+/* Return true iff we're in dry-run mode and WCPATH would have been
+ added by now if we weren't in dry-run mode.
+ Used to avoid spurious notifications (e.g. conflicts) from a merge
+ attempt into an existing target which would have been deleted if we
+ weren't in dry_run mode (issue #2584). Assumes that WCPATH is
+ still versioned (e.g. has an associated entry). */
+static APR_INLINE svn_boolean_t
+dry_run_added_p(const merge_cmd_baton_t *merge_b, const char *wcpath)
+{
+ return (merge_b->dry_run &&
+ apr_hash_get(merge_b->dry_run_added, wcpath,
+ APR_HASH_KEY_STRING) != NULL);
+}
+
/* Return whether any WC path was put in conflict by the merge
operation corresponding to MERGE_B. */
static APR_INLINE svn_boolean_t
@@ -408,20 +426,36 @@ perform_obstruction_check(svn_wc_notify_
*kind = svn_node_none;
/* In a dry run, make as if nodes "deleted" by the dry run appear so. */
- if (merge_b->dry_run && dry_run_deleted_p(merge_b, local_abspath))
+ if (merge_b->dry_run)
{
- *obstruction_state = svn_wc_notify_state_inapplicable;
+ if (dry_run_deleted_p(merge_b, local_abspath))
+ {
+ *obstruction_state = svn_wc_notify_state_inapplicable;
+
+ if (versioned)
+ *versioned = TRUE;
+ if (deleted)
+ *deleted = TRUE;
+
+ if (expected_kind != svn_node_unknown
+ && expected_kind != svn_node_none)
+ *obstruction_state = svn_wc_notify_state_obstructed;
+ return SVN_NO_ERROR;
+ }
+ else if (dry_run_added_p(merge_b, local_abspath))
+ {
+ *obstruction_state = svn_wc_notify_state_inapplicable;
- if (versioned)
- *versioned = TRUE;
- if (deleted)
- *deleted = TRUE;
-
- if (expected_kind != svn_node_unknown
- && expected_kind != svn_node_none)
- *obstruction_state = svn_wc_notify_state_obstructed;
- return SVN_NO_ERROR;
- }
+ if (versioned)
+ *versioned = TRUE;
+ if (added)
+ *added = TRUE;
+ if (kind)
+ *kind = svn_node_dir; /* Currently only used for dirs */
+
+ return SVN_NO_ERROR;
+ }
+ }
if (kind == NULL)
kind = &wc_kind;
@@ -1224,6 +1258,13 @@ merge_dir_props_changed(svn_wc_notify_st
return SVN_NO_ERROR;
}
+ if (dir_was_added
+ && merge_b->dry_run
+ && dry_run_added_p(merge_b, local_abspath))
+ {
+ return SVN_NO_ERROR; /* We can't do a real prop merge for added dirs */
+ }
+
return svn_error_return(merge_props_changed(state,
tree_conflicted,
local_abspath,
@@ -1544,8 +1585,6 @@ merge_file_added(svn_wc_notify_state_t *
*content_state = svn_wc_notify_state_unchanged;
if (prop_state)
*prop_state = svn_wc_notify_state_unchanged;
- if (tree_conflicted)
- *tree_conflicted = FALSE;
return SVN_NO_ERROR;
}
@@ -1555,9 +1594,6 @@ merge_file_added(svn_wc_notify_state_t *
if (prop_state)
*prop_state = svn_wc_notify_state_unknown;
- if (tree_conflicted)
- *tree_conflicted = FALSE;
-
/* Apply the prop changes to a new hash table. */
file_props = apr_hash_copy(subpool, original_props);
for (i = 0; i < prop_changes->nelts; ++i)
@@ -1996,8 +2032,6 @@ merge_dir_added(svn_wc_notify_state_t *s
svn_pool_destroy(subpool);
if (state)
*state = svn_wc_notify_state_unchanged;
- if (tree_conflicted)
- *tree_conflicted = FALSE;
return SVN_NO_ERROR;
}
@@ -2069,7 +2103,11 @@ merge_dir_added(svn_wc_notify_state_t *s
case svn_node_none:
/* Unversioned or schedule-delete */
if (merge_b->dry_run)
- merge_b->added_path = apr_pstrdup(merge_b->pool, local_abspath);
+ {
+ merge_b->added_path = apr_pstrdup(merge_b->pool, local_abspath);
+ apr_hash_set(merge_b->dry_run_added, merge_b->added_path,
+ APR_HASH_KEY_STRING, merge_b->added_path);
+ }
else
{
SVN_ERR(svn_io_dir_make(local_abspath, APR_OS_DEFAULT, subpool));
@@ -2183,12 +2221,8 @@ merge_dir_deleted(svn_wc_notify_state_t
svn_pool_destroy(subpool);
if (state)
*state = svn_wc_notify_state_unchanged;
- if (tree_conflicted)
- *tree_conflicted = FALSE;
return SVN_NO_ERROR;
}
- if (tree_conflicted)
- *tree_conflicted = FALSE;
/* Check for an obstructed or missing node on disk. */
{
@@ -2434,12 +2468,6 @@ merge_dir_closed(svn_wc_notify_state_t *
apr_pool_t *scratch_pool)
{
merge_cmd_baton_t *merge_b = baton;
- if (contentstate)
- *contentstate = svn_wc_notify_state_unknown;
- if (propstate)
- *propstate = svn_wc_notify_state_unknown;
- if (tree_conflicted)
- *tree_conflicted = FALSE;
if (merge_b->dry_run)
svn_hash__clear(merge_b->dry_run_deletions, scratch_pool);
@@ -8654,6 +8682,8 @@ do_merge(apr_hash_t **modified_subtrees,
merge_cmd_baton.add_necessitated_merge = FALSE;
merge_cmd_baton.dry_run_deletions =
dry_run ? apr_hash_make(subpool) : NULL;
+ merge_cmd_baton.dry_run_added =
+ dry_run ? apr_hash_make(subpool) : NULL;
merge_cmd_baton.conflicted_paths = NULL;
merge_cmd_baton.paths_with_new_mergeinfo = NULL;
merge_cmd_baton.paths_with_deleted_mergeinfo = NULL;
Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1096921&r1=1096920&r2=1096921&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/repos_diff.c Tue Apr 26 21:38:34 2011
@@ -983,40 +983,12 @@ close_directory(void *dir_baton,
struct edit_baton *eb = b->edit_baton;
svn_wc_notify_state_t content_state = svn_wc_notify_state_unknown;
svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
- svn_node_kind_t kind;
- const char *local_dir_abspath;
+ svn_boolean_t skipped = FALSE;
/* Skip *everything* within a newly tree-conflicted directory. */
if (b->skip)
return SVN_NO_ERROR;
- if (eb->wc_ctx)
- SVN_ERR(svn_wc_read_kind(&kind, eb->wc_ctx, b->wcpath, FALSE, pool));
- else
- kind = svn_node_dir;
-
- if (kind != svn_node_dir)
- {
- /* ### maybe try to stat the local b->wcpath? */
- /* If the path doesn't exist, then send a 'skipped' notification.
- Don't notify added directories as they triggered notification
- in add_directory. */
- if (! b->added && eb->notify_func)
- {
- svn_wc_notify_t *notify
- = svn_wc_create_notify(b->wcpath,
- b->tree_conflicted
- ? svn_wc_notify_tree_conflict
- : svn_wc_notify_skip,
- pool);
- notify->kind = svn_node_dir;
- notify->content_state = notify->prop_state
- = svn_wc_notify_state_missing;
- (*eb->notify_func)(eb->notify_baton, notify, pool);
- }
- return SVN_NO_ERROR;
- }
-
/* Don't do the props_changed stuff if this is a dry_run and we don't
have an access baton, since in that case the directory will already
have been recognised as added, in which case they cannot conflict. */
@@ -1030,23 +1002,30 @@ close_directory(void *dir_baton,
b->edit_baton->diff_cmd_baton, pool));
if (tree_conflicted)
b->tree_conflicted = TRUE;
+
+ if (prop_state == svn_wc_notify_state_obstructed
+ || prop_state == svn_wc_notify_state_missing)
+ {
+ content_state = prop_state;
+ skipped = TRUE;
+ }
}
- SVN_ERR(eb->diff_callbacks->dir_closed(
- NULL, NULL, NULL,
- b->wcpath, b->added,
- b->edit_baton->diff_cmd_baton, pool));
+ SVN_ERR(eb->diff_callbacks->dir_closed(NULL, NULL, NULL,
+ b->wcpath, b->added,
+ b->edit_baton->diff_cmd_baton,
+ pool));
/* Don't notify added directories as they triggered notification
in add_directory. */
- if (!b->added && eb->notify_func)
+ if (!skipped && !b->added && eb->notify_func)
{
- svn_wc_notify_t *notify;
apr_hash_index_t *hi;
for (hi = apr_hash_first(pool, eb->deleted_paths); hi;
hi = apr_hash_next(hi))
{
+ svn_wc_notify_t *notify;
const char *deleted_path = svn__apr_hash_index_key(hi);
deleted_path_notify_t *dpn = svn__apr_hash_index_val(hi);
@@ -1058,12 +1037,21 @@ close_directory(void *dir_baton,
apr_hash_set(eb->deleted_paths, deleted_path,
APR_HASH_KEY_STRING, NULL);
}
+ }
+
+ if (!b->added && eb->notify_func)
+ {
+ svn_wc_notify_t *notify;
+ svn_wc_notify_action_t action;
+
+ if (b->tree_conflicted)
+ action = svn_wc_notify_tree_conflict;
+ else if (skipped)
+ action = svn_wc_notify_skip;
+ else
+ action = svn_wc_notify_update_update;
- notify = svn_wc_create_notify(b->wcpath,
- b->tree_conflicted
- ? svn_wc_notify_tree_conflict
- : svn_wc_notify_update_update,
- pool);
+ notify = svn_wc_create_notify(b->wcpath, action, pool);
notify->kind = svn_node_dir;
/* In case of a tree conflict during merge, the diff callback
Modified: subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py?rev=1096921&r1=1096920&r2=1096921&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Tue Apr 26 21:38:34 2011
@@ -1348,6 +1348,13 @@ def tree_conflicts_merge_edit_onto_missi
expected_skip = svntest.wc.State('', {
'F/alpha' : Item(),
+ # BH: After fixing several issues in the obstruction handling
+ # I get the following Skip notifications. Please review!
+ 'D/D1' : Item(),
+ 'DD/D1' : Item(),
+ 'DF/D1' : Item(),
+ 'DDD/D1' : Item(),
+ 'DDF/D1' : Item(),
})
@@ -1424,6 +1431,13 @@ def tree_conflicts_merge_del_onto_missin
expected_skip = svntest.wc.State('', {
'F/alpha' : Item(),
'D/D1' : Item(),
+ # BH: After fixing several issues in the obstruction handling
+ # I get the following Skip notifications. Please review!
+ 'D/D1' : Item(),
+ 'DD/D1' : Item(),
+ 'DF/D1' : Item(),
+ 'DDD/D1' : Item(),
+ 'DDF/D1' : Item(),
})
svntest.actions.deep_trees_run_tests_scheme_for_merge(sbox,
Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1096921&r1=1096920&r2=1096921&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue Apr 26 21:38:34 2011
@@ -1084,6 +1084,7 @@ def lock_update_only(sbox):
# subversion/libsvn_wc/lock.c:1437: (apr_err=155005)
# svn: E155005: No write-lock in '/.../svn-test-work/working_copies/tree_conflict_tests-22/E'
@Issue(3469)
+@XFail()
def at_directory_external(sbox):
"tree conflict at directory external"
Re: svn commit: r1096921 - in /subversion/trunk/subversion:
libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_tree_conflict_tests.py
tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Apr 26, 2011 at 5:38 PM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Apr 26 21:38:34 2011
> New Revision: 1096921
>
> URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
> Log:
> Remove the last bit of working copy obstruction detection from the repository
> diff editor.
>
> This patch makes the merge editor aware of the list of added directories to
> allow dry run property merges to directories.
>
> * subversion/libsvn_client/merge.c
> (merge_cmd_baton_t): Add hashtable of merge added paths.
> (dry_run_added_p): New function.
> (perform_obstruction_check): Specialize handling of merge-added paths.
> (merge_dir_props_changed): Don't perform actual prop merging for added
> directories when doing a dry-run merge.
> (merge_file_added,
> merge_dir_added,
> merge_dir_deleted,
> merge_dir_closed): Don't set optional output arguments to their default.
>
> * subversion/libsvn_client/repos_diff.c
> (close_directory): When an obstruction is noticed, update the content state
> and use updated notifications.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
> (tree_conflicts_merge_edit_onto_missing,
> tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (at_directory_external): Mark as XFail again. This change moves the failure
> to where we step into the external for the first time. The merge code
> detects this properly now, but wants to apply mergeinfo on the external
> to notice that it wasn't merged. But it can't and shouldn't store this
> information.
Fixed the failing tree_conflict test in r1097598.
Paul
Re: svn commit: r1096921 - in /subversion/trunk/subversion:
libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_tree_conflict_tests.py
tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Apr 26, 2011 at 5:38 PM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Apr 26 21:38:34 2011
> New Revision: 1096921
>
> URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
> Log:
> Remove the last bit of working copy obstruction detection from the repository
> diff editor.
>
> This patch makes the merge editor aware of the list of added directories to
> allow dry run property merges to directories.
>
> * subversion/libsvn_client/merge.c
> (merge_cmd_baton_t): Add hashtable of merge added paths.
> (dry_run_added_p): New function.
> (perform_obstruction_check): Specialize handling of merge-added paths.
> (merge_dir_props_changed): Don't perform actual prop merging for added
> directories when doing a dry-run merge.
> (merge_file_added,
> merge_dir_added,
> merge_dir_deleted,
> merge_dir_closed): Don't set optional output arguments to their default.
>
> * subversion/libsvn_client/repos_diff.c
> (close_directory): When an obstruction is noticed, update the content state
> and use updated notifications.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
> (tree_conflicts_merge_edit_onto_missing,
> tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (at_directory_external): Mark as XFail again. This change moves the failure
> to where we step into the external for the first time. The merge code
> detects this properly now, but wants to apply mergeinfo on the external
> to notice that it wasn't merged. But it can't and shouldn't store this
> information.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
> subversion/trunk/subversion/libsvn_client/repos_diff.c
> subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Tue Apr 26 21:38:34 2011
> @@ -276,6 +276,10 @@ typedef struct merge_cmd_baton_t {
> dry_run mode. */
> apr_hash_t *dry_run_deletions;
>
> + /* The list of paths for entries we've added, used only when in
> + dry_run mode. */
> + apr_hash_t *dry_run_added;
> +
> /* The list of any paths which remained in conflict after a
> resolution attempt was made. We track this in-memory, rather
> than just using WC entry state, since the latter doesn't help us
> @@ -353,6 +357,20 @@ dry_run_deleted_p(const merge_cmd_baton_
> APR_HASH_KEY_STRING) != NULL);
> }
>
> +/* Return true iff we're in dry-run mode and WCPATH would have been
> + added by now if we weren't in dry-run mode.
> + Used to avoid spurious notifications (e.g. conflicts) from a merge
> + attempt into an existing target which would have been deleted if we
> + weren't in dry_run mode (issue #2584). Assumes that WCPATH is
> + still versioned (e.g. has an associated entry). */
> +static APR_INLINE svn_boolean_t
> +dry_run_added_p(const merge_cmd_baton_t *merge_b, const char *wcpath)
> +{
> + return (merge_b->dry_run &&
> + apr_hash_get(merge_b->dry_run_added, wcpath,
> + APR_HASH_KEY_STRING) != NULL);
> +}
> +
> /* Return whether any WC path was put in conflict by the merge
> operation corresponding to MERGE_B. */
> static APR_INLINE svn_boolean_t
> @@ -408,20 +426,36 @@ perform_obstruction_check(svn_wc_notify_
> *kind = svn_node_none;
>
> /* In a dry run, make as if nodes "deleted" by the dry run appear so. */
> - if (merge_b->dry_run && dry_run_deleted_p(merge_b, local_abspath))
> + if (merge_b->dry_run)
> {
> - *obstruction_state = svn_wc_notify_state_inapplicable;
> + if (dry_run_deleted_p(merge_b, local_abspath))
> + {
> + *obstruction_state = svn_wc_notify_state_inapplicable;
> +
> + if (versioned)
> + *versioned = TRUE;
> + if (deleted)
> + *deleted = TRUE;
> +
> + if (expected_kind != svn_node_unknown
> + && expected_kind != svn_node_none)
> + *obstruction_state = svn_wc_notify_state_obstructed;
> + return SVN_NO_ERROR;
> + }
> + else if (dry_run_added_p(merge_b, local_abspath))
> + {
> + *obstruction_state = svn_wc_notify_state_inapplicable;
>
> - if (versioned)
> - *versioned = TRUE;
> - if (deleted)
> - *deleted = TRUE;
> -
> - if (expected_kind != svn_node_unknown
> - && expected_kind != svn_node_none)
> - *obstruction_state = svn_wc_notify_state_obstructed;
> - return SVN_NO_ERROR;
> - }
> + if (versioned)
> + *versioned = TRUE;
> + if (added)
> + *added = TRUE;
> + if (kind)
> + *kind = svn_node_dir; /* Currently only used for dirs */
> +
> + return SVN_NO_ERROR;
> + }
> + }
>
> if (kind == NULL)
> kind = &wc_kind;
> @@ -1224,6 +1258,13 @@ merge_dir_props_changed(svn_wc_notify_st
> return SVN_NO_ERROR;
> }
>
> + if (dir_was_added
> + && merge_b->dry_run
> + && dry_run_added_p(merge_b, local_abspath))
> + {
> + return SVN_NO_ERROR; /* We can't do a real prop merge for added dirs */
> + }
> +
> return svn_error_return(merge_props_changed(state,
> tree_conflicted,
> local_abspath,
> @@ -1544,8 +1585,6 @@ merge_file_added(svn_wc_notify_state_t *
> *content_state = svn_wc_notify_state_unchanged;
> if (prop_state)
> *prop_state = svn_wc_notify_state_unchanged;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> return SVN_NO_ERROR;
> }
>
> @@ -1555,9 +1594,6 @@ merge_file_added(svn_wc_notify_state_t *
> if (prop_state)
> *prop_state = svn_wc_notify_state_unknown;
>
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> -
> /* Apply the prop changes to a new hash table. */
> file_props = apr_hash_copy(subpool, original_props);
> for (i = 0; i < prop_changes->nelts; ++i)
> @@ -1996,8 +2032,6 @@ merge_dir_added(svn_wc_notify_state_t *s
> svn_pool_destroy(subpool);
> if (state)
> *state = svn_wc_notify_state_unchanged;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> return SVN_NO_ERROR;
> }
>
> @@ -2069,7 +2103,11 @@ merge_dir_added(svn_wc_notify_state_t *s
> case svn_node_none:
> /* Unversioned or schedule-delete */
> if (merge_b->dry_run)
> - merge_b->added_path = apr_pstrdup(merge_b->pool, local_abspath);
> + {
> + merge_b->added_path = apr_pstrdup(merge_b->pool, local_abspath);
> + apr_hash_set(merge_b->dry_run_added, merge_b->added_path,
> + APR_HASH_KEY_STRING, merge_b->added_path);
> + }
> else
> {
> SVN_ERR(svn_io_dir_make(local_abspath, APR_OS_DEFAULT, subpool));
> @@ -2183,12 +2221,8 @@ merge_dir_deleted(svn_wc_notify_state_t
> svn_pool_destroy(subpool);
> if (state)
> *state = svn_wc_notify_state_unchanged;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> return SVN_NO_ERROR;
> }
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
>
> /* Check for an obstructed or missing node on disk. */
> {
> @@ -2434,12 +2468,6 @@ merge_dir_closed(svn_wc_notify_state_t *
> apr_pool_t *scratch_pool)
> {
> merge_cmd_baton_t *merge_b = baton;
> - if (contentstate)
> - *contentstate = svn_wc_notify_state_unknown;
> - if (propstate)
> - *propstate = svn_wc_notify_state_unknown;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
>
> if (merge_b->dry_run)
> svn_hash__clear(merge_b->dry_run_deletions, scratch_pool);
> @@ -8654,6 +8682,8 @@ do_merge(apr_hash_t **modified_subtrees,
> merge_cmd_baton.add_necessitated_merge = FALSE;
> merge_cmd_baton.dry_run_deletions =
> dry_run ? apr_hash_make(subpool) : NULL;
> + merge_cmd_baton.dry_run_added =
> + dry_run ? apr_hash_make(subpool) : NULL;
> merge_cmd_baton.conflicted_paths = NULL;
> merge_cmd_baton.paths_with_new_mergeinfo = NULL;
> merge_cmd_baton.paths_with_deleted_mergeinfo = NULL;
>
> Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
> +++ subversion/trunk/subversion/libsvn_client/repos_diff.c Tue Apr 26 21:38:34 2011
> @@ -983,40 +983,12 @@ close_directory(void *dir_baton,
> struct edit_baton *eb = b->edit_baton;
> svn_wc_notify_state_t content_state = svn_wc_notify_state_unknown;
> svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
> - svn_node_kind_t kind;
> - const char *local_dir_abspath;
> + svn_boolean_t skipped = FALSE;
>
> /* Skip *everything* within a newly tree-conflicted directory. */
> if (b->skip)
> return SVN_NO_ERROR;
>
> - if (eb->wc_ctx)
> - SVN_ERR(svn_wc_read_kind(&kind, eb->wc_ctx, b->wcpath, FALSE, pool));
> - else
> - kind = svn_node_dir;
> -
> - if (kind != svn_node_dir)
> - {
> - /* ### maybe try to stat the local b->wcpath? */
> - /* If the path doesn't exist, then send a 'skipped' notification.
> - Don't notify added directories as they triggered notification
> - in add_directory. */
> - if (! b->added && eb->notify_func)
> - {
> - svn_wc_notify_t *notify
> - = svn_wc_create_notify(b->wcpath,
> - b->tree_conflicted
> - ? svn_wc_notify_tree_conflict
> - : svn_wc_notify_skip,
> - pool);
> - notify->kind = svn_node_dir;
> - notify->content_state = notify->prop_state
> - = svn_wc_notify_state_missing;
> - (*eb->notify_func)(eb->notify_baton, notify, pool);
> - }
> - return SVN_NO_ERROR;
> - }
> -
> /* Don't do the props_changed stuff if this is a dry_run and we don't
> have an access baton, since in that case the directory will already
> have been recognised as added, in which case they cannot conflict. */
> @@ -1030,23 +1002,30 @@ close_directory(void *dir_baton,
> b->edit_baton->diff_cmd_baton, pool));
> if (tree_conflicted)
> b->tree_conflicted = TRUE;
> +
> + if (prop_state == svn_wc_notify_state_obstructed
> + || prop_state == svn_wc_notify_state_missing)
> + {
> + content_state = prop_state;
> + skipped = TRUE;
> + }
> }
>
> - SVN_ERR(eb->diff_callbacks->dir_closed(
> - NULL, NULL, NULL,
> - b->wcpath, b->added,
> - b->edit_baton->diff_cmd_baton, pool));
> + SVN_ERR(eb->diff_callbacks->dir_closed(NULL, NULL, NULL,
> + b->wcpath, b->added,
> + b->edit_baton->diff_cmd_baton,
> + pool));
>
> /* Don't notify added directories as they triggered notification
> in add_directory. */
> - if (!b->added && eb->notify_func)
> + if (!skipped && !b->added && eb->notify_func)
> {
> - svn_wc_notify_t *notify;
> apr_hash_index_t *hi;
>
> for (hi = apr_hash_first(pool, eb->deleted_paths); hi;
> hi = apr_hash_next(hi))
> {
> + svn_wc_notify_t *notify;
> const char *deleted_path = svn__apr_hash_index_key(hi);
> deleted_path_notify_t *dpn = svn__apr_hash_index_val(hi);
>
> @@ -1058,12 +1037,21 @@ close_directory(void *dir_baton,
> apr_hash_set(eb->deleted_paths, deleted_path,
> APR_HASH_KEY_STRING, NULL);
> }
> + }
> +
> + if (!b->added && eb->notify_func)
> + {
> + svn_wc_notify_t *notify;
> + svn_wc_notify_action_t action;
> +
> + if (b->tree_conflicted)
> + action = svn_wc_notify_tree_conflict;
> + else if (skipped)
> + action = svn_wc_notify_skip;
> + else
> + action = svn_wc_notify_update_update;
>
> - notify = svn_wc_create_notify(b->wcpath,
> - b->tree_conflicted
> - ? svn_wc_notify_tree_conflict
> - : svn_wc_notify_update_update,
> - pool);
> + notify = svn_wc_create_notify(b->wcpath, action, pool);
> notify->kind = svn_node_dir;
>
> /* In case of a tree conflict during merge, the diff callback
>
> Modified: subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Tue Apr 26 21:38:34 2011
> @@ -1348,6 +1348,13 @@ def tree_conflicts_merge_edit_onto_missi
>
> expected_skip = svntest.wc.State('', {
> 'F/alpha' : Item(),
> + # BH: After fixing several issues in the obstruction handling
> + # I get the following Skip notifications. Please review!
> + 'D/D1' : Item(),
> + 'DD/D1' : Item(),
> + 'DF/D1' : Item(),
> + 'DDD/D1' : Item(),
> + 'DDF/D1' : Item(),
> })
Hi Bert,
Short answer: I think your change to this test's expectations are correct.
Long answer:
So this merge tries to apply diffs to subtrees which are missing due
to an OS-level deletion. In 1.6 this would have produced tree
conflicts, but in r946186 stsp adjusted the merge behavior to skip
obstructed and missing directories during a merge (missing/obstructed
files were already skipped). The log for that change explains the
reasoning for this, so I won't rehash it here except to note that
since that change was made we now prevent merge-tracking aware merges
when subtrees are missing. So the bit in the log about
non-inheritable mergeinfo no longer applies.
Anyway, assuming r946186 is the desired behavior (I think it is) then
your changes in r1096921 actually fix a bug we missed:
Prior to r1096921we only issued notifications for missing *files*:
trunk@1096920>svn st
! local_tree_missing_incoming_leaf_edit\local\D\D1
! local_tree_missing_incoming_leaf_edit\local\F\alpha
! local_tree_missing_incoming_leaf_edit\local\DD\D1
! local_tree_missing_incoming_leaf_edit\local\DD\D1\D2
! local_tree_missing_incoming_leaf_edit\local\DF\D1
! local_tree_missing_incoming_leaf_edit\local\DF\D1\beta
! local_tree_missing_incoming_leaf_edit\local\DDD\D1
! local_tree_missing_incoming_leaf_edit\local\DDD\D1\D2
! local_tree_missing_incoming_leaf_edit\local\DDD\D1\D2\D3
! local_tree_missing_incoming_leaf_edit\local\DDF\D1
! local_tree_missing_incoming_leaf_edit\local\DDF\D1\D2
! local_tree_missing_incoming_leaf_edit\local\DDF\D1\D2\gamma
trunk@1096920>svn merge ^^/local_tree_missing_incoming_leaf_edit/incoming
local_tree_missing_incoming_leaf_edit\local --ignore-ancestry
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\F\alpha'
Summary of conflicts:
Skipped paths: 1
Uh, that seems quite wrong since the incoming change touches a lot
more than alpha:
trunk@1096920>svn log -v -r4 ^^/local_tree_missing_incoming_leaf_edit/incoming
------------------------------------------------------------------------
r4 | jrandom | 2011-04-28 09:26:12 -0400 (Thu, 28 Apr 2011) | 1 line
Changed paths:
M /local_tree_missing_incoming_leaf_edit/incoming/D/D1
A /local_tree_missing_incoming_leaf_edit/incoming/D/D1/delta
M /local_tree_missing_incoming_leaf_edit/incoming/DD/D1/D2
A /local_tree_missing_incoming_leaf_edit/incoming/DD/D1/D2/epsilon
M /local_tree_missing_incoming_leaf_edit/incoming/DDD/D1/D2/D3
A /local_tree_missing_incoming_leaf_edit/incoming/DDD/D1/D2/D3/zeta
M /local_tree_missing_incoming_leaf_edit/incoming/DDF/D1/D2/gamma
M /local_tree_missing_incoming_leaf_edit/incoming/DF/D1/beta
M /local_tree_missing_incoming_leaf_edit/incoming/F/alpha
Committing incoming actions
------------------------------------------------------------------------
trunk@1096921>svn merge ^^/local_tree_missing_incoming_leaf_edit/incoming
local_tree_missing_incoming_leaf_edit\local --ignore-ancestry --dry-run
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\D\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\F\alpha'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DD\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DF\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DDD\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DDF\D1'
Summary of conflicts:
Skipped paths: 6
Now we see a notification for the root of each missing subtree in
which we are attempting to apply a diff. No longer are those five
directories silently skipped.
> @@ -1424,6 +1431,13 @@ def tree_conflicts_merge_del_onto_missin
> expected_skip = svntest.wc.State('', {
> 'F/alpha' : Item(),
> 'D/D1' : Item(),
> + # BH: After fixing several issues in the obstruction handling
> + # I get the following Skip notifications. Please review!
> + 'D/D1' : Item(),
> + 'DD/D1' : Item(),
> + 'DF/D1' : Item(),
> + 'DDD/D1' : Item(),
> + 'DDF/D1' : Item(),
> })
Pretty much the same thing as above except prior to r1096921 we did
get a notification that the *directory* D/D1 was skipped. This was
because of yet another inconsistency in reporting skips: An incoming
delete on the root of a missing subtree produced a notification, but
not an incoming delete below the root of a missing subtree.
Fortunately you took care to avoid duplicate notifications in this
case.
Paul
> svntest.actions.deep_trees_run_tests_scheme_for_merge(sbox,
>
> Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue Apr 26 21:38:34 2011
> @@ -1084,6 +1084,7 @@ def lock_update_only(sbox):
> # subversion/libsvn_wc/lock.c:1437: (apr_err=155005)
> # svn: E155005: No write-lock in '/.../svn-test-work/working_copies/tree_conflict_tests-22/E'
> @Issue(3469)
> +@XFail()
> def at_directory_external(sbox):
> "tree conflict at directory external"
Re: svn commit: r1096921 - in /subversion/trunk/subversion:
libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_tree_conflict_tests.py
tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Apr 26, 2011 at 5:38 PM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Apr 26 21:38:34 2011
> New Revision: 1096921
>
> URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
> Log:
> Remove the last bit of working copy obstruction detection from the repository
> diff editor.
>
> This patch makes the merge editor aware of the list of added directories to
> allow dry run property merges to directories.
>
> * subversion/libsvn_client/merge.c
> (merge_cmd_baton_t): Add hashtable of merge added paths.
> (dry_run_added_p): New function.
> (perform_obstruction_check): Specialize handling of merge-added paths.
> (merge_dir_props_changed): Don't perform actual prop merging for added
> directories when doing a dry-run merge.
> (merge_file_added,
> merge_dir_added,
> merge_dir_deleted,
> merge_dir_closed): Don't set optional output arguments to their default.
>
> * subversion/libsvn_client/repos_diff.c
> (close_directory): When an obstruction is noticed, update the content state
> and use updated notifications.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
> (tree_conflicts_merge_edit_onto_missing,
> tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (at_directory_external): Mark as XFail again. This change moves the failure
> to where we step into the external for the first time. The merge code
> detects this properly now, but wants to apply mergeinfo on the external
> to notice that it wasn't merged. But it can't and shouldn't store this
> information.
Fixed the failing tree_conflict test in r1097598.
Paul
Re: svn commit: r1096921 - in /subversion/trunk/subversion:
libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_tree_conflict_tests.py
tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Apr 26, 2011 at 5:38 PM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Apr 26 21:38:34 2011
> New Revision: 1096921
>
> URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
> Log:
> Remove the last bit of working copy obstruction detection from the repository
> diff editor.
>
> This patch makes the merge editor aware of the list of added directories to
> allow dry run property merges to directories.
>
> * subversion/libsvn_client/merge.c
> (merge_cmd_baton_t): Add hashtable of merge added paths.
> (dry_run_added_p): New function.
> (perform_obstruction_check): Specialize handling of merge-added paths.
> (merge_dir_props_changed): Don't perform actual prop merging for added
> directories when doing a dry-run merge.
> (merge_file_added,
> merge_dir_added,
> merge_dir_deleted,
> merge_dir_closed): Don't set optional output arguments to their default.
>
> * subversion/libsvn_client/repos_diff.c
> (close_directory): When an obstruction is noticed, update the content state
> and use updated notifications.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
> (tree_conflicts_merge_edit_onto_missing,
> tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (at_directory_external): Mark as XFail again. This change moves the failure
> to where we step into the external for the first time. The merge code
> detects this properly now, but wants to apply mergeinfo on the external
> to notice that it wasn't merged. But it can't and shouldn't store this
> information.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
> subversion/trunk/subversion/libsvn_client/repos_diff.c
> subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Tue Apr 26 21:38:34 2011
> @@ -276,6 +276,10 @@ typedef struct merge_cmd_baton_t {
> dry_run mode. */
> apr_hash_t *dry_run_deletions;
>
> + /* The list of paths for entries we've added, used only when in
> + dry_run mode. */
> + apr_hash_t *dry_run_added;
> +
> /* The list of any paths which remained in conflict after a
> resolution attempt was made. We track this in-memory, rather
> than just using WC entry state, since the latter doesn't help us
> @@ -353,6 +357,20 @@ dry_run_deleted_p(const merge_cmd_baton_
> APR_HASH_KEY_STRING) != NULL);
> }
>
> +/* Return true iff we're in dry-run mode and WCPATH would have been
> + added by now if we weren't in dry-run mode.
> + Used to avoid spurious notifications (e.g. conflicts) from a merge
> + attempt into an existing target which would have been deleted if we
> + weren't in dry_run mode (issue #2584). Assumes that WCPATH is
> + still versioned (e.g. has an associated entry). */
> +static APR_INLINE svn_boolean_t
> +dry_run_added_p(const merge_cmd_baton_t *merge_b, const char *wcpath)
> +{
> + return (merge_b->dry_run &&
> + apr_hash_get(merge_b->dry_run_added, wcpath,
> + APR_HASH_KEY_STRING) != NULL);
> +}
> +
> /* Return whether any WC path was put in conflict by the merge
> operation corresponding to MERGE_B. */
> static APR_INLINE svn_boolean_t
> @@ -408,20 +426,36 @@ perform_obstruction_check(svn_wc_notify_
> *kind = svn_node_none;
>
> /* In a dry run, make as if nodes "deleted" by the dry run appear so. */
> - if (merge_b->dry_run && dry_run_deleted_p(merge_b, local_abspath))
> + if (merge_b->dry_run)
> {
> - *obstruction_state = svn_wc_notify_state_inapplicable;
> + if (dry_run_deleted_p(merge_b, local_abspath))
> + {
> + *obstruction_state = svn_wc_notify_state_inapplicable;
> +
> + if (versioned)
> + *versioned = TRUE;
> + if (deleted)
> + *deleted = TRUE;
> +
> + if (expected_kind != svn_node_unknown
> + && expected_kind != svn_node_none)
> + *obstruction_state = svn_wc_notify_state_obstructed;
> + return SVN_NO_ERROR;
> + }
> + else if (dry_run_added_p(merge_b, local_abspath))
> + {
> + *obstruction_state = svn_wc_notify_state_inapplicable;
>
> - if (versioned)
> - *versioned = TRUE;
> - if (deleted)
> - *deleted = TRUE;
> -
> - if (expected_kind != svn_node_unknown
> - && expected_kind != svn_node_none)
> - *obstruction_state = svn_wc_notify_state_obstructed;
> - return SVN_NO_ERROR;
> - }
> + if (versioned)
> + *versioned = TRUE;
> + if (added)
> + *added = TRUE;
> + if (kind)
> + *kind = svn_node_dir; /* Currently only used for dirs */
> +
> + return SVN_NO_ERROR;
> + }
> + }
>
> if (kind == NULL)
> kind = &wc_kind;
> @@ -1224,6 +1258,13 @@ merge_dir_props_changed(svn_wc_notify_st
> return SVN_NO_ERROR;
> }
>
> + if (dir_was_added
> + && merge_b->dry_run
> + && dry_run_added_p(merge_b, local_abspath))
> + {
> + return SVN_NO_ERROR; /* We can't do a real prop merge for added dirs */
> + }
> +
> return svn_error_return(merge_props_changed(state,
> tree_conflicted,
> local_abspath,
> @@ -1544,8 +1585,6 @@ merge_file_added(svn_wc_notify_state_t *
> *content_state = svn_wc_notify_state_unchanged;
> if (prop_state)
> *prop_state = svn_wc_notify_state_unchanged;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> return SVN_NO_ERROR;
> }
>
> @@ -1555,9 +1594,6 @@ merge_file_added(svn_wc_notify_state_t *
> if (prop_state)
> *prop_state = svn_wc_notify_state_unknown;
>
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> -
> /* Apply the prop changes to a new hash table. */
> file_props = apr_hash_copy(subpool, original_props);
> for (i = 0; i < prop_changes->nelts; ++i)
> @@ -1996,8 +2032,6 @@ merge_dir_added(svn_wc_notify_state_t *s
> svn_pool_destroy(subpool);
> if (state)
> *state = svn_wc_notify_state_unchanged;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> return SVN_NO_ERROR;
> }
>
> @@ -2069,7 +2103,11 @@ merge_dir_added(svn_wc_notify_state_t *s
> case svn_node_none:
> /* Unversioned or schedule-delete */
> if (merge_b->dry_run)
> - merge_b->added_path = apr_pstrdup(merge_b->pool, local_abspath);
> + {
> + merge_b->added_path = apr_pstrdup(merge_b->pool, local_abspath);
> + apr_hash_set(merge_b->dry_run_added, merge_b->added_path,
> + APR_HASH_KEY_STRING, merge_b->added_path);
> + }
> else
> {
> SVN_ERR(svn_io_dir_make(local_abspath, APR_OS_DEFAULT, subpool));
> @@ -2183,12 +2221,8 @@ merge_dir_deleted(svn_wc_notify_state_t
> svn_pool_destroy(subpool);
> if (state)
> *state = svn_wc_notify_state_unchanged;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
> return SVN_NO_ERROR;
> }
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
>
> /* Check for an obstructed or missing node on disk. */
> {
> @@ -2434,12 +2468,6 @@ merge_dir_closed(svn_wc_notify_state_t *
> apr_pool_t *scratch_pool)
> {
> merge_cmd_baton_t *merge_b = baton;
> - if (contentstate)
> - *contentstate = svn_wc_notify_state_unknown;
> - if (propstate)
> - *propstate = svn_wc_notify_state_unknown;
> - if (tree_conflicted)
> - *tree_conflicted = FALSE;
>
> if (merge_b->dry_run)
> svn_hash__clear(merge_b->dry_run_deletions, scratch_pool);
> @@ -8654,6 +8682,8 @@ do_merge(apr_hash_t **modified_subtrees,
> merge_cmd_baton.add_necessitated_merge = FALSE;
> merge_cmd_baton.dry_run_deletions =
> dry_run ? apr_hash_make(subpool) : NULL;
> + merge_cmd_baton.dry_run_added =
> + dry_run ? apr_hash_make(subpool) : NULL;
> merge_cmd_baton.conflicted_paths = NULL;
> merge_cmd_baton.paths_with_new_mergeinfo = NULL;
> merge_cmd_baton.paths_with_deleted_mergeinfo = NULL;
>
> Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
> +++ subversion/trunk/subversion/libsvn_client/repos_diff.c Tue Apr 26 21:38:34 2011
> @@ -983,40 +983,12 @@ close_directory(void *dir_baton,
> struct edit_baton *eb = b->edit_baton;
> svn_wc_notify_state_t content_state = svn_wc_notify_state_unknown;
> svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
> - svn_node_kind_t kind;
> - const char *local_dir_abspath;
> + svn_boolean_t skipped = FALSE;
>
> /* Skip *everything* within a newly tree-conflicted directory. */
> if (b->skip)
> return SVN_NO_ERROR;
>
> - if (eb->wc_ctx)
> - SVN_ERR(svn_wc_read_kind(&kind, eb->wc_ctx, b->wcpath, FALSE, pool));
> - else
> - kind = svn_node_dir;
> -
> - if (kind != svn_node_dir)
> - {
> - /* ### maybe try to stat the local b->wcpath? */
> - /* If the path doesn't exist, then send a 'skipped' notification.
> - Don't notify added directories as they triggered notification
> - in add_directory. */
> - if (! b->added && eb->notify_func)
> - {
> - svn_wc_notify_t *notify
> - = svn_wc_create_notify(b->wcpath,
> - b->tree_conflicted
> - ? svn_wc_notify_tree_conflict
> - : svn_wc_notify_skip,
> - pool);
> - notify->kind = svn_node_dir;
> - notify->content_state = notify->prop_state
> - = svn_wc_notify_state_missing;
> - (*eb->notify_func)(eb->notify_baton, notify, pool);
> - }
> - return SVN_NO_ERROR;
> - }
> -
> /* Don't do the props_changed stuff if this is a dry_run and we don't
> have an access baton, since in that case the directory will already
> have been recognised as added, in which case they cannot conflict. */
> @@ -1030,23 +1002,30 @@ close_directory(void *dir_baton,
> b->edit_baton->diff_cmd_baton, pool));
> if (tree_conflicted)
> b->tree_conflicted = TRUE;
> +
> + if (prop_state == svn_wc_notify_state_obstructed
> + || prop_state == svn_wc_notify_state_missing)
> + {
> + content_state = prop_state;
> + skipped = TRUE;
> + }
> }
>
> - SVN_ERR(eb->diff_callbacks->dir_closed(
> - NULL, NULL, NULL,
> - b->wcpath, b->added,
> - b->edit_baton->diff_cmd_baton, pool));
> + SVN_ERR(eb->diff_callbacks->dir_closed(NULL, NULL, NULL,
> + b->wcpath, b->added,
> + b->edit_baton->diff_cmd_baton,
> + pool));
>
> /* Don't notify added directories as they triggered notification
> in add_directory. */
> - if (!b->added && eb->notify_func)
> + if (!skipped && !b->added && eb->notify_func)
> {
> - svn_wc_notify_t *notify;
> apr_hash_index_t *hi;
>
> for (hi = apr_hash_first(pool, eb->deleted_paths); hi;
> hi = apr_hash_next(hi))
> {
> + svn_wc_notify_t *notify;
> const char *deleted_path = svn__apr_hash_index_key(hi);
> deleted_path_notify_t *dpn = svn__apr_hash_index_val(hi);
>
> @@ -1058,12 +1037,21 @@ close_directory(void *dir_baton,
> apr_hash_set(eb->deleted_paths, deleted_path,
> APR_HASH_KEY_STRING, NULL);
> }
> + }
> +
> + if (!b->added && eb->notify_func)
> + {
> + svn_wc_notify_t *notify;
> + svn_wc_notify_action_t action;
> +
> + if (b->tree_conflicted)
> + action = svn_wc_notify_tree_conflict;
> + else if (skipped)
> + action = svn_wc_notify_skip;
> + else
> + action = svn_wc_notify_update_update;
>
> - notify = svn_wc_create_notify(b->wcpath,
> - b->tree_conflicted
> - ? svn_wc_notify_tree_conflict
> - : svn_wc_notify_update_update,
> - pool);
> + notify = svn_wc_create_notify(b->wcpath, action, pool);
> notify->kind = svn_node_dir;
>
> /* In case of a tree conflict during merge, the diff callback
>
> Modified: subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Tue Apr 26 21:38:34 2011
> @@ -1348,6 +1348,13 @@ def tree_conflicts_merge_edit_onto_missi
>
> expected_skip = svntest.wc.State('', {
> 'F/alpha' : Item(),
> + # BH: After fixing several issues in the obstruction handling
> + # I get the following Skip notifications. Please review!
> + 'D/D1' : Item(),
> + 'DD/D1' : Item(),
> + 'DF/D1' : Item(),
> + 'DDD/D1' : Item(),
> + 'DDF/D1' : Item(),
> })
Hi Bert,
Short answer: I think your change to this test's expectations are correct.
Long answer:
So this merge tries to apply diffs to subtrees which are missing due
to an OS-level deletion. In 1.6 this would have produced tree
conflicts, but in r946186 stsp adjusted the merge behavior to skip
obstructed and missing directories during a merge (missing/obstructed
files were already skipped). The log for that change explains the
reasoning for this, so I won't rehash it here except to note that
since that change was made we now prevent merge-tracking aware merges
when subtrees are missing. So the bit in the log about
non-inheritable mergeinfo no longer applies.
Anyway, assuming r946186 is the desired behavior (I think it is) then
your changes in r1096921 actually fix a bug we missed:
Prior to r1096921we only issued notifications for missing *files*:
trunk@1096920>svn st
! local_tree_missing_incoming_leaf_edit\local\D\D1
! local_tree_missing_incoming_leaf_edit\local\F\alpha
! local_tree_missing_incoming_leaf_edit\local\DD\D1
! local_tree_missing_incoming_leaf_edit\local\DD\D1\D2
! local_tree_missing_incoming_leaf_edit\local\DF\D1
! local_tree_missing_incoming_leaf_edit\local\DF\D1\beta
! local_tree_missing_incoming_leaf_edit\local\DDD\D1
! local_tree_missing_incoming_leaf_edit\local\DDD\D1\D2
! local_tree_missing_incoming_leaf_edit\local\DDD\D1\D2\D3
! local_tree_missing_incoming_leaf_edit\local\DDF\D1
! local_tree_missing_incoming_leaf_edit\local\DDF\D1\D2
! local_tree_missing_incoming_leaf_edit\local\DDF\D1\D2\gamma
trunk@1096920>svn merge ^^/local_tree_missing_incoming_leaf_edit/incoming
local_tree_missing_incoming_leaf_edit\local --ignore-ancestry
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\F\alpha'
Summary of conflicts:
Skipped paths: 1
Uh, that seems quite wrong since the incoming change touches a lot
more than alpha:
trunk@1096920>svn log -v -r4 ^^/local_tree_missing_incoming_leaf_edit/incoming
------------------------------------------------------------------------
r4 | jrandom | 2011-04-28 09:26:12 -0400 (Thu, 28 Apr 2011) | 1 line
Changed paths:
M /local_tree_missing_incoming_leaf_edit/incoming/D/D1
A /local_tree_missing_incoming_leaf_edit/incoming/D/D1/delta
M /local_tree_missing_incoming_leaf_edit/incoming/DD/D1/D2
A /local_tree_missing_incoming_leaf_edit/incoming/DD/D1/D2/epsilon
M /local_tree_missing_incoming_leaf_edit/incoming/DDD/D1/D2/D3
A /local_tree_missing_incoming_leaf_edit/incoming/DDD/D1/D2/D3/zeta
M /local_tree_missing_incoming_leaf_edit/incoming/DDF/D1/D2/gamma
M /local_tree_missing_incoming_leaf_edit/incoming/DF/D1/beta
M /local_tree_missing_incoming_leaf_edit/incoming/F/alpha
Committing incoming actions
------------------------------------------------------------------------
trunk@1096921>svn merge ^^/local_tree_missing_incoming_leaf_edit/incoming
local_tree_missing_incoming_leaf_edit\local --ignore-ancestry --dry-run
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\D\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\F\alpha'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DD\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DF\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DDD\D1'
Skipped missing target: 'local_tree_missing_incoming_leaf_edit\local\DDF\D1'
Summary of conflicts:
Skipped paths: 6
Now we see a notification for the root of each missing subtree in
which we are attempting to apply a diff. No longer are those five
directories silently skipped.
> @@ -1424,6 +1431,13 @@ def tree_conflicts_merge_del_onto_missin
> expected_skip = svntest.wc.State('', {
> 'F/alpha' : Item(),
> 'D/D1' : Item(),
> + # BH: After fixing several issues in the obstruction handling
> + # I get the following Skip notifications. Please review!
> + 'D/D1' : Item(),
> + 'DD/D1' : Item(),
> + 'DF/D1' : Item(),
> + 'DDD/D1' : Item(),
> + 'DDF/D1' : Item(),
> })
Pretty much the same thing as above except prior to r1096921 we did
get a notification that the *directory* D/D1 was skipped. This was
because of yet another inconsistency in reporting skips: An incoming
delete on the root of a missing subtree produced a notification, but
not an incoming delete below the root of a missing subtree.
Fortunately you took care to avoid duplicate notifications in this
case.
Paul
> svntest.actions.deep_trees_run_tests_scheme_for_merge(sbox,
>
> Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue Apr 26 21:38:34 2011
> @@ -1084,6 +1084,7 @@ def lock_update_only(sbox):
> # subversion/libsvn_wc/lock.c:1437: (apr_err=155005)
> # svn: E155005: No write-lock in '/.../svn-test-work/working_copies/tree_conflict_tests-22/E'
> @Issue(3469)
> +@XFail()
> def at_directory_external(sbox):
> "tree conflict at directory external"