Gabriela Gibson <gabriela.gib...@gmail.com> writes: > Index: subversion/libsvn_subr/io.c > =================================================================== > --- subversion/libsvn_subr/io.c (revision 1458203) > +++ subversion/libsvn_subr/io.c (working copy) > @@ -2831,7 +2831,105 @@ svn_io_run_cmd(const char *path, > return svn_io_wait_for_cmd(&cmd_proc, cmd, exitcode, exitwhy, pool); > } > > +svn_error_t * > +svn_io_run_diff2_2(const char *dir, > + const char *const *user_args,
Indentation looks a bit out here. > + int num_user_args, > + const char *label1, > + const char *label2, > + const char *user_label_string, > + const char *from, > + const char *to, > + int *pexitcode, > + apr_file_t *outfile, > + apr_file_t *errfile, > + const char *diff_cmd, > + svn_boolean_t ext_string_present, > + 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 (pexitcode == NULL) > + pexitcode = &exitcode; > + > + if (user_args != NULL) > + nargs += num_user_args; > + else > + /* Handling the case where '-u ""' is provided, to avoid adding -u */ > + if (! ext_string_present) > + nargs += 1; /* -u */ > + > + if (label1 != NULL) > + nargs += 2; /* the -L and the label itself */ > + if (label2 != NULL) > + nargs += 2; /* the -L and the label itself */ > + > + args = apr_palloc(subpool, nargs * sizeof(char *)); > + > + i = 0; > + args[i++] = diff_cmd; > + > + if (user_args != NULL) > + { > + int j; > + for (j = 0; j < num_user_args; ++j) > + args[i++] = user_args[j]; > + } > + else > + if (! ext_string_present) > + args[i++] = "-u"; /* assume -u if the user didn't give us any args */ > + > + if (label1 != NULL) > + { > + if (! user_label_string) > + args[i++] = "-L"; > + else > + args[i++] = user_label_string; > + args[i++] = label1; > + } > + if (label2 != NULL) > + { > + if (! user_label_string) > + args[i++] = "-L"; > + else > + args[i++] = user_label_string; > + args[i++] = label2; > + } > + > + args[i++] = svn_dirent_local_style(from, subpool); > + args[i++] = svn_dirent_local_style(to, subpool); > + args[i++] = NULL; > + > + SVN_ERR_ASSERT(i == nargs); > + > + SVN_ERR(svn_io_run_cmd(dir, diff_cmd, args, pexitcode, NULL, TRUE, > + NULL, outfile, errfile, subpool)); > + > + /* The man page for (GNU) diff describes the return value as: > + > + "An exit status of 0 means no differences were found, 1 means > + some differences were found, and 2 means trouble." > + > + A return value of 2 typically occurs when diff cannot read its input > + or write to its output, but in any case we probably ought to return an > + error for anything other than 0 or 1 as the output is likely to be > + corrupt. > + */ > + if (*pexitcode != 0 && *pexitcode != 1) > + return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL, > + _("'%s' returned %d"), > + svn_dirent_local_style(diff_cmd, pool), > + *pexitcode); > + > + svn_pool_destroy(subpool); > + > + return SVN_NO_ERROR; Don't duplicate all the code like that. Take the code from the old function svn_io_run_diff2 and use it to write a new function that implements both svn_io_run_diff2 and svn_io_run_diff2_2 (this new function may be svn_io_run_diff2_2 itself or it may be a static function). Then have svn_io_run_diff2 (and svn_io_run_diff2_2 if necessary) call the new function. > +} > + > svn_error_t * > svn_io_run_diff2(const char *dir, > const char *const *user_args, > Index: subversion/include/svn_client.h > =================================================================== > --- subversion/include/svn_client.h (revision 1458203) > +++ subversion/include/svn_client.h (working copy) > @@ -3008,6 +3008,31 @@ svn_client_blame(const char *path_or_url, > * @since New in 1.8. > */ > svn_error_t * > +svn_client_diff7(const apr_array_header_t *diff_options, > + const char *path_or_url1, > + const svn_opt_revision_t *revision1, > + const char *path_or_url2, > + const svn_opt_revision_t *revision2, > + const char *relative_to_dir, > + svn_depth_t depth, > + svn_boolean_t ignore_ancestry, > + svn_boolean_t no_diff_added, > + svn_boolean_t no_diff_deleted, > + svn_boolean_t show_copies_as_adds, > + svn_boolean_t ignore_content_type, > + svn_boolean_t ignore_properties, > + const char *user_label_string, > + svn_boolean_t ext_string_present, > + svn_boolean_t properties_only, > + svn_boolean_t use_git_diff_format, > + const char *header_encoding, > + svn_stream_t *outstream, > + svn_stream_t *errstream, > + const apr_array_header_t *changelists, > + svn_client_ctx_t *ctx, > + apr_pool_t *pool); > + > +svn_error_t * > svn_client_diff6(const apr_array_header_t *diff_options, > const char *path_or_url1, > const svn_opt_revision_t *revision1, svn_client_diff6 is new in 1.8 so until we make the 1.8 branch we can simply change the API. You only need svn_client_diff7 if your patch is applied after 1.8 has branched. What about svn_client_diff_peg6? Does it need the same change? > Index: subversion/libsvn_client/diff.c > =================================================================== > --- subversion/libsvn_client/diff.c (revision 1458203) > +++ subversion/libsvn_client/diff.c (working copy) > +svn_client_diff7(const apr_array_header_t *options, > + const char *path_or_url1, > + const svn_opt_revision_t *revision1, > + const char *path_or_url2, > + const svn_opt_revision_t *revision2, > + const char *relative_to_dir, > + svn_depth_t depth, > + svn_boolean_t ignore_ancestry, > + svn_boolean_t no_diff_added, > + svn_boolean_t no_diff_deleted, > + svn_boolean_t show_copies_as_adds, > + svn_boolean_t ignore_content_type, > + svn_boolean_t ignore_properties, > + const char *user_label_string, > + svn_boolean_t ext_string_present, > + svn_boolean_t properties_only, > + svn_boolean_t use_git_diff_format, > + const char *header_encoding, > + svn_stream_t *outstream, > + svn_stream_t *errstream, > + const apr_array_header_t *changelists, > + svn_client_ctx_t *ctx, > + apr_pool_t *pool) > +{ > + struct diff_cmd_baton diff_cmd_baton = { 0 }; > + svn_opt_revision_t peg_revision; > + > + if (ignore_properties && properties_only) > + return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL, > + _("Cannot ignore properties and show only " > + "properties at the same time")); > + > + /* We will never do a pegged diff from here. */ > + peg_revision.kind = svn_opt_revision_unspecified; > + > + /* setup callback and baton */ > + diff_cmd_baton.orig_path_1 = path_or_url1; > + diff_cmd_baton.orig_path_2 = path_or_url2; > + > + SVN_ERR(set_up_diff_cmd_and_options(&diff_cmd_baton, options, > + ctx->config, pool)); > + diff_cmd_baton.pool = pool; > + diff_cmd_baton.outstream = outstream; > + diff_cmd_baton.errstream = errstream; > + diff_cmd_baton.header_encoding = header_encoding; > + diff_cmd_baton.revnum1 = SVN_INVALID_REVNUM; > + diff_cmd_baton.revnum2 = SVN_INVALID_REVNUM; > + > + diff_cmd_baton.user_label_string = user_label_string; > + diff_cmd_baton.ext_string_present = ext_string_present; > + diff_cmd_baton.force_binary = ignore_content_type; > + diff_cmd_baton.ignore_properties = ignore_properties; > + diff_cmd_baton.properties_only = properties_only; > + diff_cmd_baton.relative_to_dir = relative_to_dir; > + diff_cmd_baton.use_git_diff_format = use_git_diff_format; > + diff_cmd_baton.no_diff_added = no_diff_added; > + diff_cmd_baton.no_diff_deleted = no_diff_deleted; > + diff_cmd_baton.no_copyfrom_on_add = show_copies_as_adds; > + > + diff_cmd_baton.wc_ctx = ctx->wc_ctx; > + diff_cmd_baton.ra_session = NULL; > + diff_cmd_baton.anchor = NULL; > + > + return do_diff(&diff_callbacks, &diff_cmd_baton, ctx, > + path_or_url1, path_or_url2, revision1, revision2, > + &peg_revision, > + depth, ignore_ancestry, show_copies_as_adds, > + use_git_diff_format, changelists, pool); > +} > + > +svn_error_t * > svn_client_diff6(const apr_array_header_t *options, > const char *path_or_url1, > const svn_opt_revision_t *revision1, Again, don't duplicate the code. This simply goes away if you modify svn_client_diff6 but if you do introduce svn_client_diff7 then move svn_client_diff6 to deprecated.c and have it call svn_client_diff7. -- Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download