You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Marvin Greenberg <mg...@dctd.saic.com> on 2004/03/08 16:30:57 UTC
Re: [Issue 1751] svn switch may corrupt working copy
> From: Ben Collins-Sussman <su...@collab.net>
> Subject: [Issue 1751] svn switch may corrupt working copy
>
> On Sun, 2004-03-07 at 18:53, marvin greenberg wrote:
> > > julianfoad@tigris.org wrote:
> > > http://subversion.tigris.org/issues/show_bug.cgi?id=1751
> > >
> >
> > Is there a work-around to this issue? I've seen this problem but don't
> > really understand from the issue when it will happen and when it won't.
> >
> > We were planning to use 'svn switch' as part of our process for merging
> > between
> > branches. But obviously that won't work if it it corrupts the WC...
> I don't think you need to be afraid of 'svn switch'. We've all been
> using it for years. Gstein seems to have found an edge case
> specifically related to using the '-N' (--nonrecursive) option with
> checkout and switch. The -N flag is pretty rarely used, and already has
> known bugs, so I'm not surprised.
> As long as you stay way from -N, you should be just fine.
If you look at the case, it specifically says it isn't just "-N". I have had
this happen doing really simple things like a switch from branches/br1 to
branches/br2. No -N flag, nothing fancy. Resulting in a wedged working copy,
with a rather painful process to recover, figure out what is changed (since all
sorts of svn commands like status and diff) don't work.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
Philip Martin wrote:
> Marvin Greenberg <mg...@dctd.saic.com> writes:
>>If you look at the case, it specifically says it isn't just "-N".
> That's odd because I tried the recipe in the issue and I only have a
> problem if I use -N.
>>I have had this happen doing really simple things like a switch from
>>branches/br1 to branches/br2. No -N flag, nothing fancy.
> Show me.
The -N is only irrelevant for the checkout of wc2. For the switch you
must use -N to reprodece the error (unless someone finds another recipe).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by Marvin Greenberg <mg...@dctd.saic.com>.
Philip Martin wrote:
> Marvin Greenberg <mg...@dctd.saic.com> writes:
>
>
>>If you look at the case, it specifically says it isn't just "-N".
>
>
> That's odd because I tried the recipe in the issue and I only have a
> problem if I use -N.
>
I was just reading what MAKL said in the case -- "Note: It is irrelevant for the
recipe whether wc2 is checked out recursively or not."
>
>>I have had this happen doing really simple things like a switch from
>>branches/br1 to branches/br2. No -N flag, nothing fancy.
>
>
> Show me.
>
For you to have anything to work with, I'll have to come up with a test case,
but this is what I did to experience the problem, the last time I saw it. Note
that I tried duplicating this today by specifying the revision and it did not
happen, and no simple case I could come up so far caused any problem. And this
is 0.37, not 1.0.0.
-------------------------------------------------------------------------
bash-2.05b$ svn co http://superclass/svn/FI2010/trunk myWorkingCopy
< bunch of files are checked out >
U myWorkingCopy
Checked out revision 2102.
bash-2.05b$ cd myWorkingCopy/
bash-2.05b$ svn switch http://superclass/svn/FI2010/branches/4.0.2
< a variety of files are added, modified, deleted >
Updated to revision 2102.
bash-2.05b$ svn info | head 2
Path: .
URL: http://superclass/svn/FI2010/branches
URL should be http://superclass/svn/FI2010/branches/4.0.2 !!!
Needless to say, the working copy is now all confused. In fact,
$ svn info TENA | head -2
Path: Utils
URL: http://superclass/svn/FI2010/branches/4.0.2 [ should have TENA!]
$ svn info DCT | head -2
Path: DCT
URL: http://superclass/svn/FI2010/trunk/DCT
$ svn info Utils | head -2
Path: Utils
URL: http://superclass/svn/FI2010/branches/4.0.2 [should have Utils!]
And
$ svn up
svn: REPORT request failed on '/svn/FI2010/!svn/vcc/default'
svn:
File not found: revision '2102', path '/branches/4.0.2/pub'
Which should be Utils/pub...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by Philip Martin <ph...@codematters.co.uk>.
Marvin Greenberg <mg...@dctd.saic.com> writes:
> If you look at the case, it specifically says it isn't just "-N".
That's odd because I tried the recipe in the issue and I only have a
problem if I use -N.
> I have had this happen doing really simple things like a switch from
> branches/br1 to branches/br2. No -N flag, nothing fancy.
Show me.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
Julian Foad wrote:
> Oh yes - I'm sorry - I had not read your second recipe carefully.
>
> So, we have reproduction recipes in which a plain "switch" command
> currupts the WC when it fails. Now we just need to start debugging!
But note that the second recipe only works with http. So it may be an
edge case of mod_dav.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> makl <ma...@tigris.org> writes:
>
>>After a little debugging I have found the following:
>>
>>In function 'drive' in reporter.c there is a call to
>>'b->editor->open_root'. This call sets the URL of the wc to
>>'file:///home/mk/test/repo/branch'.
>
> Well spotted! Try this patch
>
> - if (*eb->target) /* anchor is also target */
> + if (! *eb->target) /* anchor is also target */
Well done makl and Philip. That fixes some of the "svn switch" problems.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by Philip Martin <ph...@codematters.co.uk>.
makl <ma...@tigris.org> writes:
> After a little debugging I have found the following:
>
> In function 'drive' in reporter.c there is a call to
> 'b->editor->open_root'. This call sets the URL of the wc to
> 'file:///home/mk/test/repo/branch'.
Well spotted! Try this patch
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 8954)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -251,7 +251,7 @@
another target). */
if (! pb)
{
- if (*eb->target) /* anchor is also target */
+ if (! *eb->target) /* anchor is also target */
d->new_URL = apr_pstrdup (pool, eb->switch_url);
else
d->new_URL = svn_path_dirname (eb->switch_url, pool);
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
I have finally found the reason why the file in case 1 is still at trunk.
In the non-recursive part of svn_wc__do_update_cleanup, only the entry
for the directory is updated, not the entries for the files.
The remaining 'dir not locked' error may be a duplicate of issue 1768.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
And finally a recipe that doesn't mix recursive and non-recursive
functions. This is a variant of the 'dir not locked' problem.
Another way is to interrupt the client during the switch.
svnadmin create --bdb-txn-nosync repo
svn co $REPO wc1
mkdir wc1/trunk
mkdir wc1/branch
mkdir wc1/branch/file
touch wc1/trunk/file
svn add wc1/trunk
svn add wc1/branch
svn ci -m "" wc1
svn co $REPO/trunk wc2
echo Hi >wc2/file
svn switch $REPO/branch wc2
svn info wc2
# Note the wrong url
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
After a little debugging I have found the following:
In function 'drive' in reporter.c there is a call to
'b->editor->open_root'. This call sets the URL of the wc to
'file:///home/mk/test/repo/branch'.
At the end of the function the call to 'b->editor->close_edit' set the
URL to the correct value.
If something goes wrong between the two calls, the wrong URL remains in
the entry of the wc (case 3 in my last mail).
In addition close_edit do something silly when the switch is
non-recursive (case 2 and possibly case 1).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in source
and destination
Posted by makl <ma...@tigris.org>.
Philip Martin wrote:
> Subversion's standard pool pattern is described in HACKING.
I know. I have only interpreted the code to get the intended behaviour.
> As it happens svn_wc_entries_read is special, the entries are
> allocated from the pool used to open the access baton and not from the
> pool parameter.
Seems I missed this part in the documentation of svn_wc_entries_read.
With this detail in mind my arguments do not hold.
I will post a modified patch in the next days.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in
source and destination
Posted by Philip Martin <ph...@codematters.co.uk>.
makl <ma...@tigris.org> writes:
>>>+ /* Read DIRPATH's entries. */
>>>+ SVN_ERR (svn_wc_entries_read (&entries, dirpath, TRUE, subpool));
>>
>> I know you copied this code, but the pool handling isn't correct.
>> It should be using pool here, not subpool.
>
> Potentially we read a big bunch of entries and i think the memory should
> be freed as soon as possible. And for the recursive case, after
> completing the operation we have all entries of the complete working
> copy in memory. This sounds not correct for me.
Subversion's standard pool pattern is described in HACKING. The point
of the subpool is to handle the iteration, so it *must* be cleared
within the loop. This means that any memory that must persist from
one iteration to the next cannot be allocated from the subpool. You
don't need to worry about the amount of memory allocated by one-shot,
non-iterative, allocations as it is the callers responsibility to
provide a suitable pool, and clear it if required.
As it happens svn_wc_entries_read is special, the entries are
allocated from the pool used to open the access baton and not from the
pool parameter. In most cases calling svn_wc_entries_read doesn't
allocate any additional memory.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in source
and destination
Posted by makl <ma...@tigris.org>.
Philip Martin wrote:
> makl <ma...@tigris.org> writes:
> I feel there is too much code duplication, would it be possible to use
> a single tweak_entries function with a recurse flag?
No problem.
>>+ /* Read DIRPATH's entries. */
>>+ SVN_ERR (svn_wc_entries_read (&entries, dirpath, TRUE, subpool));
>
> I know you copied this code, but the pool handling isn't correct.
> It should be using pool here, not subpool.
Potentially we read a big bunch of entries and i think the memory should
be freed as soon as possible. And for the recursive case, after
completing the operation we have all entries of the complete working
copy in memory. This sounds not correct for me.
>> # Check out the trunk as "wc2"
>>- svntest.main.run_svn(None, 'co', '-N', trunk_url, wc2_dir)
>>+ svntest.main.run_svn(None, 'co', trunk_url, wc2_dir)
>
>
> That's quite a change, is this test still testing whatever it was
> supposed to test originally?
Remember the recipe. For this two cases it's irrelevant whether the WC
is checked out recursively or not.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in source
and destination
Posted by Julian Foad <ju...@btopenworld.com>.
makl wrote:
> Version 2 of the patch.
>
> [[[
> Fix remaining part of issue 1751. File not switched if it is the same
> in source and destination.
>
> Patch from <ma...@tigris.org>
>
> * subversion/tests/clients/cmdline/switch_tests.py
> (main): Remove XFAIL from nonrecursive_switching.
It is called 'test_list', not 'main'.
> (nonrecursive_switching): Extend the test to ensure that
> subdirectories are not switched.
>
> * subversion/libsvn_wc/adm_ops.c
> (tweak_entries): Change of non_recursively_tweak_entries to be
> non recursiv if it is necessary.
It looks like it is 'recursively_tweak_entries' that was changed and renamed. The pool usage was also changed. Perhaps write it like this:
* subversion/libsvn_wc/adm_ops.c
(tweak_entries): Rename from 'recursively_tweak_entries'. Add a
'recurse' flag so that recursion is optional. Fix pool usage.
> (svn_wc__do_update_cleanup): Call tweak_entries.
> ]]]
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 9193)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -98,19 +101,18 @@
> if (base_url)
> child_url = svn_path_url_add_component (base_url, name, subpool);
>
> - /* If a file, or deleted or absent dir, then tweak the entry but
> - don't recurse. */
> + /* If a file, or deleted or absent dir in recursive mode, then tweak the
> + entry but don't recurse. */
> if ((current_entry->kind == svn_node_file)
> - || current_entry->deleted
> - || current_entry->absent)
> + || (recurse && (current_entry->deleted || current_entry->absent)))
> {
> SVN_ERR (svn_wc__tweak_entry (entries, name,
> child_url, new_rev, &write_required,
> svn_wc_adm_access_pool (dirpath)));
> }
>
> - /* If a directory... */
> - else if (current_entry->kind == svn_node_dir)
> + /* If a directory and recursiv... */
"recursive"
The rest of the patch looks OK.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in source
and destination
Posted by makl <ma...@tigris.org>.
Philip Martin wrote:
> makl <ma...@tigris.org> writes:
>
>
>>Index: subversion/libsvn_wc/adm_ops.c
>>===================================================================
>>--- subversion/libsvn_wc/adm_ops.c (revision 9193)
>>+++ subversion/libsvn_wc/adm_ops.c (working copy)
>
>
>>@@ -200,20 +202,13 @@
>> SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access, path, pool));
>>
>> if (! recursive)
>>- {
>>- svn_boolean_t write_required = FALSE;
>>- SVN_ERR (svn_wc_entries_read (&entries, dir_access, TRUE, pool));
>>- SVN_ERR (svn_wc__tweak_entry (entries, SVN_WC_ENTRY_THIS_DIR,
>>- base_url, new_revision, &write_required,
>>- svn_wc_adm_access_pool (dir_access)));
>>- if (write_required)
>>- SVN_ERR (svn_wc__entries_write (entries, dir_access, pool));
>>- }
>>+ SVN_ERR (tweak_entries (dir_access, base_url, new_revision,
>>+ NULL, NULL, FALSE,
>>+ FALSE, pool));
>
>
> Why is this passing FALSE for remove_missing_dirs (and thus NULL for
> the notify stuff)? I'm not saying it's wrong, I just don't understand
> it.
remove_missing_dirs and the notify stuff are only used in the recursive
case. Passing them as FALSE and null, everyone can see at the first
sight that they are irrelevant for the non-recursive case.
I have no problem if you prefer one function call and loosing the
"if(!recursive)".
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in
source and destination
Posted by Philip Martin <ph...@codematters.co.uk>.
makl <ma...@tigris.org> writes:
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 9193)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -200,20 +202,13 @@
> SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access, path, pool));
>
> if (! recursive)
> - {
> - svn_boolean_t write_required = FALSE;
> - SVN_ERR (svn_wc_entries_read (&entries, dir_access, TRUE, pool));
> - SVN_ERR (svn_wc__tweak_entry (entries, SVN_WC_ENTRY_THIS_DIR,
> - base_url, new_revision, &write_required,
> - svn_wc_adm_access_pool (dir_access)));
> - if (write_required)
> - SVN_ERR (svn_wc__entries_write (entries, dir_access, pool));
> - }
> + SVN_ERR (tweak_entries (dir_access, base_url, new_revision,
> + NULL, NULL, FALSE,
> + FALSE, pool));
Why is this passing FALSE for remove_missing_dirs (and thus NULL for
the notify stuff)? I'm not saying it's wrong, I just don't understand
it.
> else
> - SVN_ERR (recursively_tweak_entries (dir_access, base_url,
> - new_revision, notify_func,
> - notify_baton, remove_missing_dirs,
> - pool));
> + SVN_ERR (tweak_entries (dir_access, base_url, new_revision,
> + notify_func, notify_baton, remove_missing_dirs,
> + TRUE, pool));
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [Issue 1751] file not switched if it is the same in source
and destination
Posted by makl <ma...@tigris.org>.
Version 2 of the patch.
[[[
Fix remaining part of issue 1751. File not switched if it is the same
in source and destination.
Patch from <ma...@tigris.org>
* subversion/tests/clients/cmdline/switch_tests.py
(main): Remove XFAIL from nonrecursive_switching.
(nonrecursive_switching): Extend the test to ensure that
subdirectories are not switched.
* subversion/libsvn_wc/adm_ops.c
(tweak_entries): Change of non_recursively_tweak_entries to be
non recursiv if it is necessary.
(svn_wc__do_update_cleanup): Call tweak_entries.
]]]
Re: [PATCH] [Issue 1751] file not switched if it is the same in
source and destination
Posted by Philip Martin <ph...@codematters.co.uk>.
makl <ma...@tigris.org> writes:
> [[[
> Fix file not switched if it is the same in source and destination
>
> Patch from <ma...@tigris.org>
>
> * subversion/tests/clients/cmdline/switch_tests.py
> (main): Remove XFAIL from nonrecursive_switching
> (nonrecursive_switching): Extend the test to ensure that
> subdirectories are not switched.
>
> * subversion/libsvn_wc/adm_ops.c
> (non_recursively_tweak_entries): New function similar to
> recursively_tweak_entries but non recursive.
I feel there is too much code duplication, would it be possible to use
a single tweak_entries function with a recurse flag?
> (svn_wc__do_update_cleanup): Call non_recursively_tweak_entries
> ]]]
>
>
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 9041)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -161,6 +161,69 @@
>
>
> +/* The main non-recursive body of svn_wc__do_update_cleanup. */
> +static svn_error_t *
> +non_recursively_tweak_entries (svn_wc_adm_access_t *dirpath,
> + const char *base_url,
> + svn_revnum_t new_rev,
> + apr_pool_t *pool)
> +{
> + apr_hash_t *entries;
> + apr_hash_index_t *hi;
> + apr_pool_t *subpool = svn_pool_create (pool);
> + svn_boolean_t write_required = FALSE;
> +
> + /* Read DIRPATH's entries. */
> + SVN_ERR (svn_wc_entries_read (&entries, dirpath, TRUE, subpool));
I know you copied this code, but the pool handling isn't correct.
It should be using pool here, not subpool.
> +
> + /* Tweak "this_dir" */
> + SVN_ERR (svn_wc__tweak_entry (entries, SVN_WC_ENTRY_THIS_DIR,
> + base_url, new_rev, &write_required,
> + svn_wc_adm_access_pool (dirpath)));
> +
> + /* Recursively loop over all children. */
> + for (hi = apr_hash_first (subpool, entries); hi; hi = apr_hash_next (hi))
Use pool here.
> + {
> + const void *key;
> + void *val;
> + const char *name;
> + svn_wc_entry_t *current_entry;
> + const char *child_url = NULL;
> +
Add svn_pool_clear here.
> + apr_hash_this (hi, &key, NULL, &val);
> + name = key;
> + current_entry = val;
> +
> + /* Ignore the "this dir" entry. */
> + if (! strcmp (name, SVN_WC_ENTRY_THIS_DIR))
> + continue;
> +
> + /* Derive the new URL for the current (child) entry */
> + if (base_url)
> + child_url = svn_path_url_add_component (base_url, name, subpool);
> +
> + /* If a file, or deleted or absent dir, then tweak the entry but
> + don't recurse. */
> + if (current_entry->kind == svn_node_file)
> + {
> + SVN_ERR (svn_wc__tweak_entry (entries, name,
> + child_url, new_rev, &write_required,
> + svn_wc_adm_access_pool (dirpath)));
> + }
> + }
> +
> + /* Write a shiny new entries file to disk. */
> + if (write_required)
> + SVN_ERR (svn_wc__entries_write (entries, dirpath, subpool));
> +
> Index: subversion/tests/clients/cmdline/switch_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/switch_tests.py (revision 9041)
> +++ subversion/tests/clients/cmdline/switch_tests.py (working copy)
> @@ -636,9 +636,12 @@
> wc1_new_file = os.path.join(wc1_dir, 'branch', 'version1', 'newfile')
> wc2_new_file = os.path.join(wc2_dir, 'newfile')
> wc2_mu_file = os.path.join(wc2_dir, 'mu')
> + wc2_B_dir = os.path.join(wc2_dir, 'B')
> + wc2_C_dir = os.path.join(wc2_dir, 'C')
> + wc2_D_dir = os.path.join(wc2_dir, 'D')
>
> # Check out the trunk as "wc2"
> - svntest.main.run_svn(None, 'co', '-N', trunk_url, wc2_dir)
> + svntest.main.run_svn(None, 'co', trunk_url, wc2_dir)
That's quite a change, is this test still testing whatever it was
supposed to test originally?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] [Issue 1751] file not switched if it is the same in source
and destination
Posted by makl <ma...@tigris.org>.
[[[
Fix file not switched if it is the same in source and destination
Patch from <ma...@tigris.org>
* subversion/tests/clients/cmdline/switch_tests.py
(main): Remove XFAIL from nonrecursive_switching
(nonrecursive_switching): Extend the test to ensure that
subdirectories are not switched.
* subversion/libsvn_wc/adm_ops.c
(non_recursively_tweak_entries): New function similar to
recursively_tweak_entries but non recursive.
(svn_wc__do_update_cleanup): Call non_recursively_tweak_entries
]]]
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
Julian Foad wrote:
> So, we have reproduction recipes in which a plain "switch" command
> currupts the WC when it fails. Now we just need to start debugging!
I think that we should split the issue into three.
1. 'file' not switched if it is the same in source and destination
----------------------------------------------------------------------
svnadmin create repo
svn co file:///g:/repos/repo wc1
mkdir wc1\trunk\dir
mkdir wc1\branch
time /t >wc1\trunk\file
svn add wc1/trunk
svn add wc1/branch
svn ci -m "" wc1
svn cp -m "" file:///g:/repos/repo/trunk file:///g:/repos/repo/branch/version1
svn co file:///g:/repos/repo/trunk wc2
svn switch -N file:///g:/repos/repo/branch/version1 wc2
svn info wc2/dir
# still on trunk is correct
svn info wc2/file
# still on trunk is incorrect
----------------------------------------------------------------------
2. Invalid URL for file after switch
----------------------------------------------------------------------
svnadmin create repo
svn co file:///g:/repos/repo wc1
mkdir wc1\trunk
mkdir wc1\branch\version1
date /t >wc1\trunk\file
date /t >wc1\branch\version1\file
svn add wc1/trunk
svn add wc1/branch
svn ci -m "" wc1
svn co file:///g:/repos/repo/trunk wc2
svn switch -N file:///g:/repos/repo/branch/version1 wc2
svn info wc2
# URL is correct
svn info wc2/file
# URL is incorrect
----------------------------------------------------------------------
3. dir not locked error if switching a non-recursive wc
----------------------------------------------------------------------
svnadmin create repo
svn co file:///g:/repos/repo wc1
mkdir wc1\trunk\dir
mkdir wc1\branch\version1\dir
svn add wc1/trunk
svn add wc1/branch
svn ci -m "" wc1
svn co -N file:///g:/repos/repo/trunk wc2
svn switch file:///g:/repos/repo/branch/version1 wc2
svn info wc2
# URL of wc2 is incorrect
# This may be the same problem as in case two. But this time with
# a directory instead of a file.
----------------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by Julian Foad <ju...@btopenworld.com>.
makl wrote:
> Julian Foad wrote:
>
>> http://subversion.tigris.org/issues/show_bug.cgi?id=1751
>
>> It says "Note: It is irrelevant for the recipe whether wc2 is checked
>> out recursively or not." But, as I understand it, the "switch"
>> command must be non-recursive for that recipe. I did some experiments
>> to confirm this.
>
> Correct for the first recipe. For the second, wc2 must be checked out
> non recursively.
>
>> And it says "Changed summary, since a part of the problem occurs with
>> recursive working copies too, and in all cases the working copy is
>> dead." Again, as I understand it, this still referred only to
>> problems caused by "switch --non-recursive".
>
> Correct for the first recipe.
>
>> There are other, similar failures such as the one in the issue in
>> which the destination URL does exist but the switch fails with
>> "Working copy 'www' not locked".
>
> For another example see the second recipe in the issue.
Oh yes - I'm sorry - I had not read your second recipe carefully.
So, we have reproduction recipes in which a plain "switch" command currupts the WC when it fails. Now we just need to start debugging!
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by makl <ma...@tigris.org>.
Julian Foad wrote:
> http://subversion.tigris.org/issues/show_bug.cgi?id=1751
> It says "Note: It is irrelevant for the recipe whether wc2 is checked
> out recursively or not." But, as I understand it, the "switch" command
> must be non-recursive for that recipe. I did some experiments to
> confirm this.
Correct for the first recipe. For the second, wc2 must be checked out
non recursively.
> And it says "Changed summary, since a part of the problem occurs with
> recursive working copies too, and in all cases the working copy is
> dead." Again, as I understand it, this still referred only to problems
> caused by "switch --non-recursive".
Correct for the first recipe.
> There are other, similar failures such as the one in the issue in which
> the destination URL does exist but the switch fails with "Working copy
> 'www' not locked".
For another example see the second recipe in the issue.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 1751] svn switch may corrupt working copy
Posted by Julian Foad <ju...@btopenworld.com>.
http://subversion.tigris.org/issues/show_bug.cgi?id=1751
Marvin Greenberg wrote:
> > From: Ben Collins-Sussman <su...@collab.net>
> >
> > As long as you stay way from -N, you should be just fine.
>
> If you look at the case, it specifically says it isn't just "-N".
It says "Note: It is irrelevant for the recipe whether wc2 is checked out recursively or not." But, as I understand it, the "switch" command must be non-recursive for that recipe. I did some experiments to confirm this.
And it says "Changed summary, since a part of the problem occurs with recursive working copies too, and in all cases the working copy is dead." Again, as I understand it, this still referred only to problems caused by "switch --non-recursive".
However, there difinitely are problems also with fully recursive operation:
> I have had this happen doing really simple things like a switch from
> branches/br1 to branches/br2. No -N flag, nothing fancy. Resulting in
> a wedged working copy, with a rather painful process to recover, [...]
If you can make a recipe that reproduces the problem, that would be very helpful.
The very first thing mentioned in that issue is a (failed) plain switch command messing up a non-recursive WC (leaving it with a bad URL). I can reproduce something like that:
# Make a repository
svnadmin create repos
REPOS=file://`pwd`/repos
# Make a branch
svn mkdir -m "" $REPOS/branch1
# Check out a WC from branch1
svn co $REPOS/branch1 wc
# Show the correct info
svn info wc
# If "branch2" exists (empty), it is OK. If it does not exist, then it fails
# with "Cannot replace a directory from within", and leaves the WC with the
# wrong repository URL.
#svn mkdir -m "" $REPOS/branch2
# Try to switch WC to "branch2".
svn switch $REPOS/branch2 wc # Error: "Cannot replace a directory from within"
# Check the result
svn info wc # The URL is now wrong
I think we will agree that, while it would be acceptable to fail and leave the WC in an incomplete state (or any state that can easily be recovered from), a failure such as this should not leave the WC broken. There are other, similar failures such as the one in the issue in which the destination URL does exist but the switch fails with "Working copy 'www' not locked".
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org