[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [Fwd: svn commit: r33855 - in branches/tree-conflicts-notify/subversion: include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest]

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 23 Oct 2008 16:00:16 +0100

Neels J. Hofmeyr wrote:
> >>> > I suggest we un-deprecate the original svn_wc_conflicted_p() and use
> >>> > it for reading text and prop conflict state only. And use the
> >>> existing
> >>> > svn_wc_get_tree_conflict() to read tree conflict state.
> >
> > Any arguments against this?
> Yes, semi. I agree with taking away the has_tree_conflicted_children_p
> parameter, but I would endorse keeping the tree_conflicted_p. For the simple
> reason that anyone outside of the callbacks functions calls
> svn_wc_conflicted_p.*() to see whether the node has a conflict that should
> prevent it from changing something.

OK, you're saying keep svn_wc_conflicted_p2() but change its
tree_conflict output to report on the path as a victim:

  svn_error_t *
  svn_wc_conflicted_p2(svn_boolean_t *path_is_text_conflicted,
                       svn_boolean_t *path_is_prop_conflicted,
                       svn_boolean_t *path_is_tree_conflicted,
                       path, entry, pool);

> And I suddenly grasp this: Even in the
> callbacks, tree_conflicted_p is useful to report any tree-conflicts that
> have been around before the current operation was launched.

Yes. Each part of the up/sw/merge operation is trying to say:

  if target was already in conflict:
    skip this action
  if this action will cause a tree conflict:
    record the tree conflict
    skip this action
    do this action

> So we need both
> the return value of check_tree_conflict() *and* tree_conflicted_p from
> svn_wc_conflicted_p2(), right? (Unless check_tree_conflict() already covers
> persistence of older conflicts, and then this point still holds true for use
> outside the callbacks.)

check_tree_conflict() doesn't detect pre-existing conflict, it only
detects whether the current action is causing a conflict, so yes we need
it and also (logically before it) a check for pre-existing conflicts.

But Steve's point was that to check for pre-existing conflicts we have
the API svn_wc_get_tree_conflict() to ask if a path is the victim of a
tree conflict. We can use svn_wc_get_tree_conflict() together with
svn_wc_conflicted_p() to answer the same question as your proposed
svn_wc_conflicted_p2() will do.

When svn_wc_conflicted_p2() was ansering "does this path have any
tree-conflicted children", I thought it was better to separate out that
question into a separate API, but now that we can answer all three of
  * is there a text conflict on this path?
  * is there a property conflict on this path?
  * is there a tree conflict on this path?

with one svn_wc_conflicted_p2() API, it seems neat and useful to do so.
I'm not entirely convinced it's the most useful API in the world: most
callers of it simply want to know whether there are any conflicts on the
path, and don't care what they are. But it's OK. Go for it. (First step:
stop using the current svn_wc_conflicted_p2()'s
contains-tree-conflicted-children parameter.)

- Julian

To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-23 17:00:47 CEST

This is an archived mail posted to the Subversion Dev mailing list.