On Wed, Feb 17, 2010 at 12:22, Matthew Bentham <mjb67_at_artvps.com> wrote:
>...
> Revised patch attached fixes the unit test (and hopefully the indentation,
> if I've understood the style correctly), but I have no real instinct as to
> whether I should be modifying a libsvn_wc routine in this way. Should I be
> able to implement the client using functionality libsvn_wc already exports?
You "should", yes. However, we aren't quite sure what the new WC API
should look like after we remove entry_t. The "node" functions are
internal for now, so that we can explore and figure out what kinds of
information the client library needs to perform its work. Once we've
removed all of the entry_t uses, then we can review all the node calls
and come up with a new API design for the WC. We'll then go through
and rewrite all of those node calls into uses of the new public API.
Yes, this means we're rewriting the code a couple times, but we just
don't have a good handle on the target API. So that means we gotta do
this iteratively. Thankfully, iterations in small-ish steps is also
quite a bit safer.
> [[[
> wc-ng: work towards eliminating svn_wc_entry_t
>
> * subversion/libsvn_client/commit_util.c
> (add_lock_token): Replace a use of svn_wc__maybe_get_entry with
> use of svn_wc__node_get_*
> * subversion/libsvn_wc/node.c
> (svn_wc__internal_node_get_url): Find the URL of deleted nodes
>
> Patch by: Matthew Bentham <mjb67{_AT_}artvps.com>
> ]]]
The changes to add_lock_token() look fine, but we need something
different for get_url().
Let's step back a moment, and look at what we're really trying to
accomplish. The add_lock_token() function is being called to look for
nodes that have locks associated with them, *in a deleted subtree*.
Further note that locks are associated with repository nodes, not the
nodes in a working copy. This means nodes from the BASE tree rather
than WORKING.
Thus, if a lock token is present, then a BASE node should be present.
That means svn_wc__db_scan_base_repos() will fetch the URL information
for the lock.
It would be nice to just compute the relative/child path and use that,
but it doesn't take into consideration switched subtrees. (tho
deletions across a switch are certainly interesting, and maybe not
even possible...)
I think the right answer is to craft a new node function here.
Something with a signature like:
svn_wc__node_get_lock_info(&lock_token, &url, wc_ctx, local_abspath,
result_pool, scratch_pool);
The implementation would be similar to add_lock_token(): examine the
lock token, then get the URL. In this case, if a lock token is
present, then you can just use scan_base_repos() to get the URL
information. Even better, the code can use base_get_info() (ignore
errors about the node not being present, and return "no lock"!). That
might return the repos info in one shot, without a need to do the
scan.
How does that sound?
Thank you for your patience as we work through this. The code is
definitely getting better!
Cheers,
-g
Received on 2010-02-17 19:41:27 CET