On Tue, Mar 2, 2010 at 04:45, Matthew Bentham <mjb67_at_artvps.com> wrote:
> Trying to remove more uses of svn_wc_entry_t, it seems there is no way at
> the moment to get the equivalent of entry->copied using node routines. Is
> this the sort of thing that is needed?
>
> [[[
> wc-ng: work towards eliminating svn_wc_entry_t
>
> * libsvn_wc/node.c, include/private/svn_wc_private.h
> Add svn_wc__node_is_status_copied()
>
> * subversion/libsvn_client/copy.c
> (calculate_target_mergeinfo): Remove unused 'no_repos_access'
> parameter, replace use of svn_wc__get_entry with node routines.
>
> Patch by: Matthew Bentham <mjb67{_AT_}artvps.com>
> ]]]
Hey...
Outside of the issues around the node functions that are being
discussed, I kind of worry about this code. entry->copied does not
truly correspond to status_copied (or status_moved_here). If you look
in entries.c where entry->copied is *set*, then you'll see the logic
is rather ugly and opaque and (from a human brain's standpoint) maybe
even non-deterministic :-P
When walking into this area, it is helpful to step back and ensure
that the merge code is really looking for the right thing. It is
entirely possible it was looking at entry->copied, when it should have
been looking for status_(copied|moved_here).
Because of the fragility around entry->copied, I can't really tell
whether this patch is Right from a simple inspection. I presume that
you've run the full test suite, and it continues to pass with this
change?
Assuming so, then I might request some liberal sprinking of ###
comments to explain that the code might need further tweaking, but
that it seems to work to expectation. That future work on the section
may need additional review to compare against entry->copied's
definition. Something like that.
And thanks! I appreciate the work you're doing to eliminate the
demonspawn known as entry_t!
Cheers,
-g
Received on 2010-03-02 14:06:46 CET