RE: svn commit: r39153 - in trunk/subversion: libsvn_wc tests/cmdline

> -----Original Message-----
> From: Bert Huijben [mailto:[email protected]]
> Sent: zaterdag 5 september 2009 21:51
> To: dev-lmwclWVctOZK/[email protected]; svn-lmwclWVctOZK/[email protected]
> Subject: RE: svn commit: r39153 - in trunk/subversion: libsvn_wc
> tests/cmdline
> 
> > -----Original Message-----
> > From: Hyrum K. Wright [mailto:[email protected]]
> > Sent: zaterdag 5 september 2009 20:27
> > To: svn-lmwclWVctOZK/[email protected]
> > Subject: svn commit: r39153 - in trunk/subversion: libsvn_wc
> > tests/cmdline
> >
> > Author: hwright
> > Date: Sat Sep  5 11:26:48 2009
> > New Revision: 39153
> >
> > Log:
> > When opening access batons, recursively lock all its subdirectories
> to
> > an
> > infinite depth as well.  In wc-ng, we don't have the notion of
> > recursively
> > opening a database; it's either open or it's not.  By making this
> > change now,
> > we can remove the existing complexity of determining how many levels
> to
> > lock
> > and where to lock them.
> 
> Opening a database doesn't have to be the same as locking everything
> contained in it. It is completely possible to support other models with
> a
> single SQLite database and properly designed transactions.
> 
> > We also gain another advantage.  When attempting to open a baton, we
> > can check
> > and see if that baton is already opened, and just return it if it is.
> 
> This is not an advantage. Just returning it will make the new 'owner'
> close
> the access baton that is really owned by some other part of our code!!!
> This just introduces another kind of bugs to search for (and breaks
> backwards compatibility with old code that assumed you couldn't open an
> access baton twice!)
> 
> > By
> > always recursively locking, we can guarantee that a found baton is
> also
> > recursively locked, which is what the original caller may have
> > intended.
> > All this simplifies our current access baton usage, and the path to
> > removal.
> 
> I think we should handle this always recursive locking at the caller
> side
> (with a new function called by svn_client_*()), or just calling
> svn_wc__adm_open_in_context with depth -1; not deeply buried inside the
> access baton implementation.
> 
> I'm 99.9% sure the 1.6 client would crash on this new behavior, so this
> would be a breaking change and would probably require switching to
> developing a 2.0.

I'm guessing that the only reason this does not crash the client is that
r39150,r39151 made the last few open locations that didn't use the shared
context now do use the shared context. (Which made your patch unnecessary)

	Bert
> 
> > Oh, and it incidentally fixes copy test 76.
> 
> Yes, this tested for a too shallow lock.
> 
> 	Bert
> 
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=2391335

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2391337