You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by pb...@apache.org on 2010/05/26 01:28:46 UTC
svn commit: r948250 - in /subversion/trunk/subversion: libsvn_client/merge.c
libsvn_client/mergeinfo.h tests/cmdline/merge_tests.py
Author: pburba
Date: Tue May 25 23:28:46 2010
New Revision: 948250
URL: http://svn.apache.org/viewvc?rev=948250&view=rev
Log:
Fox issue #3642 'immediate depth merges don't create proper subtree
mergeinfo'.
* subversion/libsvn_client/mergeinfo.h
(svn_client__merge_path_t): Add new member immediate_child_dir.
* subversion/libsvn_client/merge.c
(get_mergeinfo_walk_cb): Set new svn_client__merge_path_t member.
(log_find_operative_subtree_revs): New svn_log_entry_receiver_t callback.
(get_inoperative_immediate_children): New.
(record_mergeinfo_for_dir_merge): Don't record mergeinfo on immediate
directory children of the merge target during --depth immediates merges
if those children wouldn't be affected by the merge even at --depth
infinity.
* subversion/tests/cmdline/merge_tests.py
(immedate_depth_merge_creates_minimal_subtree_mergeinfo): Renamed from
this...
(immediate_depth_merge_creates_minimal_subtree_mergeinfo): ...to this.
Removed comment about failure status.
(test_list): Fix misspelled test name, remove XFail from
immediate_depth_merge_creates_minimal_subtree_mergeinfo.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/libsvn_client/mergeinfo.h
subversion/trunk/subversion/tests/cmdline/merge_tests.py
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=948250&r1=948249&r2=948250&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue May 25 23:28:46 2010
@@ -5244,6 +5244,7 @@ get_mergeinfo_walk_cb(const char *local_
svn_boolean_t deleted;
svn_boolean_t absent;
svn_boolean_t obstructed;
+ svn_boolean_t immediate_child_dir;
/* TODO(#2843) How to deal with a excluded item on merge? */
@@ -5287,6 +5288,11 @@ get_mergeinfo_walk_cb(const char *local_
SVN_ERR(svn_wc__node_get_depth(&depth, wb->ctx->wc_ctx, local_abspath,
scratch_pool));
+ immediate_child_dir = ((wb->depth == svn_depth_immediates)
+ &&(kind == svn_node_dir)
+ && (strcmp(abs_parent_path,
+ wb->merge_target_abspath) == 0));
+
/* Store PATHs with explict mergeinfo, which are switched, are missing
children due to a sparse checkout, are scheduled for deletion are absent
from the WC, are first level sub directories relative to merge target if
@@ -5299,9 +5305,7 @@ get_mergeinfo_walk_cb(const char *local_
|| depth == svn_depth_empty
|| depth == svn_depth_files
|| absent
- || ((wb->depth == svn_depth_immediates) &&
- (kind == svn_node_dir) &&
- (strcmp(abs_parent_path, wb->merge_target_abspath) == 0))
+ || immediate_child_dir
|| ((wb->depth == svn_depth_files) &&
(kind == svn_node_file) &&
(strcmp(abs_parent_path, wb->merge_target_abspath) == 0))
@@ -5334,7 +5338,9 @@ get_mergeinfo_walk_cb(const char *local_
if (!child->has_noninheritable
&& (depth == svn_depth_empty
|| depth == svn_depth_files))
- child->has_noninheritable = TRUE;
+ child->has_noninheritable = TRUE;
+
+ child->immediate_child_dir = immediate_child_dir;
APR_ARRAY_PUSH(wb->children_with_mergeinfo,
svn_client__merge_path_t *) = child;
@@ -6897,6 +6903,120 @@ do_mergeinfo_unaware_dir_merge(const cha
merge_b, pool);
}
+/* A svn_log_entry_receiver_t callback for
+ get_inoperative_immediate_children(). */
+static svn_error_t *
+log_find_operative_subtree_revs(void *baton,
+ svn_log_entry_t *log_entry,
+ apr_pool_t *pool)
+{
+ apr_hash_t *immediate_children = baton;
+ apr_hash_index_t *hi, *hi2;
+ svn_revnum_t revision;
+
+ revision = log_entry->revision;
+
+ for (hi = apr_hash_first(pool, log_entry->changed_paths2);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *path = svn__apr_hash_index_key(hi);
+ for (hi2 = apr_hash_first(pool, immediate_children);
+ hi2;
+ hi2 = apr_hash_next(hi2))
+ {
+ const char *immediate_path = svn__apr_hash_index_val(hi2);
+ if (svn_dirent_is_ancestor(immediate_path, path))
+ {
+ apr_hash_set(immediate_children, svn__apr_hash_index_key(hi2),
+ APR_HASH_KEY_STRING, NULL);
+ /* A path can't be the child of more than
+ one immediate child of the merge target. */
+ break;
+ }
+ }
+ }
+ return SVN_NO_ERROR;
+}
+
+/* Issue #3642.
+
+ Find inoperative subtrees when record_mergeinfo_for_dir_merge() is
+ recording mergeinfo describing a merge at depth=immediates.
+
+ CHILDREN_WITH_MERGEINFO is the standard array of subtrees that are
+ interesting from a merge tracking perspective, see the global comment
+ 'THE CHILDREN_WITH_MERGEINFO ARRAY'.
+
+ MERGE_SOURCE_REPOS_ABSPATH is the absolute repository path of the merge
+ source. OLDEST_REV and YOUNGEST_REV are the revisions merged from
+ MERGE_SOURCE_REPOS_ABSPATH to MERGE_TARGET_ABSPATH.
+
+ RA_SESSION points to MERGE_SOURCE_REPOS_ABSPATH.
+
+ Set *IMMEDIATE_CHILDREN to a hash (mapping const char * WC absolute
+ paths to const char * repos absolute paths) containing all the
+ subtrees which would be inoperative if MERGE_SOURCE_REPOS_PATH
+ -r(OLDEST_REV - 1):YOUNGEST_REV were merged to MERGE_TARGET_ABSPATH
+ at --depth infinity. The keys of the hash point to copies of the ABSPATH
+ members of the svn_client__merge_path_t * in CHILDREN_WITH_MERGEINFO that
+ are inoperative. The hash values are the key's corresponding repository
+ absolute merge source path.
+
+ RESULT_POOL is used to allocate the contents of *IMMEDIATE_CHILDREN.
+ SCRATCH_POOL is used for temporary allocations. */
+static svn_error_t *
+get_inoperative_immediate_children(apr_hash_t **immediate_children,
+ apr_array_header_t *children_with_mergeinfo,
+ const char *merge_source_repos_abspath,
+ svn_revnum_t oldest_rev,
+ svn_revnum_t youngest_rev,
+ const char *merge_target_abspath,
+ svn_ra_session_t *ra_session,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ int i;
+ *immediate_children = apr_hash_make(result_pool);
+
+ SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(oldest_rev));
+ SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
+ SVN_ERR_ASSERT(oldest_rev <= youngest_rev);
+
+ /* Find all the children in CHILDREN_WITH_MERGEINFO with the
+ immediate_child_dir flag set and store them in *IMMEDIATE_CHILDREN. */
+ for (i = 0; i < children_with_mergeinfo->nelts; i++)
+ {
+ svn_client__merge_path_t *child =
+ APR_ARRAY_IDX(children_with_mergeinfo, i, svn_client__merge_path_t *);
+
+ if (child->immediate_child_dir)
+ apr_hash_set(*immediate_children,
+ apr_pstrdup(result_pool, child->abspath),
+ APR_HASH_KEY_STRING,
+ svn_uri_join(merge_source_repos_abspath,
+ svn_dirent_is_child(merge_target_abspath,
+ child->abspath,
+ result_pool),
+ result_pool));
+ }
+
+ /* Now remove any paths from *IMMEDIATE_CHILDREN that are inoperative when
+ merging MERGE_SOURCE_REPOS_PATH -r(OLDEST_REV - 1):YOUNGEST_REV to
+ MERGE_TARGET_ABSPATH at --depth infinity. */
+ if (apr_hash_count(*immediate_children))
+ {
+ apr_array_header_t *log_targets = apr_array_make(scratch_pool, 1,
+ sizeof(const char *));
+ APR_ARRAY_PUSH(log_targets, const char *) = "";
+ SVN_ERR(svn_ra_get_log2(ra_session, log_targets, youngest_rev,
+ oldest_rev, 0, TRUE, FALSE, FALSE,
+ NULL, log_find_operative_subtree_revs,
+ *immediate_children, scratch_pool));
+ }
+ return SVN_NO_ERROR;
+}
+
/* Helper for do_directory_merge().
Record mergeinfo describing a merge of MERGED_RANGE->START:
@@ -6922,6 +7042,7 @@ record_mergeinfo_for_dir_merge(const svn
int i;
svn_boolean_t is_rollback = (merged_range->start > merged_range->end);
svn_boolean_t operative_merge = FALSE;
+ apr_hash_t *inoperative_immediate_children = NULL;
/* Update the WC mergeinfo here to account for our new
merges, minus any unresolved conflicts and skips. */
@@ -6959,6 +7080,19 @@ record_mergeinfo_for_dir_merge(const svn
remove_absent_children(merge_b->target_abspath,
notify_b->children_with_mergeinfo, notify_b);
+
+ /* Find all issue #3642 children (i.e immediate child directories of the
+ merge target, with no pre-existing explicit mergeinfo, during a --depth
+ immediates merge). Stash those that are inoperative at any depth in
+ INOPERATIVE_IMMEDIATE_CHILDREN. */
+ if (!merge_b->record_only && range.start <= range.end)
+ SVN_ERR(get_inoperative_immediate_children(
+ &inoperative_immediate_children,
+ notify_b->children_with_mergeinfo,
+ mergeinfo_path, range.start + 1, range.end,
+ merge_b->target_abspath, merge_b->ra_session1,
+ pool, iterpool));
+
/* Record mergeinfo on any subtree affected by the merge or for
every subtree if this is a --record-only merge. Always record
mergeinfo on the merge target CHILDREN_WITH_MERGEINFO[0]. */
@@ -6986,6 +7120,7 @@ record_mergeinfo_for_dir_merge(const svn
was affected by the merge. */
if (i > 0 /* Always record mergeinfo on the merge target. */
&& (!merge_b->record_only || merge_b->reintegrate_merge)
+ && (!child->immediate_child_dir || child->pre_merge_mergeinfo)
&& (!operative_merge
|| !subtree_touched_by_merge(child->abspath, notify_b,
iterpool)))
@@ -7017,6 +7152,14 @@ record_mergeinfo_for_dir_merge(const svn
if (child_is_deleted)
continue;
+ /* We don't need to record mergeinfo on those issue #3642 children
+ that are inoperative at any depth. */
+ if (inoperative_immediate_children
+ && apr_hash_get(inoperative_immediate_children,
+ child->abspath,
+ APR_HASH_KEY_STRING))
+ continue;
+
child_repos_path = svn_dirent_is_child(merge_b->target_abspath,
child->abspath, iterpool);
if (!child_repos_path)
Modified: subversion/trunk/subversion/libsvn_client/mergeinfo.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/mergeinfo.h?rev=948250&r1=948249&r2=948250&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/mergeinfo.h (original)
+++ subversion/trunk/subversion/libsvn_client/mergeinfo.h Tue May 25 23:28:46 2010
@@ -73,6 +73,12 @@ typedef struct svn_client__merge_path_t
explicit or inherited. */
svn_boolean_t scheduled_for_deletion; /* ABSPATH is scheduled for
deletion. */
+ svn_boolean_t immediate_child_dir; /* ABSPATH is an immediate child
+ directory of the merge target,
+ has no explicit mergeinfo prior
+ to the merge, and the operational
+ depth of the merge is
+ svn_depth_immediates. */
} svn_client__merge_path_t;
/* Return a deep copy of the merge-path structure OLD, allocated in POOL. */
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=948250&r1=948249&r2=948250&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Tue May 25 23:28:46 2010
@@ -19062,7 +19062,7 @@ def foreign_repos_del_and_props(sbox):
# Test for issue #3642 'immediate depth merges don't create proper subtree
# mergeinfo'. See http://subversion.tigris.org/issues/show_bug.cgi?id=3642
-def immedate_depth_merge_creates_minimal_subtree_mergeinfo(sbox):
+def immediate_depth_merge_creates_minimal_subtree_mergeinfo(sbox):
"no spurious mergeinfo from immediate depth merges "
sbox.build()
@@ -19082,9 +19082,6 @@ def immedate_depth_merge_creates_minimal
# affect that subtree. The other child of the merge target, A_COPY/B/F
# would never be affected by r5, so it doesn't need any explicit
# mergeinfo.
- #
- # Currently this test fails because *no* immediate directory children
- # of the merge target get explicit mergeinfo set.
expected_output = wc.State(B_COPY_path, {})
expected_mergeinfo_output = wc.State(B_COPY_path, {
'' : Item(status=' U'),
@@ -19358,7 +19355,7 @@ test_list = [ None,
reintegrate_with_self_referential_mergeinfo,
reintegrate_with_subtree_merges,
foreign_repos_del_and_props,
- XFail(immedate_depth_merge_creates_minimal_subtree_mergeinfo),
+ immediate_depth_merge_creates_minimal_subtree_mergeinfo,
]
if __name__ == '__main__':
Re: svn commit: r948250 - in /subversion/trunk/subversion:
libsvn_client/merge.c libsvn_client/mergeinfo.h tests/cmdline/merge_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Wed, May 26, 2010 at 12:29 PM, Philip Martin
<ph...@wandisco.com> wrote:
> pburba@apache.org writes:
>
>> Author: pburba
>> Date: Tue May 25 23:28:46 2010
>> New Revision: 948250
>
>> +/* A svn_log_entry_receiver_t callback for
>> + get_inoperative_immediate_children(). */
>> +static svn_error_t *
>> +log_find_operative_subtree_revs(void *baton,
>> + svn_log_entry_t *log_entry,
>> + apr_pool_t *pool)
>
> Any reason why this is pool rather than scratch_pool?
Hi Philip,
I was simply using the same argument name in the implementation as is
used in the declaration of svn_log_entry_receiver_t. The POOL
argument to svn_log_entry_receiver_t should essentially be a
scratch_pool/iterpool per the docstring:
* Use @a pool for temporary allocation. If the caller is iterating
* over log messages, invoking this receiver on each, we recommend the
* standard pool loop recipe: create a subpool, pass it as @a pool to
* each call, clear it after each iteration, destroy it after the loop
* is done. (For allocation that must last beyond the lifetime of a
* given receiver call, use a pool in @a baton.)
>> +{
>> + apr_hash_t *immediate_children = baton;
>> + apr_hash_index_t *hi, *hi2;
>> + svn_revnum_t revision;
>> +
>> + revision = log_entry->revision;
>
> revision is not used.
Fixed.
>> +
>> + for (hi = apr_hash_first(pool, log_entry->changed_paths2);
>> + hi;
>> + hi = apr_hash_next(hi))
>> + {
>> + const char *path = svn__apr_hash_index_key(hi);
>> + for (hi2 = apr_hash_first(pool, immediate_children);
>
> It's only a small allocation so I suppose we can get away without an
> iteration pool.
As mentioned above, the POOL passed to this svn_log_entry_receiver_t
is effectively a scratch pool/iterpool, so we wouldn't gain anything.
>> + hi2;
>> + hi2 = apr_hash_next(hi2))
>> + {
>> + const char *immediate_path = svn__apr_hash_index_val(hi2);
>> + if (svn_dirent_is_ancestor(immediate_path, path))
>> + {
>> + apr_hash_set(immediate_children, svn__apr_hash_index_key(hi2),
>> + APR_HASH_KEY_STRING, NULL);
>> + /* A path can't be the child of more than
>> + one immediate child of the merge target. */
>> + break;
>> + }
>> + }
>> + }
>> + return SVN_NO_ERROR;
>> +}
>
>> +static svn_error_t *
>> +get_inoperative_immediate_children(apr_hash_t **immediate_children,
>> + apr_array_header_t *children_with_mergeinfo,
>> + const char *merge_source_repos_abspath,
>> + svn_revnum_t oldest_rev,
>> + svn_revnum_t youngest_rev,
>> + const char *merge_target_abspath,
>> + svn_ra_session_t *ra_session,
>> + apr_pool_t *result_pool,
>> + apr_pool_t *scratch_pool)
>> +{
>> + int i;
>> + *immediate_children = apr_hash_make(result_pool);
>> +
>> + SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(oldest_rev));
>> + SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
>> + SVN_ERR_ASSERT(oldest_rev <= youngest_rev);
>> +
>> + /* Find all the children in CHILDREN_WITH_MERGEINFO with the
>> + immediate_child_dir flag set and store them in *IMMEDIATE_CHILDREN. */
>> + for (i = 0; i < children_with_mergeinfo->nelts; i++)
>> + {
>> + svn_client__merge_path_t *child =
>> + APR_ARRAY_IDX(children_with_mergeinfo, i, svn_client__merge_path_t *);
>> +
>> + if (child->immediate_child_dir)
>> + apr_hash_set(*immediate_children,
>> + apr_pstrdup(result_pool, child->abspath),
>> + APR_HASH_KEY_STRING,
>> + svn_uri_join(merge_source_repos_abspath,
>> + svn_dirent_is_child(merge_target_abspath,
>> + child->abspath,
>> + result_pool),
>
> Arguments to svn_uri_join don't need to be in result pool. Use
> scratch_pool or an iteration pool.
Added an iterpool.
http://svn.apache.org/viewvc?view=revision&revision=948910
Thanks,
Paul
Re: svn commit: r948250 - in /subversion/trunk/subversion: libsvn_client/merge.c libsvn_client/mergeinfo.h tests/cmdline/merge_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
pburba@apache.org writes:
> Author: pburba
> Date: Tue May 25 23:28:46 2010
> New Revision: 948250
> +/* A svn_log_entry_receiver_t callback for
> + get_inoperative_immediate_children(). */
> +static svn_error_t *
> +log_find_operative_subtree_revs(void *baton,
> + svn_log_entry_t *log_entry,
> + apr_pool_t *pool)
Any reason why this is pool rather than scratch_pool?
> +{
> + apr_hash_t *immediate_children = baton;
> + apr_hash_index_t *hi, *hi2;
> + svn_revnum_t revision;
> +
> + revision = log_entry->revision;
revision is not used.
> +
> + for (hi = apr_hash_first(pool, log_entry->changed_paths2);
> + hi;
> + hi = apr_hash_next(hi))
> + {
> + const char *path = svn__apr_hash_index_key(hi);
> + for (hi2 = apr_hash_first(pool, immediate_children);
It's only a small allocation so I suppose we can get away without an
iteration pool.
> + hi2;
> + hi2 = apr_hash_next(hi2))
> + {
> + const char *immediate_path = svn__apr_hash_index_val(hi2);
> + if (svn_dirent_is_ancestor(immediate_path, path))
> + {
> + apr_hash_set(immediate_children, svn__apr_hash_index_key(hi2),
> + APR_HASH_KEY_STRING, NULL);
> + /* A path can't be the child of more than
> + one immediate child of the merge target. */
> + break;
> + }
> + }
> + }
> + return SVN_NO_ERROR;
> +}
> +static svn_error_t *
> +get_inoperative_immediate_children(apr_hash_t **immediate_children,
> + apr_array_header_t *children_with_mergeinfo,
> + const char *merge_source_repos_abspath,
> + svn_revnum_t oldest_rev,
> + svn_revnum_t youngest_rev,
> + const char *merge_target_abspath,
> + svn_ra_session_t *ra_session,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + int i;
> + *immediate_children = apr_hash_make(result_pool);
> +
> + SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(oldest_rev));
> + SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
> + SVN_ERR_ASSERT(oldest_rev <= youngest_rev);
> +
> + /* Find all the children in CHILDREN_WITH_MERGEINFO with the
> + immediate_child_dir flag set and store them in *IMMEDIATE_CHILDREN. */
> + for (i = 0; i < children_with_mergeinfo->nelts; i++)
> + {
> + svn_client__merge_path_t *child =
> + APR_ARRAY_IDX(children_with_mergeinfo, i, svn_client__merge_path_t *);
> +
> + if (child->immediate_child_dir)
> + apr_hash_set(*immediate_children,
> + apr_pstrdup(result_pool, child->abspath),
> + APR_HASH_KEY_STRING,
> + svn_uri_join(merge_source_repos_abspath,
> + svn_dirent_is_child(merge_target_abspath,
> + child->abspath,
> + result_pool),
Arguments to svn_uri_join don't need to be in result pool. Use
scratch_pool or an iteration pool.
> + result_pool));
> + }
> +
> + /* Now remove any paths from *IMMEDIATE_CHILDREN that are inoperative when
> + merging MERGE_SOURCE_REPOS_PATH -r(OLDEST_REV - 1):YOUNGEST_REV to
> + MERGE_TARGET_ABSPATH at --depth infinity. */
> + if (apr_hash_count(*immediate_children))
> + {
> + apr_array_header_t *log_targets = apr_array_make(scratch_pool, 1,
> + sizeof(const char *));
> + APR_ARRAY_PUSH(log_targets, const char *) = "";
> + SVN_ERR(svn_ra_get_log2(ra_session, log_targets, youngest_rev,
> + oldest_rev, 0, TRUE, FALSE, FALSE,
> + NULL, log_find_operative_subtree_revs,
> + *immediate_children, scratch_pool));
> + }
> + return SVN_NO_ERROR;
> +}
--
Philip