On Wed, Jan 09, 2002 at 04:18:21PM -0800, Greg Stein wrote:
> On Tue, Jan 08, 2002 at 09:36:22PM -0500, Garrett Rooney wrote:
> >...
> > * subversion/libsvn_wc/get_editor.c
> > (close_file): return an error when diff returns an unexpected exit
> > code.
> > * subversion/libsvn_client/diff.c
> > (svn_client__diff_cmd): ditto
> > * subversion/svnlook/main.c
> > (print_diff_tree): ditto
>
> Those "ditto" calls raise a yellow flag... It would seem to be nice to have
> a function that knows about the vagaries of invoking diff, and just Do The
> Right Thing. It could simplify a number of the params to get it called, too.
> *shrug* Not familiar enough with those sections of code to really know...
excellent point.
revised diff attached.
(incedentally, libsvn_wc/get_editor.c is currently hard coding diff
instead of using SVN_CLIENT_DIFF, which is bad, and should be fixed
with or without this patch.)
* subversion/includes/svn_error_codes.h
(SVN_ERR_EXTERNAL_PROGRAM): new error type for when an external
program fails in an unexpected way.
* subversion/include/svn_io.h
(svn_io_run_diff): new prototype.
* subversion/libsvn_wc/get_editor.c
(close_file): use svn_io_run_diff instead of calling diff by hand.
* subversion/libsvn_subr/io.c
(svn_io_run_diff): new function that calls diff (via svn_io_run_cmd),
so we don't have to reproduce the logic all over the place. most of
this logic comes from libsvn_client/diff.c
* subversion/libsvn_client/diff.c
(svn_client__diff_cmd): use svn_io_run_diff instead of calling diff
by hand.
* subversion/svnlook/main.c
(print_diff_tree): ditto.
Index: ./subversion/include/svn_error_codes.h
===================================================================
--- ./subversion/include/svn_error_codes.h
+++ ./subversion/include/svn_error_codes.h Wed Jan 9 21:46:14 2002
@@ -274,6 +274,9 @@
SVN_ERRDEF (SVN_ERR_REPOS_HOOK_FAILURE,
"A repository hook failed.")
+ SVN_ERRDEF (SVN_ERR_EXTERNAL_PROGRAM,
+ "Error calling external program")
+
SVN_ERRDEF (SVN_ERR_BERKELEY_DB,
"Berkeley DB error")
Index: ./subversion/include/svn_io.h
===================================================================
--- ./subversion/include/svn_io.h
+++ ./subversion/include/svn_io.h Wed Jan 9 21:46:14 2002
@@ -339,6 +339,27 @@
apr_file_t *errfile,
apr_pool_t *pool);
+/* Invoke SVN_CLIENT_DIFF, with USER_ARGS (which is an array of NUM_USER_ARGS
+ arguments), if they are specified, or "-u" if they are not.
+
+ Diff runs in DIR, and it's exit status is stored in EXITCODE, if it is not
+ NULL.
+
+ If LABEL is given, it will be passed in as the argument to the "-L" option.
+
+ FROM is the first file passed to diff, and TO is the second. The stdout of
+ diff will be sent to OUTFILE, and the stderr to ERRFILE. All memory will be
+ allocated using POOL. */
+svn_error_t *svn_io_run_diff (const char *dir,
+ const char *const *user_args,
+ const int num_user_args,
+ const char *label,
+ const char *from,
+ const char *to,
+ int *exitcode,
+ apr_file_t *outfile,
+ apr_file_t *errfile,
+ apr_pool_t *pool);
/* Examine FILE to determine if it can be described by a known (as in,
known by this function) Multipurpose Internet Mail Extension (MIME)
Index: ./subversion/libsvn_wc/get_editor.c
===================================================================
--- ./subversion/libsvn_wc/get_editor.c
+++ ./subversion/libsvn_wc/get_editor.c Wed Jan 9 21:46:14 2002
@@ -1322,9 +1322,8 @@
{
/* Run the external `diff' command immediately and
create a temporary .diff file. */
- apr_proc_t diff_proc;
- apr_procattr_t *diffproc_attr;
- const char *diff_args[6];
+ int exitcode;
+ const char *diff_args[2];
apr_file_t *received_diff_file;
if (eol_style == svn_wc__eol_style_native)
@@ -1396,68 +1395,14 @@
FALSE,
fb->pool));
- /* Create the process attributes. */
- apr_err = apr_procattr_create (&diffproc_attr, fb->pool);
- if (! APR_STATUS_IS_SUCCESS (apr_err))
- return svn_error_create
- (apr_err, 0, NULL, fb->pool,
- "close_file: error creating diff process attributes");
-
- /* Make sure we invoke diff directly, not thru a shell. */
- apr_err =
- apr_procattr_cmdtype_set (diffproc_attr, APR_PROGRAM);
- if (! APR_STATUS_IS_SUCCESS (apr_err))
- return svn_error_create
- (apr_err, 0, NULL, fb->pool,
- "close_file: error setting diff process cmdtype");
-
- /* Set io style. */
- apr_err =
- apr_procattr_io_set (diffproc_attr, 0,
- APR_CHILD_BLOCK, APR_CHILD_BLOCK);
- if (! APR_STATUS_IS_SUCCESS (apr_err))
- return svn_error_create
- (apr_err, 0, NULL, fb->pool,
- "close_file: error setting diff process io attributes");
-
- /* Tell it to send output to the diff file. */
- apr_err = apr_procattr_child_out_set (diffproc_attr,
- received_diff_file,
- NULL);
- if (! APR_STATUS_IS_SUCCESS (apr_err))
- return svn_error_create
- (apr_err, 0, NULL, fb->pool,
- "close_file: error setting diff process child output");
-
/* Build the diff command. */
- diff_args[0] = "diff";
- diff_args[1] = "-c";
- diff_args[2] = "--";
- diff_args[3] = txtb_full_path->data;
- diff_args[4] = tmp_txtb_full_path->data;
- diff_args[5] = NULL;
-
- /* Start the diff command. kff todo: path to diff program
- should be determined through various levels of fallback,
- of course, not hardcoded. */
- apr_err = apr_proc_create (&diff_proc,
- SVN_CLIENT_DIFF,
- diff_args,
- NULL,
- diffproc_attr,
- fb->pool);
- if (! APR_STATUS_IS_SUCCESS (apr_err))
- return svn_error_createf
- (apr_err, 0, NULL, fb->pool,
- "close_file: error starting diff process");
-
- /* Wait for the diff command to finish. */
- apr_err = apr_proc_wait (&diff_proc, NULL, NULL, APR_WAIT);
- if (APR_STATUS_IS_CHILD_NOTDONE (apr_err))
- return svn_error_createf
- (apr_err, 0, NULL, fb->pool,
- "close_file: error waiting for diff process");
+ diff_args[0] = "-c";
+ diff_args[1] = "--";
+ SVN_ERR(svn_io_run_diff
+ (".", diff_args, 2, NULL, txtb_full_path->data,
+ tmp_txtb_full_path->data, &exitcode, received_diff_file,
+ NULL, fb->pool));
/* The diff is finished. If we had to create
temporary 'translated' text-bases, write log
Index: ./subversion/libsvn_subr/io.c
===================================================================
--- ./subversion/libsvn_subr/io.c
+++ ./subversion/libsvn_subr/io.c Wed Jan 9 21:50:00 2002
@@ -34,7 +34,7 @@
#include "svn_error.h"
#include "svn_io.h"
#include "svn_pools.h"
-
+#include "svn_private_config.h" /* for SVN_CLIENT_DIFF */
struct svn_stream_t {
@@ -1303,6 +1303,86 @@
return SVN_NO_ERROR;
}
+/* Invoke SVN_CLIENT_DIFF, with USER_ARGS (which is an array of NUM_USER_ARGS
+ arguments), if they are specified, or "-u" if they are not.
+
+ Diff runs in DIR, and it's exit status is stored in EXITCODE, if it is not
+ NULL.
+
+ If LABEL is given, it will be passed in as the argument to the "-L" option.
+
+ FROM is the first file passed to diff, and TO is the second. The stdout of
+ diff will be sent to OUTFILE, and the stderr to ERRFILE. All memory will be
+ allocated using POOL. */
+svn_error_t *
+svn_io_run_diff (const char *dir,
+ const char *const *user_args,
+ const int num_user_args,
+ const char *label,
+ const char *from,
+ const char *to,
+ int *ecode,
+ apr_file_t *outfile,
+ apr_file_t *errfile,
+ apr_pool_t *pool)
+{
+ const char **args;
+ int i;
+ int *exitcode;
+ int nargs = 4; /* the diff command itself, two paths, plus a trailing NULL */
+
+ apr_pool_t *subpool = svn_pool_create (pool);
+
+ if (ecode == NULL)
+ exitcode = NULL;
+ else
+ exitcode = ecode;
+
+ if (user_args != NULL)
+ nargs += num_user_args;
+ else
+ nargs += 1; /* -u */
+
+ if (label != NULL)
+ nargs += 2; /* the -L and the label itself */
+
+ args = apr_palloc(subpool, nargs * sizeof(char *));
+
+ i = 0;
+ args[i++] = SVN_CLIENT_DIFF;
+
+ if (user_args != NULL)
+ {
+ int j;
+ for (j = 0; j < num_user_args; ++j)
+ args[i++] = user_args[j];
+ }
+ else
+ args[i++] = "-u"; /* assume -u if the user didn't give us any args */
+
+ if (label != NULL)
+ {
+ args[i++] = "-L";
+ args[i++] = label;
+ }
+
+ args[i++] = from;
+ args[i++] = to;
+ args[i++] = NULL;
+
+ assert (i == nargs);
+
+ SVN_ERR(svn_io_run_cmd (dir, SVN_CLIENT_DIFF, args, exitcode, NULL, NULL,
+ outfile, errfile, subpool));
+
+ if (*exitcode < 0 || *exitcode > 2)
+ return svn_error_createf (SVN_ERR_EXTERNAL_PROGRAM, 0, NULL, subpool,
+ "Error calling %s.", SVN_CLIENT_DIFF);
+
+ svn_pool_destroy (subpool);
+
+ return SVN_NO_ERROR;
+}
svn_error_t *
svn_io_detect_mimetype (const char **mimetype,
Index: ./subversion/libsvn_client/diff.c
===================================================================
--- ./subversion/libsvn_client/diff.c
+++ ./subversion/libsvn_client/diff.c Wed Jan 9 21:46:14 2002
@@ -54,7 +54,6 @@
apr_status_t status;
const char **args;
int i, nargs, exitcode;
- apr_exit_why_e exitwhy;
apr_file_t *outhandle, *errhandle;
/* Use a subpool here because the pool comes right through from the top
@@ -74,17 +73,13 @@
/* Execute local diff command on these two paths, print to stdout. */
- nargs = 4; /* Eek! A magic number! It's checked by a later assert */
- if (label)
- nargs += 2; /* Another magic number checked by the later assert */
- if (diff_cmd_baton->options->nelts)
- nargs += diff_cmd_baton->options->nelts;
- else
- ++nargs;
- args = apr_palloc(subpool, nargs*sizeof(char*));
+ nargs = diff_cmd_baton->options->nelts;
+ if (nargs)
+ args = apr_palloc(subpool, nargs*sizeof(char*));
+ else
+ args = NULL;
i = 0;
- args[i++] = SVN_CLIENT_DIFF; /* the autoconfiscated system diff program */
if (diff_cmd_baton->options->nelts)
{
/* Add the user specified diff options to the diff command line. */
@@ -93,19 +88,6 @@
args[i++]
= ((svn_stringbuf_t **)(diff_cmd_baton->options->elts))[j]->data;
}
- else
- {
- /* The user didn't specify any options, default to unified diff. */
- args[i++] = "-u";
- }
- if (label)
- {
- args[i++] = "-L";
- args[i++] = label->data;
- }
- args[i++] = path1->data;
- args[i++] = path2->data;
- args[i++] = NULL;
assert (i==nargs);
/* ### TODO: This printf is NOT "my final answer" -- placeholder for
@@ -116,8 +98,9 @@
apr_file_printf (outhandle, "Index: %s\n", path1->data);
apr_file_printf (outhandle, "===================================================================\n");
- SVN_ERR(svn_io_run_cmd (".", SVN_CLIENT_DIFF, args, &exitcode, &exitwhy, NULL,
- outhandle, errhandle, subpool));
+ SVN_ERR(svn_io_run_diff
+ (".", args, nargs, label ? label->data : NULL, path1->data, path2->data,
+ &exitcode, outhandle, errhandle, subpool));
/* TODO: Handle exit code == 2 (i.e. errors with diff) here */
Index: ./subversion/svnlook/main.c
===================================================================
--- ./subversion/svnlook/main.c
+++ ./subversion/svnlook/main.c Wed Jan 9 21:46:14 2002
@@ -34,7 +34,6 @@
#include "svn_fs.h"
#include "svn_repos.h"
#include "svn_time.h"
-#include "svn_private_config.h" /* for SVN_CLIENT_DIFF */
/*** Some convenience macros and types. ***/
@@ -469,9 +468,9 @@
if (orig_path && new_path)
{
- const char *args[5];
apr_file_t *outhandle;
apr_status_t apr_err;
+ int exitcode;
printf ("%s: %s\n",
((tmp_node->action == 'A') ? "Added" :
@@ -482,12 +481,6 @@
===============\n");
fflush (stdout);
- args[0] = SVN_CLIENT_DIFF;
- args[1] = "-u";
- args[2] = orig_path->data;
- args[3] = new_path->data;
- args[4] = NULL;
-
/* Get an apr_file_t representing stdout, which is where we'll have
the diff program print to. */
apr_err = apr_file_open_stdout (&outhandle, pool);
@@ -496,8 +489,9 @@
(apr_err, 0, NULL, pool,
"print_diff_tree: can't open handle to stdout");
- SVN_ERR(svn_io_run_cmd (".", SVN_CLIENT_DIFF, args, NULL, NULL,
- NULL, outhandle, NULL, pool));
+ SVN_ERR(svn_io_run_diff
+ (".", NULL, 0, NULL, orig_path->data, new_path->data, &exitcode,
+ outhandle, NULL, pool));
/* TODO: Handle exit code == 2 (i.e. diff error) here. */
--
garrett rooney Unix was not designed to stop you from
rooneg@electricjellyfish.net doing stupid things, because that would
http://electricjellyfish.net/ stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:55 2006