wc-ng code organisation

I've been looking at a delete bug: delete can remove a directory NODES
row and leave rows for children, thus creating an invalid metadata
state. The code is structured as

     svn_wc_delete4(path):
        various checks on path
        for each child of path:
           various checks on child
           svn_wc_delete4(child)
        svn_wc__db_temp_op_delete(path)
        notify
        cleanup

The recursion happens in svn_wc_delete4 while svn_wc__db_temp_op_delete
is where the database is modifed (non-recursively). I can fix the bug
by modify the checks in svn_wc_delete4 but I think this is the wrong way
to do it. I wrote svn_wc__db_temp_op_delete and the _temp_ part is
simply because I didn't really know how it should work.

Our metadata API should be responsible for maintaining the implied
database invariants. Thus the svn_wc__db_op_delete needs to do the
checks to ensure that children don't become orphans. It either needs to
reject such a change, or it needs to be recursive and remove the
children.

Making the checks in svn_wc__db_op_delete also solves a race. At
present the checks made by svn_wc_delete4 are only protected by a
wclock, putting them in svn_wc__db_op_delete means they can go inside
the same SQLite transaction that modifies the database.

Once svn_wc__db_op_delete is making the checks there is very little
point in svn_wc_delete4 making the same checks, and a good performance
reason for only making the checks once. It's probably more efficient to
move the recursion into svn_wc__db_op_delete as well and doing so also
solves the problem that currently op_depth needs to be rewritten as each
parent gets deleted.

That all seems to be good until we come to notification and workqueues.
svn_wc__db_op_delete can't really do notification as it would be making
callbacks from within an SQLite transaction. So svn_wc_delete4 must
continue to notify, and as it can't reliably identify which paths were
deleted svn_wc__db_op_delete must return a list of affected paths.

Delete doesn't currently use a workqueue. It could, and perhaps it
should, although I'm not sure it's strictly necessary. If it were to
use a workqueue then the function that modifies the metadata,
svn_wc__db_op_delete, should install any wq items (one per path or one
per tree).

So the delete code ends up looking like

    svn_wc_delete4(path):
      deleted = svn_wc__db_op_delete(path)
      for name in deleted:
        notify(name)
      run workqueue
      cleanup

Does that sound sensible?

Finally, while this mail refers to delete there are similar problems in
other functions: revert, copy, etc.

-- 
Philip