You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/09/11 00:57:48 UTC
svn commit: r1521673 - in /subversion/branches/1.7.x-issue4421/subversion:
libsvn_ra_neon/fetch.c libsvn_ra_serf/update.c tests/cmdline/diff_tests.py
Author: breser
Date: Tue Sep 10 22:57:47 2013
New Revision: 1521673
URL: http://svn.apache.org/r1521673
Log:
Fix issue #4421, diff --summarize crashes when diffing deeply added trees.
This issue basically boils down to our diff implementation starting to use
multiple component targets after r1201824 (which was backported to 1.7.x in
1207351). However, ra_serf and ra_neon don't properly deal with multiple
component targets since our API says you can't use them. This change only
is known to make using svn_ra_do_diff*() with a multiple component target safe
since the other update report paths are still protected against multiple
component targets with assertions.
1.8.x and trunk have already fixed this by a much more complicated change to
the diff implementation to avoid using multiple component targets entirely.
* subversion/libsvn_ra_neon/fetch.c
(start_element): calculate the proper root to account for multiple component
targets.
* subversion/libsvn_ra_serf/update.c
(open_dir): use the lack of a parent_dir as indication of root, not an
empty base_name.
(start_report): calculate the proper root to account for multiple component
targets.
* subversion/tests/cmdline/diff_tests.py
(basic_diff_summarize): Add test cases that excercise the issue.
Modified:
subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_neon/fetch.c
subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_serf/update.c
subversion/branches/1.7.x-issue4421/subversion/tests/cmdline/diff_tests.py
Modified: subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_neon/fetch.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_neon/fetch.c?rev=1521673&r1=1521672&r2=1521673&view=diff
==============================================================================
--- subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_neon/fetch.c (original)
+++ subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_neon/fetch.c Tue Sep 10 22:57:47 2013
@@ -1486,8 +1486,16 @@ start_element(int *elem, void *userdata,
if (DIR_DEPTH(rb) == 0)
{
- /* pathbuf has to live for the whole edit! */
- pathbuf = svn_stringbuf_create("", rb->pool);
+ /* Technically this should always be "" since the root of the update
+ * report should always match the session root. However, if the
+ * target provided has more than one component then they won't
+ * match. Our API says you can't use multiple component targets but
+ * the client libraries diff implmenetation does (see r1201824 where
+ * issue #2873 was fixed for why). When you do this
+ * the root of the report ends up being the parent of the target.
+ * pathbuf has to live for the whole edit! */
+ pathbuf = svn_stringbuf_create(svn_relpath_dirname(rb->target,
+ rb->pool), rb->pool);
/* During switch operations, we need to invalidate the
tree's version resource URLs in case something goes
Modified: subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_serf/update.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_serf/update.c?rev=1521673&r1=1521672&r2=1521673&view=diff
==============================================================================
--- subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_serf/update.c (original)
+++ subversion/branches/1.7.x-issue4421/subversion/libsvn_ra_serf/update.c Tue Sep 10 22:57:47 2013
@@ -556,7 +556,7 @@ open_dir(report_dir_t *dir)
return SVN_NO_ERROR;
}
- if (dir->base_name[0] == '\0')
+ if (!dir->parent_dir)
{
dir->dir_baton_pool = svn_pool_create(dir->pool);
@@ -1374,20 +1374,36 @@ start_report(svn_ra_serf__xml_parser_t *
info->dir->target_rev = ctx->target_rev;
info->fetch_props = TRUE;
- info->dir->base_name = "";
- info->dir->name = "";
+ /* Technically this should always be "" since the root of the update
+ * report should always match the session root. However, if the target
+ * provided has more than one component then they won't match. Our API
+ * says you can't use multiple component targets but the client
+ * libraries diff implementation does (see r1201824 where issue #2873
+ * was fixed for why). When you do this the root of
+ * the report ends up being the parent of the target. */
+ info->dir->name = svn_relpath_dirname(ctx->update_target,
+ info->dir->pool);
+ info->dir->base_name = svn_relpath_basename(info->dir->name,
+ info->dir->pool);
info->base_name = info->dir->base_name;
info->name = info->dir->name;
- info->dir->repos_relpath = apr_hash_get(ctx->switched_paths, "",
+ info->dir->repos_relpath = apr_hash_get(ctx->switched_paths,
+ info->dir->name,
APR_HASH_KEY_STRING);
if (!info->dir->repos_relpath)
- SVN_ERR(svn_ra_serf__get_relative_path(&info->dir->repos_relpath,
- ctx->sess->session_url.path,
- ctx->sess, ctx->conn,
- info->dir->pool));
+ {
+ SVN_ERR(svn_ra_serf__get_relative_path(&info->dir->repos_relpath,
+ ctx->sess->session_url.path,
+ ctx->sess, ctx->conn,
+ info->dir->pool));
+
+ info->dir->repos_relpath =
+ svn_relpath_join(info->dir->repos_relpath,
+ info->name, info->dir->pool);
+ }
}
else if (state == NONE)
{
Modified: subversion/branches/1.7.x-issue4421/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x-issue4421/subversion/tests/cmdline/diff_tests.py?rev=1521673&r1=1521672&r2=1521673&view=diff
==============================================================================
--- subversion/branches/1.7.x-issue4421/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/branches/1.7.x-issue4421/subversion/tests/cmdline/diff_tests.py Tue Sep 10 22:57:47 2013
@@ -2701,6 +2701,22 @@ def basic_diff_summarize(sbox):
svntest.actions.run_and_verify_diff_summarize(expected_reverse_diff,
p('P'), '-c-3')
+ # Get the differences between a deep newly added dir
+ expected_diff = svntest.wc.State(wc_dir, {
+ 'Q/R' : Item(status='A '),
+ 'Q/R/newfile' : Item(status='A '),
+ })
+ expected_reverse_diff = svntest.wc.State(wc_dir, {
+ 'Q/R' : Item(status='D '),
+ 'Q/R/newfile' : Item(status='D '),
+ })
+ svntest.actions.run_and_verify_diff_summarize(expected_diff,
+ p('Q/R'), '-c3')
+ svntest.actions.run_and_verify_diff_summarize(expected_reverse_diff,
+ p('Q/R'), '-c-3')
+
+
+
#----------------------------------------------------------------------
def diff_weird_author(sbox):
"diff with svn:author that has < in it"