Re: svn commit: r1104610 - in /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h
On Wed, May 18, 2011 at 11:13 AM, Bert Huijben wrote: > > >> -Original Message- >> From: Hyrum K Wright [mailto:hy...@hyrumwright.org] >> Sent: woensdag 18 mei 2011 11:11 >> To: Bert Huijben >> Cc: d...@subversion.apache.org; commits@subversion.apache.org >> Subject: Re: svn commit: r1104610 - in >> /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h >> >> On Wed, May 18, 2011 at 1:48 AM, Bert Huijben wrote: >> >> -Original Message- >> >> From: Hyrum K Wright [mailto:hy...@hyrumwright.org] >> >> Sent: woensdag 18 mei 2011 1:11 >> >> To: d...@subversion.apache.org >> >> Cc: commits@subversion.apache.org >> >> Subject: Re: svn commit: r1104610 - in >> >> /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h >> >> >> >> I understand the desire to get the buildbots green again, and I'm >> >> sorry these revisions which I committed broke the bots, but a little >> >> patience might have been useful here. We have a long tradition of >> >> allowing folks to attempt to fix problems, rather than reverting their >> >> commits without consultation. I kinda wish you'd have given me >> >> another 12 hours to attempt to fix it, rather than reverting. >> > >> > We also have the generic rule that any committer (full or partial) may >> > revert something that makes it impossible for them to do further >> > development. (See hacking) >> >> No, we have a policy that people can revert changes to the *build >> system* which prevent productivity: >> "To prevent loss of productivity, any committer (full or partial) can >> immediately revert any build system change that breaks their ability >> to effectively do development on their platform of choice, as a matter >> of ordinary routing, without fear of accusations of an over-reaction." >> (From: http://subversion.apache.org/docs/community- >> guide/building.html#configury >> ) >> >> I'm not trying to play process obstructionist, just noting that a mail >> mentioning the breakage and indicating your intent to revert would >> have been a nice consideration. >> >> > And tomorrow morning the asf repository will be readonly for quite some >> > time, so waiting till after that will probably cause more delays. >> > >> > Besides, you just mailed that you weren't going to fix this issue... :-) >> >> I guess I should have been more clear: I'm happy to fix my own >> breakage to the buildbots. When indicating I was moving on to other >> things, I didn't know I'd broken the test world. >> >> > Somehow the test that should have picked up the original failure is > broken. >> > It thinks that no output at all for a recursive proplist is ok. >> > >> > So two different bugs (the local changes one; and the base-deleted one) >> > together kept the prop_tests.py 15 test succeeding. >> >> Has this bug in the test suite been fixed? If not, I suppose that's a >> place to start... > > r1104641 fixes the test suite Great. Thanks for doing this. > And r1104631, probably fixes most of the other problems of the patch. > (Except for the performance regression of single node property reads, such > as used by the merge code) Alrighty, I'll reapply it and see what happens. Is the performance regression theoretical or practical? -Hyrum
RE: svn commit: r1104610 - in /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h
> -Original Message- > From: Hyrum K Wright [mailto:hy...@hyrumwright.org] > Sent: woensdag 18 mei 2011 11:11 > To: Bert Huijben > Cc: d...@subversion.apache.org; commits@subversion.apache.org > Subject: Re: svn commit: r1104610 - in > /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h > > On Wed, May 18, 2011 at 1:48 AM, Bert Huijben wrote: > >> -Original Message- > >> From: Hyrum K Wright [mailto:hy...@hyrumwright.org] > >> Sent: woensdag 18 mei 2011 1:11 > >> To: d...@subversion.apache.org > >> Cc: commits@subversion.apache.org > >> Subject: Re: svn commit: r1104610 - in > >> /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h > >> > >> I understand the desire to get the buildbots green again, and I'm > >> sorry these revisions which I committed broke the bots, but a little > >> patience might have been useful here. We have a long tradition of > >> allowing folks to attempt to fix problems, rather than reverting their > >> commits without consultation. I kinda wish you'd have given me > >> another 12 hours to attempt to fix it, rather than reverting. > > > > We also have the generic rule that any committer (full or partial) may > > revert something that makes it impossible for them to do further > > development. (See hacking) > > No, we have a policy that people can revert changes to the *build > system* which prevent productivity: > "To prevent loss of productivity, any committer (full or partial) can > immediately revert any build system change that breaks their ability > to effectively do development on their platform of choice, as a matter > of ordinary routing, without fear of accusations of an over-reaction." > (From: http://subversion.apache.org/docs/community- > guide/building.html#configury > ) > > I'm not trying to play process obstructionist, just noting that a mail > mentioning the breakage and indicating your intent to revert would > have been a nice consideration. > > > And tomorrow morning the asf repository will be readonly for quite some > > time, so waiting till after that will probably cause more delays. > > > > Besides, you just mailed that you weren't going to fix this issue... :-) > > I guess I should have been more clear: I'm happy to fix my own > breakage to the buildbots. When indicating I was moving on to other > things, I didn't know I'd broken the test world. > > > Somehow the test that should have picked up the original failure is broken. > > It thinks that no output at all for a recursive proplist is ok. > > > > So two different bugs (the local changes one; and the base-deleted one) > > together kept the prop_tests.py 15 test succeeding. > > Has this bug in the test suite been fixed? If not, I suppose that's a > place to start... r1104641 fixes the test suite And r1104631, probably fixes most of the other problems of the patch. (Except for the performance regression of single node property reads, such as used by the merge code) Bert
Re: svn commit: r1104610 - in /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h
On Wed, May 18, 2011 at 1:48 AM, Bert Huijben wrote: >> -Original Message- >> From: Hyrum K Wright [mailto:hy...@hyrumwright.org] >> Sent: woensdag 18 mei 2011 1:11 >> To: d...@subversion.apache.org >> Cc: commits@subversion.apache.org >> Subject: Re: svn commit: r1104610 - in >> /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h >> >> I understand the desire to get the buildbots green again, and I'm >> sorry these revisions which I committed broke the bots, but a little >> patience might have been useful here. We have a long tradition of >> allowing folks to attempt to fix problems, rather than reverting their >> commits without consultation. I kinda wish you'd have given me >> another 12 hours to attempt to fix it, rather than reverting. > > We also have the generic rule that any committer (full or partial) may > revert something that makes it impossible for them to do further > development. (See hacking) No, we have a policy that people can revert changes to the *build system* which prevent productivity: "To prevent loss of productivity, any committer (full or partial) can immediately revert any build system change that breaks their ability to effectively do development on their platform of choice, as a matter of ordinary routing, without fear of accusations of an over-reaction." (From: http://subversion.apache.org/docs/community-guide/building.html#configury ) I'm not trying to play process obstructionist, just noting that a mail mentioning the breakage and indicating your intent to revert would have been a nice consideration. > And tomorrow morning the asf repository will be readonly for quite some > time, so waiting till after that will probably cause more delays. > > Besides, you just mailed that you weren't going to fix this issue... :-) I guess I should have been more clear: I'm happy to fix my own breakage to the buildbots. When indicating I was moving on to other things, I didn't know I'd broken the test world. > Somehow the test that should have picked up the original failure is broken. > It thinks that no output at all for a recursive proplist is ok. > > So two different bugs (the local changes one; and the base-deleted one) > together kept the prop_tests.py 15 test succeeding. Has this bug in the test suite been fixed? If not, I suppose that's a place to start... -Hyrum
RE: svn commit: r1104610 - in /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h
> -Original Message- > From: Hyrum K Wright [mailto:hy...@hyrumwright.org] > Sent: woensdag 18 mei 2011 1:11 > To: d...@subversion.apache.org > Cc: commits@subversion.apache.org > Subject: Re: svn commit: r1104610 - in > /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h > > I understand the desire to get the buildbots green again, and I'm > sorry these revisions which I committed broke the bots, but a little > patience might have been useful here. We have a long tradition of > allowing folks to attempt to fix problems, rather than reverting their > commits without consultation. I kinda wish you'd have given me > another 12 hours to attempt to fix it, rather than reverting. We also have the generic rule that any committer (full or partial) may revert something that makes it impossible for them to do further development. (See hacking) And tomorrow morning the asf repository will be readonly for quite some time, so waiting till after that will probably cause more delays. Besides, you just mailed that you weren't going to fix this issue... :-) Somehow the test that should have picked up the original failure is broken. It thinks that no output at all for a recursive proplist is ok. So two different bugs (the local changes one; and the base-deleted one) together kept the prop_tests.py 15 test succeeding. Bert
Re: svn commit: r1104610 - in /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h
I understand the desire to get the buildbots green again, and I'm
sorry these revisions which I committed broke the bots, but a little
patience might have been useful here. We have a long tradition of
allowing folks to attempt to fix problems, rather than reverting their
commits without consultation. I kinda wish you'd have given me
another 12 hours to attempt to fix it, rather than reverting.
-Hyrum
On Tue, May 17, 2011 at 10:40 PM, wrote:
> Author: rhuijben
> Date: Tue May 17 22:40:07 2011
> New Revision: 1104610
>
> URL: http://svn.apache.org/viewvc?rev=1104610&view=rev
> Log:
> Temporarily revert r1104383 (and to resolve conflicts also r1104400) as
> these revisions cause some very hard to diagnose problems in the property
> handling, which breaks on all buildbots.
>
> * libsvn_wc/props.c
> * libsvn_wc/wc_db.c
> * libsvn_wc/wc_db.h
> svn merge -c -1104400,-1104383 ^/subversion/trunk
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/props.c
> subversion/trunk/subversion/libsvn_wc/wc_db.c
> subversion/trunk/subversion/libsvn_wc/wc_db.h
>
> Modified: subversion/trunk/subversion/libsvn_wc/props.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1104610&r1=1104609&r2=1104610&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/props.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/props.c Tue May 17 22:40:07 2011
> @@ -1665,34 +1665,6 @@ svn_wc_prop_list2(apr_hash_t **props,
> scratch_pool));
> }
>
> -struct propname_filter_baton_t {
> - svn_wc__proplist_receiver_t receiver_func;
> - void *receiver_baton;
> - const char *propname;
> -};
> -
> -static svn_error_t *
> -propname_filter_receiver(void *baton,
> - const char *local_abspath,
> - apr_hash_t *props,
> - apr_pool_t *scratch_pool)
> -{
> - struct propname_filter_baton_t *pfb = baton;
> - const svn_string_t *propval = apr_hash_get(props, pfb->propname,
> - APR_HASH_KEY_STRING);
> -
> - if (propval)
> - {
> - props = apr_hash_make(scratch_pool);
> - apr_hash_set(props, pfb->propname, APR_HASH_KEY_STRING, propval);
> -
> - SVN_ERR(pfb->receiver_func(pfb->receiver_baton, local_abspath, props,
> - scratch_pool));
> - }
> -
> - return SVN_NO_ERROR;
> -}
> -
> svn_error_t *
> svn_wc__prop_list_recursive(svn_wc_context_t *wc_ctx,
> const char *local_abspath,
> @@ -1706,23 +1678,40 @@ svn_wc__prop_list_recursive(svn_wc_conte
> void *cancel_baton,
> apr_pool_t *scratch_pool)
> {
> - svn_wc__proplist_receiver_t receiver = receiver_func;
> - void *baton = receiver_baton;
> -
> - struct propname_filter_baton_t pfb = { receiver_func, receiver_baton,
> - propname };
> -
> - if (propname)
> + switch (depth)
> {
> - baton = &pfb;
> - receiver = propname_filter_receiver;
> + case svn_depth_empty:
> + {
> + apr_hash_t *props;
> +
> + if (pristine)
> + SVN_ERR(svn_wc__db_read_pristine_props(&props, wc_ctx->db,
> + local_abspath,
> + scratch_pool,
> scratch_pool));
> + else
> + SVN_ERR(svn_wc__db_read_props(&props, wc_ctx->db, local_abspath,
> + scratch_pool, scratch_pool));
> +
> + if (receiver_func && props && apr_hash_count(props) > 0)
> + SVN_ERR((*receiver_func)(receiver_baton, local_abspath, props,
> + scratch_pool));
> + }
> + break;
> + case svn_depth_files:
> + case svn_depth_immediates:
> + case svn_depth_infinity:
> + SVN_ERR(svn_wc__db_read_props_streamily(wc_ctx->db, local_abspath,
> + propname, depth,
> + base_props, pristine,
> + receiver_func, receiver_baton,
> + cancel_func, cancel_baton,
> + scratch_pool));
> + break;
> + default:
> + SVN_ERR_MALFUNCTION();
> }
>
> - return svn_wc__db_read_props_streamily(wc_ctx->db, local_abspath, depth,
> - base_props, pristine,
> - receiver, baton,
> - cancel_func, cancel_baton,
> - scratch_pool);
> + return SVN_NO_ERROR;
> }
>
> svn_error_t *
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/tr
svn commit: r1104610 - in /subversion/trunk/subversion/libsvn_wc: props.c wc_db.c wc_db.h
Author: rhuijben
Date: Tue May 17 22:40:07 2011
New Revision: 1104610
URL: http://svn.apache.org/viewvc?rev=1104610&view=rev
Log:
Temporarily revert r1104383 (and to resolve conflicts also r1104400) as
these revisions cause some very hard to diagnose problems in the property
handling, which breaks on all buildbots.
* libsvn_wc/props.c
* libsvn_wc/wc_db.c
* libsvn_wc/wc_db.h
svn merge -c -1104400,-1104383 ^/subversion/trunk
Modified:
subversion/trunk/subversion/libsvn_wc/props.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
subversion/trunk/subversion/libsvn_wc/wc_db.h
Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1104610&r1=1104609&r2=1104610&view=diff
==
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Tue May 17 22:40:07 2011
@@ -1665,34 +1665,6 @@ svn_wc_prop_list2(apr_hash_t **props,
scratch_pool));
}
-struct propname_filter_baton_t {
- svn_wc__proplist_receiver_t receiver_func;
- void *receiver_baton;
- const char *propname;
-};
-
-static svn_error_t *
-propname_filter_receiver(void *baton,
- const char *local_abspath,
- apr_hash_t *props,
- apr_pool_t *scratch_pool)
-{
- struct propname_filter_baton_t *pfb = baton;
- const svn_string_t *propval = apr_hash_get(props, pfb->propname,
- APR_HASH_KEY_STRING);
-
- if (propval)
-{
- props = apr_hash_make(scratch_pool);
- apr_hash_set(props, pfb->propname, APR_HASH_KEY_STRING, propval);
-
- SVN_ERR(pfb->receiver_func(pfb->receiver_baton, local_abspath, props,
- scratch_pool));
-}
-
- return SVN_NO_ERROR;
-}
-
svn_error_t *
svn_wc__prop_list_recursive(svn_wc_context_t *wc_ctx,
const char *local_abspath,
@@ -1706,23 +1678,40 @@ svn_wc__prop_list_recursive(svn_wc_conte
void *cancel_baton,
apr_pool_t *scratch_pool)
{
- svn_wc__proplist_receiver_t receiver = receiver_func;
- void *baton = receiver_baton;
-
- struct propname_filter_baton_t pfb = { receiver_func, receiver_baton,
- propname };
-
- if (propname)
+ switch (depth)
{
- baton = &pfb;
- receiver = propname_filter_receiver;
+case svn_depth_empty:
+ {
+apr_hash_t *props;
+
+if (pristine)
+ SVN_ERR(svn_wc__db_read_pristine_props(&props, wc_ctx->db,
+ local_abspath,
+ scratch_pool, scratch_pool));
+else
+ SVN_ERR(svn_wc__db_read_props(&props, wc_ctx->db, local_abspath,
+scratch_pool, scratch_pool));
+
+if (receiver_func && props && apr_hash_count(props) > 0)
+ SVN_ERR((*receiver_func)(receiver_baton, local_abspath, props,
+ scratch_pool));
+ }
+ break;
+case svn_depth_files:
+case svn_depth_immediates:
+case svn_depth_infinity:
+ SVN_ERR(svn_wc__db_read_props_streamily(wc_ctx->db, local_abspath,
+ propname, depth,
+ base_props, pristine,
+ receiver_func, receiver_baton,
+ cancel_func, cancel_baton,
+ scratch_pool));
+ break;
+default:
+ SVN_ERR_MALFUNCTION();
}
- return svn_wc__db_read_props_streamily(wc_ctx->db, local_abspath, depth,
- base_props, pristine,
- receiver, baton,
- cancel_func, cancel_baton,
- scratch_pool);
+ return SVN_NO_ERROR;
}
svn_error_t *
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1104610&r1=1104609&r2=1104610&view=diff
==
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May 17 22:40:07 2011
@@ -7629,6 +7629,7 @@ cache_props_recursive(void *cb_baton,
svn_error_t *
svn_wc__db_read_props_streamily(svn_wc__db_t *db,
const char *local_abspath,
+const char *propname,
svn_depth_t depth,
svn_boolean_t base_props,
