You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2016/10/12 12:34:19 UTC
svn commit: r1764447 - in /subversion/trunk/subversion:
libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
Author: stsp
Date: Wed Oct 12 12:34:18 2016
New Revision: 1764447
URL: http://svn.apache.org/viewvc?rev=1764447&view=rev
Log:
Don't flag an unnecessary tree conflict while resolving 'add vs add'.
* subversion/libsvn_client/conflicts.c
(merge_added_files): New helper function.
(diff_file_added): If a file already exists then just merge with the
incoming file and return.
* subversion/tests/libsvn_client/conflicts-test.c
(test_merge_incoming_added_dir_merge, test_merge_incoming_added_dir_merge2,
test_merge_incoming_added_dir_merge3): Adjust test expectations accordingly.
Modified:
subversion/trunk/subversion/libsvn_client/conflicts.c
subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1764447&r1=1764446&r2=1764447&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Oct 12 12:34:18 2016
@@ -5640,6 +5640,66 @@ diff_dir_added(const char *relpath,
return SVN_NO_ERROR;
}
+static svn_error_t *
+merge_added_files(const char *local_abspath,
+ const char *incoming_added_file_abspath,
+ apr_hash_t *incoming_added_file_props,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc_merge_outcome_t merge_content_outcome;
+ svn_wc_notify_state_t merge_props_outcome;
+ apr_file_t *empty_file;
+ const char *empty_file_abspath;
+ apr_array_header_t *propdiffs;
+ apr_hash_t *working_props;
+
+ /* Create an empty file as fake "merge-base" for the two added files.
+ * The files are not ancestrally related so this is the best we can do. */
+ SVN_ERR(svn_io_open_unique_file3(&empty_file, &empty_file_abspath, NULL,
+ svn_io_file_del_on_pool_cleanup,
+ scratch_pool, scratch_pool));
+
+ /* Get a copy of the working file's properties. */
+ SVN_ERR(svn_wc_prop_list2(&working_props, ctx->wc_ctx, local_abspath,
+ scratch_pool, scratch_pool));
+
+ /* Create a property diff for the files. */
+ SVN_ERR(svn_prop_diffs(&propdiffs, incoming_added_file_props,
+ working_props, scratch_pool));
+
+ /* Perform the file merge. */
+ SVN_ERR(svn_wc_merge5(&merge_content_outcome, &merge_props_outcome,
+ ctx->wc_ctx, empty_file_abspath,
+ incoming_added_file_abspath, local_abspath,
+ NULL, NULL, NULL, /* labels */
+ NULL, NULL, /* conflict versions */
+ FALSE, /* dry run */
+ NULL, NULL, /* diff3_cmd, merge_options */
+ NULL, propdiffs,
+ NULL, NULL, /* conflict func/baton */
+ NULL, NULL, /* don't allow user to cancel here */
+ scratch_pool));
+
+ if (ctx->notify_func2)
+ {
+ svn_wc_notify_t *notify = svn_wc_create_notify(
+ local_abspath,
+ svn_wc_notify_update_update,
+ scratch_pool);
+
+ if (merge_content_outcome == svn_wc_merge_conflict)
+ notify->content_state = svn_wc_notify_state_conflicted;
+ else
+ notify->content_state = svn_wc_notify_state_merged;
+ notify->prop_state = merge_props_outcome;
+ notify->kind = svn_node_file;
+ ctx->notify_func2(ctx->notify_baton2, notify, scratch_pool);
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* An svn_diff_tree_processor_t callback. */
static svn_error_t *
diff_file_added(const char *relpath,
@@ -5666,6 +5726,18 @@ diff_file_added(const char *relpath,
FALSE, FALSE, scratch_pool));
SVN_ERR(svn_io_check_path(local_abspath, &on_disk_kind, scratch_pool));
+ if (db_kind == svn_node_file && db_kind == svn_node_file)
+ {
+ propsarray = svn_prop_hash_to_array(right_props, scratch_pool);
+ SVN_ERR(svn_categorize_props(propsarray, NULL, NULL, ®ular_props,
+ scratch_pool));
+ SVN_ERR(merge_added_files(local_abspath, right_file,
+ svn_prop_array_to_hash(regular_props,
+ scratch_pool),
+ b->ctx, scratch_pool));
+ return SVN_NO_ERROR;
+ }
+
if (db_kind != svn_node_none && db_kind != svn_node_unknown)
{
SVN_ERR(raise_tree_conflict(
Modified: subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c?rev=1764447&r1=1764446&r2=1764447&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c Wed Oct 12 12:34:18 2016
@@ -792,31 +792,10 @@ test_merge_incoming_added_dir_merge(cons
props_conflicted->nelts == 0 &&
!tree_conflicted);
-
- /* There should now be an 'add vs add' conflict on the new file. */
new_file_path = svn_relpath_join(branch_path,
svn_relpath_join(new_dir_name,
new_file_name, b->pool),
b->pool);
- SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, new_file_path),
- ctx, b->pool, b->pool));
- SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
- &props_conflicted,
- &tree_conflicted,
- conflict, b->pool, b->pool));
- SVN_TEST_ASSERT(!text_conflicted &&
- props_conflicted->nelts == 0 &&
- tree_conflicted);
-
- /* Resolve the tree conflict. */
- SVN_ERR(svn_test__create_client_ctx(&ctx, b, b->pool));
- SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, new_file_path),
- ctx, b->pool, b->pool));
- SVN_ERR(svn_client_conflict_tree_get_details(conflict, ctx, b->pool));
- SVN_ERR(svn_client_conflict_tree_resolve_by_id(
- conflict,
- svn_client_conflict_option_incoming_added_file_text_merge, ctx,
- b->pool));
/* Ensure that the file has the expected status. */
SVN_ERR(svn_client_status6(NULL, ctx, sbox_wc_path(b, new_file_path),
@@ -923,30 +902,10 @@ test_merge_incoming_added_dir_merge2(con
props_conflicted->nelts == 0 &&
!tree_conflicted);
- /* There should now be an 'add vs add' conflict on the new file. */
new_file_path = svn_relpath_join(branch_path,
svn_relpath_join(new_dir_name,
new_file_name, b->pool),
b->pool);
- SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, new_file_path),
- ctx, b->pool, b->pool));
- SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
- &props_conflicted,
- &tree_conflicted,
- conflict, b->pool, b->pool));
- SVN_TEST_ASSERT(!text_conflicted &&
- props_conflicted->nelts == 0 &&
- tree_conflicted);
-
- /* Resolve the tree conflict. */
- SVN_ERR(svn_test__create_client_ctx(&ctx, b, b->pool));
- SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, new_file_path),
- ctx, b->pool, b->pool));
- SVN_ERR(svn_client_conflict_tree_get_details(conflict, ctx, b->pool));
- SVN_ERR(svn_client_conflict_tree_resolve_by_id(
- conflict,
- svn_client_conflict_option_incoming_added_file_text_merge, ctx,
- b->pool));
/* Ensure that the file has the expected status. */
SVN_ERR(svn_client_status6(NULL, ctx, sbox_wc_path(b, new_file_path),
@@ -1063,25 +1022,6 @@ test_merge_incoming_added_dir_merge3(con
svn_relpath_join(new_dir_name,
new_file_name, b->pool),
b->pool);
- SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, new_file_path),
- ctx, b->pool, b->pool));
- SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
- &props_conflicted,
- &tree_conflicted,
- conflict, b->pool, b->pool));
- SVN_TEST_ASSERT(!text_conflicted &&
- props_conflicted->nelts == 0 &&
- tree_conflicted);
-
- /* Resolve the tree conflict. */
- SVN_ERR(svn_test__create_client_ctx(&ctx, b, b->pool));
- SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, new_file_path),
- ctx, b->pool, b->pool));
- SVN_ERR(svn_client_conflict_tree_get_details(conflict, ctx, b->pool));
- SVN_ERR(svn_client_conflict_tree_resolve_by_id(
- conflict,
- svn_client_conflict_option_incoming_added_file_text_merge, ctx,
- b->pool));
/* Ensure that the file has the expected status. */
SVN_ERR(svn_client_status6(NULL, ctx, sbox_wc_path(b, new_file_path),
Re: svn commit: r1764447 - in /subversion/trunk/subversion:
libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
Posted by Stefan Sperling <st...@apache.org>.
On Mon, Oct 24, 2016 at 11:18:51AM +0200, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: Stefan Sperling [mailto:stsp@apache.org]
> > Sent: maandag 24 oktober 2016 11:08
> > To: dev@subversion.apache.org
> > Subject: Re: svn commit: r1764447 - in /subversion/trunk/subversion:
> > libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
> >
> > On Sun, Oct 23, 2016 at 03:31:22PM -0400, James McCoy wrote:
> > > On Wed, Oct 12, 2016 at 12:34:19PM -0000, stsp@apache.org wrote:
> > > > Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
> > > > URL:
> >
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/confl
> ic
> > ts.c?rev=1764447&r1=1764446&r2=1764447&view=diff
> > > >
> > ================================================================
> > ==============
> > > > --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
> > > > +++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Oct 12
> > 12:34:18 2016
> > > > @@ -5666,6 +5726,18 @@ diff_file_added(const char *relpath,
> > > > FALSE, FALSE, scratch_pool));
> > > > SVN_ERR(svn_io_check_path(local_abspath, &on_disk_kind,
> > scratch_pool));
> > > >
> > > > + if (db_kind == svn_node_file && db_kind == svn_node_file)
> > >
> > > Coverity noticed both sides of the && are the same. Should one side be
> > > "on_disk_kind == svn_node_file" instead?
> > >
> >
> > Yes indeed, it should. Thanks! I'll fix this when I find time
> > if it's not already been fixed by then.
>
> Be careful when committing this as an 'obvious fix'. I've seen a very
> similar problem in the conflict code in the past that has very explicit
> tests in our testsuite that verify the current behavior. It is very well
> possible that this is just the same code moved to a different location.
>
> If this is that specific case the problem should be fixed, but it won't be
> an obvious fix... more a case of careful review of all the cases that
> trigger this code and then updating expected results.
>
> Bert
Yeah, that's why I didn't commit it myself on the spot.
It needs at least one run through the test suite.
RE: svn commit: r1764447 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@apache.org]
> Sent: maandag 24 oktober 2016 11:08
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1764447 - in /subversion/trunk/subversion:
> libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
>
> On Sun, Oct 23, 2016 at 03:31:22PM -0400, James McCoy wrote:
> > On Wed, Oct 12, 2016 at 12:34:19PM -0000, stsp@apache.org wrote:
> > > Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
> > > URL:
>
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/confl
ic
> ts.c?rev=1764447&r1=1764446&r2=1764447&view=diff
> > >
> ================================================================
> ==============
> > > --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
> > > +++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Oct 12
> 12:34:18 2016
> > > @@ -5666,6 +5726,18 @@ diff_file_added(const char *relpath,
> > > FALSE, FALSE, scratch_pool));
> > > SVN_ERR(svn_io_check_path(local_abspath, &on_disk_kind,
> scratch_pool));
> > >
> > > + if (db_kind == svn_node_file && db_kind == svn_node_file)
> >
> > Coverity noticed both sides of the && are the same. Should one side be
> > "on_disk_kind == svn_node_file" instead?
> >
>
> Yes indeed, it should. Thanks! I'll fix this when I find time
> if it's not already been fixed by then.
Be careful when committing this as an 'obvious fix'. I've seen a very
similar problem in the conflict code in the past that has very explicit
tests in our testsuite that verify the current behavior. It is very well
possible that this is just the same code moved to a different location.
If this is that specific case the problem should be fixed, but it won't be
an obvious fix... more a case of careful review of all the cases that
trigger this code and then updating expected results.
Bert
Re: svn commit: r1764447 - in /subversion/trunk/subversion:
libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
Posted by Stefan Sperling <st...@apache.org>.
On Sun, Oct 23, 2016 at 03:31:22PM -0400, James McCoy wrote:
> On Wed, Oct 12, 2016 at 12:34:19PM -0000, stsp@apache.org wrote:
> > Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1764447&r1=1764446&r2=1764447&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
> > +++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Oct 12 12:34:18 2016
> > @@ -5666,6 +5726,18 @@ diff_file_added(const char *relpath,
> > FALSE, FALSE, scratch_pool));
> > SVN_ERR(svn_io_check_path(local_abspath, &on_disk_kind, scratch_pool));
> >
> > + if (db_kind == svn_node_file && db_kind == svn_node_file)
>
> Coverity noticed both sides of the && are the same. Should one side be
> "on_disk_kind == svn_node_file" instead?
>
Yes indeed, it should. Thanks! I'll fix this when I find time
if it's not already been fixed by then.
> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7 2D23 DFE6 91AE 331B A3DB
Re: svn commit: r1764447 - in /subversion/trunk/subversion:
libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
Posted by James McCoy <ja...@jamessan.com>.
On Wed, Oct 12, 2016 at 12:34:19PM -0000, stsp@apache.org wrote:
> Author: stsp
> Date: Wed Oct 12 12:34:18 2016
> New Revision: 1764447
>
> URL: http://svn.apache.org/viewvc?rev=1764447&view=rev
> Log:
> Don't flag an unnecessary tree conflict while resolving 'add vs add'.
>
> * subversion/libsvn_client/conflicts.c
> (merge_added_files): New helper function.
> (diff_file_added): If a file already exists then just merge with the
> incoming file and return.
>
> * subversion/tests/libsvn_client/conflicts-test.c
> (test_merge_incoming_added_dir_merge, test_merge_incoming_added_dir_merge2,
> test_merge_incoming_added_dir_merge3): Adjust test expectations accordingly.
>
>
>
> Modified:
> subversion/trunk/subversion/libsvn_client/conflicts.c
> subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
>
> Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1764447&r1=1764446&r2=1764447&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
> +++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Oct 12 12:34:18 2016
> @@ -5666,6 +5726,18 @@ diff_file_added(const char *relpath,
> FALSE, FALSE, scratch_pool));
> SVN_ERR(svn_io_check_path(local_abspath, &on_disk_kind, scratch_pool));
>
> + if (db_kind == svn_node_file && db_kind == svn_node_file)
Coverity noticed both sides of the && are the same. Should one side be
"on_disk_kind == svn_node_file" instead?
Cheers,
--
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7 2D23 DFE6 91AE 331B A3DB