Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@subversion.apache.org
Date: Tue, 28 Jul 2015 15:07:39 +0200
From: Stefan Sperling <stsp@elego.de>
To: dev@subversion.apache.org
Cc: "commits@subversion.apache.org" <commits@subversion.apache.org>
Subject: Re: svn commit: r1687906 - in /subversion/trunk/subversion:
 include/svn_client.h libsvn_client/resolved.c
Message-ID: <20150728130739.GC13460@ted.stsp.name>
Mail-Followup-To: dev@subversion.apache.org,
	"commits@subversion.apache.org" <commits@subversion.apache.org>
References: <20150627103542.F0528AC012B@hades.apache.org>
 <558f10a1.82f8c20a.fcd88.4f1e@mx.google.com>
 <558f11e4.cb09b40a.d8a8b.ffff90fc@mx.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <558f11e4.cb09b40a.d8a8b.ffff90fc@mx.google.com>
User-Agent: Mutt/1.5.23 (2014-03-12)

On Sat, Jun 27, 2015 at 09:08:29PM +0000, Bert Huijben wrote:
> > > Author: stsp
> > > Date: Sat Jun 27 10:35:42 2015
> > > New Revision: 1687906
> > > 
> > > URL: http://svn.apache.org/r1687906
> > > Log:
> > > Add a conflict walker to the new svn_client_conflict API.
> > > 
> > > Contrary to our current libsvn_wc conflict walker, this walker guarantees
> > > that new conflicts created during conflict resolution (which can happen
> > > when resolving tree conflicts) are visited as well. The libsvn_wc walker
> > > will visit such new conflicts only if they happen to be created in parts
> > > of the working copy which the walker has not yet visited.
> > > 
> > > * subversion/include/svn_client.h
> > >   (svn_client_conflict_walk_func_t, svn_client_conflict_walk): Declare.
> > > 
> > > * subversion/libsvn_client/resolved.c
> > >   (svn_client_conflict_walk): Implement.
> > > 
> > > Modified:
> > >     subversion/trunk/subversion/include/svn_client.h
> > >     subversion/trunk/subversion/libsvn_client/resolved.c
> >
> > The problem you describe in the conflict walker of libsvn_wc is already fixed in the 1.9 codebase. And if not we should fix it there.
> > 
> > A bigger problem was that it even forgot to report new conflicts deep below the current node, in case the node type changed. But all of that was already fixed before 1.9.0-rc1.
> > (The fixes were part of the big catchup merge, after the initial branch)
> > 
> > The restart of the walk as implemented here was an intermediate version of the libsvn_wc version, but one that didn't catch all of the problems… as new conflicts may also appear outside the tree that is walked for conflicts. Catching new conflicts via the notify handler works much better.
> > 
> > Trying to fix this in libsvn_client is very hard, as you don’t know where to apply the working copy lock (especially on moves). Libsvn_wc has the code to do that, but I don't see a usage of that in your code here.
> > 
> Another reason to do this in libsvn_wc is that we need to retry resolving conflicts, as some conflicts might be in the way of others. The global retry as implemented here also has the side effect of hiding certain error types around these retries. (See changes of the libsvn wc version)
> 
> Just providing the error that we can't resolve everything, while hiding the specific error reason why was one of the errors I found. Specializing the depth empty status walk at least allows access to all the error messages.
> 
> 
> Bert

Ack. Thanks for pointing this out.

So one possible path forward would be to make svn_wc__resolve_conflicts()
expect a new callback type that maps to svn_client_conflict_walk_func_t, with
an option to fall back to the old callback type for backwards compatibility.
Then we can use the existing working copy walker for both APIs and fix any
remaining issues in the libsvn_wc walker there, if any.