You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2011/04/04 11:56:58 UTC
svn commit: r1088533 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/tree_conflict_tests.py
Author: danielsh
Date: Mon Apr 4 09:56:58 2011
New Revision: 1088533
URL: http://svn.apache.org/viewvc?rev=1088533&view=rev
Log:
Resolve issue #3469 (tree conflict under a directory external).
Review by: rhuijben
* subversion/libsvn_client/merge.c
(merge_cmd_baton_t): Add TARGET_WCROOT_ABSPATH member.
(do_merge): Initialize new member.
(merge_file_added):
Skip files that belong to externals or to disjoint working copies.
Exploit knowledge of libsvn_wc implementations details (queries on
directories are faster) to make the check cheaper.
* subversion/tests/cmdline/tree_conflict_tests.py
(at_directory_external):
Expect it to pass.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
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=1088533&r1=1088532&r2=1088533&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Mon Apr 4 09:56:58 2011
@@ -249,6 +249,8 @@ typedef struct merge_cmd_baton_t {
versioned dir (dry-run only) */
const char *target_abspath; /* Absolute working copy target of
the merge. */
+ const char *target_wcroot_abspath; /* Absolute path to root of wc that
+ TARGET_ABSPATH belongs to. */
/* The left and right URLs and revs. The value of this field changes to
reflect the merge_source_t *currently* being merged by do_merge(). */
@@ -1555,6 +1557,32 @@ merge_file_added(const char *local_dir_a
if (tree_conflicted)
*tree_conflicted = FALSE;
+ /* Easy out: not the same working copy. (So, a disjoint working copy or
+ an external.) */
+ {
+ const char *mine_wcroot_abspath;
+
+ /* ### White-box optimization:
+
+ The code knows that MINE_ABSPATH is not a directory (it's an added
+ file), and we know that internally libsvn_wc queries are faster for
+ directories (this is an implementation detail). Therefore, query for
+ the wcroot of the containing directory of MINE_ABSPATH.
+
+ (We rely on the implementation detail only for performance, not for
+ correctness; under any implementation it would be valid to query for
+ the parent's wcroot.)
+ */
+ SVN_ERR(svn_wc_get_wc_root(&mine_wcroot_abspath, merge_b->ctx->wc_ctx,
+ svn_dirent_dirname(mine_abspath, scratch_pool),
+ scratch_pool, scratch_pool));
+ if (strcmp(mine_wcroot_abspath, merge_b->target_wcroot_abspath))
+ {
+ *content_state = svn_wc_notify_state_obstructed;
+ return SVN_NO_ERROR;
+ }
+ }
+
/* 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)
@@ -8674,6 +8702,9 @@ do_merge(apr_hash_t **modified_subtrees,
merge_cmd_baton.target_missing_child = FALSE;
merge_cmd_baton.reintegrate_merge = reintegrate_merge;
merge_cmd_baton.target_abspath = target_abspath;
+ SVN_ERR(svn_wc_get_wc_root(&merge_cmd_baton.target_wcroot_abspath,
+ ctx->wc_ctx, merge_cmd_baton.target_abspath,
+ pool, subpool));
merge_cmd_baton.pool = subpool;
merge_cmd_baton.merge_options = merge_options;
merge_cmd_baton.diff3_cmd = diff3_cmd;
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=1088533&r1=1088532&r2=1088533&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Mon Apr 4 09:56:58 2011
@@ -1078,13 +1078,12 @@ def lock_update_only(sbox):
#----------------------------------------------------------------------
-# Currently, it fails at the merge that adds a file:
+# This used to at the merge that adds a file:
# subversion/libsvn_client/repos_diff.c:984: (apr_err=155005)
# subversion/libsvn_client/merge.c:1708: (apr_err=155005)
# subversion/libsvn_wc/update_editor.c:5055: (apr_err=155005)
# 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'
-@XFail()
@Issue(3469)
def at_directory_external(sbox):
"tree conflict at directory external"
Re: svn commit: r1088533 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/tree_conflict_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Apr 04, 2011 at 10:37:04AM -0500, Hyrum K Wright wrote:
> On Mon, Apr 4, 2011 at 4:56 AM, <da...@apache.org> wrote:
> > Author: danielsh
> > Date: Mon Apr 4 09:56:58 2011
> > New Revision: 1088533
> >
> > URL: http://svn.apache.org/viewvc?rev=1088533&view=rev
> > Log:
> > Resolve issue #3469 (tree conflict under a directory external).
>
> Does this mean the issue can be closed?
Depends. There is still a can of worms here.
In 1.6.x, we flag a tree conflict on the external, and this triggered
a bug leading to the assertion failure described in the issue.
The reproduction script for this assertion failure showed a different
bug in 1.7.x. This bug ended up being tracked by the same issue.
In 1.7.x, we now skip files a merge wants to add to an external.
I think this is OK since it doesn't make much sense for a merge to touch
multiple working copies. We don't record mergeinfo on externals either
if the merge was started in the parent working copy.
However, if we cherry-pick a file modification to a non-existent
file within the external, we still get a tree conflict flagged
within the external (try the issue-3469.sh script from the issue
with the small tweak below).
Merges should either always skip externals and never flag any conflicts
in them, or should consistently flag conflicts.
I think we should open a new issue with milestone 1.7-consider (since
this is an edge case) and close #3469 with milestone 1.6.x,
As an aside, the conflict description generated in 1.6.x doesn't make
much sense. It says "local delete, incoming edit upon merge".
But that's another issue. I'm not even sure what the description should say.
--- issue-3469.sh.orig Mon Apr 4 19:26:42 2011
+++ issue-3469.sh Mon Apr 4 19:22:26 2011
@@ -50,6 +50,9 @@
svn add $scratch/test/file.txt
svn ci $scratch -m "add a file"
+echo test >> $scratch/test/file.txt
+svn ci $scratch -m "change file"
+
rev=`svnlook youngest $repos`
svn merge -c $rev $scratch_url $ext
Re: svn commit: r1088533 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/tree_conflict_tests.py
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Mon, Apr 4, 2011 at 4:56 AM, <da...@apache.org> wrote:
> Author: danielsh
> Date: Mon Apr 4 09:56:58 2011
> New Revision: 1088533
>
> URL: http://svn.apache.org/viewvc?rev=1088533&view=rev
> Log:
> Resolve issue #3469 (tree conflict under a directory external).
Does this mean the issue can be closed?
-Hyrum
Re: svn commit: r1088533 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/tree_conflict_tests.py
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Mon, Apr 4, 2011 at 4:56 AM, <da...@apache.org> wrote:
> Author: danielsh
> Date: Mon Apr 4 09:56:58 2011
> New Revision: 1088533
>
> URL: http://svn.apache.org/viewvc?rev=1088533&view=rev
> Log:
> Resolve issue #3469 (tree conflict under a directory external).
Does this mean the issue can be closed?
-Hyrum