On Thu, 2008-10-23 at 21:21 +0100, Julian Foad wrote:
> 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.
(4) OK, I see why it's called "dir_path": (name-of-a-directory,
entry-from-the-directory) is one way of identifying an item in the WC.
We need to change (1) and (4) together, so that the new interface is:
(name-of-an-item, adm-access-for-the-item). This achieves (3).
> 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-24 01:05:59 CEST