You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2014/02/19 17:05:48 UTC
svn commit: r1569804 - /subversion/trunk/subversion/libsvn_client/diff.c
Author: rhuijben
Date: Wed Feb 19 16:05:48 2014
New Revision: 1569804
URL: http://svn.apache.org/r1569804
Log:
In libsvn_client diff handling, move the creation of diff processors up
the callchain to allow reusing more code code between diff and diff
summarize.
* subversion/libsvn_client/diff.c
(diff_wc_wc): Allow passing NULL for callback_baton. No functional
changes.
(diff_repos_wc): Add argument to allow disabling a tree wrapping.
Allow a NULL callback baton, and always initialize the urls in the baton.
(do_diff): Update arguments to accept a diff processor instead of callbacks.
(diff_summarize_repos_wc,
diff_summarize_wc_wc): Remove now unused functions.
(do_diff_summarize): Construct diff processor and pass it to the common
diff functions.
(svn_client_diff6,
svn_client_diff_peg6): Wrap callbacks to update call of do_diff.
Modified:
subversion/trunk/subversion/libsvn_client/diff.c
Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1569804&r1=1569803&r2=1569804&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Wed Feb 19 16:05:48 2014
@@ -1637,7 +1637,6 @@ diff_wc_wc(const char **anchor_path,
apr_pool_t *scratch_pool)
{
const char *abspath1;
- svn_error_t *err;
SVN_ERR_ASSERT(! svn_path_is_url(path1));
SVN_ERR_ASSERT(! svn_path_is_url(path2));
@@ -1656,25 +1655,6 @@ diff_wc_wc(const char **anchor_path,
)));
- /* Resolve named revisions to real numbers. */
- err = svn_client__get_revision_number(&callback_baton->revnum1, NULL,
- ctx->wc_ctx, abspath1, NULL,
- revision1, scratch_pool);
-
- /* In case of an added node, we have no base rev, and we show a revision
- * number of 0. Note that this code is currently always asking for
- * svn_opt_revision_base.
- * ### TODO: get rid of this 0 for added nodes. */
- if (err && (err->apr_err == SVN_ERR_CLIENT_BAD_REVISION))
- {
- svn_error_clear(err);
- callback_baton->revnum1 = 0;
- }
- else
- SVN_ERR(err);
-
- callback_baton->revnum2 = SVN_INVALID_REVNUM; /* WC */
-
if (! show_copies_as_adds && !use_git_diff_format)
{
/* ### Eventually we want to get rid of this wrapping as it loses
@@ -1683,17 +1663,39 @@ diff_wc_wc(const char **anchor_path,
diff_processor, scratch_pool);
}
- {
- svn_node_kind_t kind;
- SVN_ERR(svn_wc_read_kind2(&kind, ctx->wc_ctx, abspath1,
- TRUE, FALSE, scratch_pool));
+ if (callback_baton)
+ {
+ svn_node_kind_t kind;
+ svn_error_t *err;
- if (kind != svn_node_dir)
- callback_baton->anchor = svn_dirent_dirname(path1, scratch_pool);
- else
- callback_baton->anchor = path1;
- }
+ /* Resolve named revisions to real numbers. */
+ err = svn_client__get_revision_number(&callback_baton->revnum1, NULL,
+ ctx->wc_ctx, abspath1, NULL,
+ revision1, scratch_pool);
+
+ /* In case of an added node, we have no base rev, and we show a revision
+ * number of 0. Note that this code is currently always asking for
+ * svn_opt_revision_base.
+ * ### TODO: get rid of this 0 for added nodes. */
+ if (err && (err->apr_err == SVN_ERR_CLIENT_BAD_REVISION))
+ {
+ svn_error_clear(err);
+ callback_baton->revnum1 = 0;
+ }
+ else
+ SVN_ERR(err);
+
+ callback_baton->revnum2 = SVN_INVALID_REVNUM; /* WC */
+
+ SVN_ERR(svn_wc_read_kind2(&kind, ctx->wc_ctx, abspath1,
+ TRUE, FALSE, scratch_pool));
+
+ if (kind != svn_node_dir)
+ callback_baton->anchor = svn_dirent_dirname(path1, scratch_pool);
+ else
+ callback_baton->anchor = path1;
+ }
/* --git and --show-copies-as-adds imply --notice-ancestry */
if (use_git_diff_format || show_copies_as_adds)
@@ -1892,6 +1894,7 @@ diff_repos_wc(const char **anchor_path,
svn_boolean_t reverse,
svn_depth_t depth,
svn_boolean_t ignore_ancestry,
+ svn_boolean_t wrap_create,
svn_boolean_t show_copies_as_adds,
svn_boolean_t use_git_diff_format,
const apr_array_header_t *changelists,
@@ -1960,6 +1963,10 @@ diff_repos_wc(const char **anchor_path,
peg_revision,
revision1, NULL,
ctx, pool));
+ }
+
+ if (cmd_baton)
+ {
if (!reverse)
{
cmd_baton->orig_path_1 = url1;
@@ -1988,13 +1995,16 @@ diff_repos_wc(const char **anchor_path,
SVN_ERR(svn_wc_read_kind2(&kind2, ctx->wc_ctx, abspath2,
TRUE, FALSE, pool));
- cmd_baton->ra_session = ra_session;
- cmd_baton->anchor = anchor;
+ if (cmd_baton)
+ {
+ cmd_baton->ra_session = ra_session;
+ cmd_baton->anchor = anchor;
- if (!reverse)
- cmd_baton->revnum1 = rev;
- else
- cmd_baton->revnum2 = rev;
+ if (!reverse)
+ cmd_baton->revnum1 = rev;
+ else
+ cmd_baton->revnum2 = rev;
+ }
/* Check if our diff target is a copied node. */
SVN_ERR(svn_wc__node_get_origin(&is_copy,
@@ -2009,7 +2019,7 @@ diff_repos_wc(const char **anchor_path,
diff_processor = svn_diff__tree_processor_reverse_create(
diff_processor, NULL, pool);
- if (! show_copies_as_adds)
+ if (wrap_create)
diff_processor = svn_diff__tree_processor_copy_as_changed_create(
diff_processor, pool);
@@ -2051,7 +2061,8 @@ diff_repos_wc(const char **anchor_path,
const char *copyfrom_parent_url;
const char *copyfrom_basename;
- cmd_baton->repos_wc_diff_target_is_copy = TRUE;
+ if (cmd_baton)
+ cmd_baton->repos_wc_diff_target_is_copy = TRUE;
/* We're diffing a locally copied/moved node.
* Describe the copy source to the reporter instead of the copy itself.
@@ -2139,9 +2150,7 @@ diff_repos_wc(const char **anchor_path,
/* This is basically just the guts of svn_client_diff[_peg]6(). */
static svn_error_t *
do_diff(const char **anchor_path,
- const svn_wc_diff_callbacks4_t *callbacks,
struct diff_cmd_baton *callback_baton,
- svn_client_ctx_t *ctx,
const char *path_or_url1,
const char *path_or_url2,
const svn_opt_revision_t *revision1,
@@ -2152,22 +2161,18 @@ do_diff(const char **anchor_path,
svn_boolean_t show_copies_as_adds,
svn_boolean_t use_git_diff_format,
const apr_array_header_t *changelists,
+ const svn_diff_tree_processor_t *diff_processor,
+ svn_client_ctx_t *ctx,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
svn_boolean_t is_repos1;
svn_boolean_t is_repos2;
- const svn_diff_tree_processor_t *diff_processor;
/* Check if paths/revisions are urls/local. */
SVN_ERR(check_paths(&is_repos1, &is_repos2, path_or_url1, path_or_url2,
revision1, revision2, peg_revision));
- SVN_ERR(svn_wc__wrap_diff_callbacks(&diff_processor,
- callbacks, callback_baton,
- TRUE /* walk_deleted_dirs */,
- scratch_pool, scratch_pool));
-
if (is_repos1)
{
if (is_repos2)
@@ -2187,7 +2192,9 @@ do_diff(const char **anchor_path,
SVN_ERR(diff_repos_wc(anchor_path,
path_or_url1, revision1, peg_revision,
path_or_url2, revision2, FALSE, depth,
- ignore_ancestry, show_copies_as_adds,
+ ignore_ancestry,
+ ! show_copies_as_adds/* wrap_create */,
+ show_copies_as_adds,
use_git_diff_format, changelists,
callback_baton,
diff_processor, ctx,
@@ -2201,7 +2208,9 @@ do_diff(const char **anchor_path,
SVN_ERR(diff_repos_wc(anchor_path,
path_or_url2, revision2, peg_revision,
path_or_url1, revision1, TRUE, depth,
- ignore_ancestry, show_copies_as_adds,
+ ignore_ancestry,
+ ! show_copies_as_adds/* wrap_create */,
+ show_copies_as_adds,
use_git_diff_format, changelists,
callback_baton,
diff_processor, ctx,
@@ -2241,111 +2250,6 @@ do_diff(const char **anchor_path,
return SVN_NO_ERROR;
}
-/* Perform a diff between a repository path and a working-copy path.
-
- PATH_OR_URL1 may be either a URL or a working copy path. PATH2 is a
- working copy path. REVISION1 and REVISION2 are their respective
- revisions. If REVERSE is TRUE, the diff will be done in reverse.
- If PEG_REVISION is specified, then PATH_OR_URL1 is the path in the peg
- revision, and the actual repository path to be compared is
- determined by following copy history.
-
- All other options are the same as those passed to svn_client_diff6(). */
-static svn_error_t *
-diff_summarize_repos_wc(svn_client_diff_summarize_func_t summarize_func,
- void *summarize_baton,
- const char *path_or_url1,
- const svn_opt_revision_t *revision1,
- const svn_opt_revision_t *peg_revision,
- const char *path2,
- const svn_opt_revision_t *revision2,
- svn_boolean_t reverse,
- svn_depth_t depth,
- svn_boolean_t ignore_ancestry,
- const apr_array_header_t *changelists,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
-{
- struct diff_cmd_baton cmd_baton;
- const svn_diff_tree_processor_t *diff_processor;
- const char **anchor_path;
-
- SVN_ERR_ASSERT(! svn_path_is_url(path2));
-
- SVN_ERR(svn_client__get_diff_summarize_callbacks(
- &diff_processor, &anchor_path,
- summarize_func, summarize_baton,
- path2, pool, pool));
-
- if (reverse)
- diff_processor = svn_diff__tree_processor_reverse_create(
- diff_processor, NULL, pool);
-
- SVN_ERR(diff_repos_wc(anchor_path,
- path_or_url1, revision1, peg_revision,
- path2, revision2, reverse,
- depth, FALSE, TRUE, FALSE, changelists,
- &cmd_baton,
- diff_processor, ctx, pool, pool));
- return SVN_NO_ERROR;
-}
-
-/* Perform a summary diff between two working-copy paths.
-
- PATH1 and PATH2 are both working copy paths. REVISION1 and
- REVISION2 are their respective revisions.
-
- All other options are the same as those passed to svn_client_diff6(). */
-static svn_error_t *
-diff_summarize_wc_wc(svn_client_diff_summarize_func_t summarize_func,
- void *summarize_baton,
- const char *path1,
- const svn_opt_revision_t *revision1,
- const char *path2,
- const svn_opt_revision_t *revision2,
- svn_depth_t depth,
- svn_boolean_t ignore_ancestry,
- const apr_array_header_t *changelists,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
-{
- const char *abspath1;
- const svn_diff_tree_processor_t *diff_processor;
- const char **anchor_path;
-
- SVN_ERR_ASSERT(! svn_path_is_url(path1));
- SVN_ERR_ASSERT(! svn_path_is_url(path2));
-
- /* Currently we support only the case where path1 and path2 are the
- same path. */
- if ((strcmp(path1, path2) != 0)
- || (! ((revision1->kind == svn_opt_revision_base)
- && (revision2->kind == svn_opt_revision_working))))
- return unsupported_diff_error
- (svn_error_create
- (SVN_ERR_INCORRECT_PARAMS, NULL,
- _("Summarized diffs are only supported between a path's text-base "
- "and its working files at this time")));
-
- /* Find the node kind of PATH1 so that we know whether the diff drive will
- be anchored at PATH1 or its parent dir. */
- SVN_ERR(svn_dirent_get_absolute(&abspath1, path1, pool));
-
- SVN_ERR(svn_client__get_diff_summarize_callbacks(
- &diff_processor, &anchor_path,
- summarize_func, summarize_baton, path1,
- pool, pool));
-
- SVN_ERR(svn_wc__diff7(anchor_path,
- ctx->wc_ctx,
- abspath1,
- depth, ignore_ancestry, changelists,
- diff_processor,
- ctx->cancel_func, ctx->cancel_baton,
- pool, pool));
- return SVN_NO_ERROR;
-}
-
/* This is basically just the guts of svn_client_diff_summarize[_peg]2(). */
static svn_error_t *
do_diff_summarize(svn_client_diff_summarize_func_t summarize_func,
@@ -2366,6 +2270,12 @@ do_diff_summarize(svn_client_diff_summar
svn_boolean_t is_repos1;
svn_boolean_t is_repos2;
+ SVN_ERR(svn_client__get_diff_summarize_callbacks(
+ &diff_processor, &anchor_path,
+ summarize_func, summarize_baton,
+ path_or_url1,
+ pool, pool));
+
/* Check if paths/revisions are urls/local. */
SVN_ERR(check_paths(&is_repos1, &is_repos2, path_or_url1, path_or_url2,
revision1, revision2, peg_revision));
@@ -2373,14 +2283,6 @@ do_diff_summarize(svn_client_diff_summar
if (is_repos1)
{
if (is_repos2)
- {
-
- SVN_ERR(svn_client__get_diff_summarize_callbacks(
- &diff_processor,
- &anchor_path,
- summarize_func, summarize_baton,
- path_or_url1,
- pool, pool));
SVN_ERR(diff_repos_repos(anchor_path, NULL,
path_or_url1, path_or_url2,
revision1, revision2,
@@ -2388,28 +2290,29 @@ do_diff_summarize(svn_client_diff_summar
FALSE /* text_deltas */,
diff_processor, ctx,
pool, pool));
- }
else
- SVN_ERR(diff_summarize_repos_wc(summarize_func, summarize_baton,
- path_or_url1, revision1,
- peg_revision,
- path_or_url2, revision2,
- FALSE, depth,
- ignore_ancestry,
- changelists,
- ctx, pool));
+ SVN_ERR(diff_repos_wc(anchor_path,
+ path_or_url1, revision1, peg_revision,
+ path_or_url2, revision2, FALSE /* reverse*/,
+ depth, ignore_ancestry,
+ FALSE /* wrap_create */,
+ FALSE /* show_copies_as_adds */,
+ FALSE /* use_git_diff_format */,
+ changelists, NULL,
+ diff_processor, ctx, pool, pool));
}
else /* ! is_repos1 */
{
if (is_repos2)
- SVN_ERR(diff_summarize_repos_wc(summarize_func, summarize_baton,
- path_or_url2, revision2,
- peg_revision,
- path_or_url1, revision1,
- TRUE, depth,
- ignore_ancestry,
- changelists,
- ctx, pool));
+ SVN_ERR(diff_repos_wc(anchor_path,
+ path_or_url2, revision2, peg_revision,
+ path_or_url1, revision1, TRUE /* reverse */,
+ depth, ignore_ancestry,
+ FALSE /* wrap_create */,
+ FALSE /* show_copies_as_adds */,
+ FALSE /* use_git_diff_format */,
+ changelists, NULL,
+ diff_processor, ctx, pool, pool));
else
{
if (revision1->kind == svn_opt_revision_working
@@ -2420,12 +2323,6 @@ do_diff_summarize(svn_client_diff_summar
SVN_ERR(svn_dirent_get_absolute(&abspath1, path_or_url1, pool));
SVN_ERR(svn_dirent_get_absolute(&abspath2, path_or_url2, pool));
- SVN_ERR(svn_client__get_diff_summarize_callbacks(
- &diff_processor, &anchor_path,
- summarize_func, summarize_baton,
- path_or_url1,
- pool, pool));
-
SVN_ERR(svn_client__arbitrary_nodes_diff(anchor_path,
abspath1, abspath2,
depth,
@@ -2433,11 +2330,12 @@ do_diff_summarize(svn_client_diff_summar
ctx, pool, pool));
}
else
- SVN_ERR(diff_summarize_wc_wc(summarize_func, summarize_baton,
- path_or_url1, revision1,
- path_or_url2, revision2,
- depth, ignore_ancestry,
- changelists, ctx, pool));
+ SVN_ERR(diff_wc_wc(anchor_path,
+ path_or_url1, revision1,
+ path_or_url2, revision2,
+ depth, ignore_ancestry,
+ FALSE, FALSE, changelists, NULL,
+ diff_processor, ctx, pool, pool));
}
}
@@ -2570,6 +2468,7 @@ svn_client_diff6(const apr_array_header_
{
struct diff_cmd_baton diff_cmd_baton = { 0 };
svn_opt_revision_t peg_revision;
+ const svn_diff_tree_processor_t *diff_processor;
if (ignore_properties && properties_only)
return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
@@ -2605,11 +2504,18 @@ svn_client_diff6(const apr_array_header_
diff_cmd_baton.ra_session = NULL;
diff_cmd_baton.anchor = NULL;
- return do_diff(NULL, &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, pool);
+ SVN_ERR(svn_wc__wrap_diff_callbacks(&diff_processor,
+ &diff_callbacks, &diff_cmd_baton,
+ TRUE /* walk_deleted_dirs */,
+ pool, pool));
+
+ return svn_error_trace(do_diff(NULL, &diff_cmd_baton,
+ path_or_url1, path_or_url2,
+ revision1, revision2,
+ &peg_revision,
+ depth, ignore_ancestry, show_copies_as_adds,
+ use_git_diff_format, changelists,
+ diff_processor, ctx, pool, pool));
}
svn_error_t *
@@ -2636,6 +2542,7 @@ svn_client_diff_peg6(const apr_array_hea
apr_pool_t *pool)
{
struct diff_cmd_baton diff_cmd_baton = { 0 };
+ const svn_diff_tree_processor_t *diff_processor;
if (ignore_properties && properties_only)
return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
@@ -2668,11 +2575,17 @@ svn_client_diff_peg6(const apr_array_hea
diff_cmd_baton.ra_session = NULL;
diff_cmd_baton.anchor = NULL;
- return do_diff(NULL, &diff_callbacks, &diff_cmd_baton, ctx,
- path_or_url, path_or_url, start_revision, end_revision,
- peg_revision,
- depth, ignore_ancestry, show_copies_as_adds,
- use_git_diff_format, changelists, pool, pool);
+ SVN_ERR(svn_wc__wrap_diff_callbacks(&diff_processor,
+ &diff_callbacks, &diff_cmd_baton,
+ TRUE /* walk_deleted_dirs */,
+ pool, pool));
+
+ return svn_error_trace(do_diff(NULL, &diff_cmd_baton,
+ path_or_url, path_or_url,
+ start_revision, end_revision, peg_revision,
+ depth, ignore_ancestry, show_copies_as_adds,
+ use_git_diff_format, changelists,
+ diff_processor, ctx, pool, pool));
}
svn_error_t *