Re: svn commit: r1463721 - in /subversion/trunk/subversion/libsvn_client: checkout.c client.h commit.c copy.c export.c externals.c revert.c switch.c update.c
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 11 Apr 2013 13:22:54 +0100 (BST)
-- Join WANdisco's free daily demo sessions on Scaling Subversion for the Enterprise <http://www.wandisco.com/training/webinars> ----- Original Message ----- > From: Philip Martin <philip.martin_at_wandisco.com> > To: dev_at_subversion.apache.org > Cc: > Sent: Thursday, 11 April 2013, 6:27 > Subject: Re: svn commit: r1463721 - in /subversion/trunk/subversion/libsvn_client: checkout.c client.h commit.c copy.c export.c externals.c revert.c switch.c update.c > > julianfoad_at_apache.org writes: > >> Author: julianfoad >> Date: Tue Apr 2 19:53:43 2013 >> New Revision: 1463721 >> >> URL: http://svn.apache.org/r1463721 >> Log: >> Fix some sleep-for-timestamps code: avoid missing some cases where we need >> to sleep -- especially error cases -- and avoid sleeping in some cases > where >> we don't need to. Simplify the code by using a consistent pattern: > make the >> decision of whether to sleep at the lowest level possible (ideally in the > WC >> layer, but not yet), and do the sleep at the highest level possible >> (immediately below the libsvn_client API). > >> * subversion/libsvn_client/copy.c >> (do_wc_to_wc_copies_with_write_lock, >> do_wc_to_wc_moves, >> repos_to_wc_copy_single): Return a flag instead of sleeping here. A >> multi-target repos-to-WC copy will now only sleep once, not once per >> target. >> (do_wc_to_wc_copies, >> repos_to_wc_copy_locked, >> repos_to_wc_copy, >> try_copy): Pass the flag through. >> (svn_client_copy6, >> svn_client_move7): Handle the sleep here. > >> --- subversion/trunk/subversion/libsvn_client/copy.c (original) >> +++ subversion/trunk/subversion/libsvn_client/copy.c Tue Apr 2 19:53:43 > 2013 >> @@ -212,7 +214,6 @@ do_wc_to_wc_copies_with_write_lock(const >> } >> svn_pool_destroy(iterpool); >> >> - svn_io_sleep_for_timestamps(dst_parent, scratch_pool); >> SVN_ERR(err); >> return SVN_NO_ERROR; >> } > >> @@ -2292,6 +2312,9 @@ svn_client_copy6(const apr_array_header_ >> subpool); >> } >> >> + if (timestamp_sleep) >> + svn_io_sleep_for_timestamps(NULL, subpool); >> + >> svn_pool_destroy(subpool); >> return svn_error_trace(err); >> } > > Is there any reason you are passing NULL to svn_io_sleep_for_timestamps > rather than dst_path? Good catch. I suppose I was thinking I couldn't be sure that the same path was used in each possible case. Looking at it again, if dst_path is not a URL then it would be reasonable to assume that all relevant changes are on the same filesystem as dst_path. Even more so if we use the child of dst_path instead, in the copy_as_child case; then the path used would be, I think, the same as it was when the sleep_for_timestamps calls were distributed among the leaf functions before my patch. And when dst_path is a URL, timestamp_sleep shouldn't be true, but I also notice I failed to initialize 'timestamp_sleep'. The standard pattern is that the called functions don't evert set it false, they only ever set it true, so the top level function needs to initialize it. Will fix. - JulianReceived on 2013-04-11 14:23:30 CEST
This is an archived mail posted to the Subversion Dev mailing list.