Re: [PATCH] Sleep for timestamps in the right places

Philip Martin wrote:
> Julian Foad <julianfoad_at_btopenworld.com> writes:
>> The attached patch attempts to improve our 'sleep for timestamps'
>> coverage, especially in error conditions where the WC has already been
>> changed and so we should sleep as well as returning an error.  I just
>> happened to notice that we currently miss several such cases.
>>
>> The are several TODO notes in the log message.  Before I go any
>> further with this, can anybody check I'm on the right track?
>
> Looks plausible.
>
>> +  err = svn_client__checkout_internal(result_rev, URL, local_abspath,
>> +                                      peg_revision, revision, depth,
>> +                                      ignore_externals,
>> +                                      allow_unver_obstructions, &sleep_here,
>> +                                      ctx, pool);
>> +  if (sleep_here)
>> +    svn_io_sleep_for_timestamps(local_abspath, pool);
>
> Could we check use-commit-times and avoid the sleep?  Either before
> sleeping or perhaps where we set sleep_here TRUE.

OK, thanks; I did that (inside update_internal()), and a few more things, and committed r1463721.

For the future, it seems to me that the tracking of whether we need to sleep should be done inside libsvn_wc, as only there do we really know whether we have made a change that relies on the timestamp.  The current code is all in libsvn_client and can only assume that if it called something like svn_wc_revert or svn_wc_do_update then it will need to sleep, which is a crude assumption (not always true).  We would also have the ability to store more detailed information there, such as the latest timestamp that we are relying on, so that we wouldn't have to sleep for 1 second starting from when we start to sleep but only starting from when the last WC update was, which might have been some time ago in some cases.  But I'm not planning on doing any of that.

- Julian
Received on 2013-04-02 22:05:01 CEST