Julian Foad wrote:
> These functions:
> need some "consistifying".
> * adm_access
> I have come up against difficulties with WC APIs that need to support
> tree conflict (TC) victims. Such victims are not always versioned and so
> do not always have the expected adm_access available. Also, the
> implementation stores TCs in the parent dir's entry, so that's the place
> we really need the baton for. Arguably it's more convenient for callers
> if they don't have to know that and can supply the victim's adm_access
> if they already have it. So we could choose either (the strict one)
> - "adm_access is the access baton for the parent dir of the victim.
> (If there is no versioned parent, a tree conflict cannot be
> or (the lenient one):
> - "adm_access is an access baton in a set that contains an access
> baton for the parent. (If there is no versioned parent, a tree conflict
> cannot be represented.)"
> I'm going for the latter.
> A "get" function could be even more lenient: if no access baton is
> supplied, get one itself. That is how it is currently implemented, and
> it can save the caller some effort. In one sense, it saves the caller
> from needing to know that the parent dir is involved, but in another
> sense it's making too many assumptions. So I think the get function
> should be as strict as the add/del functions.
I think callers have to be aware of the access batons anyway, and IMHO it's
not asking too much from callers to always provide an access baton. It might
make some code clearer.
> * Public or private?
> The first three, the non-loggy ones, should all have the same level of
> publicity, or at the very least "add" and "del" should be the same.
> I'm going to make them all private because the very question of changing
> them today indicates that they're not stable.
> Objections? Comments?
> - Julian
> Examples of the change to 'adm_access' definition:
> Index: subversion/include/svn_wc.h
> @@ svn_wc_resolved_conflict4():
> - * @a adm_access is an access baton, with a write lock, for @a path.
> + * @a adm_access is an access baton in a set that includes a write lock for
> + * @a path (unless @a path is an unversioned victim of a tree conflict) and
> + * a write lock for @a path's parent directory (if available). If a lock on
> + * the parent is not provided, any tree conflict of which @a path itself is
> + * the victim will not be resolved.
I think encountering one lock failure on a tree-conflict bails out and
doesn't go on trying to resolve other provided targets. That would have to
be changed and/or documented.
> @@ svn_wc_get_tree_conflict():
> /** Set @a *tree_conflict to a newly allocated @c
> * svn_wc_conflict_description_t structure describing the tree
> * conflict state of @a victim_path, or to @c NULL if @a victim_path
> - * is not in a state of tree conflict. @a adm_access is the admin
> - * access baton for @a victim_path. Use @a pool for all allocations.
> + * is not in a state of tree conflict.
> + *
> + * @a adm_access should be an admin access baton in a set that includes @a
> + * victim_path's parent directory. (If there is no versioned parent, a tree
> + * conflict cannot be represented.)
> + * (### Actually, implementation tries to get the parent access baton for
> + * itself if not found in the set.)
> + *
> + * Use @a pool for all allocations.
> * @since New in 1.6.
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-12 02:44:41 CET