Re: Recursive revert behavior

On 08/31/2010 05:10 AM, Bert Huijben wrote:
>> -----Original Message-----
>> From: Philip Martin [mailto:philip.martin_at_wandisco.com]
>> Sent: dinsdag 31 augustus 2010 10:26
>> To: dev_at_subversion.apache.org
>> Subject: Re: svn commit: r990916 - in
>> /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp
>> ClientContext.h
>>
>> "Bert Huijben" <bert_at_qqmail.nl> writes:
>>
>
> <snip> See original thread
>
>>
>>> The current recursive lock code for multi-db doesn't have stable
>> behavior
>>> over added and removed directories, while the single-db code has. I
>> didn't
>>> feel like reinventing a complete lock store system like the old
>> access
>>> batons just to fix these issues for multi-db. (Single-db has a real
>>> recursive lock, so it only has to delete the lock from the root)
>>
>> The recursive lock is a problem for non-recursive revert as it no
>> longer returns SVN_ERR_WC_NOT_LOCKED, see the XFail in revert_tests.py
>> (and our client explicitly checks for that error). We can fix the
>> recursive behaviour of revert, but can we fix the return value? Is it
>> acceptable to return SVN_ERR_WC_NOT_LOCKED when we have a recursive
>> lock?
>
> This is one of the revert tests if I remember correctly?
>
> I think we have to move these checks in the revert code instead of forcing
> old lock behavior. But I really don't know what we should do here, except
> for maybe revving svn_client_revertX(), to move the old behavior in the
> deprecated wrapper.
>
> For who doesn't know the full story: This is about
> * svn revert root-of-copy-operation
> vs
> * svn revert -R root-of-copy-operation
>
> With the new fully recursive lock behavior in all libsvn_client functions
> this 'svn revert WC-TREE' just works, but it is a big behavior change.
> And in this case the old behavior warned you before performing a lossy
> operation, which was kind of useful.

I think requiring -R for any deep revert makes sense and is the behavior
that users have grown to expect.

But why not at least make 'svn revert COPY-TARGET-DIR' do something useful,
such as revert any propchanges made to the copy target? It can even still
notify with a warning that the reversion wasn't deep, suggesting the
--recursive flag as we do today.

In other words, where today we see:

   $ svn copy A A-copy
   $ svn pset foo bar A-copy
   $ svn revert A-copy
   svn: Try 'svn revert --depth infinity' instead?
   subversion/libsvn_wc/lock.c:952: (apr_err=155005)
   svn: Unable to lock 'A-copy/B'
   $

We might see:

   $ svn copy A A-copy
   $ svn pset foo bar A-copy
   $ svn revert A-copy
   Reverted property changes for 'Z'
   svn: warning: Directory 'Z' contains additional changes; use 'svn \
   revert --depth infinity' to revert the whole tree.
   $

Just a thought.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand