On Thu, 2008-10-23 at 16:00 +0100, Julian Foad wrote:
> 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
> else
> if this action will cause a tree conflict:
> record the tree conflict
> skip this action
> else:
> 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.)
Hmm. I've just had a look. I would want the new svn_wc_conflicted_p2()
to be rather different from the old ..._p().
(1) It needs to take an adm_access. It should take this INSTEAD of an
"entry", as it can easily get the entry for itself. There is no need to
make the caller do this extra step. The rules for adm_access should
probably be, "If there is an open adm access baton for path's parent,
then adm_access must be in a set that includes it; and if there is an
open adm access baton for path, then adm_access must be in a set that
includes it." or something like that.
(2) While we're there, we should make all of the output parameters
optional.
(3) The path need not be the path of a version-controlled object.
(4) The old function's path parameter is called "dir_path". Not sure
why. The new one should be called just "path" as it applies to any node
kind.
The old function looks for files on disk as well as what the "entry"
says to determine whether there are text and prop conflicts. I don't
propose changing that now, even though we have no equivalent behaviour
for tree conflicts.
- 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 22:21:45 CEST