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
