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 Tue Oct 2 01:51:10 2007