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

Re: [PATCH] Implement (l)aunch an external merge tool

From: Augie Fackler <durin42_at_gmail.com>
Date: 2007-10-03 21:31:44 CEST

I believe this addresses everything we talked about. The config
variable doesn't work yet, but I didn't fix it per our discussion on
IRC about you having added a fix for this in your wc.

Attached the patch so that our mailserver can't munge it like it
evidently did last time.

Peace,
Augie

[[[
Implement launching an external merge tool for interactive conflict
handling.

* subversion/include/svn_error_codes.h
   (SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL): New. Error code that
indicates no merge
    tool could be found.

* subversion/libsvn_subr/svn_config.h
   (SVN_CONFIG_OPTION_MERGE_TOOL_CMD): New. Run-time configuration
option for the
    external merging tool to use.

* subversion/libsvn_subr/config_file.c
   (svn_config_ensure): Add a line with some basic documentation for
the new
    run-time configuration option.

* subversion/svn/cl.h
   (svn_cl__merge_file_externally): New prototype.

* subversion/svn/util.c
   Include unistd.h.
   (svn_cl__merge_file_externally): New. Used to invoke the user's
external merge
    tool on the specified files.

* subversion/svn/conflict-callbacks.c
   Include stdio.h.
   (svn_cl__interactive_conflict_handler): Implement calling an
external merge
    tool using svn_cl__merge_file_externally.
]]]

On Oct 1, 2007, at 7:50 PM, Eric Gillespie wrote:

> After integrating your patch into my wc so I can handle the
> conflicts with my own change early, I have more comments.
>
> Augie Fackler <durin42@gmail.com> writes:
>
>> +svn_error_t *
>> +svn_cl__merge_file_externally(const char *base_path,
>> + const char *repos_path,
>> + const char *user_path,
>> + const char *merged_path,
>> + apr_pool_t *pool)
>> +{
>> + const char *merge_tool;
>> + merge_tool = getenv("SVNMERGE");
>
> Use SVN_MERGE, to be consistent with SVN_EDITOR, not to mention
> tradition.
>
>> + /* Error if there is no editor specified */
>> + if (merge_tool)
>> + {
>> + const char *c;
>>
>> + for (c = merge_tool; *c; c++)
>> + if (!svn_ctype_isspace(*c))
>> + break;
>>
>> + if (! *c)
>> + return svn_error_create
>> + (SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL, NULL,
>> + _("The SVNMERGE environment variable is empty or "
>> + "consists solely of whitespace. Expected a shell
>> command.\n"));
>> + }
>> + else
>> + return svn_error_create
>> + (SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL, NULL,
>> + _("The environment variable SVNMERGE is not set.\n"));
>> +
>> + /* command base left right destination */
>> + const char *arguments[] = { merge_tool, base_path, repos_path,
>> + user_path, merged_path, NULL};
>> + int exitcode;
>> + apr_exit_why_e exitwhy;
>> + char *sys_path = getwd(NULL);
>
> apr *does* have getcwd, but it has the craziest name I've ever seen:
>
> char *cwd;
> status = apr_filepath_get(&cwd, APR_FILEPATH_NATIVE, pool);
> if (status != 0)
> return svn_error_wrap_apr(status, NULL);
> return svn_io_run_cmd(svn_path_internal_style(cwd, pool),
>
> If we're sure that apr's sense of internal path names is the same
> as svn's (it happens to be on unix and windows, at the moment),
> we can drop the NATIVE option and internal_style call. Unless
> someone else speaks up, stick with this.
>
>> + const char *path = svn_path_internal_style(sys_path, pool);
>> + free(sys_path);
>> + SVN_ERR(svn_io_run_cmd(path, merge_tool, arguments, &exitcode,
>> &exitwhy, TRUE,
>> + NULL, NULL, NULL, pool));
>> +
>> + return SVN_NO_ERROR;
>
> You'll need to wrap this final block in { }, as you can't mix
> declarations and statements in C89. And just pass NULL instead
> of &exitcode, &exitwhy since we don't use them here.
>
>> Index: subversion/svn/conflict-callbacks.c
>> ===================================================================
>> --- subversion/svn/conflict-callbacks.c (revision 26684)
>> +++ subversion/svn/conflict-callbacks.c (working copy)
>> @@ -26,6 +26,7 @@
>> #define APR_WANT_STDIO
>> #define APR_WANT_STRFUNC
>> #include <apr_want.h>
>> +#include <stdio.h>
>
> You added this for the fflush, I see. Nothing else here flushes,
> yet. I think I managed to convince at least Sussman that we
> should set stdout and stderr to line buffering in cmdline_init;
> we can safely leave out these flushes and take care of all such
> cases one way or the other in a later change.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Received on Wed Oct 3 21:36:27 2007

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.