Re: r1477876 - setting the node kind in a prop conflict description

[Julian Foad <ju...@btopenworld.com>]

r1477876, which I proposed for back-port to 1.8.x, fills in the 'node kind' field correctly for a prop conflict.  The code in 'svn' shown below now leads to a crash because the 'file/edit/edit' case matches a prop conflict and calls the text conflict handler function, which asserts that the conflict kind is 'text'.

Stefan Sperling, can you confirm the intention there was to match only a text conflict?  (I think this is your code.)  This looks like a simple enough fix (shown below), except I'm dealing with a knock-on effect already so I thought 
it's time I posted here to get a second opinion.  I'm a bit uncertain about the 'obstruction' handling below it.  How do the obstruction cases relate to the conflict kinds 'text', 'prop', and 'tree'?

If there are any obstruction cases that are reported as conflict kind 'text', then this patch would probably break them.  I don't know if there are.

[[[
   /* Print a summary of conflicts before starting interactive resolution */
   if (! b->printed_summary)
     {
       SVN_ERR(svn_cl__print_conflict_stats(b->conflict_stats, scratch_pool));
       b->printed_summary = TRUE;
     }
 
   /* We're in interactive mode and either the user gave no --accept
      option or the option did not apply; let's prompt. */
 
   /* Handle the most common cases, which is either:
 
      Conflicting edits on a file's text, or
      Conflicting edits on a property.
   */
-  if (((desc->node_kind == svn_node_file)
+  if (((desc->kind == svn_wc_conflict_kind_text)
        && (desc->action == svn_wc_conflict_action_edit)
        && (desc->reason == svn_wc_conflict_reason_edited)))
     SVN_ERR(handle_text_conflict(*result, desc, b, scratch_pool));
   else if (desc->kind == svn_wc_conflict_kind_property)
     SVN_ERR(handle_prop_conflict(*result, desc, b, scratch_pool));
 
   /*
     Dealing with obstruction of additions can be tricky.  The
     obstructing item could be unversioned, versioned, or even
     schedule-add.  Here's a matrix of how the caller should behave,
     based on results we return.
 
                          Unversioned       Versioned       Schedule-Add
 
       choose_mine       skip addition,    skip addition     skip addition
                         add existing item
 
       choose_theirs     destroy file,    schedule-delete,   revert add,
                         add new item.    add new item.      rm file,
                                                             add new item
 
       postpone               [              bail out                 ]
 
    */
   else if ((desc->action == svn_wc_conflict_action_add)
            && (desc->reason == svn_wc_conflict_reason_obstructed))
     SVN_ERR(handle_obstructed_add(*result, desc, b, scratch_pool));
 
   else if (desc->kind == svn_wc_conflict_kind_tree)
     SVN_ERR(handle_tree_conflict(*result, desc, b, scratch_pool));

]]]

- Julian

 
--
Join WANdisco's free daily demo sessions on Scaling Subversion for the Enterprise
<http://www.wandisco.com/training/webinars>

Re: r1477876 - setting the node kind in a prop conflict description

[Stefan Sperling <st...@elego.de>]

On Wed, May 01, 2013 at 08:32:55PM +0100, Julian Foad wrote:
> r1477876, which I proposed for back-port to 1.8.x, fills in the 'node kind' field correctly for a prop conflict.  The code in 'svn' shown below now leads to a crash because the 'file/edit/edit' case matches a prop conflict and calls the text conflict handler function, which asserts that the conflict kind is 'text'.
> 
> Stefan Sperling, can you confirm the intention there was to match only a text conflict?  (I think this is your code.)

I don't think I wrote this code originally but I certainly touched
it during the 1.8-dev timeframe.

Your patch looks very sensible to me.

>   This looks like a simple enough fix (shown below), except I'm dealing with a knock-on effect already so I thought 
> it's time I posted here to get a second opinion.  I'm a bit uncertain about the 'obstruction' handling below it.  How do the obstruction cases relate to the conflict kinds 'text', 'prop', and 'tree'?

The obstruction handling dates back to 1.5, I believe.
It was added as part of the original conflict resolution callback
implementation.

> If there are any obstruction cases that are reported as conflict kind 'text', then this patch would probably break them.  I don't know if there are.
> 

That wouldn't make any sense anyway. They should be tree-conflicts.