You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sb...@apache.org on 2010/08/09 18:36:51 UTC
svn commit: r983720 - in /subversion/trunk/subversion:
libsvn_client/repos_diff.c tests/cmdline/diff_tests.py
Author: sbutler
Date: Mon Aug 9 16:36:51 2010
New Revision: 983720
URL: http://svn.apache.org/viewvc?rev=983720&view=rev
Log:
Fix issue 2333 "diff URL1 URL2 not reverse of diff URL2 URL1". When
the repository reports a deleted directory, recursively walk the
directory (in the repository) and report files as deleted.
TODO: Handle non-infinite depth correctly.
Review by: rhuijben
stsp
* subversion/libsvn_client/repos_diff.c
(edit_baton): Add a boolean field to control whether this workaround
should be used. Add a func and baton for cancellation.
(diff_deleted_dir): New function.
(delete_entry): Call diff_deleted_dir() if needed.
(svn_client__get_diff_editor): Set the new edit_baton fields.
* subversion/tests/cmdline/diff_tests.py
(diff_multiple_reverse): Remove a comment that made this test the moral
equivalent of an XFAIL.
(diff_renamed_dir): Add more test cases. Correct the expectations for
diffs within a moved directory.
(test_list): Remove XFail from diff_renamed_dir.
Modified:
subversion/trunk/subversion/libsvn_client/repos_diff.c
subversion/trunk/subversion/tests/cmdline/diff_tests.py
Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=983720&r1=983719&r2=983720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/repos_diff.c Mon Aug 9 16:36:51 2010
@@ -89,6 +89,17 @@ 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_repos_dirs;
+
+ /* A callback used to see if the client wishes to cancel the running
+ operation. */
+ svn_cancel_func_t cancel_func;
+
+ /* A baton to pass to the cancellation callback. */
+ void *cancel_baton;
+
apr_pool_t *pool;
};
@@ -443,6 +454,90 @@ open_root(void *edit_baton,
return SVN_NO_ERROR;
}
+/* Recursively walk tree rooted at DIR (at REVISION) in the repository,
+ * reporting all files as deleted. Part of a workaround for issue 2333.
+ *
+ * DIR is a repository path relative to the URL in RA_SESSION. REVISION
+ * may be NULL, in which case it defaults to HEAD. EDIT_BATON is the
+ * overall crawler editor baton. If CANCEL_FUNC is not NULL, then it
+ * should refer to a cancellation function (along with CANCEL_BATON).
+ */
+/* ### TODO: Handle depth. */
+static svn_error_t *
+diff_deleted_dir(const char *dir,
+ svn_revnum_t revision,
+ svn_ra_session_t *ra_session,
+ void *edit_baton,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ apr_pool_t *pool)
+{
+ struct edit_baton *eb = edit_baton;
+ apr_hash_t *dirents;
+ apr_pool_t *iterpool = svn_pool_create(pool);
+ apr_hash_index_t *hi;
+
+ if (cancel_func)
+ SVN_ERR(cancel_func(cancel_baton));
+
+ SVN_ERR(svn_ra_get_dir2(ra_session,
+ &dirents,
+ NULL, NULL,
+ dir,
+ revision,
+ SVN_DIRENT_KIND,
+ pool));
+
+ for (hi = apr_hash_first(pool, dirents); hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *path;
+ const char *name = svn__apr_hash_index_key(hi);
+ svn_dirent_t *dirent = svn__apr_hash_index_val(hi);
+
+ svn_pool_clear(iterpool);
+
+ path = svn_relpath_join(dir, name, iterpool);
+
+ if (dirent->kind == svn_node_file)
+ {
+ 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 */
+ b = make_file_baton(path, FALSE, eb, iterpool);
+ SVN_ERR(get_file_from_ra(b, 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));
+ }
+
+ if (dirent->kind == svn_node_dir)
+ SVN_ERR(diff_deleted_dir(path,
+ revision,
+ ra_session,
+ eb,
+ cancel_func,
+ cancel_baton,
+ iterpool));
+ }
+
+ svn_pool_destroy(iterpool);
+ return SVN_NO_ERROR;
+}
+
/* An editor function. */
static svn_error_t *
delete_entry(const char *path,
@@ -500,6 +595,20 @@ delete_entry(const char *path,
(local_dir_abspath, &state, &tree_conflicted,
svn_dirent_join(eb->target, path, pool),
eb->diff_cmd_baton, pool));
+
+ if (eb->walk_deleted_repos_dirs)
+ {
+ /* A workaround for issue 2333. The "old" dir will be
+ skipped by the repository report. Crawl it recursively,
+ diffing each file against the empty file. */
+ SVN_ERR(diff_deleted_dir(path,
+ eb->revision,
+ eb->ra_session,
+ eb,
+ eb->cancel_func,
+ eb->cancel_baton,
+ pool));
+ }
break;
}
default:
@@ -1213,6 +1322,9 @@ svn_client__get_diff_editor(const char *
eb->pool = subpool;
eb->notify_func = notify_func;
eb->notify_baton = notify_baton;
+ eb->walk_deleted_repos_dirs = TRUE;
+ eb->cancel_func = cancel_func;
+ eb->cancel_baton = cancel_baton;
tree_editor->set_target_revision = set_target_revision;
tree_editor->open_root = open_root;
Modified: subversion/trunk/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/diff_tests.py?rev=983720&r1=983719&r2=983720&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/diff_tests.py Mon Aug 9 16:36:51 2010
@@ -565,8 +565,7 @@ def diff_multiple_reverse(sbox):
repo_diff(wc_dir, 4, 1, check_add_a_file_in_a_subdir)
repo_diff(wc_dir, 4, 1, check_add_a_file)
repo_diff(wc_dir, 1, 4, check_update_a_file)
-### TODO: directory delete doesn't work yet
-# repo_diff(wc_dir, 1, 4, check_add_a_file_in_a_subdir_reverse)
+ repo_diff(wc_dir, 1, 4, check_add_a_file_in_a_subdir_reverse)
repo_diff(wc_dir, 1, 4, check_add_a_file_reverse)
# test 6
@@ -1921,6 +1920,7 @@ def diff_renamed_dir(sbox):
'A') :
raise svntest.Failure
+ # Commit
svntest.actions.run_and_verify_svn(None, None, [],
'ci', '-m', 'log msg')
@@ -1949,20 +1949,62 @@ def diff_renamed_dir(sbox):
'A') :
raise svntest.Failure
- # Test the diff while within the moved directory
- os.chdir(os.path.join('A','D','I'))
+ # repos->repos with explicit URL arg
+ exit_code, diff_output, err_output = svntest.main.run_svn(None, 'diff',
+ '-r', '1:2',
+ '^/A')
+ if check_diff_output(diff_output,
+ os.path.join('D', 'G', 'pi'),
+ 'D') :
+ raise svntest.Failure
+ if check_diff_output(diff_output,
+ os.path.join('D', 'I', 'pi'),
+ 'A') :
+ raise svntest.Failure
+ # Go to the parent of the moved directory
+ os.chdir(os.path.join('A','D'))
+
+ # repos->wc diff in the parent
exit_code, diff_output, err_output = svntest.main.run_svn(None, 'diff',
'-r', '1')
- if check_diff_output(diff_output, 'pi', 'A') :
+ if check_diff_output(diff_output,
+ os.path.join('G', 'pi'),
+ 'D') :
+ raise svntest.Failure
+ if check_diff_output(diff_output,
+ os.path.join('I', 'pi'),
+ 'A') :
raise svntest.Failure
- # Test a repos->repos diff while within the moved directory
+ # repos->repos diff in the parent
+ exit_code, diff_output, err_output = svntest.main.run_svn(None, 'diff',
+ '-r', '1:2')
+
+ if check_diff_output(diff_output,
+ os.path.join('G', 'pi'),
+ 'D') :
+ raise svntest.Failure
+ if check_diff_output(diff_output,
+ os.path.join('I', 'pi'),
+ 'A') :
+ raise svntest.Failure
+
+ # Go to the move target directory
+ os.chdir('I')
+
+ # repos->wc diff while within the moved directory (should be empty)
+ exit_code, diff_output, err_output = svntest.main.run_svn(None, 'diff',
+ '-r', '1')
+ if diff_output:
+ raise svntest.Failure
+
+ # repos->repos diff while within the moved directory (should be empty)
exit_code, diff_output, err_output = svntest.main.run_svn(None, 'diff',
'-r', '1:2')
- if check_diff_output(diff_output, 'pi', 'A') :
+ if diff_output:
raise svntest.Failure
@@ -3584,7 +3626,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,
Re: svn commit: r983720 - in /subversion/trunk/subversion:
libsvn_client/repos_diff.c tests/cmdline/diff_tests.py
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2010-08-09, sbutler@apache.org wrote:
> Fix issue 2333 "diff URL1 URL2 not reverse of diff URL2 URL1". When
> the repository reports a deleted directory, recursively walk the
> directory (in the repository) and report files as deleted.
>
> TODO: Handle non-infinite depth correctly.
>
> Review by: rhuijben
> stsp
>
> * subversion/libsvn_client/repos_diff.c
> (edit_baton): Add a boolean field to control whether this workaround
> should be used. Add a func and baton for cancellation.
> (diff_deleted_dir): New function.
> (delete_entry): Call diff_deleted_dir() if needed.
> (svn_client__get_diff_editor): Set the new edit_baton fields.
>
> * subversion/tests/cmdline/diff_tests.py
> (diff_multiple_reverse): Remove a comment that made this test the moral
> equivalent of an XFAIL.
> (diff_renamed_dir): Add more test cases. Correct the expectations for
> diffs within a moved directory.
> (test_list): Remove XFail from diff_renamed_dir.
[...]
> +/* Recursively walk tree rooted at DIR (at REVISION) in the repository,
> + * reporting all files as deleted. Part of a workaround for issue 2333.
> + *
> + * DIR is a repository path relative to the URL in RA_SESSION. REVISION
> + * may be NULL, in which case it defaults to HEAD. EDIT_BATON is the
Does REVISION really need to be able to default to HEAD? I wouldn't
have thought so. (If so, that would be SVN_INVALID_REVNUM not NULL.)
> + * overall crawler editor baton. If CANCEL_FUNC is not NULL, then it
> + * should refer to a cancellation function (along with CANCEL_BATON).
> + */
> +/* ### TODO: Handle depth. */
> +static svn_error_t *
> +diff_deleted_dir(const char *dir,
> + svn_revnum_t revision,
> + svn_ra_session_t *ra_session,
> + void *edit_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool)
- Julian
Re: svn commit: r983720 - in /subversion/trunk/subversion:
libsvn_client/repos_diff.c tests/cmdline/diff_tests.py
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2010-08-09, sbutler@apache.org wrote:
> Fix issue 2333 "diff URL1 URL2 not reverse of diff URL2 URL1". When
> the repository reports a deleted directory, recursively walk the
> directory (in the repository) and report files as deleted.
>
> TODO: Handle non-infinite depth correctly.
>
> Review by: rhuijben
> stsp
>
> * subversion/libsvn_client/repos_diff.c
> (edit_baton): Add a boolean field to control whether this workaround
> should be used. Add a func and baton for cancellation.
> (diff_deleted_dir): New function.
> (delete_entry): Call diff_deleted_dir() if needed.
> (svn_client__get_diff_editor): Set the new edit_baton fields.
>
> * subversion/tests/cmdline/diff_tests.py
> (diff_multiple_reverse): Remove a comment that made this test the moral
> equivalent of an XFAIL.
> (diff_renamed_dir): Add more test cases. Correct the expectations for
> diffs within a moved directory.
> (test_list): Remove XFail from diff_renamed_dir.
[...]
> +/* Recursively walk tree rooted at DIR (at REVISION) in the repository,
> + * reporting all files as deleted. Part of a workaround for issue 2333.
> + *
> + * DIR is a repository path relative to the URL in RA_SESSION. REVISION
> + * may be NULL, in which case it defaults to HEAD. EDIT_BATON is the
Does REVISION really need to be able to default to HEAD? I wouldn't
have thought so. (If so, that would be SVN_INVALID_REVNUM not NULL.)
> + * overall crawler editor baton. If CANCEL_FUNC is not NULL, then it
> + * should refer to a cancellation function (along with CANCEL_BATON).
> + */
> +/* ### TODO: Handle depth. */
> +static svn_error_t *
> +diff_deleted_dir(const char *dir,
> + svn_revnum_t revision,
> + svn_ra_session_t *ra_session,
> + void *edit_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool)
- Julian