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 2013/02/08 19:43:08 UTC
svn commit: r1444183 - in /subversion/trunk/subversion: libsvn_client/diff.c
libsvn_wc/diff_editor.c tests/cmdline/diff_tests.py
Author: rhuijben
Date: Fri Feb 8 18:43:08 2013
New Revision: 1444183
URL: http://svn.apache.org/r1444183
Log:
Make 'svn diff -r 123 MOVED-PATH' use the normal diff editor instead of custom
code that reimplemented an ignore-ancestry diff for files.
Also fix some ignore ancestry handling in libsvn_wc's diff editor to make the
results match what was expected by the test suite.
As long as the diff editor isn't updated to properly handle -r BASE as either
CHECKEDOUT(=op_depth 0) or PRISTINE(=working), this will be a similar hack
as before, but at least not multiple ones.
* subversion/libsvn_client/diff.c
(diff_repos_repos_added_or_deleted_file): Provide what the repos-repos diff
does.
(diff_repos_wc_file_target): Remove function.
(diff_repos_wc): Add support for reporting a switched file to the server and
use that instead of diff_repos_wc_file_target().
* subversion/libsvn_wc/diff_editor.c
(dir_baton): Allow access to parent baton. Add reference count.
(file_baton): Add parent baton.
(make_dir_baton,
make_file_baton): Initialize reference to parent baton
(maybe_done): New function.
(close_directory): Call maybe_done.
(close_file): Delay getting the add kind a bit to make it easy to implement
proper ignore-ancestry support. Call maybe_done on parent baton.
* subversion/tests/cmdline/diff_tests.py
(diff_head_of_moved_file): Expect specific output instead of any output.
(diff_url_against_local_mods): Remove XFail.
Modified:
subversion/trunk/subversion/libsvn_client/diff.c
subversion/trunk/subversion/libsvn_wc/diff_editor.c
subversion/trunk/subversion/tests/cmdline/diff_tests.py
Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1444183&r1=1444182&r2=1444183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Fri Feb 8 18:43:08 2013
@@ -1707,10 +1707,10 @@ diff_repos_repos_added_or_deleted_file(c
svn_prop_get_value(prop_hash,
SVN_PROP_MIME_TYPE),
NULL, SVN_INVALID_REVNUM,
- make_regular_props_array(prop_hash,
- scratch_pool,
- scratch_pool),
- NULL, callback_baton, scratch_pool));
+ svn_prop_hash_to_array(prop_hash,
+ scratch_pool),
+ apr_hash_make(scratch_pool),
+ callback_baton, scratch_pool));
}
return SVN_NO_ERROR;
@@ -2009,154 +2009,6 @@ diff_repos_repos(const svn_wc_diff_callb
return reporter->finish_report(reporter_baton, pool);
}
-
-/* Using CALLBACKS, show a REPOS->WC diff for a file TARGET, which in the
- * working copy is at FILE2_ABSPATH. KIND1 is the node kind of the repository
- * target (either svn_node_file or svn_node_none). REV is the revision the
- * working file is diffed against. RA_SESSION points at the URL of the file
- * in the repository and is used to get the file's repository-version content,
- * if necessary. The other parameters are as in diff_repos_wc(). */
-static svn_error_t *
-diff_repos_wc_file_target(const char *target,
- const char *file2_abspath,
- svn_node_kind_t kind1,
- svn_revnum_t rev,
- svn_boolean_t reverse,
- svn_boolean_t show_copies_as_adds,
- const svn_wc_diff_callbacks4_t *callbacks,
- void *callback_baton,
- svn_ra_session_t *ra_session,
- svn_client_ctx_t *ctx,
- apr_pool_t *scratch_pool)
-{
- const char *file1_abspath;
- svn_stream_t *file1_content;
- svn_stream_t *file2_content;
- apr_hash_t *file1_props = NULL;
- apr_hash_t *file2_props;
- svn_boolean_t is_copy = FALSE;
- apr_hash_t *keywords = NULL;
- svn_string_t *keywords_prop;
- svn_subst_eol_style_t eol_style;
- const char *eol_str;
-
- /* Get content and props of file 1 (the remote file). */
- SVN_ERR(svn_stream_open_unique(&file1_content, &file1_abspath, NULL,
- svn_io_file_del_on_pool_cleanup,
- scratch_pool, scratch_pool));
- if (kind1 == svn_node_file)
- {
- if (show_copies_as_adds)
- SVN_ERR(svn_wc__node_get_origin(&is_copy,
- NULL, NULL, NULL, NULL, NULL,
- ctx->wc_ctx, file2_abspath,
- FALSE, scratch_pool, scratch_pool));
- /* If showing copies as adds, diff against the empty file. */
- if (!(show_copies_as_adds && is_copy))
- SVN_ERR(svn_ra_get_file(ra_session, "", rev, file1_content,
- NULL, &file1_props, scratch_pool));
- }
-
- SVN_ERR(svn_stream_close(file1_content));
-
- SVN_ERR(svn_wc_prop_list2(&file2_props, ctx->wc_ctx, file2_abspath,
- scratch_pool, scratch_pool));
-
- /* We might have to create a normalised version of the working file. */
- svn_subst_eol_style_from_value(&eol_style, &eol_str,
- svn_prop_get_value(file2_props,
- SVN_PROP_EOL_STYLE));
- keywords_prop = apr_hash_get(file2_props, SVN_PROP_KEYWORDS,
- APR_HASH_KEY_STRING);
- if (keywords_prop)
- SVN_ERR(svn_subst_build_keywords2(&keywords, keywords_prop->data,
- NULL, NULL, 0, NULL,
- scratch_pool));
- if (svn_subst_translation_required(eol_style, SVN_SUBST_NATIVE_EOL_STR,
- keywords, FALSE, TRUE))
- {
- svn_stream_t *working_content;
- svn_stream_t *normalized_content;
-
- SVN_ERR(svn_stream_open_readonly(&working_content, file2_abspath,
- scratch_pool, scratch_pool));
-
- /* Create a temporary file and copy normalised data into it. */
- SVN_ERR(svn_stream_open_unique(&file2_content, &file2_abspath, NULL,
- svn_io_file_del_on_pool_cleanup,
- scratch_pool, scratch_pool));
- normalized_content = svn_subst_stream_translated(
- file2_content, SVN_SUBST_NATIVE_EOL_STR,
- TRUE, keywords, FALSE, scratch_pool);
- SVN_ERR(svn_stream_copy3(working_content, normalized_content,
- ctx->cancel_func, ctx->cancel_baton,
- scratch_pool));
- }
-
- if (kind1 == svn_node_file && !(show_copies_as_adds && is_copy))
- {
- const char *mime1 = svn_prop_get_value(file1_props, SVN_PROP_MIME_TYPE);
- const char *mime2 = svn_prop_get_value(file2_props, SVN_PROP_MIME_TYPE);
- SVN_ERR(callbacks->file_opened(NULL, NULL, target,
- reverse ? SVN_INVALID_REVNUM : rev,
- callback_baton, scratch_pool));
-
- if (reverse)
- SVN_ERR(callbacks->file_changed(NULL, NULL, NULL, target,
- file2_abspath, file1_abspath,
- SVN_INVALID_REVNUM, rev,
- mime2,
- mime1,
- make_regular_props_array(
- file1_props, scratch_pool,
- scratch_pool),
- file2_props,
- callback_baton, scratch_pool));
- else
- SVN_ERR(callbacks->file_changed(NULL, NULL, NULL, target,
- file1_abspath, file2_abspath,
- rev, SVN_INVALID_REVNUM,
- mime1,
- mime2,
- make_regular_props_array(
- file2_props, scratch_pool,
- scratch_pool),
- file1_props,
- callback_baton, scratch_pool));
- }
- else
- {
- const char *mime2 = svn_prop_get_value(file2_props, SVN_PROP_MIME_TYPE);
- if (reverse)
- {
- SVN_ERR(callbacks->file_deleted(NULL, NULL,
- target, file2_abspath, file1_abspath,
- mime2,
- NULL,
- make_regular_props_hash(
- file2_props, scratch_pool,
- scratch_pool),
- callback_baton, scratch_pool));
- }
- else
- {
- SVN_ERR(callbacks->file_added(NULL, NULL, NULL, target,
- file1_abspath, file2_abspath,
- rev, SVN_INVALID_REVNUM,
- NULL,
- mime2,
- NULL, SVN_INVALID_REVNUM,
- make_regular_props_array(
- file2_props, scratch_pool,
- scratch_pool),
- NULL,
- callback_baton, scratch_pool));
- }
- }
-
- 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
@@ -2183,8 +2035,9 @@ diff_repos_wc(const char *path_or_url1,
void *callback_baton,
struct diff_cmd_baton *cmd_baton,
svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+ apr_pool_t *scratch_pool)
{
+ apr_pool_t *pool = scratch_pool;
const char *url1, *anchor, *anchor_url, *target;
svn_revnum_t rev;
svn_ra_session_t *ra_session;
@@ -2201,9 +2054,9 @@ diff_repos_wc(const char *path_or_url1,
svn_node_kind_t kind1;
svn_node_kind_t kind2;
svn_boolean_t is_copy;
- svn_revnum_t copyfrom_rev;
- const char *copy_source_repos_relpath;
- const char *copy_source_repos_root_url;
+ svn_revnum_t cf_revision;
+ const char *cf_repos_relpath;
+ const char *cf_repos_root_url;
SVN_ERR_ASSERT(! svn_path_is_url(path2));
@@ -2279,32 +2132,14 @@ diff_repos_wc(const char *path_or_url1,
cmd_baton->revnum2 = rev;
/* Check if our diff target is a copied node. */
- SVN_ERR(svn_wc__node_get_origin(&is_copy,
- ©from_rev,
- ©_source_repos_relpath,
- ©_source_repos_root_url,
+ SVN_ERR(svn_wc__node_get_origin(&is_copy,
+ &cf_revision,
+ &cf_repos_relpath,
+ &cf_repos_root_url,
NULL, NULL,
ctx->wc_ctx, abspath2,
FALSE, pool, pool));
- /* If both diff targets can be diffed as files, fetch the appropriate
- * file content from the repository and generate a diff against the
- * local version of the file.
- * However, if comparing the repository version of the file to the BASE
- * tree version we can use the diff editor to transmit a delta instead
- * of potentially huge file content. */
- if ((!rev2_is_base || is_copy) &&
- (kind1 == svn_node_file || kind1 == svn_node_none)
- && kind2 == svn_node_file)
- {
- SVN_ERR(diff_repos_wc_file_target(target, abspath2, kind1, rev,
- reverse, show_copies_as_adds,
- callbacks, callback_baton,
- ra_session, ctx, pool));
-
- return SVN_NO_ERROR;
- }
-
/* Use the diff editor to generate the diff. */
SVN_ERR(svn_ra_has_capability(ra_session, &server_supports_depth,
SVN_RA_CAPABILITY_DEPTH, pool));
@@ -2313,7 +2148,7 @@ diff_repos_wc(const char *path_or_url1,
anchor_abspath,
target,
depth,
- ignore_ancestry,
+ ignore_ancestry || is_copy,
show_copies_as_adds,
use_git_diff_format,
rev2_is_base,
@@ -2332,30 +2167,40 @@ diff_repos_wc(const char *path_or_url1,
if (is_copy)
{
- const char *copyfrom_url;
const char *copyfrom_parent_url;
const char *copyfrom_basename;
svn_depth_t copy_depth;
cmd_baton->repos_wc_diff_target_is_copy = TRUE;
- /* We're diffing a locally copied/moved directory.
+ /* We're diffing a locally copied/moved node.
* Describe the copy source to the reporter instead of the copy itself.
* Doing the latter would generate a single add_directory() call to the
* diff editor which results in an unexpected diff (the copy would
* be shown as deleted). */
- copyfrom_url = apr_pstrcat(pool, copy_source_repos_root_url, "/",
- copy_source_repos_relpath, (char *)NULL);
- svn_uri_split(©from_parent_url, ©from_basename,
- copyfrom_url, pool);
+ if (cf_repos_relpath[0] == '\0')
+ {
+ copyfrom_parent_url = cf_repos_root_url;
+ copyfrom_basename = "";
+ }
+ else
+ {
+ const char *parent_relpath;
+ svn_relpath_split(&parent_relpath, ©from_basename,
+ cf_repos_relpath, scratch_pool);
+
+ copyfrom_parent_url = svn_path_url_add_component2(cf_repos_root_url,
+ parent_relpath,
+ scratch_pool);
+ }
SVN_ERR(svn_ra_reparent(ra_session, copyfrom_parent_url, pool));
/* Tell the RA layer we want a delta to change our txn to URL1 */
SVN_ERR(svn_ra_do_diff3(ra_session,
&reporter, &reporter_baton,
rev,
- copyfrom_basename,
+ target,
diff_depth,
ignore_ancestry,
TRUE, /* text_deltas */
@@ -2365,9 +2210,23 @@ diff_repos_wc(const char *path_or_url1,
/* Report the copy source. */
SVN_ERR(svn_wc__node_get_depth(©_depth, ctx->wc_ctx, abspath2,
pool));
- SVN_ERR(reporter->set_path(reporter_baton, "", copyfrom_rev,
- copy_depth, FALSE, NULL, pool));
-
+
+ if (copy_depth == svn_depth_unknown)
+ copy_depth = svn_depth_infinity;
+
+ SVN_ERR(reporter->set_path(reporter_baton, "",
+ cf_revision,
+ copy_depth, FALSE, NULL, scratch_pool));
+
+ if (strcmp(target, copyfrom_basename) != 0)
+ SVN_ERR(reporter->link_path(reporter_baton, target,
+ svn_path_url_add_component2(
+ cf_repos_root_url,
+ cf_repos_relpath,
+ scratch_pool),
+ cf_revision,
+ copy_depth, FALSE, NULL, scratch_pool));
+
/* Finish the report to generate the diff. */
SVN_ERR(reporter->finish_report(reporter_baton, pool));
}
Modified: subversion/trunk/subversion/libsvn_wc/diff_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/diff_editor.c?rev=1444183&r1=1444182&r2=1444183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/diff_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/diff_editor.c Fri Feb 8 18:43:08 2013
@@ -261,6 +261,9 @@ struct dir_baton {
/* Gets set if the directory is added rather than replaced/unchanged. */
svn_boolean_t added;
+ /* Reference to parent directory baton (or NULL for the root) */
+ struct dir_baton *parent_baton;
+
/* The depth at which this directory should be diffed. */
svn_depth_t depth;
@@ -295,6 +298,7 @@ struct dir_baton {
struct edit_baton *eb;
apr_pool_t *pool;
+ int users;
};
/* File level baton.
@@ -303,6 +307,8 @@ struct file_baton {
/* Gets set if the file is added rather than replaced. */
svn_boolean_t added;
+ struct dir_baton *parent_baton;
+
/* The name and path of this file as if they would be/are in the
local working copy. */
const char *name;
@@ -412,9 +418,11 @@ make_dir_baton(const char *path,
svn_depth_t depth,
apr_pool_t *result_pool)
{
- apr_pool_t *dir_pool = svn_pool_create(result_pool);
+ apr_pool_t *dir_pool = svn_pool_create(parent_baton ? parent_baton->pool
+ : eb->pool);
struct dir_baton *db = apr_pcalloc(dir_pool, sizeof(*db));
+ db->parent_baton = parent_baton;
db->eb = eb;
db->added = added;
db->depth = depth;
@@ -426,11 +434,17 @@ make_dir_baton(const char *path,
db->name = svn_dirent_basename(db->path, NULL);
if (parent_baton != NULL)
- db->local_abspath = svn_dirent_join(parent_baton->local_abspath, db->name,
- dir_pool);
+ {
+ db->local_abspath = svn_dirent_join(parent_baton->local_abspath,
+ db->name,
+ dir_pool);
+ parent_baton->users++;
+ }
else
db->local_abspath = apr_pstrdup(dir_pool, eb->anchor_abspath);
+ db->users = 1;
+
return db;
}
@@ -450,6 +464,8 @@ make_file_baton(const char *path,
struct edit_baton *eb = parent_baton->eb;
fb->eb = eb;
+ fb->parent_baton = parent_baton;
+ fb->parent_baton->users++;
fb->added = added;
fb->pool = file_pool;
fb->propchanges = apr_array_make(file_pool, 8, sizeof(svn_prop_t));
@@ -462,6 +478,25 @@ make_file_baton(const char *path,
return fb;
}
+/* Destroy DB when there are no more registered users */
+static svn_error_t *
+maybe_done(struct dir_baton *db)
+{
+ db->users--;
+
+ if (!db->users)
+ {
+ struct dir_baton *pb = db->parent_baton;
+
+ svn_pool_clear(db->pool);
+
+ if (pb != NULL)
+ SVN_ERR(maybe_done(pb));
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* Get the empty file associated with the edit baton. This is cached so
* that it can be reused, all empty files are the same.
*/
@@ -1398,7 +1433,7 @@ close_directory(void *dir_baton,
db->added, db->eb->callback_baton,
scratch_pool));
- svn_pool_destroy(db->pool); /* destroys scratch_pool */
+ SVN_ERR(maybe_done(db)); /* destroys scratch_pool */
return SVN_NO_ERROR;
}
@@ -1646,12 +1681,6 @@ close_file(void *file_baton,
pristine_props = apr_hash_make(scratch_pool);
}
- if (status == svn_wc__db_status_added)
- SVN_ERR(svn_wc__db_scan_addition(&status, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, eb->db,
- fb->local_abspath,
- scratch_pool, scratch_pool));
-
repos_props = svn_prop__patch(pristine_props, fb->propchanges, scratch_pool);
repos_mimetype = get_prop_mimetype(repos_props);
repos_file = fb->temp_file_path ? fb->temp_file_path : pristine_file;
@@ -1661,7 +1690,15 @@ close_file(void *file_baton,
and it was marked as schedule-deleted), we show either an addition
or a deletion of the complete contents of the repository file,
depending upon the direction of the diff. */
- if (fb->added || (!eb->use_text_base && status == svn_wc__db_status_deleted))
+ if (eb->ignore_ancestry && status == svn_wc__db_status_added)
+ {
+ /* Add this filename to the parent directory's list of elements that
+ have been compared. */
+ svn_hash_sets(fb->parent_baton->compared,
+ apr_pstrdup(fb->parent_baton->pool, fb->path), "");
+ }
+ else if (fb->added
+ || (!eb->use_text_base && status == svn_wc__db_status_deleted))
{
if (eb->reverse_order)
return eb->callbacks->file_added(NULL, NULL, NULL, fb->path,
@@ -1687,6 +1724,12 @@ close_file(void *file_baton,
scratch_pool);
}
+ if (status == svn_wc__db_status_added)
+ SVN_ERR(svn_wc__db_scan_addition(&status, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, eb->db,
+ fb->local_abspath,
+ scratch_pool, scratch_pool));
+
/* If the file was locally added with history, and we want to show copies
* as added, diff the file with the empty file. */
if ((status == svn_wc__db_status_copied ||
@@ -1779,6 +1822,7 @@ close_file(void *file_baton,
scratch_pool));
}
+ SVN_ERR(maybe_done(fb->parent_baton));
svn_pool_destroy(fb->pool); /* destroys scratch_pool */
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/diff_tests.py?rev=1444183&r1=1444182&r2=1444183&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/diff_tests.py Fri Feb 8 18:43:08 2013
@@ -991,7 +991,21 @@ def diff_head_of_moved_file(sbox):
# Modify the file to ensure that the diff is non-empty.
svntest.main.file_append(new_mu_path, "\nActually, it's a new mu.")
- svntest.actions.run_and_verify_svn(None, svntest.verify.AnyOutput, [],
+ mu_new = sbox.ospath('A/mu.new').replace('\\','/')
+
+ expected_output = [
+ 'Index: %s\n' % mu_new,
+ '===================================================================\n',
+ '--- %s\t(.../mu)\t(revision 1)\n' % mu_new,
+ '+++ %s\t(.../mu.new)\t(working copy)\n' % mu_new,
+ '@@ -1 +1,3 @@\n',
+ ' This is the file \'mu\'.\n',
+ '+\n',
+ '+Actually, it\'s a new mu.\n',
+ '\ No newline at end of file\n',
+ ]
+
+ svntest.actions.run_and_verify_svn(None, expected_output, [],
'diff', '-r', 'HEAD', new_mu_path)
@@ -3316,7 +3330,6 @@ def make_file_edit_del_add(dir):
svntest.main.run_svn(None, 'add', theta)
-@XFail()
@Issue(3295)
def diff_url_against_local_mods(sbox):
"diff URL against working copy with local mods"