On Wed, Feb 22, 2012 at 12:36 PM, Paul Burba <ptbu...@gmail.com> wrote:
> On Wed, Feb 22, 2012 at 12:05 PM, Philip Martin
> <philip.mar...@wandisco.com> wrote:
>> Paul Burba <ptbu...@gmail.com> writes:
>>
>>> Index: subversion/libsvn_client/copy.c
>>> ===================================================================
>>> --- subversion/libsvn_client/copy.c (revision 1292379)
>>> +++ subversion/libsvn_client/copy.c (working copy)
>>> @@ -1519,6 +1519,17 @@
>>> ctx->notify_baton2 = old_notify_baton2;
>>>
>>> SVN_ERR(err);
>>> +
>>> +#ifdef WIN32
>>> + if (!ignore_externals)
>>> + {
>>> + /* Issue #4123: We may still hold file handles to the databases
>>> + for externals under TMP_ABSPATH. We need to release these
>>> + handles before we move TMP_ABSPATH below or Windows will
>>> + raise an ERROR_ACCESS_DENIED error. */
>>> + SVN_ERR(svn_wc__externals_close(tmp_abspath, ctx->wc_ctx,
>>> pool));
>>> + }
>>> +#endif
>>
>> I'm not sure why this would be windows specific. Yes, Linux lets us
>> move the dir with the handle open but I think it would be an error for
>> the Subversion client to use the handle after the move.
>
> Hi Philip,
>
> OK, both you and Bert pointed this out, so it's gone.
>
>> Is this the best way to do it? It isn't what I was expecting. I was
>> expecting the checkout code to explicitly close the handles it was
>> responsible for opening. Then the copy code would not have to do
>> anything special.
>
> Are you thinking within svn_client__checkout_internal or from its
> caller, like the attached patch?
Doh, of course the attached patch *is* within on of
svn_client__checkout_internal's helpers; that's what I get for a quick
patch. This fixes the issue and passes all tests; committed it in
r1292827.
Paul
