Sending again with a non-null attachment MIME-type...
Steve
Quoting Stephen Butler <[email protected]>:
Hi folks,
here's a work in progress for fixing a long-standing diff bug.
"'svn diff URL1 URL2' not reverse of 'svn diff URL2 URL1'"
http://subversion.tigris.org/issues/show_bug.cgi?id=2333
The basic approach was suggested by cmpilato in a comment
to the issue. In repos_diff.c:delete_entry(), I call svn_client_list2(),
handing it a callback that prints diffs for all files in the "old"
tree. It's a bit awkward to call a client API function in repos_diff,
whose edit_baton was never yet sullied by an svn_client_ctx_t.
The ugliest hack (so far) is in diff_deleted_dir_cb(), the callback.
I need to hand get_file_from_ra() a path relative to the RA
session's URL.
TODOs include:
Find out why diff_tests.py 28 still fails.
Investigate other failing diff tests.
Include dir diffs in the list2-callback.
Handle authz failures gracefully.
Anything else?
Comments welcome.
Steve
--
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Index: subversion/tests/cmdline/diff_tests.py
===================================================================
--- subversion/tests/cmdline/diff_tests.py (revision 981661)
+++ subversion/tests/cmdline/diff_tests.py (working copy)
@@ -3533,7 +3533,7 @@ test_list = [ None,
diff_keywords,
diff_force,
diff_schedule_delete,
- XFail(diff_renamed_dir),
+ diff_renamed_dir,
diff_property_changes_to_base,
diff_mime_type_changes,
diff_prop_change_local_propmod,
Index: subversion/libsvn_client/repos_diff.c
===================================================================
--- subversion/libsvn_client/repos_diff.c (revision 981661)
+++ subversion/libsvn_client/repos_diff.c (working copy)
@@ -54,6 +54,10 @@ struct edit_baton {
repository operation. */
svn_wc_context_t *wc_ctx;
+ /* A client context passed to client APIs when fetching missing data
+ during an editor drive. May be NULL. */
+ svn_client_ctx_t *ctx;
+
/* The callback and calback argument that implement the file comparison
function */
const svn_wc_diff_callbacks4_t *diff_callbacks;
@@ -89,6 +93,14 @@ struct edit_baton {
svn_wc_notify_func2_t notify_func;
void *notify_baton;
+ /* TRUE if the operation needs to walk deleted dirs on the "old" side.
+ FALSE otherwise. */
+ svn_boolean_t walk_deleted_dirs;
+
+ /* If WALK_DELETED_DIRS is TRUE, the walk callback will need the repos
+ root URL. Otherwise NULL. */
+ const char *root_url;
+
apr_pool_t *pool;
};
@@ -443,6 +455,59 @@ open_root(void *edit_baton,
return SVN_NO_ERROR;
}
+/* This implements the svn_client_list_func_t API. To be called during
+ URL-URL diffs only. Prints diff output for each file in a deleted dir
+ in the "old" URL. */
+static svn_error_t *
+diff_deleted_tree_cb(void *baton,
+ const char *path,
+ const svn_dirent_t *dirent,
+ const svn_lock_t *lock,
+ const char *abs_path,
+ apr_pool_t *pool)
+{
+ struct edit_baton *eb = baton;
+
+ if (dirent->kind == svn_node_file)
+ {
+ const char *file_relpath, *file_path, *url;
+ struct file_baton *b;
+ const char *mimetype1, *mimetype2;
+ svn_wc_notify_state_t state = svn_wc_notify_state_inapplicable;
+ svn_boolean_t tree_conflicted = FALSE;
+
+ /* Compare a file being deleted against an empty file */
+
+ /* ### Hack! get_file_from_ra() expects the file's repository path
+ relative to the RA session's URL. */
+ file_path = svn_dirent_join(abs_path, path, pool);
+ while (*file_path && *file_path == '/')
+ file_path++;
+ url = svn_uri_join(eb->root_url, file_path, pool);
+ svn_ra_get_path_relative_to_session(eb->ra_session,
+ &file_relpath,
+ url,
+ pool);
+ b = make_file_baton(file_relpath, FALSE, eb, pool);
+ SVN_ERR(get_file_from_ra(b, eb->revision));
+
+ SVN_ERR(get_empty_file(b->edit_baton, &(b->path_end_revision)));
+
+ get_file_mime_types(&mimetype1, &mimetype2, b);
+
+ SVN_ERR(eb->diff_callbacks->file_deleted
+ (NULL, &state, &tree_conflicted, b->wcpath,
+ b->path_start_revision,
+ b->path_end_revision,
+ mimetype1, mimetype2,
+ b->pristine_props,
+ b->edit_baton->diff_cmd_baton,
+ pool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* An editor function. */
static svn_error_t *
delete_entry(const char *path,
@@ -500,6 +565,34 @@ delete_entry(const char *path,
(local_dir_abspath, &state, &tree_conflicted,
svn_dirent_join(eb->target, path, pool),
eb->diff_cmd_baton, pool));
+
+ /* A URL-URL diff's editor will skip the content of a deleted
+ dir, so we'll walk the directory's "old" URL to display all
+ file content as deleted. */
+ if (eb->walk_deleted_dirs)
+ {
+ const char *dir_url, *session_url;
+ svn_opt_revision_t revision;
+
+ SVN_ERR(svn_ra_get_session_url(eb->ra_session,
+ &session_url,
+ pool));
+ dir_url = svn_uri_join(session_url, path, pool);
+
+ revision.kind = svn_opt_revision_number;
+ revision.value.number = eb->revision;
+
+ SVN_ERR(svn_client_list2(dir_url,
+ &revision,
+ &revision,
+ svn_depth_infinity,
+ SVN_DIRENT_KIND,
+ FALSE,
+ diff_deleted_tree_cb,
+ eb,
+ eb->ctx,
+ pool));
+ }
break;
}
default:
@@ -1180,6 +1273,7 @@ absent_file(const char *path,
svn_error_t *
svn_client__get_diff_editor(const char *target,
svn_wc_context_t *wc_ctx,
+ svn_client_ctx_t *ctx,
const svn_wc_diff_callbacks4_t *diff_callbacks,
void *diff_cmd_baton,
svn_depth_t depth,
@@ -1198,10 +1292,13 @@ svn_client__get_diff_editor(const char *target,
svn_delta_editor_t *tree_editor = svn_delta_default_editor(subpool);
struct edit_baton *eb = apr_palloc(subpool, sizeof(*eb));
const char *target_abspath;
+ const char *root_url;
SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, pool));
+ SVN_ERR(svn_client_root_url_from_path(&root_url, target, ctx, pool));
eb->target = target;
eb->wc_ctx = wc_ctx;
+ eb->ctx = ctx;
eb->diff_callbacks = diff_callbacks;
eb->diff_cmd_baton = diff_cmd_baton;
eb->dry_run = dry_run;
@@ -1213,6 +1310,8 @@ svn_client__get_diff_editor(const char *target,
eb->pool = subpool;
eb->notify_func = notify_func;
eb->notify_baton = notify_baton;
+ eb->walk_deleted_dirs = TRUE;
+ eb->root_url = root_url;
tree_editor->set_target_revision = set_target_revision;
tree_editor->open_root = open_root;
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 981661)
+++ subversion/libsvn_client/client.h (working copy)
@@ -631,6 +631,9 @@ svn_client__switch_internal(svn_revnum_t *result_r
WC_CTX is a context for the working copy and should be NULL for
operations that do not involve a working copy.
+ CTX is a client context passed to client APIs when fetching missing
+ data during an editor drive. May be NULL.
+
DIFF_CMD/DIFF_CMD_BATON represent the callback and callback argument that
implement the file comparison function
@@ -650,6 +653,7 @@ svn_client__switch_internal(svn_revnum_t *result_r
svn_error_t *
svn_client__get_diff_editor(const char *target,
svn_wc_context_t *wc_ctx,
+ svn_client_ctx_t *ctx,
const svn_wc_diff_callbacks4_t *diff_cmd,
void *diff_cmd_baton,
svn_depth_t depth,
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 981661)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -4881,6 +4881,7 @@ drive_merge_report_editor(const char *target_abspa
/* Get the diff editor and a reporter with which to, ultimately,
drive it. */
SVN_ERR(svn_client__get_diff_editor(target_abspath, merge_b->ctx->wc_ctx,
+ NULL,
&merge_callbacks, merge_b, depth,
merge_b->dry_run,
merge_b->ra_session2, revision1,
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 981661)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1601,7 +1601,7 @@ diff_repos_repos(const struct diff_parameters *dif
Otherwise, we just use "". */
SVN_ERR(svn_client__get_diff_editor
(drr.base_path ? drr.base_path : "",
- NULL, callbacks, callback_baton, diff_param->depth,
+ NULL, ctx, callbacks, callback_baton, diff_param->depth,
FALSE /* doesn't matter for diff */, extra_ra_session, drr.rev1,
NULL /* no notify_func */, NULL /* no notify_baton */,
ctx->cancel_func, ctx->cancel_baton,
