On Fri, Feb 17, 2012 at 12:46 PM, Nico Kadel-Garcia <nka...@gmail.com> wrote:
> On Fri, Feb 17, 2012 at 10:54 AM, Philip Martin
> <philip.mar...@wandisco.com> wrote:
>> Paul Burba <ptbu...@gmail.com> writes:
>>
>>> I'm able to replicate this failure on my Windows box with my own
>>> builds of trunk@1245285, 1.7.0 and 1.7.3. Alexey's script works as
>>> expected with 1.6.17, so this appears to be a regression. Moving back
>>> to the dev list.
>>>
>>> Investigating...
>>
>>>> svn: E720005: Can't move 'C:\t\wc\.svn\tmp\svn-8ED6923C' to
>>>> 'C:\t\wc\externals-container-copy': Access is denied.
>>
>> I suspect this is a directory move. Perhaps the wc.db file of the
>> external is still open? Can a directory be renamed on Windows when one
>> of the files inside it is still open?
>
> Not in my experience.
I added an issue for this:
http://subversion.tigris.org/issues/show_bug.cgi?id=4123
and a test also: http://svn.apache.org/viewvc?view=revision&revision=1292090
The attached patch fixes this problem on Windows. Could any wc-ng
gurus take a look and see if this seems reasonable?
[[[
Fix issue #4123 'URL-to-WC copy of externals fails on Windows'.
* subversion/include/private/svn_wc_private.h
(svn_wc__externals_close): New declaration.
* subversion/libsvn_wc/externals.c
(svn_wc__externals_close): New definition.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy_single): Use new function to close handles to
externals' DB files before trying to rename the whole temp WC.
* subversion/tests/cmdline/externals_tests.py
(url_to_wc_copy_of_externals): Remove XFail decorator and update comments
re failure status.
]]]
Even with this fix I'm still seeing odd behavior post-copy (the
following example uses a WC-to-WC copy, but the same problem occurs
with a URL-to-WC copy):
### We have a working copy at a uniform revision with an external:
>svn up -q
>svn st
X A\C\external
Performing status on external item at 'A\C\external':
>svn pl -vR
Properties on 'A\C':
svn:externals
^/A/D/G external
### We copy the directory with the external definition:
>svn copy A/C WC-to-WC-Copy-of-C
A WC-to-WC-Copy-of-C
### But the external shows up as unversioned:
>svn st
X A\C\external
A + WC-to-WC-Copy-of-C
? WC-to-WC-Copy-of-C\external
Performing status on external item at 'WC-to-WC-Copy-of-C\external':
Performing status on external item at 'A\C\external':
### Even if we commit an update the WC we see the same problem:
>svn ci -m ""
Adding WC-to-WC-Copy-of-C
Committed revision 3.
>svn up
Updating '.':
Fetching external item into 'WC-to-WC-Copy-of-C\external':
External at revision 3.
Fetching external item into 'A\C\external':
External at revision 3.
At revision 3.
>svn st
X A\C\external
? WC-to-WC-Copy-of-C\external
Performing status on external item at 'WC-to-WC-Copy-of-C\external':
Performing status on external item at 'A\C\external':
>
To fix this we need to remove the external via the OS then update to
restore it (note that this problem occurs without my patch applied, so
this is a separate issue).
Paul
Index: subversion/include/private/svn_wc_private.h
===================================================================
--- subversion/include/private/svn_wc_private.h (revision 1292379)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -263,6 +263,15 @@
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+/* Close the DBs for any externals under LOCAL_ABSPATH. Detection of
+ externals is as per svn_wc__externals_gather_definitions.
+
+ Perform temporary allocations in SCRATCH_POOL. */
+svn_error_t *
+svn_wc__externals_close(const char *local_abspath,
+ svn_wc_context_t *wc_ctx,
+ apr_pool_t *scratch_pool);
+
/** Set @a *tree_conflict to a newly allocated @c
* svn_wc_conflict_description_t structure describing the tree
* conflict state of @a victim_abspath, or to @c NULL if @a victim_abspath
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
}
/* Schedule dst_path for addition in parent, with copy history.
Index: subversion/libsvn_wc/externals.c
===================================================================
--- subversion/libsvn_wc/externals.c (revision 1292379)
+++ subversion/libsvn_wc/externals.c (working copy)
@@ -1363,6 +1363,56 @@
}
}
+svn_error_t *
+svn_wc__externals_close(const char *local_abspath,
+ svn_wc_context_t *wc_ctx,
+ apr_pool_t *scratch_pool)
+{
+ apr_hash_t *externals;
+ apr_hash_index_t *hi;
+ apr_pool_t *iterpool;
+
+ SVN_ERR(svn_wc__externals_gather_definitions(&externals, NULL, wc_ctx,
+ local_abspath,
+ svn_depth_infinity,
+ scratch_pool, scratch_pool));
+
+ if (!apr_hash_count(externals))
+ return SVN_NO_ERROR;
+
+ iterpool = svn_pool_create(scratch_pool);
+
+ for (hi = apr_hash_first(scratch_pool, externals);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *this_abspath = svn__apr_hash_index_key(hi);
+ const char *external_value = svn__apr_hash_index_val(hi);
+ apr_array_header_t *ext_desc;
+ int i;
+
+ SVN_ERR(svn_wc_parse_externals_description3(&ext_desc, this_abspath,
+ external_value, FALSE,
+ iterpool));
+ for (i = 0; i < ext_desc->nelts; i++)
+ {
+ svn_wc_external_item2_t *ext_item =
+ APR_ARRAY_IDX(ext_desc, i, svn_wc_external_item2_t *);
+ const char *external_abspath;
+
+ SVN_ERR(svn_dirent_get_absolute(&external_abspath,
+ svn_dirent_join(this_abspath,
+ ext_item->target_dir,
+ iterpool),
+ iterpool));
+ SVN_ERR(svn_wc__db_drop_root(wc_ctx->db, external_abspath,
+ iterpool));
+ }
+ }
+ svn_pool_destroy(iterpool);
+ return SVN_NO_ERROR;
+}
+
/* Return the scheme of @a uri in @a scheme allocated from @a pool.
If @a uri does not appear to be a valid URI, then @a scheme will
not be updated. */
Index: subversion/tests/cmdline/externals_tests.py
===================================================================
--- subversion/tests/cmdline/externals_tests.py (revision 1292379)
+++ subversion/tests/cmdline/externals_tests.py (working copy)
@@ -2787,7 +2787,6 @@
# Test for issue #4123 'URL-to-WC copy of externals fails on Windows'
@Issue(4123)
-@XFail(svntest.main.is_os_windows)
def url_to_wc_copy_of_externals(sbox):
"url-to-wc copy of externals"
@@ -2805,7 +2804,8 @@
svntest.actions.run_and_verify_svn(None, None, [], 'up', wc_dir)
# Copy ^/A/C to External-WC-to-URL-Copy.
- # Currently this fails with:
+ #
+ # Previously this failed with:
# >svn copy ^^/A/C External-WC-to-URL-Copy
# U External-WC-to-URL-Copy
#
