On Wed, May 04, 2011 at 01:32:02AM +0200, Paolo Compieta wrote:
> Thanks for the reply, your decision not to block the release makes sense.
> Also, i was thinking of a temporary workaround for marking directories in
> destination branches to inhibit their deletion.. but it seems there isn't
> any.
No, there isn't. You'll have to rely on manual audits right now.
> May i help somehow to make this land in 1.7.x? Writing tests or analyzing
> code for the solution? Do you have already a clear list of shortcomings to
> be fixed before starting to implement this kind of detection?
Sure, help is always welcome!
To determine whether the deletion of a directory causes a tree conflict,
we need to know whether the directory that has been deleted in the merge
source had the same content as the corresponding deleted directory in
the merge target. If it did, there is no conflict so it can be safely
deleted. If it did not, a tree conflict needs to be flagged.
The problems we were facing are as follows:
1) At present, it is impossible to tell apart a rename from a deletion.
You seem to be concerned mostly about reliably detecting the following
situations:
incoming delete vs. local edit
incoming delete vs. local rename
incoming rename vs. local rename
incoming rename vs. local delete
It helps to keep in mind that all of the following cases look the same
to Subversion at the moment:
incoming delete vs. local rename
incoming rename vs. local delete
incoming rename vs. local rename
incoming delete vs. local delete
The last case is not really a conflict but needs to be flagged as such
because it looks the same as the former three possibilities.
So you would still have to check each reported tree conflict for validity.
This behaviour already applies for files in 1.6.x though, so it's a
problem that isn't unique to directories. It is nevertheless annoying.
But maybe less annoying than the current situation.
2) Comparing a mixed-revision working copy to a tree in the repository
(which is always at a single revision) can produce spurious differences.
However, as of 1.7, merges into mixed-revision working copies are disallowed
by default. So this problem is now less of a concern than it used to be.
3) Comparing a local tree to some tree in the repository usually implies
quite a bit of network and server-side processing overhead. It will be
quite noticeable in most environments. This comparison would need to be
performed for every incoming deletion of a directory. It's already being done
for files but comparing entire directory trees will in general take longer.
Having some kind of optimisation in place for this purpose would be good.
However, I'd favour correctness over performance.
4) We need the above comparison to be reliable. I don't remember the
details, but I think we found problems with the diff implementation
while trying to compare a working copy tree to a repository tree.
These problems may have been due to corner-case bugs since this kind
of operation is rarely tested in practice. But it might also have been
due to the mixed-revision working copy problem, which we can now ignore.
There is some research left to be done here.
I think a good start would be trying to get the code to catch the
"incoming delete vs. local edit" situation by performing a comparison
with the corresponding tree in the repository at the merge-left revision,
using the existing diff APIs (see subversion/include/svn_diff.h).
The point to start off at is in subversion/libsvn_client/merge.c,
inside the merge_dir_deleted() function, at the following TODO comment
currently at line 2222:
/* An svn_wc_diff_callbacks4_t function. */
static svn_error_t *
merge_dir_deleted(svn_wc_notify_state_t *state,
svn_boolean_t *tree_conflicted,
const char *local_abspath,
void *baton,
apr_pool_t *scratch_pool)
{
[...]
case svn_node_dir:
{
if (is_versioned && !is_deleted)
{
/* ### TODO: Before deleting, we should ensure that this dir
tree is equal to the one we're being asked to delete.
If not, mark this directory as a tree conflict victim,
because this could be use case 5 as described in
notes/tree-conflicts/detection.txt.
*/
There is code detecting various other tree conflict situations in that file.
See the function make_tree_conflict() and its callers for examples.
