Re: tree conflicts and directories discussion

On Fri, Apr 04, 2008 at 11:08:58AM +0200, Nico Schellingerhout wrote:
> Stefan Sperling <stsp_at_elego.de> wrote on 04/03/2008 06:28:13 PM:
> > I think your example is a good one, because I hadn't thought of this
> > scenario yet. But all that comes to my mind about it is that,
> > when dir1 is deleted, which contains tree conflict victim dir1/b',
> > and the deletion of dir1 itself causes a tree conflict, we can either
> >
> > 1) forget about that dir1/b' is a tree conflict victim,
> > saying that the tree conflict with victim dir1 supersedes it
> >
> > or
> >
> > 2) don't discard any information but record dir1 as
> > an additional tree conflict victim
> I think the second option is better: the user needs to understand that
> the action that led to the conflict is the delete of dir1, but also
> should get hints at what he has to do to fix the conflicts. Therefore,
> a conflict on the modified files/dirs should be raised as well.
>
> Let me see if I can sketch an example, to start off the discussion:
>
> Start on source:
> dir1/
> dir1/a
> dir1/b
> dir1/dir2/
> dir1/dir2/c
>
> Revision to merge:
> D dir1
>
> Start on target:
> dir1/
> dir1/a
> dir1/b'
> dir1/dir2/
> dir1/dir2/c
> dir1/dir2/d
>
> I would like to see the merge to have something like the following result:

> C dir1/ [could not be deleted because of local modifications in b' and dir2]

With "local modifications", you mean both modifications local to the
working copy, and modifications made in the history of the branch the
working copy is based on, which are not present in the source branch
of the merge. Right?

In which case, "dir1 could not be deleted because of local modifications
in b' and dir2" is the same as "dir1 could not be deleted because there
are tree conflicts in the tree rooted at dir1". If we could use such a
recursive definition, the implementation should not be too hard, and
an extra crawl would not be required, since we already have information
about tree conflicts inside a directory in the close_dir() callback
during merge anyway.

Also, I think the output should be:

  C . [dir1 could not be deleted because of local modifications in b' and dir2]
  D dir1/

since the conflict is marked at the parent directory of dir1,
which is the working copy root in this case.

Keep in mind that we never delete files that are tree conflict
victims from disk (at least since r30144), see
http://subversion.tigris.org/issues/show_bug.cgi?id=3149
We just mark them for deletion.

So we could define the same behaviour for directories, which would
be consistent with both the behaviour for files, and your example.
If we did that, dir1 (and all its children) would be marked for
deletion, but dir1 and its children would not be removed from disk.

But I'm not sure yet what happens if we try to mark a tree conflicted
item for deletion. Will the delete bail out?

I think we'd need to do this though -- the current code will
unconditionally mark the whole tree rooted at a directory deleted
during merge for deletion. I don't really want to change that code
unless we absolutely have to. We have changed a lot of code already.
Merging the tree-conflicts branch into trunk will get increasingly
complex if we touch even more code. So if at all possible, let's try
to find a way of dealing with directories that makes as little
changes to the current way of doing things as possible.

> dir1/a
> dir1/b'
> C dir1/dir2/ [could not be deleted because file d is not in the source
> branch]

In this case, "could not be deleted because file d is not in the source
branch" is the same as: "could not be deleted because there is a tree
conflict in the tree rooted at dir2". Is it? If so, the recursive
definition I proposed above would also work here.

> dir1/dir2/c
> dir1/dir2/d
>
> This gives the user the information needed to solve all problems in a
> systematic manner.

Yes.

> > My arguments may well be incorrect as long as I haven't fully understood
> > all problems involved yet. Which I apparently haven't, since I have
> > trouble understanding your point against not doing a crawl :(
> >
> Perhaps the above example helps. The problem is that I do not understand
> the merge algorithm in sufficient detail to predict what would happen,
> and that is why I raised the issue. Given the complexity of merging,
> we're probably both wrong ;)

:)

Well, I hope we can get some comments by people who know more
about merging than we do!

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