Help needed understanding reporter.c's delete_entry() calculation of base revisions

I'm looking off and on at issue #3324[1] (Path moved by merge but source not
deleted). Looking at Paul's initial solution (which he says breaks more
things than it fixes) has drawn my eye to an area of code that's changed a
bit since I last looked at it. The code in question is the delta driver in
libsvn_repos/reporter.c, which has grown at some point in the past the
ability to transmit a base-revision with the editor->delete_entry() call. I
*think* this was added when Paul was doing the detailed out-of-date
information tweaks for 'svn status'. (So, not a recent change.)

However, now that I'm reading the code, I find I have some questions. For
example, starting in reporter.c:798, we see:

  /* If there's a source and it's not related to the target, nuke it. */
  if (s_entry && !related)
    {
      svn_revnum_t deleted_rev;

      SVN_ERR(svn_repos_deleted_rev(svn_fs_root_fs(b->t_root), t_path,
                                    s_rev, b->t_rev, &deleted_rev,
                                    pool));
      SVN_ERR(b->editor->delete_entry(e_path, deleted_rev, dir_baton,
                                      pool));
      s_path = NULL;
    }

At first glance, it makes sense. If there's something in the source tree
that's not the same object as is in the target, delete the one (and then
later, out of the above context, add the other). (By the way, related may
also be TRUE if the "ignore_ancestry" option is set for this operation, not
because things really are related, but because the code should behave as if
they are.) Anyway, what I don't really understand is the use of the
svn_repos_delete_rev() function, and the passing of its return value through
delete_entry(). Here's why: if we know the source and target aren't
related at all, and we're merely manufacturing a deletion in the editor
drive, then why would we expect to find a real deletion of the target in the
repository?

Actually, we can even ask similar questions at a higher level, too. This
code is used to transform trees not just for 'svn update', but also for 'svn
switch', where the source and target trees might be two different branches.
 What does it even mean to try to identify a deletion-rev in such a
scenario? How do you bound the search?

It looks to me like the source revision and target revision are the bounds,
which makes total sense if the source and target trees are the same tree at
different snapshots, such as with 'svn update'. But what about 'svn
switch', where the source might be /trunk in r1234, and the target
/branches/foo in r1234. You might still need to delete something to make
that transformation, but with a revision range bounding of 1234:1234, you
aren't gonna find a real deleted-rev. And again, what is the semantic
meaning of this anyway? I can certainly construct an algorithm for finding
this answer (it involves searching backward from source to common-ancestor
for the addition of the missing thing and, failing that, searching forward
from common-ancestor to target for its deletion). But as this code can also
be used to compare completely unrelated trees, even that isn't a foolproof
deleted-rev search mechanism.

I'm concerned as I look at this code that it is well-intended, but
attempting to answer questions that in many cases have no answers, or at the
very least isn't doing enough to discern those answers where they exist.
And I'm further concerned that code originally added for a bit of reporting
goodness ('svn status' out-of-dateness stuff) is now being used more
operationally as part of 'svn merge' to unpleasant ends in certain cases.

Paul, can you help me understand this better? Is there something key that
I'm missing?

[1] http://subversion.tigris.org/issues/show_bug.cgi?id=3324

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=980157