[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] New --file-merge option for issue #4487

From: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 7 Apr 2014 18:00:21 +0200

Hi,

the patch below adds a --file-merge option, in an attempt to solve
issue #4487 "add a scriptable non-interactive option for additive merges"
http://subversion.tigris.org/issues/show_bug.cgi?id=4487

There's been a discussion on IRC about this today:
http://colabti.org/irclogger/irclogger_log/svn-dev?date=2014-04-07#l116
Before jumping there, please read my wrap-up below.

A new --file-merge option is the simplest way of solving this
problem I could come up with. It maps the options provided by
the internal merge tool to a command line flag:

  --file-merge ARG : Set a pre-defined choice ARG for the built-in file
                             merge tool, which otherwise prompts interactively.
                             --file-merge applies to text conflicts only and
                             overrides the --accept option for file merges.
                             ARG is any of 1, 2, 12, 21, e1, e2, or eb:
                             (1) use their version, (2) use your version,
                             (12) their version first, then yours,
                             (21) your version first, then theirs,
                             (e1) edit their version and use the result,
                             (e2) edit your version and use the result,
                             (eb) edit both versions and use the result

But it's just an ad-hoc idea, so I would like to get some feedback.
I'm curious if anyone else has other ideas.

Among other ideas discussed on IRC where:

 - Currently the file merge tool is only reachable from the interactive
   conflict resolution handler. We could split the internal merge tool
   off into a seperate program shipped with Subversion (let's call this
   svn-file-merge for this discussion) so it could be run standalone:

   svn-file-merge [--default-answer=(1|2|12|21)] FILE1 FILE2 FILE3 OUTPUT_FILE

   This should allow scripts to configure a merge tool as needed, e.g.

   svn update --config-option config:helpers:merge-tool-cmd \
     'svn-file-merge --default-answer=21'
  
   Note that this approach might also fix issue #4426 since the
   distinction between internal and external merge tools becomes
   meaningless (this might be of interest to Ben Reser in case he's
   reading).

 - If we're considering exposing the file-merge tool functionality via a
   seperate CLI tool, we might also take into account the possibility of
   exposing it directly via an 'svn' subcommand.

   E.g. we could add a new mode to 'svn merge' which unlike 'svn
   resolve' skips the standard conflict prompt and runs the built-in
   merge tool interactively, or with an --file-merge option similar
   to the one in the patch below.
   It would be invoked like this, for instance:

     svn merge [--file-merge=(1|2|12|21)] FILE1 FILE2 FILE3 OUTPUT_FILE

   All input files would need to be unversioned to trigger "merge tool mode".
   'svn merge' could then be invoked as an "external" merge tool and would
   also be the pre-configured default merge tool.

   svn merge --config-option config:helpers:merge-tool-cmd \
     'svn merge --file-merge=21'

   This looks a bit circular, but users who don't suffer from issue
   #4487 wouldn't even need to know about this quirk. And perhaps this
   is preferable to adding yet another separate utility or subcommand.

Would anyone prefer the above possibilities to the patch below?
Any other ideas?

Thanks!

[[[
Add a --file-merge option, exposing the interactive built-in file
merge tool via a command line switch intended to be used by scripts.
See issue #4487.

* subversion/svn/cl.h
  (svn_cl__opt_state_t): Add file_merge opt arg.
  (svn_cl__get_conflict_func_interactive_baton): Add file_merge_option
   parameter.
  (svn_cl__merge_file): Add a corresponding 'predefined_answer' parameter.

* subversion/svn/conflict-callbacks.c
  (svn_cl__interactive_conflict_baton_t,
  svn_cl__get_conflict_func_interactive_baton): Add file_merge_option.
  (handle_text_conflict): Force the built-in merge tool into interactive
   mode by passing NULL for 'predefined_answer'.
  (conflict_func_interactive): Resolve text conflicts as specified by
   the --file-merge option if it was provided.

* subversion/svn/file-merge.c
  (file_merge_baton, merge_chunks): Add 'predefined_answer' parameter.
   If set, skip interactive prompting and use the predefined answer.
  (merge_file_chunks, file_merge_output_conflict, svn_cl__merge_file):
   Propagate the new parameter.

* subversion/svn/svn.c
  (svn_cl__longopt_t): Add opt_file_merge.
  (svn_cl__options, svn_cl__cmd_table): Define and document the --file-merge
   option and add it to subcommands which also accept the --accept option.
  (sub_main): Parse and verify the --file-merge option.

]]]

Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1585458)
+++ subversion/svn/cl.h (working copy)
@@ -245,6 +245,7 @@ typedef struct svn_cl__opt_state_t
   svn_boolean_t remove_ignored; /* remove ignored items */
   svn_boolean_t no_newline; /* do not output the trailing newline */
   svn_boolean_t show_passwords; /* show cached passwords */
+ const char *file_merge; /* non-interactive file merge */
 } svn_cl__opt_state_t;
 
 
@@ -378,6 +379,7 @@ svn_cl__get_conflict_func_interactive_baton(
   apr_hash_t *config,
   const char *editor_cmd,
   svn_cl__conflict_stats_t *conflict_stats,
+ const char *file_merge_option,
   svn_cancel_func_t cancel_func,
   void *cancel_baton,
   apr_pool_t *result_pool);
@@ -532,7 +534,8 @@ svn_cl__merge_file_externally(const char *base_pat
                               apr_pool_t *pool);
 
 /* Like svn_cl__merge_file_externally, but using a built-in merge tool
- * with help from an external editor specified by EDITOR_CMD. */
+ * with help from an external editor specified by EDITOR_CMD, and
+ * also supports non-interactive use via a PREDEFINED_ANSWER. */
 svn_error_t *
 svn_cl__merge_file(svn_boolean_t *remains_in_conflict,
                    const char *base_path,
@@ -543,6 +546,7 @@ svn_cl__merge_file(svn_boolean_t *remains_in_confl
                    const char *path_prefix,
                    const char *editor_cmd,
                    apr_hash_t *config,
+ const char *predefined_answer,
                    svn_cancel_func_t cancel_func,
                    void *cancel_baton,
                    apr_pool_t *scratch_pool);
Index: subversion/svn/conflict-callbacks.c
===================================================================
--- subversion/svn/conflict-callbacks.c (revision 1585458)
+++ subversion/svn/conflict-callbacks.c (working copy)
@@ -57,6 +57,7 @@ struct svn_cl__interactive_conflict_baton_t {
   svn_boolean_t quit;
   svn_cl__conflict_stats_t *conflict_stats;
   svn_boolean_t printed_summary;
+ const char *file_merge_option;
 };
 
 svn_error_t *
@@ -66,6 +67,7 @@ svn_cl__get_conflict_func_interactive_baton(
   apr_hash_t *config,
   const char *editor_cmd,
   svn_cl__conflict_stats_t *conflict_stats,
+ const char *file_merge_option,
   svn_cancel_func_t cancel_func,
   void *cancel_baton,
   apr_pool_t *result_pool)
@@ -84,6 +86,7 @@ svn_cl__get_conflict_func_interactive_baton(
   (*b)->quit = FALSE;
   (*b)->conflict_stats = conflict_stats;
   (*b)->printed_summary = FALSE;
+ (*b)->file_merge_option = file_merge_option;
 
   return SVN_NO_ERROR;
 }
@@ -864,6 +867,7 @@ handle_text_conflict(svn_wc_conflict_result_t *res
                                          b->path_prefix,
                                          b->editor_cmd,
                                          b->config,
+ NULL,
                                          b->pb->cancel_func,
                                          b->pb->cancel_baton,
                                          iterpool));
