On Fri, Oct 14, 2016 at 03:35:48PM +0200, Stefan wrote:
> Thanks for taking the time to review the patch, Ivan (and also stsp).
> After talking to stsp, here's a different approach to solve the issue
> without the need for adding a new parameter to the public API.
>
> (all tests pass on Windows and the conflict resolution UI was tested
> manually to ensure no duplicate options are displayed for the conflict
> resolution)
+1, please commit.
Thank you!
> [[[
> Fix svn resolve --accept=working not working on binary conflicts by retrying
> to find a resolution option for the semantically corresponding
> svn_client_conflict_option_working_text option.
>
> Additionally, enable the mf option for binary conflicts, so to have the
> option displayed in the client.
>
> * subversion/libsvn_client/conflicts.c
> (svn_client_conflict_text_get_resolution_options): return the more
> suitable
> svn_client_conflict_option_working_text to resolve binary conflicts.
> (svn_client_conflict_text_resolve_by_id): retry determining resolution
> option with svn_client_conflict_option_working_text.
>
> * subversion/svn/conflict-callbacks.c
> (handle_text_conflict): add "mf" to the list of allowed resolutions for
> binary conflicts.
>
> * subversion/tests/cmdline/resolve_tests.py
> (automatic_binary_conflict_resolution): remove XFail marker
> ]]]
>
> Regards,
> Stefan
>
> Index: subversion/libsvn_client/conflicts.c
> ===================================================================
> --- subversion/libsvn_client/conflicts.c (revision 1764824)
> +++ subversion/libsvn_client/conflicts.c (working copy)
> @@ -7193,7 +7193,7 @@
> resolve_text_conflict);
>
> add_resolution_option(*options, conflict,
> - svn_client_conflict_option_merged_text,
> + svn_client_conflict_option_working_text,
> _("accept binary file as it appears in the working copy"),
> resolve_text_conflict);
> }
> @@ -8537,6 +8537,7 @@
> {
> apr_array_header_t *resolution_options;
> svn_client_conflict_option_t *option;
> + const char *mime_type;
>
> SVN_ERR(svn_client_conflict_text_get_resolution_options(
> &resolution_options, conflict, ctx,
> @@ -8543,13 +8544,29 @@
> scratch_pool, scratch_pool));
> option = svn_client_conflict_option_find_by_id(resolution_options,
> option_id);
> +
> + /* Support svn_client_conflict_option_merged_text for binary conflicts by
> + * mapping this onto the semantically equal
> + * svn_client_conflict_option_working_text.
> + */
> if (option == NULL)
> - return svn_error_createf(SVN_ERR_CLIENT_CONFLICT_OPTION_NOT_APPLICABLE,
> - NULL,
> - _("Inapplicable conflict resolution option "
> - "given for conflicted path '%s'"),
> - svn_dirent_local_style(conflict->local_abspath,
> - scratch_pool));
> + {
> + if (option_id == svn_client_conflict_option_merged_text) {
> + mime_type = svn_client_conflict_text_get_mime_type(conflict);
> + if (mime_type && svn_mime_type_is_binary(mime_type))
> + option = svn_client_conflict_option_find_by_id(resolution_options,
> + svn_client_conflict_option_working_text);
> + }
> + }
> +
> + if (option == NULL)
> + return svn_error_createf(SVN_ERR_CLIENT_CONFLICT_OPTION_NOT_APPLICABLE,
> + NULL,
> + _("Inapplicable conflict resolution option "
> + "given for conflicted path '%s'"),
> +
> svn_dirent_local_style(conflict->local_abspath,
> + scratch_pool));
> +
> SVN_ERR(svn_client_conflict_text_resolve(conflict, option, ctx,
> scratch_pool));
>
> return SVN_NO_ERROR;
> Index: subversion/svn/conflict-callbacks.c
> ===================================================================
> --- subversion/svn/conflict-callbacks.c (revision 1764824)
> +++ subversion/svn/conflict-callbacks.c (working copy)
> @@ -913,10 +913,9 @@
> if (knows_something || is_binary)
> *next_option++ = "r";
>
> - /* The 'mine-full' option selects the ".mine" file so only offer
> - * it if that file exists. It does not exist for binary files,
> - * for example (questionable historical behaviour since 1.0). */
> - if (my_abspath)
> + /* The 'mine-full' option selects the ".mine" file for texts or
> + * the current working directory file for binary files. */
> + if (my_abspath || is_binary)
> *next_option++ = "mf";
>
> *next_option++ = "tf";
> Index: subversion/tests/cmdline/resolve_tests.py
> ===================================================================
> --- subversion/tests/cmdline/resolve_tests.py (revision 1764824)
> +++ subversion/tests/cmdline/resolve_tests.py (working copy)
> @@ -602,7 +602,6 @@
>
> # Test for issue #4647 'auto resolution mine-full fails on binary file'
> @Issue(4647)
> -@XFail()
> def automatic_binary_conflict_resolution(sbox):
> "resolve -R --accept [base | mf | tf] binary file"
>
