You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <rh...@sharpsvn.net> on 2009/03/17 08:45:13 UTC
RE: svn commit: r36613 - trunk/subversion/libsvn_client
> -----Original Message-----
> From: Paul T. Burba [mailto:pburba@collab.net]
> Sent: Tuesday, March 17, 2009 1:23 AM
> To: svn@subversion.tigris.org
> Subject: svn commit: r36613 - trunk/subversion/libsvn_client
>
> Author: pburba
> Date: Mon Mar 16 17:22:43 2009
> New Revision: 36613
>
> Log:
> Avoid potential segfaults caused by NULL elements in the
> CHILDREN_WITH_MERGEINFO array by removing elements rather than setting
> them
> to NULL.
Hi,
It looks like this test generates the following failure on neon (and
probably serf):
FAIL: merge_authz_tests.py 1: skipped paths get overriding mergeinfo
(From
http://crest.ics.uci.edu/buildbot/builders/x86-macosx-gnu%20shared/builds/17
8/steps/Test%20fsfs%2Bra_neon/logs/testlog/text):
CMD: svn proplist --verbose
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/C
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ome
ga svn-test-
work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E/bet
a svn-test-work/working_copies/merge_authz_test
s-1.restricted/A_COPY_2/mu
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/gamma
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E/alp
ha
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ch
i
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/lambd
a svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/F
svn-test-work/working_copies/mer
ge_authz_tests-1.restricted/A_COPY_2 --config-dir
/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/svn
-test-work/local_tmp/config --password rayjandom --no-auth-cache --username
jrandom <TIME = 0.086657>
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/om
ega':
svn:mergeinfo
/A/D/H/omega:5-8
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D':
svn:mergeinfo
/A/D:5-8*
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H':
svn:mergeinfo
/A/D/H:5-8*
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ch
i':
svn:mergeinfo
/A/D/H/chi:5-8
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E':
svn:mergeinfo
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2':
svn:mergeinfo
/A:5-8
=============================================================
Expected 'gamma' and actual 'gamma' in disk tree are different!
=============================================================
EXPECTED NODE TO BE:
=============================================================
* Node name: gamma
Path: __SVN_ROOT_NODE/D/gamma
Contents: This is the file 'gamma'.
Properties: {'svn:mergeinfo': '/A/D/gamma:5-8'}
Attributes: {}
Children: None (node is probably a file)
=============================================================
ACTUAL NODE FOUND:
=============================================================
* Node name: gamma
Path: __SVN_ROOT_NODE/D/gamma
Contents: This is the file 'gamma'.
Properties: {}
Attributes: {}
Children: None (node is probably a file)
Unequal at node gamma
Unequal at node D
ACTUAL DISK TREE:
svntest.wc.State('', {
'C' : Item(),
'B' : Item(),
'B/E' : Item(props={'svn:mergeinfo':''}),
'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' : Item(props={'svn:mergeinfo':'/A/D:5-8*'}),
'D/H' : Item(props={'svn:mergeinfo':'/A/D/H:5-8*'}),
'D/H/omega' : Item(contents="New content",
props={'svn:mergeinfo':'/A/D/H/omega:5-8'}),
'D/H/chi' : Item(contents="This is the file 'chi'.\n",
props={'svn:mergeinfo':'/A/D/H/chi:5-8'}),
'D/gamma' : Item(contents="This is the file 'gamma'.\n"),
'mu' : Item(contents="This is the file 'mu'.\n"),
'.' : Item(props={'svn:mergeinfo':'/A:5-8'}),
})
EXCEPTION: SVNTreeUnequal
Traceback (most recent call last):
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/main.py", line 1120, in run
rc = self.pred.run(sandbox)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/testcase.py", line 185, in run
return self._delegate.run(sandbox)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/testcase.py", line 185, in run
return self._delegate.run(sandbox)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/testcase.py", line 114, in run
return self.func(sandbox)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/me
rge_authz_tests.py", line 260, in mergeinfo_and_skipped_paths
None, 1, 0)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/actions.py", line 788, in run_and_verify_merge
b_baton, check_props, dry_run)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/actions.py", line 916, in run_and_verify_merge2
check_props)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/actions.py", line 615, in verify_update
singleton_handler_b, b_baton)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/tree.py", line 626, in compare_trees
singleton_handler_b, b_baton)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/tree.py", line 626, in compare_trees
singleton_handler_b, b_baton)
File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/tree.py", line 601, in compare_trees
raise SVNTreeUnequal
SVNTreeUnequal
FAIL: merge_authz_tests.py 1: skipped paths get overriding mergeinfo
>
> Found by: Mark Eichin <ei...@gmail.com>
>
> See http://svn.haxx.se/dev/archive-2009-03/0382.shtml.
>
> * subversion/libsvn_client/merge.c
> (CHILDREN_WITH_MERGEINFO ARRAY): Tweak this global comment.
> (populate_remaining_ranges, drive_merge_report_editor,
> do_directory_merge):
> Check for NULL children_with_mergeinfo elements with SVN_ERR_ASSERT.
> (remove_child_with_mergeinfo): New.
> (remove_absent_children, remove_children_with_deleted_mergeinfo): Use
> remove_child_with_mergeinfo() to truly remove elements rather than
> setting
> them to NULL.
>
> Modified:
> trunk/subversion/libsvn_client/merge.c
>
> Modified: trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/merge.c
> ?pathrev=36613&r1=36612&r2=36613
> =======================================================================
> =======
> --- trunk/subversion/libsvn_client/merge.c Mon Mar 16 16:57:46 2009
> (r36612)
> +++ trunk/subversion/libsvn_client/merge.c Mon Mar 16 17:22:43 2009
> (r36613)
> @@ -131,7 +131,8 @@
> *
> * CHILDREN_WITH_MERGEINFO is initially created by
> get_mergeinfo_paths()
> * and outside of that function and its helpers should always meet the
> - * criteria dictated in get_mergeinfo_paths()'s doc string.
> + * criteria dictated in get_mergeinfo_paths()'s doc string. The
> elements
> + * of CHILDREN_WITH_MERGINFO should never be NULL.
> */
>
> /*--------------------------------------------------------------------
> ---*/
> @@ -3269,7 +3270,8 @@ populate_remaining_ranges(apr_array_head
> svn_client__merge_path_t *parent = NULL;
>
> /* If the path is absent don't do subtree merge either. */
> - if (!child || child->absent)
> + SVN_ERR_ASSERT(child);
> + if (child->absent)
> continue;
>
> svn_pool_clear(iterpool);
> @@ -3634,6 +3636,37 @@ make_merge_conflict_error(const char *ta
> r->start, r->end, svn_path_local_style(target_wcpath, pool));
> }
>
> +/* Remove the element at INDEX from the array CHILDREN_WITH_MERGEINFO.
> + If INDEX is not a valid element of CHILDREN_WITH_MERGEINFO do
> nothing. */
> +static void
> +remove_child_with_mergeinfo(apr_array_header_t
> *children_with_mergeinfo,
> + int index)
> +{
> + /* Do we have a valid index? */
> + if (index >= 0 && index < children_with_mergeinfo->nelts)
> + {
> + if (index == (children_with_mergeinfo->nelts - 1))
> + {
> + /* Deleting the last or only element in an array is easy. */
> + apr_array_pop(children_with_mergeinfo);
> + }
> + else
> + {
> + int remainder = children_with_mergeinfo->nelts - 1 - index;
> + svn_client__merge_path_t *deleted_child =
> + APR_ARRAY_IDX(children_with_mergeinfo, index,
> + svn_client__merge_path_t *);
> + svn_client__merge_path_t *next_child =
> + APR_ARRAY_IDX(children_with_mergeinfo, index + 1,
> + svn_client__merge_path_t *);
> +
> + memmove(deleted_child, next_child,
> + children_with_mergeinfo->elt_size * remainder);
> + --(children_with_mergeinfo->nelts);
> + }
> + }
> +}
> +
> /* Helper for do_directory_merge().
>
> TARGET_WCPATH is a directory and CHILDREN_WITH_MERGEINFO is filled
> @@ -3657,15 +3690,13 @@ remove_absent_children(const char *targe
> svn_client__merge_path_t *child =
> APR_ARRAY_IDX(children_with_mergeinfo,
> i, svn_client__merge_path_t *);
> - if (child
> - && (child->absent || child->scheduled_for_deletion)
> + if ((child->absent || child->scheduled_for_deletion)
> && svn_path_is_ancestor(target_wcpath, child->path))
> {
> if (notify_b->skipped_paths)
> apr_hash_set(notify_b->skipped_paths, child->path,
> APR_HASH_KEY_STRING, NULL);
> - APR_ARRAY_IDX(children_with_mergeinfo, i,
> - svn_client__merge_path_t *) = NULL;
> + remove_child_with_mergeinfo(children_with_mergeinfo, i);
> }
> }
> }
> @@ -3695,13 +3726,12 @@ remove_children_with_deleted_mergeinfo(m
> svn_client__merge_path_t *child =
> APR_ARRAY_IDX(notify_b->children_with_mergeinfo,
> i, svn_client__merge_path_t *);
> - if (child
> - && apr_hash_get(merge_b->paths_with_deleted_mergeinfo,
> - child->path,
> - APR_HASH_KEY_STRING))
> + if (apr_hash_get(merge_b->paths_with_deleted_mergeinfo,
> + child->path,
> + APR_HASH_KEY_STRING))
> {
> - APR_ARRAY_IDX(notify_b->children_with_mergeinfo, i,
> - svn_client__merge_path_t *) = NULL;
> + remove_child_with_mergeinfo(notify_b-
> >children_with_mergeinfo,
> + i);
> }
> }
> }
> @@ -3822,7 +3852,7 @@ drive_merge_report_editor(const char *ta
> see 'THE CHILDREN_WITH_MERGEINFO ARRAY'. */
> child = APR_ARRAY_IDX(children_with_mergeinfo, 0,
> svn_client__merge_path_t *);
> -
> + SVN_ERR_ASSERT(child);
> if (child->remaining_ranges->nelts == 0)
> {
> /* The merge target doesn't need anything merged. */
> @@ -3899,7 +3929,8 @@ drive_merge_report_editor(const char *ta
> int parent_index;
> svn_boolean_t nearest_parent_is_target;
>
> - if (!child || child->absent)
> + SVN_ERR_ASSERT(child);
> + if (child->absent)
> continue;
>
> /* Find this child's nearest wc ancestor with mergeinfo. */
> @@ -6330,7 +6361,8 @@ do_directory_merge(const char *url1,
> svn_client__merge_path_t *child =
> APR_ARRAY_IDX(notify_b-
> >children_with_mergeinfo, i,
> svn_client__merge_path_t *);
> - if (!child || child->absent)
> + SVN_ERR_ASSERT(child);
> + if (child->absent)
> continue;
>
> if (strlen(child->path) == merge_target_len)
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=1336228
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1339304
Re: svn commit: r36613 - trunk/subversion/libsvn_client
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Mar 17, 2009 at 4:45 AM, Bert Huijben <rh...@sharpsvn.net> wrote:
>> -----Original Message-----
>> From: Paul T. Burba [mailto:pburba@collab.net]
>> Sent: Tuesday, March 17, 2009 1:23 AM
>> To: svn@subversion.tigris.org
>> Subject: svn commit: r36613 - trunk/subversion/libsvn_client
>>
>> Author: pburba
>> Date: Mon Mar 16 17:22:43 2009
>> New Revision: 36613
>>
>> Log:
>> Avoid potential segfaults caused by NULL elements in the
>> CHILDREN_WITH_MERGEINFO array by removing elements rather than setting
>> them
>> to NULL.
>
> Hi,
>
> It looks like this test generates the following failure on neon (and
> probably serf):
> FAIL: merge_authz_tests.py 1: skipped paths get overriding mergeinfo
>
> (From
> http://crest.ics.uci.edu/buildbot/builders/x86-macosx-gnu%20shared/builds/17
> 8/steps/Test%20fsfs%2Bra_neon/logs/testlog/text):
>
> CMD: svn proplist --verbose
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/C
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ome
> ga svn-test-
> work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E/bet
> a svn-test-work/working_copies/merge_authz_test
> s-1.restricted/A_COPY_2/mu
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/gamma
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E/alp
> ha
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ch
> i
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/lambd
> a svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E
> svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/F
> svn-test-work/working_copies/mer
> ge_authz_tests-1.restricted/A_COPY_2 --config-dir
> /Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/svn
> -test-work/local_tmp/config --password rayjandom --no-auth-cache --username
> jrandom <TIME = 0.086657>
> Properties on
> 'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/om
> ega':
> svn:mergeinfo
> /A/D/H/omega:5-8
> Properties on
> 'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D':
> svn:mergeinfo
> /A/D:5-8*
> Properties on
> 'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H':
> svn:mergeinfo
> /A/D/H:5-8*
> Properties on
> 'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ch
> i':
> svn:mergeinfo
> /A/D/H/chi:5-8
> Properties on
> 'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E':
> svn:mergeinfo
>
> Properties on
> 'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2':
> svn:mergeinfo
> /A:5-8
> =============================================================
> Expected 'gamma' and actual 'gamma' in disk tree are different!
> =============================================================
> EXPECTED NODE TO BE:
> =============================================================
> * Node name: gamma
> Path: __SVN_ROOT_NODE/D/gamma
> Contents: This is the file 'gamma'.
>
> Properties: {'svn:mergeinfo': '/A/D/gamma:5-8'}
> Attributes: {}
> Children: None (node is probably a file)
> =============================================================
> ACTUAL NODE FOUND:
> =============================================================
> * Node name: gamma
> Path: __SVN_ROOT_NODE/D/gamma
> Contents: This is the file 'gamma'.
>
> Properties: {}
> Attributes: {}
> Children: None (node is probably a file)
> Unequal at node gamma
> Unequal at node D
> ACTUAL DISK TREE:
> svntest.wc.State('', {
> 'C' : Item(),
> 'B' : Item(),
> 'B/E' : Item(props={'svn:mergeinfo':''}),
> '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' : Item(props={'svn:mergeinfo':'/A/D:5-8*'}),
> 'D/H' : Item(props={'svn:mergeinfo':'/A/D/H:5-8*'}),
> 'D/H/omega' : Item(contents="New content",
> props={'svn:mergeinfo':'/A/D/H/omega:5-8'}),
> 'D/H/chi' : Item(contents="This is the file 'chi'.\n",
> props={'svn:mergeinfo':'/A/D/H/chi:5-8'}),
> 'D/gamma' : Item(contents="This is the file 'gamma'.\n"),
> 'mu' : Item(contents="This is the file 'mu'.\n"),
> '.' : Item(props={'svn:mergeinfo':'/A:5-8'}),
> })
> EXCEPTION: SVNTreeUnequal
> Traceback (most recent call last):
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/main.py", line 1120, in run
> rc = self.pred.run(sandbox)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/testcase.py", line 185, in run
> return self._delegate.run(sandbox)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/testcase.py", line 185, in run
> return self._delegate.run(sandbox)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/testcase.py", line 114, in run
> return self.func(sandbox)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/me
> rge_authz_tests.py", line 260, in mergeinfo_and_skipped_paths
> None, 1, 0)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/actions.py", line 788, in run_and_verify_merge
> b_baton, check_props, dry_run)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/actions.py", line 916, in run_and_verify_merge2
> check_props)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/actions.py", line 615, in verify_update
> singleton_handler_b, b_baton)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/tree.py", line 626, in compare_trees
> singleton_handler_b, b_baton)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/tree.py", line 626, in compare_trees
> singleton_handler_b, b_baton)
> File
> "/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
> ntest/tree.py", line 601, in compare_trees
> raise SVNTreeUnequal
> SVNTreeUnequal
> FAIL: merge_authz_tests.py 1: skipped paths get overriding mergeinfo
Bert,
Yeah, that was due to some stupidity on my part. Fixed in r33631.
That will teach me to run the tests over [ra_svn | ra_neon | ra_serf]
when making merge related changes (merge_authz_tests.py doesn't run
over ra_local).
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1342388