[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r937524 - in /subversion/trunk/subversion: libsvn_wc/props.c tests/cmdline/prop_tests.py tests/cmdline/svntest/sandbox.py

From: Paul Burba <ptburba_at_gmail.com>
Date: Fri, 14 May 2010 13:46:00 -0400

On Fri, Apr 23, 2010 at 5:22 PM, <gstein_at_apache.org> wrote:
> Author: gstein
> Date: Fri Apr 23 21:22:52 2010
> New Revision: 937524
>
> URL: http://svn.apache.org/viewvc?rev=937524&view=rev
> Log:
> Begin new infrastructure for generating prop conflict messages. This will
> allow us to (re)generate a property reject file at will, given a record of
> the property conflicts on a given node.
>
> There are two issues for discussion and fixing in a future revision:
> - incoming-delete will remove local-add (it should conflict?)

Hi Greg,

I think the correct behavior is: An incoming-delete removes a local
add only if the incoming base value is the *same* as the added value;
otherwise there is a conflict. This is analogous to how we treat an
incoming file deletion on a local file addition. It's only a tree
conflict if the files differ.

More below...

> - incoming-delete will crash on a local-delete
>
> * subversion/libsvn_wc/props.c:
>  (generate_conflict_message): new function to generate a property
>    conflict message given the four property values involved in a 4-way
>    merge.
>  (apply_single_prop_delete): leave two notes about behavior in here (see
>    the issues above). fix message generation: use OLD_VAL, not BASE_VAL
>  (apply_single_generic_prop_change): the OLD_VAL parameter will always be
>    not-NULL, so we can simplify an if condition.
>  (svn_wc__merge_props): save away MINE_VAL, and then if we see a conflict
>    message returned by the property merging functions, then assert that
>    our new function comes up with the same message
>
> * subversion/tests/cmdline/prop_tests.py:
>  (prop_reject_grind): new test function to grind thru all the variations
>    of property conflicts.
>  (test_list): add new test
>
> * subversion/tests/cmdline/svntest/sandbox.py:
>  (Sandbox.simple_propset, Sandbox.simple_propdel): new methods
>
> Modified:
>    subversion/trunk/subversion/libsvn_wc/props.c
>    subversion/trunk/subversion/tests/cmdline/prop_tests.py
>    subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py
>
> Modified: subversion/trunk/subversion/libsvn_wc/props.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=937524&r1=937523&r2=937524&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/props.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/props.c Fri Apr 23 21:22:52 2010
> @@ -709,6 +709,136 @@ svn_wc_merge_props3(svn_wc_notify_state_
>  }
>
>
> +/* Generate a message to describe the property conflict among these four
> +   values.
> +
> +   Note that this function (currently) interprets the property values as
> +   strings, but they could actually be binary values. We'll keep the
> +   types as svn_string_t in case we fix this in the future.  */
> +static const svn_string_t *
> +generate_conflict_message(const char *propname,
> +                          const svn_string_t *original,
> +                          const svn_string_t *mine,
> +                          const svn_string_t *incoming,
> +                          const svn_string_t *incoming_base,
> +                          apr_pool_t *result_pool)
> +{
> +  if (incoming_base == NULL)
> +    {
> +      /* Attempting to add the value INCOMING.  */
> +      SVN_ERR_ASSERT_NO_RETURN(incoming != NULL);
> +
> +      if (mine)
> +        {
> +          /* To have a conflict, these must be different.  */
> +          SVN_ERR_ASSERT_NO_RETURN(!svn_string_compare(mine, incoming));
> +
> +          /* Note that we don't care whether MINE is locally-added or
> +             edited, or just something different that is a copy of the
> +             pristine ORIGINAL.  */
> +          return svn_string_createf(result_pool,
> +                                    _("Trying to add new property '%s' with "
> +                                      "value '%s',\nbut property already "
> +                                      "exists with value '%s'."),
> +                                    propname, incoming->data, mine->data);
> +        }
> +
> +      /* To have a conflict, we must have an ORIGINAL which has been
> +         locally-deleted.  */
> +      SVN_ERR_ASSERT_NO_RETURN(original != NULL);
> +      return svn_string_createf(result_pool,
> +                                _("Trying to create property '%s' with "
> +                                  "value '%s',\nbut it has been locally "
> +                                  "deleted."),
> +                                propname, incoming->data);
> +    }
> +
> +  if (incoming == NULL)
> +    {
> +      /* Attempting to delete the value INCOMING_BASE.  */
> +      SVN_ERR_ASSERT_NO_RETURN(incoming_base != NULL);
> +
> +      /* A conflict can only occur if we originally had the property;
> +         otherwise, we would have merged the property-delete into the
> +         non-existent property.  */
> +      SVN_ERR_ASSERT_NO_RETURN(original != NULL);
> +
> +      if (mine && svn_string_compare(original, incoming_base))
> +        {
> +          /* We were trying to delete the correct property, but an edit
> +             caused the conflict.  */
> +          return svn_string_createf(result_pool,
> +                                    _("Trying to delete property '%s' with "
> +                                      "value '%s'\nbut it has been modified "
> +                                      "from '%s' to '%s'."),
> +                                    propname, incoming_base->data,
> +                                    original->data, mine->data);
> +        }
> +
> +      /* We were trying to delete INCOMING_BASE but our ORIGINAL is
> +         something else entirely.  */
> +      SVN_ERR_ASSERT_NO_RETURN(!svn_string_compare(original, incoming_base));
> +
> +      /* ### wait. what if we had a different property and locally
> +         ### deleted it? the statement below is gonna blow up.
> +         ### we could have: local-add, local-edit, local-del, or just
> +         ### something different (and unchanged).  */

<snip>

> @@ -1166,6 +1296,8 @@ apply_single_prop_delete(svn_wc_notify_s
>
> if (! base_val)
> {
> + /* ### what about working_val? what if we locally-added? */
> +
> apr_hash_set(working_props, propname, APR_HASH_KEY_STRING, NULL);
> if (old_val)
> /* This is a merge, merging a delete into non-existent */
> @@ -1216,11 +1348,13 @@ apply_single_prop_delete(svn_wc_notify_s
> cancel_func, cancel_baton,
> dry_run, scratch_pool));
> if (got_conflict)
> + /* ### wait. what if we had a different property and locally
> + ### deleted it? the statement below is gonna blow up. */

Attached is a patch that fixes the segfault and makes an incoming
deletion on a local addition, where the incoming base value differs
from the added value, a conflict, rather than unconditionally deleting
the addition.

I also tweaked prop_test.py 32 to check the results of the *.prej file.

This patch adds two new potential conflicts messages:

Incoming delete on local add of different value:

   Trying to delete property 'del.add' with value 'repos',
   but property has been locally added with value 'local'.

Incoming delete on local delete of different value:

   Trying to delete property 'del.del' with value 'repos',
   but property with value 'local' is locally deleted.

What do you think?

Paul

[[[
Fix some property merge conflict bugs.

1) Incoming delete on a local add of a different value is now a
   conflict. Previously it was a clean merge and the prop was
   deleted.

2) Incoming delete on a local delete where the incoming base value
   differs from the local value is now a conflict. Previously
   this caused a segfault.

* subversion/libsvn_wc/props.c

  (generate_conflict_message): Handle incoming delete on local add and
   incoming delete on local delete of a different prop value. Consistently
   use a trailing ',' after the first line of each prej conflict message.

  (apply_single_prop_delete): Stop considering an incoming delete on a local
   add as a merge.

* subversion/tests/cmdline/prop_tests.py

  (prop_reject_grind): Start testing incoming delete on local delete of
   different prop value. Verify the resulting *.prej file.
]]]

Received on 2010-05-14 19:46:29 CEST

This is an archived mail posted to the Subversion Dev mailing list.