Possible bug in conflict resolver
Hi all Not sure if this is a bug or expected behaviour. Using Subversion 1.11.1 Two people with separate checkouts working on the same file. Person A edits a line and the renames the file and commits. Person B edits the same line in the same file and then does an update. The conflict resolver first flags the tree conflict which is resolved with a move and merge. The subsequent merge then produces the second conflict. Here’s were the behaviour is not what I expected. If I select “tf” so accepting all incoming changes I end up with a file that is resolved but is showing as modified. I check the contents and it contains my changes. Conversely if I resolve using “mf” so rejecting incoming changes I end up with a file that is resolved but is showing as normal and is identical to the head version and I have lost all my changes. I’m pretty sure this not the intended behaviour but I could be wrong. Regards Jonathan
New lines in notify.c
Hi all Just out of curiosity in notify.c we have the two lines SVN_ERR(svn_cmdline_printf(pool, "\rChecking r ")); SVN_ERR(svn_cmdline_printf(pool, "\rChecking r%ld...", n->revision)); Is there a specific reason \r is used and not \n? Thanks Jonathan Guy
Re: [PATCH] Suppress conflict resolver in dry-run merge
Ok I looked into this a bit more and I see what's going on now.
The post merge conflict resolver runs because the merge operation reported
conflicts (via the conflict stats).
This calls
svn_client_conflict_walk
which calls
svn_wc_walk_status
which reports no conflict at that path because the wc was never changed because
it was a dry run. So the whole operation gets dropped here.
I’m still convinced the whole thing is a pointless exercise.
A dry-run merge will not produce any actual conflicts on-disk so the resolver
will never do anything.
I guess there could be other reasons I don’t know of to run the resolver so
we’ll just leave it as is.
Thanks again for your time.
Regards
Jonathan
> On 31 Oct 2018, at 12:35, Stefan Sperling wrote:
>
> On Wed, Oct 31, 2018 at 09:46:23AM +, Jonathan Guy wrote:
>> Hi Stephan
>> Thanks for the reply.
>> I did see code section you refer to however the
>> svn_client__resolve_conflicts will only be called if you have
>> ctx->conflict_func2.
>> That function (AFAIK) is only installed if an “accept” option is supplied as
>> the code below
>>
>>
>> /* Install a legacy conflict handler if the --accept option was given.
>> * Else, svn_client_merge5() may abort the merge in an undesirable way.
>> * See the docstring at conflict_func_merge_cmd() for details */
>> if (opt_state->accept_which != svn_cl__accept_unspecified)
>>{
>> struct conflict_func_merge_cmd_baton *b = apr_pcalloc(pool, sizeof(*b));
>>
>> b->accept_which = opt_state->accept_which;
>> SVN_ERR(svn_dirent_get_absolute(>path_prefix, "", pool));
>> b->conflict_stats = conflict_stats;
>>
>> ctx->conflict_func2 = conflict_func_merge_cmd;
>> ctx->conflict_baton2 = b;
>>}
>>
>>
>> The resolver that runs post run_merge is however run regardless if there is
>> an accept argument or not.
>> If no accept option is given the user is ultimately prompted to resolve the
>> conflict (dry-run or no dry-run).
>>
>>
>> /* If we are in interactive mode and either the user gave no --accept
>> * option or the option did not apply, then prompt. */
>> if (option_id == svn_client_conflict_option_unspecified)
>>{
>> svn_boolean_t resolved = FALSE;
>> svn_boolean_t postponed = FALSE;
>> svn_boolean_t printed_description = FALSE;
>> svn_error_t *err;
>>
>> *quit = FALSE;
>>
>> while (!resolved && !postponed && !*quit)
>>{
>> err = resolve_conflict_interactively(, ,
>>
>>
>> Sorry I’m no trying to be argumentative I just need to clear on this one
>> aspect.
>> Thanks for any help.
>
> Can you provide an example? I cannot reproduce the problem like this:
>
> $ svn merge ^/trunk
> --- Merging r2 through r5 into '.':
> C alpha
> Aalpha2
> --- Recording mergeinfo for merge of r2 through r5 into '.':
> U .
> Summary of conflicts:
> Tree conflicts: 1
> Searching tree conflict details for 'alpha' in repository:
> Checking r4... done
> Tree conflict on 'alpha':
> File merged from
> '^/trunk/alpha@1'
> to
> '^/trunk/alpha@5'
> was moved to '^/trunk/alpha2' by stsp in r4.
> A file which differs from the corresponding file on the merge source branch
> was found in the working copy.
> Applying recommended resolution 'Move and merge':
> Calpha2
> Tree conflict at 'alpha' marked as resolved.
> Merge conflict discovered in file 'alpha2'.
> Select: (p) Postpone, (df) Show diff, (e) Edit file, (m) Merge,
>(s) Show all options: q
> Summary of conflicts:
> Text conflicts: 1 remaining (and 0 already resolved)
> Tree conflicts: 0 remaining (and 1 already resolved)
> $ sv revert
> $ pwd
> /tmp/svn-sandbox/branch
> $ svn revert -R .
> Reverted '.'
> Reverted 'alpha'
> Reverted 'alpha2'
> $ svn merge ^/trunk --dry-run
> --- Merging r2 through r5 into '.':
> C alpha
> Aalpha2
> Summary of conflicts:
> Tree conflicts: 1
> $
>
>
Re: [PATCH] Suppress conflict resolver in dry-run merge
Hi Stephan
Thanks for the reply.
I did see code section you refer to however the svn_client__resolve_conflicts
will only be called if you have ctx->conflict_func2.
That function (AFAIK) is only installed if an “accept” option is supplied as
the code below
/* Install a legacy conflict handler if the --accept option was given.
* Else, svn_client_merge5() may abort the merge in an undesirable way.
* See the docstring at conflict_func_merge_cmd() for details */
if (opt_state->accept_which != svn_cl__accept_unspecified)
{
struct conflict_func_merge_cmd_baton *b = apr_pcalloc(pool, sizeof(*b));
b->accept_which = opt_state->accept_which;
SVN_ERR(svn_dirent_get_absolute(>path_prefix, "", pool));
b->conflict_stats = conflict_stats;
ctx->conflict_func2 = conflict_func_merge_cmd;
ctx->conflict_baton2 = b;
}
The resolver that runs post run_merge is however run regardless if there is an
accept argument or not.
If no accept option is given the user is ultimately prompted to resolve the
conflict (dry-run or no dry-run).
/* If we are in interactive mode and either the user gave no --accept
* option or the option did not apply, then prompt. */
if (option_id == svn_client_conflict_option_unspecified)
{
svn_boolean_t resolved = FALSE;
svn_boolean_t postponed = FALSE;
svn_boolean_t printed_description = FALSE;
svn_error_t *err;
*quit = FALSE;
while (!resolved && !postponed && !*quit)
{
err = resolve_conflict_interactively(, ,
Sorry I’m no trying to be argumentative I just need to clear on this one aspect.
Thanks for any help.
Regards
Jonathan
> On 31 Oct 2018, at 08:32, Stefan Sperling wrote:
>
> On Tue, Oct 30, 2018 at 06:39:49PM +, Jonathan Guy wrote:
>> [[[
>> *subversion/svn/merge-cmd.c
>> (svn_cl__merge): Suppress the interactive conflict resolver
>> if a merge has been performed with the dry-run option.
>> ]]]
>
> Hi Jonathan,
>
> Thank your for the patch. I am afraid I misled you earlier by suggesting
> that you send a patch for the issue. This is in fact not a problem in
> the current implementation because the dry_run flag gets passed down
> into libsvn_client, where it takes part in the decision about running
> the conflict resolver.
>
> Specifically, this part of the do_merge() function in the file
> subversion/libsvn_client/merge.c checks the flag:
>
> /* Give the conflict resolver callback the opportunity to
> * resolve any conflicts that were raised. If it resolves all
> * of them, go around again to merge the next sub-range (if any). */
> if (conflicted_range_report && ctx->conflict_func2 && ! dry_run)
>{
> svn_boolean_t conflicts_remain;
>
> SVN_ERR(svn_client__resolve_conflicts(
>_remain, merge_cmd_baton.conflicted_paths,
>ctx, iterpool));
> if (conflicts_remain)
>break;
>
> So your patch is redundant and we don't need to apply it.
> I should have tested the current behaviour before recommending that
> you send a patch.
>
> Regardless, thank you for your contribution!
>
> Stefan
[PATCH] Suppress conflict resolver in dry-run merge
[[[ *subversion/svn/merge-cmd.c (svn_cl__merge): Suppress the interactive conflict resolver if a merge has been performed with the dry-run option. ]]] merge-cmd.c.patch Description: Binary data
