On Sat, Jun 11, 2011 at 03:05:59PM -0500, Brian Neal wrote:
> Hello -
>
> Attached is a patch for issue #3919 [1]. Please review when you have a
> chance. Thanks!
>
> Possible commit message:
>
> [[[
> Fix issue #3919. During a merge of a property, add a check against the
> incoming new property value and the working copy value. If they
> already match, then the merge trivially succeeds.
>
> * subversion/libsvn_wc/props.c
> (apply_single_generic_prop_change): Do nothing if the incoming new
> property value already matches the working value.
>
> ]]]
>
>
> [1]: http://subversion.tigris.org/issues/show_bug.cgi?id=3919
>
>
> Best regards,
> BN
Hi Brian,
thanks for this patch.
I have one suggestion:
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c (revision 1134717)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -1280,8 +1280,9 @@ apply_single_mergeinfo_prop_change(svn_wc_notify_s
> }
>
> /* Merge a change to a property, using the rule that if the working value
> - is the same as OLD_VAL then apply the change as a simple update
> - (replacement), otherwise invoke maybe_generate_propconflict().
> + is equal to the new value then there is nothing we need to do. Else, if
> + the working value is the same as the old value then apply the change as
> + a simple update (replacement), otherwise invoke maybe_generate_propconflict().
> The definition of the arguments and behaviour is the same as
> apply_single_prop_change(). */
> static svn_error_t *
> @@ -1308,8 +1309,14 @@ apply_single_generic_prop_change(svn_wc_notify_sta
>
> SVN_ERR_ASSERT(old_val != NULL);
>
> + /* If working_val is the same as new_val already then there is nothing to do */
> + if (working_val && new_val
> + && svn_string_compare(working_val, new_val))
> + {
> + ; /* do nothing */
Instead of doing nothing, I think this should set the *state to 'merged'
using
set_prop_merge_state(state, svn_wc_notify_state_merged);
Just like apply_single_prop_add() already does.
Do you agree? If so I will apply this patch with that modification.
Also, did you already run the regression tests with your patch ("make
check")? My suggestion might affect the output of 'svn' so tests would
need to be run again (but I will run them either way before committing).
> + }
> /* If working_val is the same as old_val... */
> - if (working_val && old_val
> + else if (working_val && old_val
> && svn_string_compare(working_val, old_val))
> {
> /* A trivial update: change it to new_val. */
Received on 2011-06-15 13:35:44 CEST