@@ -1183,6 +1187,35 @@ conflict_func_interactive(svn_wc_conflict_result_t
   *result = svn_wc_create_conflict_result(svn_wc_conflict_choose_postpone,
                                           NULL, result_pool);
 
+ /* If the user specified the --file-merge option and
+ * this is a text conflict, resolve the conflict with
+ * the built-in merge tool and the pre-defined answer. */
+ if (b->file_merge_option &&
+ ((desc->kind == svn_wc_conflict_kind_text)
+ && (desc->action == svn_wc_conflict_action_edit)
+ && (desc->reason == svn_wc_conflict_reason_edited)))
+ {
+ svn_boolean_t remains_in_conflict;
+
+ SVN_ERR(svn_cl__merge_file(&remains_in_conflict,
+ desc->base_abspath,
+ desc->their_abspath,
+ desc->my_abspath,
+ desc->merged_file,
+ desc->local_abspath,
+ b->path_prefix,
+ b->editor_cmd,
+ b->config,
+ b->file_merge_option,
+ b->pb->cancel_func,
+ b->pb->cancel_baton,
+ scratch_pool));
+ if (!remains_in_conflict)
+ (*result)->choice = svn_wc_conflict_choose_merged;
+
+ return SVN_NO_ERROR;
+ }
+
   switch (b->accept_which)
     {
     case svn_cl__accept_invalid:
Index: subversion/svn/file-merge.c
===================================================================
--- subversion/svn/file-merge.c (revision 1585458)
+++ subversion/svn/file-merge.c (working copy)
@@ -78,6 +78,9 @@ struct file_merge_baton {
   /* Whether the merge should be aborted. */
   svn_boolean_t abort_merge;
 
+ /* A pre-defined answer, for non-interactive use. */
+ const char *predefined_answer;
+
   /* Pool for temporary allocations. */
   apr_pool_t *scratch_pool;
 };
@@ -579,6 +582,7 @@ merge_chunks(apr_array_header_t **merged_chunk,
              svn_linenum_t current_line2,
              const char *editor_cmd,
              apr_hash_t *config,
+ const char *predefined_answer,
              apr_pool_t *result_pool,
              apr_pool_t *scratch_pool)
 {
@@ -670,7 +674,23 @@ merge_chunks(apr_array_header_t **merged_chunk,
 
       svn_pool_clear(iterpool);
 
- SVN_ERR(svn_cmdline_prompt_user2(&answer, prompt->data, NULL, iterpool));
+ if (predefined_answer)
+ {
+ answer = predefined_answer;
+
+ /* The predefined answer must be a valid choice.
+ * Otherwise we'll never leave this while-loop. */
+ SVN_ERR_ASSERT(strcmp(answer, "1") == 0 ||
+ strcmp(answer, "2") == 0 ||
+ strcmp(answer, "12") == 0 ||
+ strcmp(answer, "21") == 0 ||
+ strcmp(answer, "e1") == 0 ||
+ strcmp(answer, "e2") == 0 ||
+ strcmp(answer, "eb") == 0);
+ }
+ else
+ SVN_ERR(svn_cmdline_prompt_user2(&answer, prompt->data, NULL, iterpool));
+
       if (strcmp(answer, "1") == 0)
         {
           *merged_chunk = chunk1;
@@ -759,6 +779,7 @@ merge_file_chunks(svn_boolean_t *remains_in_confli
                   svn_linenum_t *current_line2,
                   const char *editor_cmd,
                   apr_hash_t *config,
+ const char *predefined_answer,
                   apr_pool_t *scratch_pool)
 {
   apr_array_header_t *chunk1;
@@ -774,7 +795,7 @@ merge_file_chunks(svn_boolean_t *remains_in_confli
 
   SVN_ERR(merge_chunks(&merged_chunk, abort_merge, chunk1, chunk2,
                        *current_line1, *current_line2,
- editor_cmd, config,
+ editor_cmd, config, predefined_answer,
                        scratch_pool, scratch_pool));
 
   if (*abort_merge)
@@ -839,6 +860,7 @@ file_merge_output_conflict(void *output_baton,
                             &b->current_line_latest,
                             b->editor_cmd,
                             b->config,
+ b->predefined_answer,
                             b->scratch_pool));
   return SVN_NO_ERROR;
 }
@@ -862,6 +884,7 @@ svn_cl__merge_file(svn_boolean_t *remains_in_confl
                    const char *path_prefix,
                    const char *editor_cmd,
                    apr_hash_t *config,
+ const char *predefined_answer,
                    svn_cancel_func_t cancel_func,
                    void *cancel_baton,
                    apr_pool_t *scratch_pool)
@@ -918,6 +941,7 @@ svn_cl__merge_file(svn_boolean_t *remains_in_confl
   fmb.editor_cmd = editor_cmd;
   fmb.config = config;
   fmb.abort_merge = FALSE;
+ fmb.predefined_answer = predefined_answer;
   fmb.scratch_pool = scratch_pool;
 
   SVN_ERR(svn_diff_output2(diff, &fmb, &file_merge_diff_output_fns,
Index: subversion/svn/svn.c
===================================================================
--- subversion/svn/svn.c (revision 1585458)
+++ subversion/svn/svn.c (working copy)
@@ -139,7 +139,8 @@ typedef enum svn_cl__longopt_t {
   opt_remove_unversioned,
   opt_remove_ignored,
   opt_no_newline,
- opt_show_passwords
+ opt_show_passwords,
+ opt_file_merge,
 } svn_cl__longopt_t;
 
 
@@ -394,6 +395,28 @@ const apr_getopt_option_t svn_cl__options[] =
   {"remove-ignored", opt_remove_ignored, 0, N_("remove ignored items")},
   {"no-newline", opt_no_newline, 0, N_("do not output trailing newline")},
   {"show-passwords", opt_show_passwords, 0, N_("show cached passwords")},
+ {"file-merge", opt_file_merge, 1,
+ N_("Set a pre-defined choice ARG for the built-in file\n"
+ " "
+ "merge tool, which otherwise prompts interactively.\n"
+ " "
+ "--file-merge applies to text conflicts only and\n"
+ " "
+ "overrides the --accept option for file merges.\n"
+ " "
+ "ARG is any of 1, 2, 12, 21, e1, e2, or eb:\n"
+ " "
+ "(1) use their version, (2) use your version,\n"
+ " "
+ "(12) their version first, then yours,\n"
+ " "
+ "(21) your version first, then theirs,\n"
+ " "
+ "(e1) edit their version and use the result,\n"
+ " "
+ "(e2) edit your version and use the result,\n"
+ " "
+ "(eb) edit both versions and use the result\n")},
 
   /* Long-opt Aliases
    *
@@ -1141,7 +1164,7 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
 " repositories.\n"),
     {'r', 'c', 'N', opt_depth, 'q', opt_force, opt_dry_run, opt_merge_cmd,
      opt_record_only, 'x', opt_ignore_ancestry, opt_accept, opt_reintegrate,
- opt_allow_mixed_revisions, 'v'},
+ opt_allow_mixed_revisions, 'v', opt_file_merge},
     { { opt_force, N_("force deletions even if deleted contents don't match") } }
   },
 
@@ -1442,7 +1465,7 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
      " The --accept=ARG option prevents interactive prompting and forces\n"
      " conflicts on PATH to be resolved in the manner specified by ARG.\n"
      " In this mode, the command is not recursive by default (depth 'empty').\n"),
- {opt_targets, 'R', opt_depth, 'q', opt_accept},
+ {opt_targets, 'R', opt_depth, 'q', opt_accept, opt_file_merge},
     {{opt_accept, N_("specify automatic conflict resolution source\n"
                      " "
                      "('base', 'working', 'mine-conflict',\n"
@@ -1608,7 +1631,8 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
      " svn switch --relocate http://www.example.com/repo/project \\\n"
      " svn://svn.example.com/repo/project\n"),
     { 'r', 'N', opt_depth, opt_set_depth, 'q', opt_merge_cmd, opt_relocate,
- opt_ignore_externals, opt_ignore_ancestry, opt_force, opt_accept},
+ opt_ignore_externals, opt_ignore_ancestry, opt_force, opt_accept,
+ opt_file_merge},
     {{opt_ignore_ancestry,
      N_("allow switching to a node with no common ancestor")},
      {opt_force,
@@ -1670,7 +1694,7 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
      " targets of this operation.\n"),
     {'r', 'N', opt_depth, opt_set_depth, 'q', opt_merge_cmd, opt_force,
      opt_ignore_externals, opt_changelist, opt_editor_cmd, opt_accept,
- opt_parents},
+ opt_parents, opt_file_merge},
     { {opt_force,
        N_("handle unversioned obstructions as changes")} } },
 
@@ -2344,6 +2368,22 @@ sub_main(int *exit_code, int argc, const char *arg
       case opt_show_passwords:
         opt_state.show_passwords = TRUE;
         break;
+ case opt_file_merge:
+ SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
+ if (strcmp(utf8_opt_arg, "1") != 0 &&
+ strcmp(utf8_opt_arg, "2") != 0 &&
+ strcmp(utf8_opt_arg, "12") != 0 &&
+ strcmp(utf8_opt_arg, "21") != 0 &&
+ strcmp(utf8_opt_arg, "e1") != 0 &&
+ strcmp(utf8_opt_arg, "e2") != 0 &&
+ strcmp(utf8_opt_arg, "eb") != 0)
+ {
+ return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Invalid file merge choice '%s'"),
+ utf8_opt_arg);
+ }
+ opt_state.file_merge = utf8_opt_arg;
+ break;
       default:
         /* Hmmm. Perhaps this would be a good place to squirrel away
            opts that commands like svn diff might need. Hmmm indeed. */
@@ -2941,6 +2981,7 @@ sub_main(int *exit_code, int argc, const char *arg
                 &b,
                 opt_state.accept_which,
                 ctx->config, opt_state.editor_cmd, conflict_stats,
+ opt_state.file_merge,
                 ctx->cancel_func, ctx->cancel_baton, pool));
     ctx->conflict_baton2 = b;
   }
Received on 2014-04-07 18:01:06 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.