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 <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).

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 <stsp_at_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