You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2011/05/04 18:37:26 UTC
Making delete resolve tree conflicts
To make the new single-txn delete pass the regression tests I had to
make it leave ACTUAL_NODE rows with tree conflicts. This preserved the
behaviour of the old per-node delete code, but I'm wondering if that is
the correct thing to do. I suspect that the current behaviour is, in
part at least, accidental--I think is stems from the time when tree
conflicts were stored on the parent node and so deleting a conflicted
node would leave the tree conflict unless the parent node was also
modified.
The current behaviour is that if I have a node in conflict:
$ svn st wc
R C wc/A/f
> local add, incoming add upon update
Summary of conflicts:
Tree conflicts: 1
and I delete the conflicted node:
$ svn rm --force wc/A/f
D wc/A/f
it remains in conflict:
$ svn st wc
D C wc/A/f
> local add, incoming add upon update
Summary of conflicts:
Tree conflicts: 1
I'd like to change this so that delete removes the ACTUAL_NODE row and
thus automatically resolves the conflict.
Anybody see any problems that would arise?
It also solves one of the orphaned ACTUAL_NODE problems: if the
conflicted node is within an added tree then deleting the tree and
leaving the ACTUAL_NODE row could result in an ACTUAL_NODE that has no
parents.
--
Philip
Re: Making delete resolve tree conflicts
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> On Thu, May 5, 2011 at 15:16, Bert Huijben <be...@qqmail.nl> wrote:
>>...
>>> wc_db could do this, as long as we put a check for conflicts up in
>>> svn_wc_delete() to look for unresolved conflicts.
>>
>> The easiest place to block this is probably libsvn_client. It does already
>
> That feels too high level to me. It allows third party code to call
> svn_wc_delete() and wipe out conflict info too easily.
I don't understand this. svn_wc_delete deletes text changes, text
conflicts, property changes, property conflicts and tree changes. Why
should tree conflicts be preserved when all those other things are
deleted? My view is that text changes, which get deleted, are more
valuable than tree conflicts.
>> Is making 'svn rm' fail on conflicts backwards compatible?
>
> I say "no". That just seems the safe way to approach this. That we
> didn't stop the operation before seems like a big mistake. We looked
> for local mods, and I view conflicts in the same boat ("hey! you've
> got something in this subtree").
We check for modifications, including tree conflicts, in the client
layer. The user has to use --force, or --keep-local, to get the delete
to run at all.
--
Philip
Re: Making delete resolve tree conflicts
Posted by Greg Stein <gs...@gmail.com>.
On Thu, May 5, 2011 at 15:16, Bert Huijben <be...@qqmail.nl> wrote:
>...
>> wc_db could do this, as long as we put a check for conflicts up in
>> svn_wc_delete() to look for unresolved conflicts.
>
> The easiest place to block this is probably libsvn_client. It does already
That feels too high level to me. It allows third party code to call
svn_wc_delete() and wipe out conflict info too easily.
> perform a recursive status on the delete target to notice changes when you
> don't add --force, so it would be easy to add this check.
> And it would also allow deleting the conflict with --force.... Nice :-)
param: svn_boolean_t ignore_conflicts
> ....
>
> Is making 'svn rm' fail on conflicts backwards compatible?
I say "no". That just seems the safe way to approach this. That we
didn't stop the operation before seems like a big mistake. We looked
for local mods, and I view conflicts in the same boat ("hey! you've
got something in this subtree").
Cheers,
-g
RE: Making delete resolve tree conflicts
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: donderdag 5 mei 2011 21:04
> To: Philip Martin; Julian Foad
> Cc: dev@subversion.apache.org
> Subject: Re: Making delete resolve tree conflicts
>
> >> - delete all conflicts
> >
> >> I still favour making delete remove all tree conflicts. It's both easy
> >> to implement and easy to explain. It's also what happens with text and
> >> property conflicts.
> >
> > I have to admit that simply deleting all conflicts seems like a nice
> > simple approach. Simple in conceptual behaviour, I mean, and if it's
> > simple to implement too that's a bonus.
>
> wc_db could do this, as long as we put a check for conflicts up in
> svn_wc_delete() to look for unresolved conflicts.
The easiest place to block this is probably libsvn_client. It does already
perform a recursive status on the delete target to notice changes when you
don't add --force, so it would be easy to add this check.
And it would also allow deleting the conflict with --force.... Nice :-)
....
Is making 'svn rm' fail on conflicts backwards compatible?
Bert
Re: Making delete resolve tree conflicts
Posted by Greg Stein <gs...@gmail.com>.
On Thu, May 5, 2011 at 10:58, Julian Foad <ju...@wandisco.com> wrote:
> On Thu, 2011-05-05 at 14:48 +0100, Philip Martin wrote:
>...
>> - fail, with an error about the orphaned conflict
>
> Doesn't feel right. The high-level "svn delete" operation is welcome to
> fail if there are any unresolved conflicts, like it does when there are
> local modifications (unless "--force" is given), but if at all possible
> this low-level operation should be designed so as not to fail on such
> things.
This seems right. The user has not handled a conflict. We don't allow
a number of operations when there are outstanding conflicts. Simply
letting the user toss out the information without a resolution does
not seem right.
iirc, update will skip a node that is in a conflicted state. In the
same way, a delete would do the same, but... you can't delete the
parent if you've skipped a child. Thus, I think the delete should
fail.
High or low level... the wc_db API allows you to do a lot of monkeying
around. So yeah, I don't think it needs to enforce this level of
semantics. That should be at the svn_wc_delete() level.
>> - delete all conflicts
>
>> I still favour making delete remove all tree conflicts. It's both easy
>> to implement and easy to explain. It's also what happens with text and
>> property conflicts.
>
> I have to admit that simply deleting all conflicts seems like a nice
> simple approach. Simple in conceptual behaviour, I mean, and if it's
> simple to implement too that's a bonus.
wc_db could do this, as long as we put a check for conflicts up in
svn_wc_delete() to look for unresolved conflicts.
Cheers,
-g
Re: Making delete resolve tree conflicts
Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2011-05-05 at 14:48 +0100, Philip Martin wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
> > svnadmin create repo
> > svn mkdir -mm file://`pwd`/repo/A
> > svn co -r0 file://`pwd`/repo wc
> > svn mkdir --parents wc/X/Y/A
> > svn merge -c1 ^/ wc/X/Y
> > svn rm --force wc/X
> >
> > sqlite3 wc/.svn/wc.db "select op_depth, local_relpath from nodes"
> > 0|
> >
> > sqlite3 wc/.svn/wc.db "select local_relpath from actual_node"
> > X/Y/A
>
> After discussion on IRC i think we agree that leaving an "orphaned"
> actual-node for X/Y/A is wrong. So how should svn_wc__db_op_delete
> behave? It could:
>
> - delete the orphaned conflict but leave any non-orphaned conflicts
This is probably the option that most closely mirrors 1.6.x operation.
In 1.6.x, a tree conflict would remain if the parent directory remained
(or something like that). I thought that behaviour was entirely
intentional but now I'm not so sure.
> - move the orphaned conflict to some non-orphaned row
You mean convert it into some kind of tree conflict description on a
parent directory that indicates there was a conflict somewhere inside
the subtree? No, I don't think that makes sense within our idea of
conflicts. And if we were going to do that kind of logical
transformation, it should be done by higher level code.
> - fail, with an error about the orphaned conflict
Doesn't feel right. The high-level "svn delete" operation is welcome to
fail if there are any unresolved conflicts, like it does when there are
local modifications (unless "--force" is given), but if at all possible
this low-level operation should be designed so as not to fail on such
things.
> - delete all conflicts
> I still favour making delete remove all tree conflicts. It's both easy
> to implement and easy to explain. It's also what happens with text and
> property conflicts.
I have to admit that simply deleting all conflicts seems like a nice
simple approach. Simple in conceptual behaviour, I mean, and if it's
simple to implement too that's a bonus.
- Julian
Re: Making delete resolve tree conflicts
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> svnadmin create repo
> svn mkdir -mm file://`pwd`/repo/A
> svn co -r0 file://`pwd`/repo wc
> svn mkdir --parents wc/X/Y/A
> svn merge -c1 ^/ wc/X/Y
> svn rm --force wc/X
>
> sqlite3 wc/.svn/wc.db "select op_depth, local_relpath from nodes"
> 0|
>
> sqlite3 wc/.svn/wc.db "select local_relpath from actual_node"
> X/Y/A
After discussion on IRC i think we agree that leaving an "orphaned"
actual-node for X/Y/A is wrong. So how should svn_wc__db_op_delete
behave? It could:
- delete the orphaned conflict but leave any non-orphaned conflicts
- move the orphaned conflict to some non-orphaned row
- fail, with an error about the orphaned conflict
- delete all conflicts
I still favour making delete remove all tree conflicts. It's both easy
to implement and easy to explain. It's also what happens with text and
property conflicts.
--
Philip
Re: Making delete resolve tree conflicts
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@wandisco.com> writes:
>> > It also solves one of the orphaned ACTUAL_NODE problems: if the
>> > conflicted node is within an added tree then deleting the tree and
>> > leaving the ACTUAL_NODE row could result in an ACTUAL_NODE that has no
>> > parents.
>>
>> That would still be a "local delete, incoming add upon update". Not
>> sure whether that would be located at the child, or at the deleted
>> ancestor. The update editor has a notion of conflicted parents, and
>> this seems to follow that case.
>
> I can't see how the no-parents thing could happen, because there would
> be a tree conflict only at the root-most parent that conflicts.
svnadmin create repo
svn mkdir -mm file://`pwd`/repo/A
svn co -r0 file://`pwd`/repo wc
svn mkdir --parents wc/X/Y/A
svn merge -c1 ^/ wc/X/Y
svn rm --force wc/X
sqlite3 wc/.svn/wc.db "select op_depth, local_relpath from nodes"
0|
sqlite3 wc/.svn/wc.db "select local_relpath from actual_node"
X/Y/A
--
Philip
Re: Making delete resolve tree conflicts
Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2011-05-04 at 12:42 -0400, Greg Stein wrote:
> On Wed, May 4, 2011 at 12:37, Philip Martin <ph...@wandisco.com> wrote:
> > To make the new single-txn delete pass the regression tests I had to
> > make it leave ACTUAL_NODE rows with tree conflicts. This preserved the
> > behaviour of the old per-node delete code, but I'm wondering if that is
> > the correct thing to do. I suspect that the current behaviour is, in
> > part at least, accidental--I think is stems from the time when tree
> > conflicts were stored on the parent node and so deleting a conflicted
> > node would leave the tree conflict unless the parent node was also
> > modified.
> >
> > The current behaviour is that if I have a node in conflict:
> >
> > $ svn st wc
> > R C wc/A/f
> > > local add, incoming add upon update
> > Summary of conflicts:
> > Tree conflicts: 1
> >
> > and I delete the conflicted node:
> >
> > $ svn rm --force wc/A/f
> > D wc/A/f
> >
> > it remains in conflict:
> >
> > $ svn st wc
> > D C wc/A/f
> > > local add, incoming add upon update
> > Summary of conflicts:
> > Tree conflicts: 1
> >
> > I'd like to change this so that delete removes the ACTUAL_NODE row and
> > thus automatically resolves the conflict.
> >
> > Anybody see any problems that would arise?
Yes, I do. A tree conflict should remain marked until the user declares
it resolved. The user should be able to review the conflicts that were
raised, make a series of local modifications, review again the conflicts
that were originally raised, and then mark them as resolved when the
user is ready to do so.
> I think the conflict should turn into "local delete, incoming add upon update".
No, it should not change. Your suggestion would mean "an incoming add
conflicted with the local delete", which is not true. The "local"
information provided here should not be a duplicate of the node's
current status; it should simply show what local change conflicted with
the incoming change.
> > It also solves one of the orphaned ACTUAL_NODE problems: if the
> > conflicted node is within an added tree then deleting the tree and
> > leaving the ACTUAL_NODE row could result in an ACTUAL_NODE that has no
> > parents.
>
> That would still be a "local delete, incoming add upon update". Not
> sure whether that would be located at the child, or at the deleted
> ancestor. The update editor has a notion of conflicted parents, and
> this seems to follow that case.
I can't see how the no-parents thing could happen, because there would
be a tree conflict only at the root-most parent that conflicts.
- Julian
Re: Making delete resolve tree conflicts
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> On Wed, May 4, 2011 at 12:37, Philip Martin <ph...@wandisco.com> wrote:
>> To make the new single-txn delete pass the regression tests I had to
>> make it leave ACTUAL_NODE rows with tree conflicts. This preserved the
>> behaviour of the old per-node delete code, but I'm wondering if that is
>> the correct thing to do. I suspect that the current behaviour is, in
>> part at least, accidental--I think is stems from the time when tree
>> conflicts were stored on the parent node and so deleting a conflicted
>> node would leave the tree conflict unless the parent node was also
>> modified.
>>
>> The current behaviour is that if I have a node in conflict:
>>
>> $ svn st wc
>> R C wc/A/f
>> > local add, incoming add upon update
>> Summary of conflicts:
>> Tree conflicts: 1
>>
>> and I delete the conflicted node:
>>
>> $ svn rm --force wc/A/f
>> D wc/A/f
>>
>> it remains in conflict:
>>
>> $ svn st wc
>> D C wc/A/f
>> > local add, incoming add upon update
>> Summary of conflicts:
>> Tree conflicts: 1
>>
>> I'd like to change this so that delete removes the ACTUAL_NODE row and
>> thus automatically resolves the conflict.
>>
>> Anybody see any problems that would arise?
>
> I think the conflict should turn into "local delete, incoming add upon
> update".
I suppose that would make the conflict more accurate, but is it better
than simply resolving the conflict?
>
>> It also solves one of the orphaned ACTUAL_NODE problems: if the
>> conflicted node is within an added tree then deleting the tree and
>> leaving the ACTUAL_NODE row could result in an ACTUAL_NODE that has no
>> parents.
>
> That would still be a "local delete, incoming add upon update". Not
> sure whether that would be located at the child, or at the deleted
> ancestor. The update editor has a notion of conflicted parents, and
> this seems to follow that case.
Again, is that better than resolving the conflict?
We would have to move the conflict, the wc code barely handles
actual-only nodes, I expect if will do even worse on actual-only nodes
that don't have parents.
--
Philip
Re: Making delete resolve tree conflicts
Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 4, 2011 at 12:37, Philip Martin <ph...@wandisco.com> wrote:
> To make the new single-txn delete pass the regression tests I had to
> make it leave ACTUAL_NODE rows with tree conflicts. This preserved the
> behaviour of the old per-node delete code, but I'm wondering if that is
> the correct thing to do. I suspect that the current behaviour is, in
> part at least, accidental--I think is stems from the time when tree
> conflicts were stored on the parent node and so deleting a conflicted
> node would leave the tree conflict unless the parent node was also
> modified.
>
> The current behaviour is that if I have a node in conflict:
>
> $ svn st wc
> R C wc/A/f
> > local add, incoming add upon update
> Summary of conflicts:
> Tree conflicts: 1
>
> and I delete the conflicted node:
>
> $ svn rm --force wc/A/f
> D wc/A/f
>
> it remains in conflict:
>
> $ svn st wc
> D C wc/A/f
> > local add, incoming add upon update
> Summary of conflicts:
> Tree conflicts: 1
>
> I'd like to change this so that delete removes the ACTUAL_NODE row and
> thus automatically resolves the conflict.
>
> Anybody see any problems that would arise?
I think the conflict should turn into "local delete, incoming add upon update".
> It also solves one of the orphaned ACTUAL_NODE problems: if the
> conflicted node is within an added tree then deleting the tree and
> leaving the ACTUAL_NODE row could result in an ACTUAL_NODE that has no
> parents.
That would still be a "local delete, incoming add upon update". Not
sure whether that would be located at the child, or at the deleted
ancestor. The update editor has a notion of conflicted parents, and
this seems to follow that case.
Cheers,
-g