[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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 14:24:15 +0100 (BST)

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

This is an archived mail posted to the Subversion Dev mailing list.