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 2011/12/06 23:04:22 UTC
svn commit: r1211199 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_tests.py
tests/cmdline/mergeinfo_tests.py
Author: pburba
Date: Tue Dec 6 22:04:22 2011
New Revision: 1211199
URL: http://svn.apache.org/viewvc?rev=1211199&view=rev
Log:
Fix issue #4057 "don't record non-inheritable mergeinfo in shallow merge
if entire diff is within requested depth".
* subversion/libsvn_client/merge.c
(calculate_merge_inheritance): Don't unconditionally set non-inheritable
mergeinfo on the merge target if the operational depth is shallow.
Instead rely on the caller to determine if this is necessary.
(log_find_operative_subtree_baton_t): New.
(log_find_operative_subtree_revs): Build up a hash of operative immediate
children rather than paring down a hash of potentially inoperative
immediate children.
(get_inoperative_immediate_children): Rename this specialized issue #3642
fix from this...
(get_operative_immediate_children): ...to this and become more general
purpose in the process. As the name change indicates this function now
gets operative rather than inoperative subtrees. It also handles shallow
depths other than svn_depth_immediates.
(flag_subtrees_needing_mergeinfo): Account for the fact that the primary
helper for this function now gets operative, rather than inoperative
immediate children.
* subversion/tests/cmdline/merge_tests.py
(merge_away_subtrees_noninheritable_ranges,
reverse_merge_adds_subtree): Adjust expectations to account for new
behavior.
(unnecessary_noninheritable_mergeinfo_shallow_merge): Remove XFail
decorator and adjust comments re failure status.
* subversion/tests/cmdline/mergeinfo_tests.py
(noninheritabled_mergeinfo_not_always_eligible): Replicate pre-1.8 behavior
to keep this test for issue #4050 valid.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/tests/cmdline/merge_tests.py
subversion/trunk/subversion/tests/cmdline/mergeinfo_tests.py
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1211199&r1=1211198&r2=1211199&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue Dec 6 22:04:22 2011
@@ -4518,7 +4518,9 @@ populate_remaining_ranges(apr_array_head
WC_PATH_HAS_MISSING_CHILD is true if WC_PATH is missing an immediate child
because the child is switched or absent from the WC, or due to a sparse
- checkout -- see get_mergeinfo_paths().
+ checkout (see get_mergeinfo_paths) or because DEPTH is shallow
+ (i.e. < svn_depth_infinity) and the merge would affect a child if
+ performed at a deeper depth.
Perform any temporary allocations in SCRATCH_POOL. */
static svn_error_t *
@@ -4543,9 +4545,7 @@ calculate_merge_inheritance(svn_boolean_
{
if (wc_path_is_merge_target)
{
- if (wc_path_has_missing_child
- || depth == svn_depth_files
- || depth == svn_depth_empty)
+ if (wc_path_has_missing_child)
{
*non_inheritable = TRUE;
}
@@ -7266,6 +7266,23 @@ do_mergeinfo_unaware_dir_merge(const cha
merge_b, pool);
}
+/* A svn_log_entry_receiver_t baton for log_find_operative_subtree_revs(). */
+typedef struct log_find_operative_subtree_baton_t
+{
+ /* Mapping of const char * absolute working copy paths to those
+ path's const char * repos absolute paths. */
+ apr_hash_t *operative_children;
+
+ /* As per the arguments of the same name to
+ get_operative_immediate_children(). */
+ const char *merge_source_fspath;
+ const char *merge_target_abspath;
+ svn_depth_t depth;
+
+ /* A pool to allocate additions to the hashes in. */
+ apr_pool_t *result_pool;
+} log_find_operative_subtree_baton_t;
+
/* A svn_log_entry_receiver_t callback for
get_inoperative_immediate_children(). */
static svn_error_t *
@@ -7273,41 +7290,74 @@ log_find_operative_subtree_revs(void *ba
svn_log_entry_t *log_entry,
apr_pool_t *pool)
{
- apr_hash_t *immediate_children = baton;
- apr_hash_index_t *hi, *hi2;
+ log_find_operative_subtree_baton_t *log_baton = baton;
+ apr_hash_index_t *hi;
+ apr_pool_t *iterpool;
/* It's possible that authz restrictions on the merge source prevent us
from knowing about any of the changes for LOG_ENTRY->REVISION. */
if (!log_entry->changed_paths2)
return SVN_NO_ERROR;
+ iterpool = svn_pool_create(pool);
+
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;
+ svn_log_changed_path2_t *change = svn__apr_hash_index_val(hi);
+
+ {
+ const char *child;
+ const char *potential_child;
+ const char *rel_path =
+ svn_fspath__skip_ancestor(log_baton->merge_source_fspath, path);
+
+ /* Some affected paths might be the root of the merge source or
+ entirely otuside our subtree of interest. In either case they
+ are not operative *immediate* children. */
+ if (rel_path == NULL
+ || rel_path[0] == '\0')
+ continue;
+
+ svn_pool_clear(iterpool);
+
+ child = svn_relpath_dirname(rel_path, iterpool);
+ if (child[0] == '\0')
+ {
+ /* We only care about immediate directory children if
+ DEPTH is svn_depth_files. */
+ if (log_baton->depth == svn_depth_files
+ && change->node_kind == svn_node_file)
+ continue;
+ child = rel_path;
}
- }
+
+ potential_child = svn_dirent_join(log_baton->merge_target_abspath,
+ child, iterpool);
+
+ if (change->action == 'A'
+ || !apr_hash_get(log_baton->operative_children, potential_child,
+ APR_HASH_KEY_STRING))
+ {
+ apr_hash_set(log_baton->operative_children,
+ apr_pstrdup(log_baton->result_pool,
+ potential_child),
+ APR_HASH_KEY_STRING,
+ apr_pstrdup(log_baton->result_pool, path));
+ }
+ }
}
+ svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
-/* Issue #3642.
-
- Find inoperative subtrees when record_mergeinfo_for_dir_merge() is
- recording mergeinfo describing a merge at depth=immediates.
+/* Find operative subtrees if record_mergeinfo_for_dir_merge() were
+ recording mergeinfo describing a merge at svn_depth_infinity, rather
+ than at DEPTH (which is assumed to be shallow; if
+ DEPTH == svn_depth_infinity then this function does nothing beyond
+ setting *OPERATIVE_CHILDREN to an empty hash).
CHILDREN_WITH_MERGEINFO is the standard array of subtrees that are
interesting from a merge tracking perspective, see the global comment
@@ -7319,73 +7369,54 @@ log_find_operative_subtree_revs(void *ba
RA_SESSION points to MERGE_SOURCE_FSPATH.
- 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.
+ Set *OPERATIVE_CHILDREN to a hash (mapping const char * absolute
+ working copy paths to those path's 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 svn_depth_infinity.
- RESULT_POOL is used to allocate the contents of *IMMEDIATE_CHILDREN.
+ RESULT_POOL is used to allocate the contents of *OPERATIVE_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_fspath,
- 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)
+get_operative_immediate_children(apr_hash_t **operative_children,
+ apr_array_header_t *children_with_mergeinfo,
+ const char *merge_source_fspath,
+ svn_revnum_t oldest_rev,
+ svn_revnum_t youngest_rev,
+ const char *merge_target_abspath,
+ svn_depth_t depth,
+ svn_wc_context_t *wc_ctx,
+ svn_ra_session_t *ra_session,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
- int i;
- apr_pool_t *iterpool;
+ apr_array_header_t *log_targets;
+ log_find_operative_subtree_baton_t log_baton;
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);
- *immediate_children = apr_hash_make(result_pool);
- iterpool = svn_pool_create(scratch_pool);
-
- /* 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 *);
-
- svn_pool_clear(iterpool);
-
- if (child->immediate_child_dir)
- apr_hash_set(*immediate_children,
- apr_pstrdup(result_pool, child->abspath),
- APR_HASH_KEY_STRING,
- svn_fspath__join(merge_source_fspath,
- svn_dirent_is_child(merge_target_abspath,
- child->abspath,
- iterpool),
- result_pool));
- }
+ *operative_children = apr_hash_make(result_pool);
- svn_pool_destroy(iterpool);
+ if (depth == svn_depth_infinity)
+ return SVN_NO_ERROR;
/* 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));
- }
+ log_baton.operative_children = *operative_children;
+ log_baton.merge_source_fspath = merge_source_fspath;
+ log_baton.merge_target_abspath = merge_target_abspath;
+ log_baton.depth = depth;
+ log_baton.result_pool = result_pool;
+ 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,
+ &log_baton, scratch_pool));
+
return SVN_NO_ERROR;
}
@@ -7420,20 +7451,16 @@ flag_subtrees_needing_mergeinfo(svn_bool
{
apr_pool_t *iterpool = svn_pool_create(scratch_pool);
int i;
- apr_hash_t *inoperative_immediate_children = NULL;
+ apr_hash_t *operative_immediate_children = NULL;
- /* 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
&& merged_range->start <= merged_range->end
- && depth == svn_depth_immediates)
- SVN_ERR(get_inoperative_immediate_children(
- &inoperative_immediate_children,
+ && (depth < svn_depth_infinity))
+ SVN_ERR(get_operative_immediate_children(
+ &operative_immediate_children,
notify_b->children_with_mergeinfo,
mergeinfo_fspath, merged_range->start + 1, merged_range->end,
- merge_b->target_abspath, merge_b->ra_session1,
+ merge_b->target_abspath, depth, merge_b->ctx->wc_ctx, merge_b->ra_session1,
scratch_pool, iterpool));
/* Issue #4056: Walk NOTIFY_B->CHILDREN_WITH_MERGEINFO reverse depth-first
@@ -7455,7 +7482,7 @@ flag_subtrees_needing_mergeinfo(svn_bool
APR_HASH_KEY_STRING))
continue;
- /* ### ptb: Yes, we could combine the follwing into a single
+ /* ### ptb: Yes, we could combine the following into a single
### conditional, but clarity would suffer (even more than
### it does now). */
if (i == 0)
@@ -7470,8 +7497,8 @@ flag_subtrees_needing_mergeinfo(svn_bool
}
else if (child->immediate_child_dir
&& !child->pre_merge_mergeinfo
- && inoperative_immediate_children
- && !apr_hash_get(inoperative_immediate_children,
+ && operative_immediate_children
+ && apr_hash_get(operative_immediate_children,
child->abspath,
APR_HASH_KEY_STRING))
{
@@ -7554,11 +7581,23 @@ flag_subtrees_needing_mergeinfo(svn_bool
if (child->record_mergeinfo)
{
+ svn_boolean_t missing_child =
+ child->missing_child || child->switched_child;
+
+ /* If, during a shallow merge, the merge target has immediate
+ children that would be affected by a deeper merge, then
+ consider the merge target as missing a child for the purposes
+ of recording non-inheritable mergeinfo. */
+ if (i == 0
+ && depth < svn_depth_immediates
+ && operative_immediate_children
+ && apr_hash_count(operative_immediate_children))
+ missing_child = TRUE;
+
SVN_ERR(calculate_merge_inheritance(&(child->record_noninheritable),
child->abspath,
i == 0,
- (child->missing_child
- || child->switched_child),
+ missing_child,
depth,
merge_b->ctx->wc_ctx,
iterpool));
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1211199&r1=1211198&r2=1211199&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Tue Dec 6 22:04:22 2011
@@ -7276,7 +7276,7 @@ def merge_with_depth_files(sbox):
#
# Test issue #3407 'Shallow merges incorrectly set mergeinfo on children'.
@SkipUnless(server_has_mergeinfo)
-@Issues(2976,3392,3407)
+@Issues(2976,3392,3407,4057)
def merge_away_subtrees_noninheritable_ranges(sbox):
"subtrees can lose non-inheritable ranges"
@@ -7595,8 +7595,9 @@ def merge_away_subtrees_noninheritable_r
svntest.actions.run_and_verify_svn(None, None, [], 'revert', '-R', wc_dir)
svntest.actions.run_and_verify_svn(None, None, [], 'up', wc_dir)
- # Merge r8 from A/D/H to A_COPY_D/H at depth empty, creating non-inheritable
- # mergeinfo on the target. Commit this merge as r13.
+ # Merge r8 from A/D/H to A_COPY_D/H at depth empty. Since r8 affects only
+ # A_COPY/D/H itself, the resulting mergeinfo is inheritabled. Commit this
+ # merge as r13.
expected_output = wc.State(H_COPY_2_path, {
'' : Item(status=' U'),
})
@@ -7612,7 +7613,7 @@ def merge_away_subtrees_noninheritable_r
'chi' : Item(status=' ', wc_rev=12),
})
expected_disk = wc.State('', {
- '' : Item(props={SVN_PROP_MERGEINFO : '/A/D/H:8*',
+ '' : Item(props={SVN_PROP_MERGEINFO : '/A/D/H:8',
"prop:name" : "propval"}),
'psi' : Item("This is the file 'psi'.\n"),
'omega' : Item("This is the file 'omega'.\n"),
@@ -16845,7 +16846,7 @@ def merge_adds_subtree_with_mergeinfo(sb
#----------------------------------------------------------------------
# A test for issue #3978 'reverse merge which adds subtree fails'.
-@Issue(3978)
+@Issue(3978,4057)
@SkipUnless(server_has_mergeinfo)
def reverse_merge_adds_subtree(sbox):
"reverse merge adds subtree"
@@ -16872,6 +16873,9 @@ def reverse_merge_adds_subtree(sbox):
'Cherry-pick r7 from A to A_COPY', wc_dir)
# r9 - File depth sync merge from A/D/H to A_COPY/D/H/
+ # This shallow merge does not create non-inheritable mergeinfo because of
+ # the issue #4057 fix; all subtrees affected by the diff are present, so
+ # non-inheritable mergeinfo is not required.
svntest.actions.run_and_verify_svn(None, None, [], 'up', wc_dir)
svntest.actions.run_and_verify_svn(None, None, [], 'merge',
sbox.repo_url + '/A/D/H',
@@ -16958,12 +16962,10 @@ def reverse_merge_adds_subtree(sbox):
'D/G/rho' : Item("This is the file 'rho'.\n"),
'D/G/tau' : Item("This is the file 'tau'.\n"),
'D/gamma' : Item("This is the file 'gamma'.\n"),
- 'D/H' : Item(props={SVN_PROP_MERGEINFO : '/A/D/H:2-6*,8*'}),
+ 'D/H' : Item(props={SVN_PROP_MERGEINFO : '/A/D/H:2-6,8'}),
'D/H/chi' : Item("This is the file 'chi'.\n"),
- 'D/H/psi' : Item("New content",
- props={SVN_PROP_MERGEINFO : '/A/D/H/psi:2-8'}),
- 'D/H/omega' : Item("New content",
- props={SVN_PROP_MERGEINFO : '/A/D/H/omega:2-8'}),
+ 'D/H/psi' : Item("New content"),
+ 'D/H/omega' : Item("New content"),
})
expected_skip = wc.State('.', { })
svntest.actions.run_and_verify_merge(A_COPY_path, 7, 6,
@@ -17239,7 +17241,6 @@ def unnecessary_noninheritable_mergeinfo
# Test for issue #4057 "don't record non-inheritable mergeinfo in shallow
# merge if entire diff is within requested depth".
@Issue(4057)
-@XFail()
@SkipUnless(server_has_mergeinfo)
def unnecessary_noninheritable_mergeinfo_shallow_merge(sbox):
"shallow merge reaches all neccessary subtrees"
@@ -17253,7 +17254,7 @@ def unnecessary_noninheritable_mergeinfo
# Merge r3 from ^/A/B to branch/B at operational depth=files
#
- # Currently this fails because merge isn't smart enough to
+ # Previously this failed because merge wasn't smart enough to
# realize that despite being a shallow merge, the diff can
# only affect branch/B/lambda, which is within the specified
# depth, so there is no need to record non-inheritable mergeinfo
Modified: subversion/trunk/subversion/tests/cmdline/mergeinfo_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/mergeinfo_tests.py?rev=1211199&r1=1211198&r2=1211199&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/mergeinfo_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/mergeinfo_tests.py Tue Dec 6 22:04:22 2011
@@ -706,15 +706,14 @@ def noninheritabled_mergeinfo_not_always
svntest.actions.run_and_verify_svn(None, None, [], 'merge',
sbox.repo_url + '/A', branch_path,
'-c3', '--depth=empty')
+ # Forcibly set non-inheritable mergeinfo to replicate the pre-1.8 behavior,
+ # where prior to the fix for issue #4057, non-inheritable mergeinfo was
+ # unconditionally set for merges with shallow operational depths.
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'propset', SVN_PROP_MERGEINFO,
+ '/A:3*\n', branch_path)
svntest.main.run_svn(None, 'commit', '-m', 'shallow merge', wc_dir)
- # A sanity check that we really have non-inheritable mergeinfo.
- # If issue #4057 is ever fixed then the above merge will record
- # inheritable mergeinfo and this test will spuriously pass.
- svntest.actions.run_and_verify_svn(None, ["/A:3*\n"], [],
- 'propget', SVN_PROP_MERGEINFO,
- branch_path)
-
# Now check that r3 is reported as fully merged from ^/A to ^/branch
# and does not show up all when asking for eligible revs.
svntest.actions.run_and_verify_mergeinfo(
Re: svn commit: r1211199 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_tests.py tests/cmdline/mergeinfo_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Dec 8, 2011 at 10:08 AM, Paul Burba <pt...@gmail.com> wrote:
> On Wed, Dec 7, 2011 at 6:45 PM, Philip Martin
> <ph...@wandisco.com> wrote:
>> pburba@apache.org writes:
>>
>>> Author: pburba
>>> Date: Tue Dec 6 22:04:22 2011
>>> New Revision: 1211199
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1211199&view=rev
>>> Log:
>>> Fix issue #4057 "don't record non-inheritable mergeinfo in shallow merge
>>> if entire diff is within requested depth".
>>
>> This commit causes merge_tests.py 120 and 124 to fail for BDB
>> repositories on one of the buildbots and on my machine. The tests pass
>> with FSFS repositories. valgrind doesn't give any warnings.
>
> Hi Philip,
>
> Thanks for the heads-up, I'm aware of the problem and working on a fix.
Done r1212015.
> Paul
Re: svn commit: r1211199 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_tests.py tests/cmdline/mergeinfo_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Dec 7, 2011 at 6:45 PM, Philip Martin
<ph...@wandisco.com> wrote:
> pburba@apache.org writes:
>
>> Author: pburba
>> Date: Tue Dec 6 22:04:22 2011
>> New Revision: 1211199
>>
>> URL: http://svn.apache.org/viewvc?rev=1211199&view=rev
>> Log:
>> Fix issue #4057 "don't record non-inheritable mergeinfo in shallow merge
>> if entire diff is within requested depth".
>
> This commit causes merge_tests.py 120 and 124 to fail for BDB
> repositories on one of the buildbots and on my machine. The tests pass
> with FSFS repositories. valgrind doesn't give any warnings.
Hi Philip,
Thanks for the heads-up, I'm aware of the problem and working on a fix.
Paul
Re: svn commit: r1211199 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tests.py tests/cmdline/mergeinfo_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
pburba@apache.org writes:
> Author: pburba
> Date: Tue Dec 6 22:04:22 2011
> New Revision: 1211199
>
> URL: http://svn.apache.org/viewvc?rev=1211199&view=rev
> Log:
> Fix issue #4057 "don't record non-inheritable mergeinfo in shallow merge
> if entire diff is within requested depth".
This commit causes merge_tests.py 120 and 124 to fail for BDB
repositories on one of the buildbots and on my machine. The tests pass
with FSFS repositories. valgrind doesn't give any warnings.
On the buildbot:
http://ci.apache.org/builders/svn-x64-centos-gcc/builds/5157
On my machine:
$ ../../../../src/subversion/tests/cmdline/merge_tests.py 120 --fs-type=bdb
=============================================================
Expected 'H' and actual 'H' in disk tree are different!
=============================================================
EXPECTED NODE TO BE:
=============================================================
* Node name: H
Path: __SVN_ROOT_NODE/D/H
Contents: N/A (node is a directory)
Properties: {'svn:mergeinfo': '/A/D/H:2-6,8'}
Attributes: {}
Children: 3
=============================================================
ACTUAL NODE FOUND:
=============================================================
* Node name: H
Path: __SVN_ROOT_NODE/D/H
Contents: N/A (node is a directory)
Properties: {u'svn:mergeinfo': u'/A/D/H:2-6*,8*'}
Attributes: {}
Children: 3
Unequal at node H
Unequal at node D
EXPECTED DISK TREE:
svntest.wc.State(wc_dir, {
'C' : Item(),
'B/E/beta' : Item(contents="This is the file 'beta'.\n"),
'B/E/alpha' : Item(contents="This is the file 'alpha'.\n"),
'B/lambda' : Item(contents="This is the file 'lambda'.\n"),
'B/F' : Item(),
'D/H' : Item(props={'svn:mergeinfo':'/A/D/H:2-6,8'}),
'D/H/omega' : Item(contents="New content"),
'D/H/psi' : Item(contents="New content"),
'D/H/chi' : Item(contents="This is the file 'chi'.\n"),
'D/G/rho' : Item(contents="This is the file 'rho'.\n"),
'D/G/tau' : Item(contents="This is the file 'tau'.\n"),
'D/G/pi' : Item(contents="This is the file 'pi'.\n"),
'D/gamma' : Item(contents="This is the file 'gamma'.\n"),
'mu' : Item(contents="This is the file 'mu'.\n"),
})ACTUAL DISK TREE:
svntest.wc.State(wc_dir, {
'C' : Item(),
'B/E/alpha' : Item(contents="This is the file 'alpha'.\n"),
'B/E/beta' : Item(contents="This is the file 'beta'.\n"),
'B/lambda' : Item(contents="This is the file 'lambda'.\n"),
'B/F' : Item(),
'D/H' : Item(props={'svn:mergeinfo':'/A/D/H:2-6*,8*'}),
'D/H/omega' : Item(contents="New content", props={'svn:mergeinfo':'/A/D/H/omega:2-8'}),
'D/H/psi' : Item(contents="New content", props={'svn:mergeinfo':'/A/D/H/psi:2-8'}),
'D/H/chi' : Item(contents="This is the file 'chi'.\n"),
'D/G/pi' : Item(contents="This is the file 'pi'.\n"),
'D/G/rho' : Item(contents="This is the file 'rho'.\n"),
'D/G/tau' : Item(contents="This is the file 'tau'.\n"),
'D/gamma' : Item(contents="This is the file 'gamma'.\n"),
'mu' : Item(contents="This is the file 'mu'.\n"),
})CWD: /home/pm/sw/subversion/obj/subversion/tests/cmdline
EXCEPTION: SVNTreeUnequal
Traceback (most recent call last):
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/main.py", line 1326, in run
rc = self.pred.run(sandbox)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/testcase.py", line 254, in run
return self._delegate.run(sandbox)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/testcase.py", line 176, in run
return self.func(sandbox)
File "../../../../src/subversion/tests/cmdline/merge_tests.py", line 16980, in reverse_merge_adds_subtree
None, 1, False)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/actions.py", line 1133, in run_and_verify_merge
check_props)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/actions.py", line 779, in verify_update
singleton_handler_b, b_baton)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/tree.py", line 686, in compare_trees
singleton_handler_b, b_baton)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/tree.py", line 686, in compare_trees
singleton_handler_b, b_baton)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/tree.py", line 672, in compare_trees
raise SVNTreeUnequal
SVNTreeUnequal
FAIL: merge_tests.py 120: reverse merge adds subtree
$ ../../../../src/subversion/tests/cmdline/merge_tests.py 124 --fs-type=bdb
=============================================================
Expected '__SVN_ROOT_NODE' and actual '__SVN_ROOT_NODE' in elision_output tree are different!
=============================================================
EXPECTED NODE TO BE:
=============================================================
* Node name: __SVN_ROOT_NODE
Path: __SVN_ROOT_NODE
Contents: N/A (node is a directory)
Properties: {}
Attributes: {}
Children: 1
=============================================================
ACTUAL NODE FOUND:
=============================================================
* Node name: __SVN_ROOT_NODE
Path: __SVN_ROOT_NODE
Contents: None
Properties: {}
Attributes: {}
Children: None (node is probably a file)
Unequal Types: one Node is a file, the other is a directory
ACTUAL ELISION OUTPUT TREE:
svntest.wc.State(wc_dir, {})CWD: /home/pm/sw/subversion/obj/subversion/tests/cmdline
EXCEPTION: SVNTreeUnequal
Traceback (most recent call last):
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/main.py", line 1326, in run
rc = self.pred.run(sandbox)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/testcase.py", line 254, in run
return self._delegate.run(sandbox)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/testcase.py", line 176, in run
return self.func(sandbox)
File "../../../../src/subversion/tests/cmdline/merge_tests.py", line 17280, in unnecessary_noninheritable_mergeinfo_shallow_merge
'--depth', 'files', B_branch_path)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/actions.py", line 1133, in run_and_verify_merge
check_props)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/actions.py", line 766, in verify_update
elision_output_tree)
File "/home/pm/sw/subversion/src/subversion/tests/cmdline/svntest/tree.py", line 694, in compare_trees
raise SVNTreeUnequal
SVNTreeUnequal
FAIL: merge_tests.py 124: shallow merge reaches all neccessary subtrees
--
Philip
Re: svn commit: r1211199 - Fix issue #4057 "don't record non-inheritable mergeinfo ..."
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> Yes that's what I meant. But it's a moot point now: In fixing another
> issue #4057 bug (see r1211647) I moved this function's code inline to
> its only caller. I also tried to improve the comments there to better
> explain what is going on. Let me know if that doesn't clear things
> up.
Thanks; that looks good AFAICT.
- Julian
Re: svn commit: r1211199 - Fix issue #4057 "don't record
non-inheritable mergeinfo ..."
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Dec 7, 2011 at 6:26 AM, Julian Foad <ju...@btopenworld.com> wrote:
>> Author: pburba
>
>> Date: Tue Dec 6 22:04:22 2011
>> New Revision: 1211199
>>
>> URL: http://svn.apache.org/viewvc?rev=1211199&view=rev
>> Log:
>> Fix issue #4057 "don't record non-inheritable mergeinfo in shallow
>> merge if entire diff is within requested depth".
>>
>> * subversion/libsvn_client/merge.c
>>
>> (calculate_merge_inheritance): Don't unconditionally set non-inheritable
>> mergeinfo on the merge target if the operational depth is shallow.
>> Instead rely on the caller to determine if this is necessary.
> [...]
>
>> WC_PATH_HAS_MISSING_CHILD is true if WC_PATH is missing an immediate child
>> because the child is switched or absent from the WC, or due to a sparse
>> - checkout -- see get_mergeinfo_paths().
>> + checkout (see get_mergeinfo_paths) or because DEPTH is shallow
>> + (i.e. < svn_depth_infinity) and the merge would affect a child if
>> + performed at a deeper depth.
>
> Does the phrase "and the merge would affect a child" apply to the parameter as a whole? It sounds like it's asking the caller to determine whether the diff would have affected a missing child in some cases but not in other cases. (I haven't tried to see whether the caller actually does make that determination in the other cases.) More concretely: if WC_PATH is missing an immediate child because the child is switched or absent from the WC, or due to a sparse checkout, then I think the caller should pass TRUE only if the diff would affect that child. Maybe that's what you meant already.
Hi Julian,
Yes that's what I meant. But it's a moot point now: In fixing another
issue #4057 bug (see r1211647) I moved this function's code inline to
its only caller. I also tried to improve the comments there to better
explain what is going on. Let me know if that doesn't clear things
up.
Paul
> - Julian
>
>
>> Perform any temporary allocations in SCRATCH_POOL. */
>> static svn_error_t *
>> @@ -4543,9 +4545,7 @@ calculate_merge_inheritance(svn_boolean_
>> {
>> if (wc_path_is_merge_target)
>> {
>> - if (wc_path_has_missing_child
>> - || depth == svn_depth_files
>> - || depth == svn_depth_empty)
>> + if (wc_path_has_missing_child)
>> {
>> *non_inheritable = TRUE;
>> }
Re: svn commit: r1211199 - Fix issue #4057 "don't record non-inheritable mergeinfo ..."
Posted by Julian Foad <ju...@btopenworld.com>.
> Author: pburba
> Date: Tue Dec 6 22:04:22 2011
> New Revision: 1211199
>
> URL: http://svn.apache.org/viewvc?rev=1211199&view=rev
> Log:
> Fix issue #4057 "don't record non-inheritable mergeinfo in shallow
> merge if entire diff is within requested depth".
>
> * subversion/libsvn_client/merge.c
>
> (calculate_merge_inheritance): Don't unconditionally set non-inheritable
> mergeinfo on the merge target if the operational depth is shallow.
> Instead rely on the caller to determine if this is necessary.
[...]
> WC_PATH_HAS_MISSING_CHILD is true if WC_PATH is missing an immediate child
> because the child is switched or absent from the WC, or due to a sparse
> - checkout -- see get_mergeinfo_paths().
> + checkout (see get_mergeinfo_paths) or because DEPTH is shallow
> + (i.e. < svn_depth_infinity) and the merge would affect a child if
> + performed at a deeper depth.
Does the phrase "and the merge would affect a child" apply to the parameter as a whole? It sounds like it's asking the caller to determine whether the diff would have affected a missing child in some cases but not in other cases. (I haven't tried to see whether the caller actually does make that determination in the other cases.) More concretely: if WC_PATH is missing an immediate child because the child is switched or absent from the WC, or due to a sparse checkout, then I think the caller should pass TRUE only if the diff would affect that child. Maybe that's what you meant already.
- Julian
> Perform any temporary allocations in SCRATCH_POOL. */
> static svn_error_t *
> @@ -4543,9 +4545,7 @@ calculate_merge_inheritance(svn_boolean_
> {
> if (wc_path_is_merge_target)
> {
> - if (wc_path_has_missing_child
> - || depth == svn_depth_files
> - || depth == svn_depth_empty)
> + if (wc_path_has_missing_child)
> {
> *non_inheritable = TRUE;
> }