[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: disallowing commit of a deletion of a deleted file (was: Re: deleted-with-history [sic] after merge)

From: David Glasser <glasser_at_davidglasser.net>
Date: Mon, 31 Mar 2008 14:14:00 -0700

On Wed, Mar 19, 2008 at 2:42 PM, Stefan Sperling <stsp_at_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 <sbutler_at_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,

David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-31 23:14:11 CEST

This is an archived mail posted to the Subversion Dev mailing list.