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;

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));

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;

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 <[hidden email]> wrote:
>
> On Tue, Oct 30, 2018 at 06:39:49PM +0000, 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(
> &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

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.

Re: [PATCH] Suppress conflict resolver in dry-run merge

On Thu, Nov 01, 2018 at 10:32:56AM +0000, Jonathan Guy wrote:

> 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.

Thanks for digging into this further.

I think you are raising a good point. The fact that it works as expected
right now is due to a side-effect. So your proposed change is still worth
some consideration.

While many SVN operations support a dry-run mode, at present the conflict
resolver does not. It might actually be nice to see what the resolver would
do while in dry-run mode. A dry-run merge could show the result of successful
resolution in cases where a recommended resolution option exists, rather
than showing a conflict. For instance, a merge to the new location of a
moved item could be shown, instead of showing a conflict at the old location.
This would require some work in the conflict resolver which would mostly
be trivial; it's just that there's a lot of code in the resolver so
completing even trivial changes in a consistent manner takes time.

Until the resolver grows such a dry-run mode I think your patch makes sense.
Do we agree here?

Re: [PATCH] Suppress conflict resolver in dry-run merge

On 01.11.2018 17:25, Stefan Sperling wrote:

> On Thu, Nov 01, 2018 at 10:32:56AM +0000, Jonathan Guy wrote:
>> 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.
> Thanks for digging into this further.
>
> I think you are raising a good point. The fact that it works as expected
> right now is due to a side-effect. So your proposed change is still worth
> some consideration.
>
> While many SVN operations support a dry-run mode, at present the conflict
> resolver does not. It might actually be nice to see what the resolver would
> do while in dry-run mode. A dry-run merge could show the result of successful
> resolution in cases where a recommended resolution option exists, rather
> than showing a conflict. For instance, a merge to the new location of a
> moved item could be shown, instead of showing a conflict at the old location.

Wouldn't that only work if the nature of the conflict was recorded
somewhere in the working copy? And wouldn't recording it be exactly
opposite of what 'svn merge --dry-run' really means?

On the other hand, we've had requests for 'svn merge --dry-run' to show
potential text conflicts and even launch an external diff or merge tool
to display them. So there's clearly room for these kinds of features.
Maybe a '--moist-run' option is what we need. :)

Re: [PATCH] Suppress conflict resolver in dry-run merge

> On 01.11.2018 17:25, Stefan Sperling wrote:
> > While many SVN operations support a dry-run mode, at present the conflict
> > resolver does not. It might actually be nice to see what the resolver would
> > do while in dry-run mode. A dry-run merge could show the result of successful
> > resolution in cases where a recommended resolution option exists, rather
> > than showing a conflict. For instance, a merge to the new location of a
> > moved item could be shown, instead of showing a conflict at the old location.
>
> Wouldn't that only work if the nature of the conflict was recorded
> somewhere in the working copy? And wouldn't recording it be exactly
> opposite of what 'svn merge --dry-run' really means?

Indeed, the resolver only reads such information from on-disk working
copy state. So this idea would probably not be worth the effort.