You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2008/03/19 21:42:16 UTC
disallowing commit of a deletion of a deleted file (was: Re:
deleted-with-history [sic] after merge)
[Now that I have my tree-conflicts hat back on, I'd like to revive
this thread, which didn't come to a conclusion last time around.
Stephen's original mail in this thread talks about two separate issues,
actually. One of them, the commit of a delete of an already deleted
file, will be addressed below. The other one, namely that of the diff
editor visiting all files in deleted directories during merge, should
be discussed separately. In fact, Stephen discovered the latter issue
while investigating the former.]
On Fri, Feb 29, 2008 at 08:50:56AM -0600, Ben Collins-Sussman wrote:
> 2008/2/29 Stephen Butler <sb...@elego.de>:
> > Hi folks,
> >
> > If an item has been modified or deleted in the repository since
> > the item was last updated in the working copy, Subversion should
> > disallow committing that item. This is a prerequisite for tree
> > conflict detection. The user should have to update in order to
> > integrate the repo changes, possibly revealing tree conflicts.
> >
> > I had assumed that Subversion would block a commit that includes an
> > item that has been modified or deleted on the repo. But that's not
> > true in all cases:
> >
> > WC State Repo State Blocked?
> > ======== ========== ========
> > Modified Modified Yes
> > Modified Deleted Yes
> > Deleted Modified Yes
> > Deleted Deleted No
> >
> > We'd like to plug that gap.
>
> Let's back up a moment. Why is that a 'gap'? That's a very
> deliberate design decision, IIRC. The 'auto merging' loop that's part
> of commit finalization considers some changes to be mergeable, and
> some not. If two people are in a commit race and change the same
> file, that's not auto-mergeable: somebody wins the race, the other
> person's commit outright fails. But if the only 'conflict' is that
> they both happened to delete the same file, that's a perfectly safe
> auto-mergeable change: the overlapping changes are in agreement.
> Nobody will be destroying or overwriting somebody else's change by
> allowing the changes to be merged.
>
> In theory, if the two commits made absolutely identical textual
> changes to the same file, that would also be an example of a 'safe'
> auto-mergeable thing; we just never made the commit-finalization loop
> sophisticated enough to notice that case.
>
> In any case, I certainly don't consider this a loophole or oversight;
> it's a deliberate feature. Is this behavior presenting a real problem
> for your model of tree-conflict resolution?
Ben, I've talked to Stephen again today about this issue. I think
I understand now why the current behaviour of accepting a delete
of a deleted file as a valid commit operation does indeed present
a real problem for tree-conflict detection (not resolution -- automatic
tree-conflict resolution isn't covered by the current implementation).
If you are talking only about file deletions, the current behaviour
is perfectly acceptable -- after all, a delete of a delete adds up
to a delete, so there's no conflict, right?
Now, looking at this from the tree-conflict perspective, things are
a bit more complicated.
Let us assume, for a minute, that Subversion had true renames, and
that the update editor had a callback function like move_file(source, dest).
Imagine a user who has locally deleted a file, and updates the working
copy before committing. As part of the update, the editor tries to run
the move_file() operation on the locally deleted file. Since the source
has been deleted, the operation obviously cannot be carried out as
intended (a non-existent file cannot be moved).
A move transforms the tree in a different way than a delete does,
and, semantically, one operation excludes the other, so we have a
tree-conflict.
Since move_file() does not exist in reality, and since the update and
diff editors handle deletes and adds with history in essentially separate
contexts (i.e. callbacks), we have to try to make semi-educated guesses
about the user's intention by looking *only* at deletes.
The whole tree-conflict branch is essentially about reverse engineering
the developer's real intention behind a tree transformation. We can only
do this after the fact, by looking at the small amount of information
we have left on the client side during update and merge operations.
Rename information is tossed out right in libsvn_client when the
user runs "svn move". We therefore have to live with false positives,
since a delete callback invocation may always be due to an actual delete,
not a move.
So to catch the case where one developer deletes a file, and another
removes it (a sub-case of our "use case 3" in
notes/tree-conflicts/use-cases.txt, in which both operations are
moves), Subversion must not allow a commit of a deleted file if the
file has already been deleted in the repository. Because it could be
that the file was not actually deleted in the repository, but moved!
Instead of the commit succeeding (leaving the developer with the impression
that the file was deleted, even though it was renamed), the developer
should be forced to update (the deleted file being out-of-date).
The developer will then receive the delete of the already locally deleted
file, which our current implementation will flag as a potential tree-conflict.
The developer should then take this a warning sign, check the resulting tree
for sanity, take appropriate action if need be and mark the conflict as
resolved. Now the commit can take place without risk of causing trouble
further down the road.
I admit that this sounds a bit unwieldy, since we could essentially
already flag a tree-conflict during the commit phase, upon discovering
that the file is missing in the repository, without requiring the extra
update. But in any case, the commit needs to be blocked if we want to have
a chance at informing the developer whether a delete about to be committed
may actually end up being a rename.
--
Stefan Sperling <st...@elego.de> Software Developer
elego Software Solutions GmbH HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12 Tel: +49 30 23 45 86 96
13355 Berlin Fax: +49 30 23 45 86 95
http://www.elego.de Geschaeftsfuehrer: Olaf Wagner
Re: disallowing commit of a deletion of a deleted file
Posted by Julian Foad <ju...@btopenworld.com>.
David Glasser wrote:
> Just for extra super duper fun, Subversion today isn't actually
> consistent about whether or not a double-delete should be a conflict
> or OK.
>
> Specifically, in the libsvn_repos commit editor, if a delete_entry is
> sent from the client on an entry that doesn't exist in the
> transactions' root (and note that the transaction root is always based
> off of the youngest rev when the commit is started, not anything found
> in the WC or otherwise client-specified), then it'll just ignore that
> call and not pass anything to the FS layer: the concurrent deletion is
> ignored. This is the usual case, where we commit deletions not
> simultaneously, but without an update in between.
>
> However! *If* two people are actually trying to delete the entry
> nearly simultaneously (specifically, if both transactions are created
> before either is committed), then the merge() code in
> libsvn_fs_{base,fs}/tree.c will actually notice the double-delete and
> throw a conflict error. (I managed to reproduce this a few months ago
> with careful placement of breakpoints on the server.)
Filed as issue #3156.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: disallowing commit of a deletion of a deleted file
Posted by David Glasser <gl...@davidglasser.net>.
On Wed, Apr 2, 2008 at 4:02 PM, Karl Fogel <kf...@red-bean.com> wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
> > Just for extra super duper fun, Subversion today isn't actually
> > consistent about whether or not a double-delete should be a conflict
> > or OK.
> >
> > Specifically, in the libsvn_repos commit editor, if a delete_entry is
> > sent from the client on an entry that doesn't exist in the
> > transactions' root (and note that the transaction root is always based
> > off of the youngest rev when the commit is started, not anything found
> > in the WC or otherwise client-specified), then it'll just ignore that
> > call and not pass anything to the FS layer: the concurrent deletion is
> > ignored. This is the usual case, where we commit deletions not
> > simultaneously, but without an update in between.
> >
> > However! *If* two people are actually trying to delete the entry
> > nearly simultaneously (specifically, if both transactions are created
> > before either is committed), then the merge() code in
> > libsvn_fs_{base,fs}/tree.c will actually notice the double-delete and
> > throw a conflict error. (I managed to reproduce this a few months ago
> > with careful placement of breakpoints on the server.)
>
> How odd. That means that simply re-attempting the failed commit would
> succeed, even though no update had happened in between, right?
I believe so, yes.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: disallowing commit of a deletion of a deleted file
Posted by Karl Fogel <kf...@red-bean.com>.
"David Glasser" <gl...@davidglasser.net> writes:
> Just for extra super duper fun, Subversion today isn't actually
> consistent about whether or not a double-delete should be a conflict
> or OK.
>
> Specifically, in the libsvn_repos commit editor, if a delete_entry is
> sent from the client on an entry that doesn't exist in the
> transactions' root (and note that the transaction root is always based
> off of the youngest rev when the commit is started, not anything found
> in the WC or otherwise client-specified), then it'll just ignore that
> call and not pass anything to the FS layer: the concurrent deletion is
> ignored. This is the usual case, where we commit deletions not
> simultaneously, but without an update in between.
>
> However! *If* two people are actually trying to delete the entry
> nearly simultaneously (specifically, if both transactions are created
> before either is committed), then the merge() code in
> libsvn_fs_{base,fs}/tree.c will actually notice the double-delete and
> throw a conflict error. (I managed to reproduce this a few months ago
> with careful placement of breakpoints on the server.)
How odd. That means that simply re-attempting the failed commit would
succeed, even though no update had happened in between, right?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: disallowing commit of a deletion of a deleted file (was: Re: deleted-with-history [sic] after merge)
Posted by David Glasser <gl...@davidglasser.net>.
On Wed, Mar 19, 2008 at 2:42 PM, Stefan Sperling <st...@elego.de> wrote:
>
> [Now that I have my tree-conflicts hat back on, I'd like to revive
> this thread, which didn't come to a conclusion last time around.
>
> Stephen's original mail in this thread talks about two separate issues,
> actually. One of them, the commit of a delete of an already deleted
> file, will be addressed below. The other one, namely that of the diff
> editor visiting all files in deleted directories during merge, should
> be discussed separately. In fact, Stephen discovered the latter issue
> while investigating the former.]
>
>
> On Fri, Feb 29, 2008 at 08:50:56AM -0600, Ben Collins-Sussman wrote:
> > 2008/2/29 Stephen Butler <sb...@elego.de>:
> > > Hi folks,
> > >
> > > If an item has been modified or deleted in the repository since
> > > the item was last updated in the working copy, Subversion should
> > > disallow committing that item. This is a prerequisite for tree
> > > conflict detection. The user should have to update in order to
> > > integrate the repo changes, possibly revealing tree conflicts.
> > >
> > > I had assumed that Subversion would block a commit that includes an
> > > item that has been modified or deleted on the repo. But that's not
> > > true in all cases:
> > >
> > > WC State Repo State Blocked?
> > > ======== ========== ========
> > > Modified Modified Yes
> > > Modified Deleted Yes
> > > Deleted Modified Yes
> > > Deleted Deleted No
> > >
> > > We'd like to plug that gap.
> >
> > Let's back up a moment. Why is that a 'gap'? That's a very
> > deliberate design decision, IIRC. The 'auto merging' loop that's part
> > of commit finalization considers some changes to be mergeable, and
> > some not. If two people are in a commit race and change the same
> > file, that's not auto-mergeable: somebody wins the race, the other
> > person's commit outright fails. But if the only 'conflict' is that
> > they both happened to delete the same file, that's a perfectly safe
> > auto-mergeable change: the overlapping changes are in agreement.
> > Nobody will be destroying or overwriting somebody else's change by
> > allowing the changes to be merged.
> >
> > In theory, if the two commits made absolutely identical textual
> > changes to the same file, that would also be an example of a 'safe'
> > auto-mergeable thing; we just never made the commit-finalization loop
> > sophisticated enough to notice that case.
> >
> > In any case, I certainly don't consider this a loophole or oversight;
> > it's a deliberate feature. Is this behavior presenting a real problem
> > for your model of tree-conflict resolution?
>
>
> Ben, I've talked to Stephen again today about this issue. I think
> I understand now why the current behaviour of accepting a delete
> of a deleted file as a valid commit operation does indeed present
> a real problem for tree-conflict detection (not resolution -- automatic
> tree-conflict resolution isn't covered by the current implementation).
Just for extra super duper fun, Subversion today isn't actually
consistent about whether or not a double-delete should be a conflict
or OK.
Specifically, in the libsvn_repos commit editor, if a delete_entry is
sent from the client on an entry that doesn't exist in the
transactions' root (and note that the transaction root is always based
off of the youngest rev when the commit is started, not anything found
in the WC or otherwise client-specified), then it'll just ignore that
call and not pass anything to the FS layer: the concurrent deletion is
ignored. This is the usual case, where we commit deletions not
simultaneously, but without an update in between.
However! *If* two people are actually trying to delete the entry
nearly simultaneously (specifically, if both transactions are created
before either is committed), then the merge() code in
libsvn_fs_{base,fs}/tree.c will actually notice the double-delete and
throw a conflict error. (I managed to reproduce this a few months ago
with careful placement of breakpoints on the server.)
just muddying the waters,
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: disallowing commit of a deletion of a deleted file (was: Re:
deleted-with-history [sic] after merge)
Posted by Stephen Butler <sb...@elego.de>.
Quoting Stefan Sperling <st...@elego.de>:
> Instead of the commit succeeding (leaving the developer with the impression
> that the file was deleted, even though it was renamed), the developer
> should be forced to update (the deleted file being out-of-date).
> The developer will then receive the delete of the already locally deleted
> file, which our current implementation will flag as a potential
> tree-conflict.
> The developer should then take this a warning sign, check the resulting tree
> for sanity, take appropriate action if need be and mark the conflict as
> resolved. Now the commit can take place without risk of causing trouble
> further down the road.
>
> I admit that this sounds a bit unwieldy, since we could essentially
> already flag a tree-conflict during the commit phase, upon discovering
> that the file is missing in the repository, without requiring the extra
> update.
A cautionary note about flagging tree conflicts during commit:
The current tree-conflicts plan has a simple division of labor. The
update/switch/merge commands detect tree conflicts and flag them in
the working copy. The commit command stops with an error if it
finds a flagged conflict.
The user then removes the conflict flags using the resolved or revert
commands, and tries to commit again.
Suppose we extend commit to detect tree conflicts and flag them in
the working copy. The user would have no chance to resolve the
conflicts. We would need a special list of "tree conflicts detected
by commit" and would have to track which of them were already
resolved.
Being a little more strict about requiring an up-to-date working
copy is much simpler.
Steve
> But in any case, the commit needs to be blocked if we want to have
> a chance at informing the developer whether a delete about to be committed
> may actually end up being a rename.
--
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org