I (Julian Foad) wrote:
> Philip Martin wrote:
>> julianfoad_at_apache.org writes:
>>> URL: http://svn.apache.org/r1463721
[...]
>>> * 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
>>> @@ -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.
Committed r1466876.
- Julian
Received on 2013-04-11 15:33:13 CEST