You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2011/04/07 14:34:33 UTC
svn commit: r1089856 -
/subversion/trunk/subversion/tests/cmdline/switch_tests.py
Author: philip
Date: Thu Apr 7 12:34:33 2011
New Revision: 1089856
URL: http://svn.apache.org/viewvc?rev=1089856&view=rev
Log:
Make switch_tests.py 38 into a PASS. It was failing because of an
'R'/'A' difference between WCNG status and the old entries-based
status (for which we have testsuite support) but apart from that it
was behaving in the way the test expected.
* subversion/tests/cmdline/switch_tests.py
(copy_with_switched_subdir): Use entry_status to tweak expectations.
Modified:
subversion/trunk/subversion/tests/cmdline/switch_tests.py
Modified: subversion/trunk/subversion/tests/cmdline/switch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/switch_tests.py?rev=1089856&r1=1089855&r2=1089856&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/switch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/switch_tests.py Thu Apr 7 12:34:33 2011
@@ -3079,7 +3079,7 @@ def relocate_with_switched_children(sbox
expected_info = { 'URL' : pattern }
svntest.actions.run_and_verify_info([expected_info], path)
-@XFail()
+
def copy_with_switched_subdir(sbox):
"copy directory with switched subdir"
sbox.build()
@@ -3095,7 +3095,7 @@ def copy_with_switched_subdir(sbox):
# Verify before switching
svntest.actions.run_and_verify_status(wc_dir, state)
- # Switch D
+ # Switch A/D/G
svntest.actions.run_and_verify_svn(None, None, [], 'switch',
'--ignore-ancestry', E_url, G)
@@ -3107,7 +3107,7 @@ def copy_with_switched_subdir(sbox):
})
svntest.actions.run_and_verify_status(wc_dir, state)
- # And now copy A and everything below it to R
+ # And now copy A/D and everything below it to R
svntest.actions.run_and_verify_svn(None, None, [], 'cp', D, R)
state.add({
@@ -3119,25 +3119,17 @@ def copy_with_switched_subdir(sbox):
'R/H/chi' : Item(status=' ', copied='+', wc_rev='-'),
'R/H/omega' : Item(status=' ', copied='+', wc_rev='-'),
'R/H/psi' : Item(status=' ', copied='+', wc_rev='-'),
-
- # This should be:
- # 'R/G' : Item(status='A ', copied='+', wc_rev='-'),
- # But is:
- 'R/G' : Item(status=' ', copied='+', wc_rev='-'),
+ 'R/G' : Item(status='R ', copied='+', wc_rev='-', entry_status='A '),
})
svntest.actions.run_and_verify_status(wc_dir, state)
svntest.main.run_svn(None, 'ci', '-m', 'Commit added folder', wc_dir)
- # Additional test. Enable when the invalid copy is fixed.
- # It should either commit to R/G/alpha or to A/D/G/alpha when the copy
- # keeps the switch.. or it must be that there is no alpha file.
-
- # (Don't forget to bump the wc_rev below for the extra commit)
- #svntest.main.run_svn(None, 'up', wc_dir)
- #svntest.main.file_append(os.path.join(wc_dir, 'R/G/alpha'), "apple")
- #svntest.main.run_svn(None, 'ci', '-m', 'Commit changed file', wc_dir)
+ # Additional test, it should commit to R/G/alpha.
+ svntest.main.run_svn(None, 'up', wc_dir)
+ svntest.main.file_append(os.path.join(wc_dir, 'R/G/alpha'), "apple")
+ svntest.main.run_svn(None, 'ci', '-m', 'Commit changed file', wc_dir)
# Checkout working copy to verify result
svntest.main.safe_rmtree(wc_dir, 1)
@@ -3146,17 +3138,15 @@ def copy_with_switched_subdir(sbox):
'checkout',
sbox.repo_url, wc_dir)
- # Switch D again to recreate state
+ # Switch A/D/G again to recreate state
svntest.actions.run_and_verify_svn(None, None, [], 'switch',
'--ignore-ancestry', E_url, G)
# Clear the statuses
- state.tweak(status=' ', copied=None, wc_rev='2')
+ state.tweak(status=' ', copied=None, wc_rev='3', entry_status=None)
# But reset the switched state
state.tweak('A/D/G', switched='S')
- # This fails because the original tree of D is under R, while there
- # should either be the tree of E, or nothing at all.
svntest.actions.run_and_verify_status(wc_dir, state)
### regression test for issue #3597
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
> Ok, if I read this right you are assuming that that the switched path
> doesn't have local modifications.
No.
Here's an example:
svnadmin create repo
svn mkdir -mm --parents file://`pwd`/repo/A/B/C
svn cp -mm file://`pwd`/repo/A file://`pwd`/repo/X
svn co file://`pwd`/repo/A wc
svn sw ^/X/B/C wc/B/C
svn cp wc/B wc/Z
svn ps p v wc/Z/C
So Z in the working copy is a copy of B and Z/C is a copy of the
switched B/C. Status sould show something like:
$ svn st wc
S wc/B/C
A + wc/Z
M S wc/Z/C
The copied switch doesn't get committed, it's just a switch, and changes
to Z/C affect the switch source /A/B/C not some future new URL. So a
commit would create /A/Z in the repository as a copy of /A/B@2 and
change the properties on /X/B/C. After the commit Z/C in the working
copy is still switched.
I don't know whether this is feasible in the current 1.7 model. It
probably involves putting Z/C at op-depth=0 inside the copy, which may
or may not work. It would also mean, as you pointed out, that the user
would want to be able to run 'svn up' on the op-depth=0 part of copy
which is likely to be difficult.
Once we can update inside the copy we can also implement switch inside
the copy, with that "switch-copy-commit", "copy-switch-commit" and
"copy-commit-switch" would all produce the same result.
--
Philip
RE: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: dinsdag 12 april 2011 12:32
> To: Bert Huijben
> Cc: 'C. Michael Pilato'; dev@subversion.apache.org
> Subject: Re: svn commit: r1089856 -
> /subversion/trunk/subversion/tests/cmdline/switch_tests.py
>
> "Bert Huijben" <be...@qqmail.nl> writes:
>
> > I like to think of it this way (and the current WC-NG tree model
agrees):
> >
> > A wc-wc copy is created from what is currently in your working copy and
> > copies are not affected by BASE operations; just like a non svn copy
would
> > do.
> >
> > So following this reasoning a copy should register exactly what it
copied,
> > recording the original locations on switches etc.
> > (I really hope it currently does; if that is not the case our copy
operation
> > doesn't introduce the new op-roots where it should).
>
> It's still not clear to me that it is correct for switch to become a
> replace when copied.
>
> Consider a working tree (not copied) with depth changes and switches.
> Neither depth changes or switches count as local modifications that can
> be committed, they just affect the "view" of the repository.
>
> If we wc-to-wc copy such a tree the depth changes and switches are going
> to be "present" in the copy, that seems reasonable--it's a working copy
> only operation so only the items in the wc can get copied. It's how
> that copy gets committed that is the issue. In 1.7 the switch in the
> copy has become a local modification that gets committed as a replace.
> That's a change from 1.6 and it's also inconsistent with how switch
> behaves outside the copy, it's inconsistent with wc-to-URL copy, and
> it's inconsistent with the way the depth changes behave in the copy.
Ok, if I read this right you are assuming that that the switched path
doesn't have local modifications.
If that switched path does have local modifications then it must match the
base version of the copy source (or the commit cannot be processed).
We can't commit a copy from a different source then where it was copied from
(in this case from the switched location), without introducing all kinds of
assumptions like this 'it was not modified'
Yes, it would be nice to have the copy from the unswitched path.... Nice use
case, and certainly nice for better merge behavior later on.
But that requires unswitching it, by merging the differences.
We can't just make the commit processing provide a different copyfrom_url,
without handling the consequences.
In the old entries world, things were simple: We had only one url, so we had
to use that.... and we ignored the consequences (which would be cought by a
failing commit), which happened to work in some of the cases.
Once we start changing copy origins in arbitrary ways, how are we going to
guarantee that the commit processor knows how to apply them?
For the current model/implementation, designed by Erik and Greg, this
behavior is very well defined: better than ever before.
For a new translate switches in copies behavior it isn't.
And that we see a replacement, is probably because something was added as
op_depth 0 under the copy.
Our 'R'eplaced is currently ill-defined as there is an existing node in
BASE, and a normal node in working, while it should really look at op-roots,
etc.
(If you replace a subdir via a single copy, then you see the 'R' status
there, while it is not an op_root)
Bert
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/12/2011 06:32 AM, Philip Martin wrote:
> "Bert Huijben" <be...@qqmail.nl> writes:
>
>> I like to think of it this way (and the current WC-NG tree model agrees):
>>
>> A wc-wc copy is created from what is currently in your working copy and
>> copies are not affected by BASE operations; just like a non svn copy would
>> do.
>>
>> So following this reasoning a copy should register exactly what it copied,
>> recording the original locations on switches etc.
>> (I really hope it currently does; if that is not the case our copy operation
>> doesn't introduce the new op-roots where it should).
>
> It's still not clear to me that it is correct for switch to become a
> replace when copied.
>
> Consider a working tree (not copied) with depth changes and switches.
> Neither depth changes or switches count as local modifications that can
> be committed, they just affect the "view" of the repository.
>
> If we wc-to-wc copy such a tree the depth changes and switches are going
> to be "present" in the copy, that seems reasonable--it's a working copy
> only operation so only the items in the wc can get copied. It's how
> that copy gets committed that is the issue. In 1.7 the switch in the
> copy has become a local modification that gets committed as a replace.
> That's a change from 1.6 and it's also inconsistent with how switch
> behaves outside the copy, it's inconsistent with wc-to-URL copy, and
> it's inconsistent with the way the depth changes behave in the copy.
>
> I don't know what the answer is, perhaps switched nodes should be
> op-depth=0 within the copy, perhaps the code should recognise some
> copied nodes at op-depth>0 as switched.
>
> My worry is that we have arrived at the current 1.7 behaviour by
> accident, rather than as a conscious decision, and that supporting this
> behaviour in future will prevent us making switch in a copy behave more
> like switch outside a copy.
I also get the sense that what we have today is just unexpected fallout from
the work we've done for everything else in WC-NG. If I may wax quaintly
poetic for a second...
The storage layer's design should not your features define.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
> I like to think of it this way (and the current WC-NG tree model agrees):
>
> A wc-wc copy is created from what is currently in your working copy and
> copies are not affected by BASE operations; just like a non svn copy would
> do.
>
> So following this reasoning a copy should register exactly what it copied,
> recording the original locations on switches etc.
> (I really hope it currently does; if that is not the case our copy operation
> doesn't introduce the new op-roots where it should).
It's still not clear to me that it is correct for switch to become a
replace when copied.
Consider a working tree (not copied) with depth changes and switches.
Neither depth changes or switches count as local modifications that can
be committed, they just affect the "view" of the repository.
If we wc-to-wc copy such a tree the depth changes and switches are going
to be "present" in the copy, that seems reasonable--it's a working copy
only operation so only the items in the wc can get copied. It's how
that copy gets committed that is the issue. In 1.7 the switch in the
copy has become a local modification that gets committed as a replace.
That's a change from 1.6 and it's also inconsistent with how switch
behaves outside the copy, it's inconsistent with wc-to-URL copy, and
it's inconsistent with the way the depth changes behave in the copy.
I don't know what the answer is, perhaps switched nodes should be
op-depth=0 within the copy, perhaps the code should recognise some
copied nodes at op-depth>0 as switched.
My worry is that we have arrived at the current 1.7 behaviour by
accident, rather than as a conscious decision, and that supporting this
behaviour in future will prevent us making switch in a copy behave more
like switch outside a copy.
--
Philip
RE: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: maandag 11 april 2011 19:09
> To: 'Philip Martin'; 'C. Michael Pilato'
> Cc: dev@subversion.apache.org
> Subject: RE: svn commit: r1089856 -
> /subversion/trunk/subversion/tests/cmdline/switch_tests.py
>
>
>
> > -----Original Message-----
> > From: Philip Martin [mailto:philip.martin@wandisco.com]
> > Sent: maandag 11 april 2011 18:15
> > To: C. Michael Pilato
> > Cc: dev@subversion.apache.org
> > Subject: Re: svn commit: r1089856 -
> > /subversion/trunk/subversion/tests/cmdline/switch_tests.py
> >
> > "C. Michael Pilato" <cm...@collab.net> writes:
> >
> > > On 04/11/2011 05:53 AM, Philip Martin wrote:
> > >> "C. Michael Pilato" <cm...@collab.net> writes:
> > >>
> > >>> But we obviously have precedent for supporting committed copies
> > >>> of deeply switched things, so perhaps this isn't the best use of our
> time
> > >>> right now.
> > >>
> > >> "Support" is generous, we only really support copied switches with no
> > >> modifications:
> > >>
> > >> svnadmin create repo
> > >> svn import -mm repo/format file://`pwd`/repo/A/B/f
> > >> svn import -mm repo/format file://`pwd`/repo/A/B/C/g
> > >> svn co file://`pwd`/repo wc
> > >> svn sw ^/A/B/C wc/A/B
> > >> svn cp wc/A wc/X
> > >>
> > >> Using 1.6 the copy of the switch does not show up in status. Using
1.6
> > >> the switch does not count as a local modification and gets ignored by
> > >> the the commit harvester. After the commit 1.6 shows wc/X/B as
> > >> switched.
> > >>
> > >> If I make a text modification within the switched subdir before
commit:
> > >>
> > >> echo xx >> wc/X/B/g
> > >>
> > >> then the commit fails because it attempts to modify /X/B/g in the
> > >> repoository and that file does not exist. The fact that 1.6 attempts
> to
> > >> commit modifications to the wrong file is a definite bug, if the path
> > >> existed and the checksums matched the commit would go through.
> > >>
> > >> 1.7 treats the copy of the switch as a local modification that gets
> > >> committed as a replace; after the commit there is no switch. The
test
> > >> is new in 1.7 and it's not clear to me that the new behaviour is
> > >> correct.
> > >
> > > Man, my gut says to just not allow folks to copy trees with switched
> > > children until we have a more-fully-formed vision regarding how to
deal
> > with
> > > the overlap of the copy and switch concepts. (Of course, that sanity
> check
> > > would need to *not* block copies of trees with file externals ... our
> > > now-routine "when is switched not *really* switched" exception.) Yes,
I
> > > realize that this might be considered the cop-out position to take.
I'm
> > > okay with that.
> >
> > I tend to agree.
>
> I like to think of it this way (and the current WC-NG tree model agrees):
>
> A wc-wc copy is created from what is currently in your working copy and
> copies are not affected by BASE operations; just like a non svn copy would
> do.
>
> So following this reasoning a copy should register exactly what it copied,
> recording the original locations on switches etc.
> (I really hope it currently does; if that is not the case our copy
operation
> doesn't introduce the new op-roots where it should).
>
> Update and switch (and checkout) only update the BASE and actual trees.
> Any
> case that would affect the WORKING tree raises some kind of tree conflict
> (some of which are automatically resolved for historical reasons).
I forgot one very important point here:
Additions (adds and copies) are always seens as a copy as a new child of the
parent node.
So switches inside (or below) the copy are not followed for the commit
processing for determining where to commit. The entire working (or copied)
tree should be committed below that single point.
And this might be the reason we are talking about this: I wouldn't be
surprised if we have some failures here, where we are just looking in the
BASE tree to find the URL of where we want to commit instead of using the
proper svn_wc__db_scan_addition() call.
Bert
RE: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: maandag 11 april 2011 18:15
> To: C. Michael Pilato
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1089856 -
> /subversion/trunk/subversion/tests/cmdline/switch_tests.py
>
> "C. Michael Pilato" <cm...@collab.net> writes:
>
> > On 04/11/2011 05:53 AM, Philip Martin wrote:
> >> "C. Michael Pilato" <cm...@collab.net> writes:
> >>
> >>> But we obviously have precedent for supporting committed copies
> >>> of deeply switched things, so perhaps this isn't the best use of our
time
> >>> right now.
> >>
> >> "Support" is generous, we only really support copied switches with no
> >> modifications:
> >>
> >> svnadmin create repo
> >> svn import -mm repo/format file://`pwd`/repo/A/B/f
> >> svn import -mm repo/format file://`pwd`/repo/A/B/C/g
> >> svn co file://`pwd`/repo wc
> >> svn sw ^/A/B/C wc/A/B
> >> svn cp wc/A wc/X
> >>
> >> Using 1.6 the copy of the switch does not show up in status. Using 1.6
> >> the switch does not count as a local modification and gets ignored by
> >> the the commit harvester. After the commit 1.6 shows wc/X/B as
> >> switched.
> >>
> >> If I make a text modification within the switched subdir before commit:
> >>
> >> echo xx >> wc/X/B/g
> >>
> >> then the commit fails because it attempts to modify /X/B/g in the
> >> repoository and that file does not exist. The fact that 1.6 attempts
to
> >> commit modifications to the wrong file is a definite bug, if the path
> >> existed and the checksums matched the commit would go through.
> >>
> >> 1.7 treats the copy of the switch as a local modification that gets
> >> committed as a replace; after the commit there is no switch. The test
> >> is new in 1.7 and it's not clear to me that the new behaviour is
> >> correct.
> >
> > Man, my gut says to just not allow folks to copy trees with switched
> > children until we have a more-fully-formed vision regarding how to deal
> with
> > the overlap of the copy and switch concepts. (Of course, that sanity
check
> > would need to *not* block copies of trees with file externals ... our
> > now-routine "when is switched not *really* switched" exception.) Yes, I
> > realize that this might be considered the cop-out position to take. I'm
> > okay with that.
>
> I tend to agree.
I like to think of it this way (and the current WC-NG tree model agrees):
A wc-wc copy is created from what is currently in your working copy and
copies are not affected by BASE operations; just like a non svn copy would
do.
So following this reasoning a copy should register exactly what it copied,
recording the original locations on switches etc.
(I really hope it currently does; if that is not the case our copy operation
doesn't introduce the new op-roots where it should).
Update and switch (and checkout) only update the BASE and actual trees. Any
case that would affect the WORKING tree raises some kind of tree conflict
(some of which are automatically resolved for historical reasons).
When we want update/switch to affect the working tree we immediately have
the problem that we can only update a node once in Editor v1. We need the
BASE change for detecting tree conflicts, so we can't just use the update to
also update the switched location.
So affecting the working layers would require some smart thinkering with the
update editor: Maybe two editor runs, or maybe a virtual tree that isn't
directly mapped on local paths, ....
Note that we don't need this for handling file moves post 1.7: We already
record the changes on the BASE tree, so all we have to do is replay the
changes to that BASE location to the moved_to location.
> > Philip, you say that it's not clear to you that the new behavior is
> > correct. What are you currently leaning towards?
>
> I like the 1.6 behaviour where switch is not a modification that gets
> committed. That's how switch behaves outside copies, and it means that
> "switch-copy-commit" produces the same result as "copy-commit-switch",
> which seems like a plausible definition of what copy of a switch should
> mean.
>
> If we adopted that behaviour we would have to fix the behaviour for
> commits of modifications within the switch. The 1.6 behaviour is
> obviously wrong, the commit will usually fail, and when it succeeds the
> wrong node is modified in the repository. Fixing it would mean
> committing the modifications to the source of the switched URLs, just
> like any other switch. The commit logic is mostly URL driven so I
> imagine it could be adjusted to do this. Doing this would probably
> allow modifications to switches inside a copy to be committed without
> committing the copy itself.
>
> I've just thought of a related problem: deletes that contain a
> switch. Given:
>
> svnadmin create repo
> svn mkdir -mm --parents file://`pwd`/repo/A/B/C/D
> svn cp -mm file://`pwd`/repo/A file://`pwd`/repo/X
> svn co file://`pwd`/repo/A wc
> svn sw ^/X/B/C wc/B/C
> svn rm wc/B
> svn st wc
> D 2 1 pm wc/B
> D S 2 2 pm wc/B/C
> D 2 2 pm wc/B/C/D
>
> (1.7 doesn't show the S flag after the delete, but it comes back if the
> delete is reverted so it's just a display issue)
>
> If I commit the above should ^/X/B/C or ^/X/B/C/D be deleted? 1.6 and
> 1.7 both just delete ^/A/B.
This current behavior follows from the three trees model. Once a node is
overlaid/shadowed we stop looking at its lower layers.
Just deleting the root is what I would expect.
Bert
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/11/2011 01:04 PM, Bob Archer wrote:
> I would even ask... why allow switched children? Is this really a heavily
> used feature of svn... or is it just a byproduct of the current WC
> implementation that a small percentage of users are taking advantage. I
> know people having switch children where I work has caused problems. Yes
> mostly due to lack of understanding.
No, switch isn't a byproduct of the WC implementation. It was designed to
work the way it does today with reasoning at a higher level. If you recall
the CVS-tinged roots of this software, you'll recognize the correlation
between 'cvs up -r SOME_BRANCH' and 'svn switch ^/branches/SOME_BRANCH'.
When a working copy represents a branch root and nothing more, then sure,
'svn switch' is just an optimization over: 'svn diff wc > PATCH && rm -rf
wc && svn co ^/branches/SOME_BRANCH wc && patch -p0 < PATCH'. Why allow
switches of subtree items? Because Subversion has never forced folks to
only "branch" at project root locations. You can branch any subtree of your
project, so it stands to reason that you might want to switch only the
subtree to reflect that branch.
Of course, all this functionality is pointless if we allow folks to shoot
themselves in the foot, commit to the wrong location, hork their working
copies, or whatever other ill can result from the state of things today.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
RE: svn commit: r1089856 -
/subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Bob Archer <Bo...@amsi.com>.
> "C. Michael Pilato" <cm...@collab.net> writes:
>
> > On 04/11/2011 05:53 AM, Philip Martin wrote:
> >> "C. Michael Pilato" <cm...@collab.net> writes:
> >>
> >>> But we obviously have precedent for supporting committed copies
> >>> of deeply switched things, so perhaps this isn't the best use
> of our time
> >>> right now.
> >>
> >> "Support" is generous, we only really support copied switches
> with no
> >> modifications:
> >>
> >> svnadmin create repo
> >> svn import -mm repo/format file://`pwd`/repo/A/B/f
> >> svn import -mm repo/format file://`pwd`/repo/A/B/C/g
> >> svn co file://`pwd`/repo wc
> >> svn sw ^/A/B/C wc/A/B
> >> svn cp wc/A wc/X
> >>
> >> Using 1.6 the copy of the switch does not show up in status.
> Using 1.6
> >> the switch does not count as a local modification and gets
> ignored by
> >> the the commit harvester. After the commit 1.6 shows wc/X/B as
> >> switched.
> >>
> >> If I make a text modification within the switched subdir before
> commit:
> >>
> >> echo xx >> wc/X/B/g
> >>
> >> then the commit fails because it attempts to modify /X/B/g in
> the
> >> repoository and that file does not exist. The fact that 1.6
> attempts to
> >> commit modifications to the wrong file is a definite bug, if the
> path
> >> existed and the checksums matched the commit would go through.
> >>
> >> 1.7 treats the copy of the switch as a local modification that
> gets
> >> committed as a replace; after the commit there is no switch.
> The test
> >> is new in 1.7 and it's not clear to me that the new behaviour is
> >> correct.
> >
> > Man, my gut says to just not allow folks to copy trees with
> switched
> > children until we have a more-fully-formed vision regarding how
> to deal with
> > the overlap of the copy and switch concepts. (Of course, that
> sanity check
> > would need to *not* block copies of trees with file externals ...
> our
> > now-routine "when is switched not *really* switched" exception.)
> Yes, I
> > realize that this might be considered the cop-out position to
> take. I'm
> > okay with that.
I would even ask... why allow switched children? Is this really a heavily used feature of svn... or is it just a byproduct of the current WC implementation that a small percentage of users are taking advantage. I know people having switch children where I work has caused problems. Yes mostly due to lack of understanding.
If you want children to be from a different path than the root of the WC shouldn't you be using externals?
BOb
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> On 04/11/2011 05:53 AM, Philip Martin wrote:
>> "C. Michael Pilato" <cm...@collab.net> writes:
>>
>>> But we obviously have precedent for supporting committed copies
>>> of deeply switched things, so perhaps this isn't the best use of our time
>>> right now.
>>
>> "Support" is generous, we only really support copied switches with no
>> modifications:
>>
>> svnadmin create repo
>> svn import -mm repo/format file://`pwd`/repo/A/B/f
>> svn import -mm repo/format file://`pwd`/repo/A/B/C/g
>> svn co file://`pwd`/repo wc
>> svn sw ^/A/B/C wc/A/B
>> svn cp wc/A wc/X
>>
>> Using 1.6 the copy of the switch does not show up in status. Using 1.6
>> the switch does not count as a local modification and gets ignored by
>> the the commit harvester. After the commit 1.6 shows wc/X/B as
>> switched.
>>
>> If I make a text modification within the switched subdir before commit:
>>
>> echo xx >> wc/X/B/g
>>
>> then the commit fails because it attempts to modify /X/B/g in the
>> repoository and that file does not exist. The fact that 1.6 attempts to
>> commit modifications to the wrong file is a definite bug, if the path
>> existed and the checksums matched the commit would go through.
>>
>> 1.7 treats the copy of the switch as a local modification that gets
>> committed as a replace; after the commit there is no switch. The test
>> is new in 1.7 and it's not clear to me that the new behaviour is
>> correct.
>
> Man, my gut says to just not allow folks to copy trees with switched
> children until we have a more-fully-formed vision regarding how to deal with
> the overlap of the copy and switch concepts. (Of course, that sanity check
> would need to *not* block copies of trees with file externals ... our
> now-routine "when is switched not *really* switched" exception.) Yes, I
> realize that this might be considered the cop-out position to take. I'm
> okay with that.
I tend to agree.
> Philip, you say that it's not clear to you that the new behavior is
> correct. What are you currently leaning towards?
I like the 1.6 behaviour where switch is not a modification that gets
committed. That's how switch behaves outside copies, and it means that
"switch-copy-commit" produces the same result as "copy-commit-switch",
which seems like a plausible definition of what copy of a switch should
mean.
If we adopted that behaviour we would have to fix the behaviour for
commits of modifications within the switch. The 1.6 behaviour is
obviously wrong, the commit will usually fail, and when it succeeds the
wrong node is modified in the repository. Fixing it would mean
committing the modifications to the source of the switched URLs, just
like any other switch. The commit logic is mostly URL driven so I
imagine it could be adjusted to do this. Doing this would probably
allow modifications to switches inside a copy to be committed without
committing the copy itself.
I've just thought of a related problem: deletes that contain a
switch. Given:
svnadmin create repo
svn mkdir -mm --parents file://`pwd`/repo/A/B/C/D
svn cp -mm file://`pwd`/repo/A file://`pwd`/repo/X
svn co file://`pwd`/repo/A wc
svn sw ^/X/B/C wc/B/C
svn rm wc/B
svn st wc
D 2 1 pm wc/B
D S 2 2 pm wc/B/C
D 2 2 pm wc/B/C/D
(1.7 doesn't show the S flag after the delete, but it comes back if the
delete is reverted so it's just a display issue)
If I commit the above should ^/X/B/C or ^/X/B/C/D be deleted? 1.6 and
1.7 both just delete ^/A/B.
--
Philip
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/11/2011 05:53 AM, Philip Martin wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
>> But we obviously have precedent for supporting committed copies
>> of deeply switched things, so perhaps this isn't the best use of our time
>> right now.
>
> "Support" is generous, we only really support copied switches with no
> modifications:
>
> svnadmin create repo
> svn import -mm repo/format file://`pwd`/repo/A/B/f
> svn import -mm repo/format file://`pwd`/repo/A/B/C/g
> svn co file://`pwd`/repo wc
> svn sw ^/A/B/C wc/A/B
> svn cp wc/A wc/X
>
> Using 1.6 the copy of the switch does not show up in status. Using 1.6
> the switch does not count as a local modification and gets ignored by
> the the commit harvester. After the commit 1.6 shows wc/X/B as
> switched.
>
> If I make a text modification within the switched subdir before commit:
>
> echo xx >> wc/X/B/g
>
> then the commit fails because it attempts to modify /X/B/g in the
> repoository and that file does not exist. The fact that 1.6 attempts to
> commit modifications to the wrong file is a definite bug, if the path
> existed and the checksums matched the commit would go through.
>
> 1.7 treats the copy of the switch as a local modification that gets
> committed as a replace; after the commit there is no switch. The test
> is new in 1.7 and it's not clear to me that the new behaviour is
> correct.
Man, my gut says to just not allow folks to copy trees with switched
children until we have a more-fully-formed vision regarding how to deal with
the overlap of the copy and switch concepts. (Of course, that sanity check
would need to *not* block copies of trees with file externals ... our
now-routine "when is switched not *really* switched" exception.) Yes, I
realize that this might be considered the cop-out position to take. I'm
okay with that.
Philip, you say that it's not clear to you that the new behavior is correct.
What are you currently leaning towards?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> But we obviously have precedent for supporting committed copies
> of deeply switched things, so perhaps this isn't the best use of our time
> right now.
"Support" is generous, we only really support copied switches with no
modifications:
svnadmin create repo
svn import -mm repo/format file://`pwd`/repo/A/B/f
svn import -mm repo/format file://`pwd`/repo/A/B/C/g
svn co file://`pwd`/repo wc
svn sw ^/A/B/C wc/A/B
svn cp wc/A wc/X
Using 1.6 the copy of the switch does not show up in status. Using 1.6
the switch does not count as a local modification and gets ignored by
the the commit harvester. After the commit 1.6 shows wc/X/B as
switched.
If I make a text modification within the switched subdir before commit:
echo xx >> wc/X/B/g
then the commit fails because it attempts to modify /X/B/g in the
repoository and that file does not exist. The fact that 1.6 attempts to
commit modifications to the wrong file is a definite bug, if the path
existed and the checksums matched the commit would go through.
1.7 treats the copy of the switch as a local modification that gets
committed as a replace; after the commit there is no switch. The test
is new in 1.7 and it's not clear to me that the new behaviour is
correct.
--
Philip
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/08/2011 08:39 AM, Philip Martin wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
>> On 04/08/2011 04:48 AM, Philip Martin wrote:
>>> Not sure I understand. Are you saying that "copy then switch then
>>> commit" should be the same as "copy then commit then switch"?
>>
>> I'm suggesting that "copy A Z; switch Z/D; commit Z" should be the same as
>> "switch A/D; copy A Z; commit Z".
>
> I still confused: "copy/switch/commit" isn't supported; one cannot
> switch a copy. So is this just a roundabout way of saying that the
> original test case, "switch/copy/commit", should not be supported or are
> you proposing some behaviour that should be supported?
It's apparently a roundabout way of saying that I'm a knucklehead! Sorry,
Philip -- I clearly wasn't thinking fully through this. My general concern
remains, of course, that it seems weird (to me) for a switch to be treated
as a local mod in that it can have some affect on the results of a copy
operation. But we obviously have precedent for supporting committed copies
of deeply switched things, so perhaps this isn't the best use of our time
right now.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> On 04/08/2011 04:48 AM, Philip Martin wrote:
>> Not sure I understand. Are you saying that "copy then switch then
>> commit" should be the same as "copy then commit then switch"?
>
> I'm suggesting that "copy A Z; switch Z/D; commit Z" should be the same as
> "switch A/D; copy A Z; commit Z".
I still confused: "copy/switch/commit" isn't supported; one cannot
switch a copy. So is this just a roundabout way of saying that the
original test case, "switch/copy/commit", should not be supported or are
you proposing some behaviour that should be supported?
> In other words, stop treating a switch
> like a local mod. A switch isn't a local mod -- it's a client-only view
> option, kinda like a sparse directory is. Or just say, "Sorry, I can't do
> that copy, because you won't get the result you might think you'd get."
--
Philip
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/08/2011 04:48 AM, Philip Martin wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
>> On 04/07/2011 08:44 AM, Philip Martin wrote:
>>> I'm not really sure how a copied switch should behave when committed, is
>>> the above correct?
>>
>> This use-case doesn't even make sense to me. Switch is a working copy
>> operation concept -- causing local elements to reflect an alternate line of
>> history.
>>
>> Forgetting the copy operation for a second, if you simply commit in a
>> working copy that has a switched subdir, there's no "coalescing" of the tree
>> on the server side -- changes made in the subdir wind up in the repos tree
>> to which the subdir is switched, and changes made outside the subdir wind up
>> in their respective original repos locations. Now we add 'copy' to this
>> situation -- a copy of a tree with a switched subdir. I rather think that
>> this should behave as if it was a BASE deep copy with the switches
>> re-applied after the copy. In other words, when looking at the copy result,
>> switch followed by copy should have the same effect as copy followed by
>> switch, or something like that.
>
> Not sure I understand. Are you saying that "copy then switch then
> commit" should be the same as "copy then commit then switch"?
I'm suggesting that "copy A Z; switch Z/D; commit Z" should be the same as
"switch A/D; copy A Z; commit Z". In other words, stop treating a switch
like a local mod. A switch isn't a local mod -- it's a client-only view
option, kinda like a sparse directory is. Or just say, "Sorry, I can't do
that copy, because you won't get the result you might think you'd get."
> We do have precedent for preventing copies: we disallow copies with
> absent (excluded by authz) nodes, since the server does not allow the
> parent of an absent node to be copied.
... and yet we allow copies of directories locally configured to be
--set-depth=empty, where the result on the server is a full recursive copy.
Yes, I even pushed in favor of that use-case recently. But now I'm not so
sure. I think the average user's expectation is always going to be, "If I
do a copy, the result will look just like the original did at the time of
the copy." We can't promise that today in many cases. Still, folks are
making use of those very cases. So now I've gone and split my mind about
the matter. :-\
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> On 04/07/2011 08:44 AM, Philip Martin wrote:
>> I'm not really sure how a copied switch should behave when committed, is
>> the above correct?
>
> This use-case doesn't even make sense to me. Switch is a working copy
> operation concept -- causing local elements to reflect an alternate line of
> history.
>
> Forgetting the copy operation for a second, if you simply commit in a
> working copy that has a switched subdir, there's no "coalescing" of the tree
> on the server side -- changes made in the subdir wind up in the repos tree
> to which the subdir is switched, and changes made outside the subdir wind up
> in their respective original repos locations. Now we add 'copy' to this
> situation -- a copy of a tree with a switched subdir. I rather think that
> this should behave as if it was a BASE deep copy with the switches
> re-applied after the copy. In other words, when looking at the copy result,
> switch followed by copy should have the same effect as copy followed by
> switch, or something like that.
Not sure I understand. Are you saying that "copy then switch then
commit" should be the same as "copy then commit then switch"?
> This also implies that, for user sanity, we
> should disallow WC-to-REPOS copies of trees containing switched items.
I suppose making switch within a copy behave like other switches, i.e.
affecting the switched location in the repository, would be possible but
likely confusing.
We do have precedent for preventing copies: we disallow copies with
absent (excluded by authz) nodes, since the server does not allow the
parent of an absent node to be copied.
I was going to say I'd wait for Julian's input, but I see Bert added
this test in 2009...
--
Philip
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/07/2011 08:44 AM, Philip Martin wrote:
> I'm not really sure how a copied switch should behave when committed, is
> the above correct?
This use-case doesn't even make sense to me. Switch is a working copy
operation concept -- causing local elements to reflect an alternate line of
history.
Forgetting the copy operation for a second, if you simply commit in a
working copy that has a switched subdir, there's no "coalescing" of the tree
on the server side -- changes made in the subdir wind up in the repos tree
to which the subdir is switched, and changes made outside the subdir wind up
in their respective original repos locations. Now we add 'copy' to this
situation -- a copy of a tree with a switched subdir. I rather think that
this should behave as if it was a BASE deep copy with the switches
re-applied after the copy. In other words, when looking at the copy result,
switch followed by copy should have the same effect as copy followed by
switch, or something like that. This also implies that, for user sanity, we
should disallow WC-to-REPOS copies of trees containing switched items.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1089856 - /subversion/trunk/subversion/tests/cmdline/switch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
philip@apache.org writes:
> Author: philip
> Date: Thu Apr 7 12:34:33 2011
> New Revision: 1089856
>
> URL: http://svn.apache.org/viewvc?rev=1089856&view=rev
> Log:
> Make switch_tests.py 38 into a PASS. It was failing because of an
> 'R'/'A' difference between WCNG status and the old entries-based
> status (for which we have testsuite support) but apart from that it
> was behaving in the way the test expected.
> - # This should be:
> - # 'R/G' : Item(status='A ', copied='+', wc_rev='-'),
> - # But is:
> - 'R/G' : Item(status=' ', copied='+', wc_rev='-'),
> + 'R/G' : Item(status='R ', copied='+', wc_rev='-', entry_status='A '),
This is the bit that makes the test PASS. It'a a test that switches a
subdir and then copies the parent of the switched subdir. The copied,
switched subdir shows up as 'R' and when committed and when the copy is
committed it gives:
------------------------------------------------------------------------
r2 | jrandom | 2011-04-07 13:11:28 +0100 (Thu, 07 Apr 2011) | 1 line
Changed paths:
A /R (from /A/D:1)
R /R/G (from /A/B/E:1)
Commit added folder
------------------------------------------------------------------------
I made it a PASS because the behaviour we are getting is essentially
what the test expected.
I'm not really sure how a copied switch should behave when committed, is
the above correct?
--
Philip