Reporting subtree deletes through svn_delta_editor_t?

[Sorry, long message. Summary: I'd like to change the editor interface
to inline reports of deleted subtrees, if that makes sense.]

Hello all,

I notice that Garrett has changed the svn_delta_editor_t interface in
the atomic renames branch, which has reminded me about something I've
been toying with for a while.

Issue #2333 is about my favourite topic: svn diff. Specifically,
reporting diffs against a repository revision for deleted directories.

When we do a wc-repos diff, we effectively request that BASE be updated
to the target repos revision, and use the editor to drive our diff output.
Repos-repos diffs are similar.

The problem occurs when we see a deleted directory, because we don't
get any detail about what the contents of that directory were. So if
we have a file /A/B/x and we delete /A/B, we'll only see the following
calls to the editor:

open_directory A
  delete_entry B
close_directory A

There are effectively two ways to handle this that I can see.

The first is to pause whenever we see a delete_entry and separately
traverse the deleted subtree. I've already added code for repos->WORKING
diffs to do this (which is easy because we need to traverse the local wc),
but it's fragile and doesn't work for repos-BASE or WORKING->repos diffs
(where we'd need to contact the repository).

Also note that if we did this for repos-repos diffs, we'd need to use at
least three connections: one to receive the report request, a second to
traverse the deleted subtrees, and a third to fetch the contents of files.

The other way is to inline the deletions into the editor. What I've
played around with is adjusting the interface to recurse into deleted
subtrees, before deleting them. The above example then looks like this:

open_directory A
  open_directory B
    delete_entry x
  close_directory B
  delete_entry B
close_directory A

This works really quite well, and I can imagine other consumers of the
editor interface being interested in the information, so I don't see it
as a hack just for diff.

Doing this fixes simple repos-repos diffs and wc-repos diffs immediately,
because the consumers now only need to deal with file deletes, and they
all already do. I _think_ it might even be allowed by the spirit of
the existing interface as well.

But there's a problem that I can't fix, apart from the hackiness:
Currently, in a repos-WORKING diff, we wait until we see a close_directory
and then run through the wc outputting local modifications.

The problem is that if we have schedule-add files in a to-be-deleted
directory, these should be reported as deleted, but we don't know that
the directory is deleted until _after_ we close it. In other words,
we can't tell the difference between the deletion of /A/B itself, and
the deletion of all its contents, until it's too late.

So what I'd like to do is change the editor interface so that the caller
can request something like the following:

open_directory A
  open_deleted_directory B
    delete_entry x
  close_deleted_directory B
close_directory A

Or in other words, optionally inline the deleted subtree at the point
of deletion.

What do people think?

Regards,
Malcolm

PS I've attached my current patch and reproduction recipe, if anyone's
interested.

[[[
Hackily force 'deep deletes' (recursion into deleted directories).

* libsvn_repos/reporter.c
  (update_entry): recurse into deleted directories if the deep_deletes
    option is set.
  (delta_dirs): support a NULL target as well as a NULL source.
    Call update_entry() to delete deleted entries as well as replaced
    entries, rather than calling the delete_entry callback directly.

* libsvn_wc/diff.c (delete_entry): disable the wc traversal for
    repos->WORKING diffs if we already recursed into the deleted directory.
]]]

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org