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

Re: [PATCH] Tree conflicts - revert and resolve per victim

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 05 Nov 2008 23:24:48 +0000

On Wed, 2008-11-05 at 21:18 +0100, Stephen Butler wrote:
> Quoting Julian Foad <julianfoad_at_btopenworld.com>:
>
> >
> > Yes, I am. I'm implementing an augmented variant of walk_entries now,
> > and then I'll make "resolve" and "revert" and "info" and "status" use
> > it.
>
> That's interesting. I hadn't thought that status could be done
> using walk_entries, but if walk_entries is augmented for nonexistent
> items, then status might as well use callbacks like the other
> commands.

Well, I haven't actually checked that "status" doesn't do special things
that would make it impossible. It does do special things like report
status of unversioned items. I'll see when I get to it whether it makes
sense to use a generic (or fairly generic but somewhat more
specialised!) walker.

Actually I probably won't even try to touch it in this round of work. It
works as it is. It can be tidied up at some unknown time in the future.

> >> I'm working on svn resolve (your last item) currently. I
> >> assume you're attacking this list from the top down.
> >
> > Steve, could you describe what you're aiming to make happen?
>
> I'm trying to implement all of the resolve --accept options that
> we outlined via email earlier in the week. I have a handy little
> function that checks for and removes a tree conflict in one call.

That's great.

> /** If the item at @a victim_path is tree-conflicted, remove the tree
> * conflict state immediately and set @a was_present * to @c TRUE. If
> * the item has no tree conflict, set @a *was_present to @c FALSE. @a
> * adm_access is an admin access baton that can lock @a victim_path.

The wording "that can lock @a victim_path" is odd. May I suggest "in a
set that contains a write lock for @a victim_path".

> * Use @a pool for all allocations.
> *
> * @since New in 1.6.
> */
> svn_error_t *
> svn_wc_remove_tree_conflict(svn_boolean_t *was_present,
> const char *victim_path,
> svn_wc_adm_access_t *adm_access,
> apr_pool_t *pool);
>
> It's a counterpart of the {get, add} functions we already have.
> Note that it's not loggy, because resolve acts upon each conflicted
> item immediately, IIUC.

It's like my recently committed "svn_wc__del_tree_conflict()", except
with the "was_present" parameter. You might as well add "was_present" as
an optional output parameter (can be null) to the
svn_wc__del_tree_conflict() function. The callers of my function both
want that parameter and could be simplified with it.

You can also make the function public like you're proposing here.
Logically, if "add" is public then "del"/"remove" should be public too.
It seems to me just a quirk of implementation that "resolve" and
"revert" are currently implemented wholly within the WC.

> Since I hadn't figured out a solution for the recursive case, as
> you have, I haven't committed this yet. Though I suppose I can
> commit this API function alone.

- 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-11-06 00:25:08 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.