You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2004/12/10 02:52:29 UTC

Re: svn commit: r12241 - trunk/notes/locking

lundblad@tigris.org wrote:

>Author: lundblad
>Date: Wed Dec  8 08:44:45 2004
>New Revision: 12241
>  
>

>+          libsvn_wc also stores the other fields of a lock (owner, comment,
>+          creation-date, expiration-date) in the entries file, so that
>+          they are available for the info command.  Note that this
>+          information is only stored if the current WC has a lock on
>+          the file.
>  
>
Oy, wait a minute. I thought the expiration date was there only to 
support generic DAV clients, and that our client should know nothing 
about it. So why store it in the WC?

>+               When the commit is finished, if the user didn't specify
>+               the --keep-locks option, the client will issue unlock
>+               commans for the files it has locked.  (### Is it
>+               cleaner to have a new ra->get_commit_editor2() with a
>+               flag for this and tlet the server take care of it. It
>+               will avoid a lot of network round-trips in ra_svn,
>+               since the server already got the tokens.)
>  
>
I think the server should clean up in this case. Apart from avoiding 
network round trips, it helps to make the whole thing atomic. It would 
be a bit funny if the commit succeeded, but a subsequent unlock errored 
out because someone happened to break the lock in the meantime.

>          c. Breaking a lock
> 
>+            svn unlock --force will first ask the server for a lock
>+            token and use it in the ra->unlock command to break the
>+            lock.
>  
>
Should look in the WC first if the lock token is there... maybe. This is 
a small optimisation for people who break their own locks. :-)

-- Brane



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 10 Dec 2004, [UTF-8] Branko �^Libej wrote:

> lundblad@tigris.org wrote:
>
> >Author: lundblad
> >Date: Wed Dec  8 08:44:45 2004
> >New Revision: 12241
> >
> >
>
> >+          libsvn_wc also stores the other fields of a lock (owner, comment,
> >+          creation-date, expiration-date) in the entries file, so that
> >+          they are available for the info command.  Note that this
> >+          information is only stored if the current WC has a lock on
> >+          the file.
> >
> >
> Oy, wait a minute. I thought the expiration date was there only to
> support generic DAV clients, and that our client should know nothing
> about it. So why store it in the WC?
>

Good point.  It is always optional, so if we want to add it later, that's
no problem. I'll drop it if no one objects.

> >+               When the commit is finished, if the user didn't specify
> >+               the --keep-locks option, the client will issue unlock
> >+               commans for the files it has locked.  (### Is it
> >+               cleaner to have a new ra->get_commit_editor2() with a
> >+               flag for this and tlet the server take care of it. It
> >+               will avoid a lot of network round-trips in ra_svn,
> >+               since the server already got the tokens.)
> >
> >
> I think the server should clean up in this case. Apart from avoiding
> network round trips, it helps to make the whole thing atomic. It would
> be a bit funny if the commit succeeded, but a subsequent unlock errored
> out because someone happened to break the lock in the meantime.
>
Yes. That was changed later and the new APIs reflect that. It will not be
done atomically on the server, but that's no problem, since we can just
ignore not locked errors. That's a non-error in this situation.


> >+            svn unlock --force will first ask the server for a lock
> >+            token and use it in the ra->unlock command to break the
> >+            lock.
> >
> >
> Should look in the WC first if the lock token is there... maybe. This is
> a small optimisation for people who break their own locks. :-)
>
And a pesimization when the lock token is defunct, in which case we need
to unlock, get-lock, unlock. Breaking ones own valid lock should be rare.
Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r12241 - trunk/notes/locking

Posted by Branko Čibej <br...@xbc.nu>.
Ben Collins-Sussman wrote:

>
> On Dec 9, 2004, at 8:52 PM, Branko Čibej wrote:
>
>>>
>> Oy, wait a minute. I thought the expiration date was there only to 
>> support generic DAV clients, and that our client should know nothing 
>> about it. So why store it in the WC?
>>
>
> The svn_lock_t structure is defined in svn_types.h, available to every 
> library on both sides of the network, and is totally transparent.
>
> We agreed that svn clients wouldn't ever have a UI (or API) to *set* 
> expiration dates -- that 'svn lock' would always create a non-expiring 
> lock.   Only mod_dav_svn would allow generic DAV clients to set 
> expiration times.
>
> Still, I don't see why would should "hide" the expiration_time field 
> of an svn_lock_t from anyone.  It's still an important piece of 
> information, even if not every client can set the value.  (For 
> example, if I run a client command to list all locks in the 
> repository, I would know that any locks with non-zero expiration time 
> were created by DAV clients.)

Possibly.

>>> +               the --keep-locks option, the client will issue unlock
>>> +               commans for the files it has locked.  (### Is it
>>> +               cleaner to have a new ra->get_commit_editor2() with a
>>> +               flag for this and tlet the server take care of it. It
>>> +               will avoid a lot of network round-trips in ra_svn,
>>> +               since the server already got the tokens.)
>>>
>> I think the server should clean up in this case. Apart from avoiding 
>> network round trips, it helps to make the whole thing atomic. It 
>> would be a bit funny if the commit succeeded, but a subsequent unlock 
>> errored out because someone happened to break the lock in the meantime.
>
> +               When the commit is finished, if the user didn't specify
>
> Oh jeez.  So we're talking about create svn_fs_commit_txn2() now, 
> which takes a "free/keep locks" boolean?

I dunno, but if that's what it takes, I think it's not unreasonable. 
You've been munging the FS vtables anyway, haven't you?

-- Brane



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 10 Dec 2004, Ben Collins-Sussman wrote:

>
> On Dec 10, 2004, at 12:29 PM, Ben Collins-Sussman wrote:
> >
> > An svn_lock_t is a transparent object meant to be used on both sides
> > of the network, cached in the client, displayed by the client,
> > presented to users.  I so no reason to create extra complexity to hide
> > a field, just because the user can't change it.  The user can't change
> > lock->creation_date either, right?
> >
>
> I guess what worries me is that 'svn info foo.c' and 'svn info URL'
> might show different fields to describe a lock.  In my idealistic
> world, 'svn info' would always print the entire svn_lock_t.  But that
> would mean caching extra -- essentially useless -- information in
> .svn/entries, such as the expiration_date and absolute_fs_path.
>
Re expiration_date: give me a scenario where that will end up in the
working copy. That might clarify somewhat.

Re the absolute path. Is there really a point in showing it? When will it
be something other than the absolute path of the locked item?

> So which is better or worse?  Does it matter that 'svn info' always
> show the same svn_lock_t fields, regardless of local vs. remote
> operation?
>
They should print the same info as far as possible. But if you look at the
info command as a whole, it will have to print different information
depending whether the information comes from the WC and the repsitory. For
example, there is no schedule in the repo.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by Mark Phippard <Ma...@softlanding.com>.
Ben Collins-Sussman <su...@collab.net> wrote on 12/10/2004 01:53:16 PM:

> 
> On Dec 10, 2004, at 12:29 PM, Ben Collins-Sussman wrote:
> >
> > An svn_lock_t is a transparent object meant to be used on both sides 
> > of the network, cached in the client, displayed by the client, 
> > presented to users.  I so no reason to create extra complexity to hide 

> > a field, just because the user can't change it.  The user can't change 

> > lock->creation_date either, right?
> >
> 
> I guess what worries me is that 'svn info foo.c' and 'svn info URL' 
> might show different fields to describe a lock.  In my idealistic 
> world, 'svn info' would always print the entire svn_lock_t.  But that 
> would mean caching extra -- essentially useless -- information in 
> .svn/entries, such as the expiration_date and absolute_fs_path.
> 
> So which is better or worse?  Does it matter that 'svn info' always 
> show the same svn_lock_t fields, regardless of local vs. remote 
> operation?
> 

If the value will always be the same in a WC, why couldn't svn info just 
"pretend" it has the info and output what it would have said had the info 
been present?

Mark


_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 10, 2004, at 12:29 PM, Ben Collins-Sussman wrote:
>
> An svn_lock_t is a transparent object meant to be used on both sides 
> of the network, cached in the client, displayed by the client, 
> presented to users.  I so no reason to create extra complexity to hide 
> a field, just because the user can't change it.  The user can't change 
> lock->creation_date either, right?
>

I guess what worries me is that 'svn info foo.c' and 'svn info URL' 
might show different fields to describe a lock.  In my idealistic 
world, 'svn info' would always print the entire svn_lock_t.  But that 
would mean caching extra -- essentially useless -- information in 
.svn/entries, such as the expiration_date and absolute_fs_path.

So which is better or worse?  Does it matter that 'svn info' always 
show the same svn_lock_t fields, regardless of local vs. remote 
operation?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 10 Dec 2004, Ben Collins-Sussman wrote:
>
> On Dec 10, 2004, at 12:16 PM, Peter N. Lundblad wrote:
> >
> > Since the client will never set an expiration date and the lock
> > information will only be stored for locks created using the current
> > working copy, what's the point in storing an expiration time that will
> > never be there?
> >
>
> So if I ask my client to tell me about some lock in the repository,
> it's going to deliberately hide that one svn_lock_t field from me?
> Where does this hiding happen?  Does the server not send everything?
> Does the ra_svn/ra_dav library hide something?  Or does libsvn_client
> just not print that one field in svn_lock_t?
>
I thought we wwere talking aobut the WC. Asking for a lock in the repo
will contact the server. If a lock is stored in the WC, it has to have
been created by our client. So, I don't see what information we are
hiding. We just add support for a field that will never be there.

> Similarly, if I have a locktoken in my wc, I would expect the whole
> svn_lock_t to be cached in .svn/ as well.  Or do we only cache *most*
> of it?
>
Shoudl we cache the absolute path as well? When we have the repository
root stored in the entries file,, that would be information duplication,
but before that happens we can't derive it.

 > I guess I feel that the client will definitely be manipulating
> svn_lock_t objects, and so it seems like "extra work" for .svn/entries
> to deliberately store only 'some' of the fields, or for various layers
> to worry about screening things out.  It seems like we're creating
> extra complication for no reason.
>
Check out the current client APIs. They handle svn_lock_t and return it,
but there is nothing that reads a svn_lock_t from the WC. That's because
we don't currently have the absolute path, only the lock token is required
to talk to the server, and "svn info" will have the whole entry anyway.

> "Why so difficult?"
>
If you feel strongly about storing a field that will never be there, then
let's go for it:-)

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 10, 2004, at 12:16 PM, Peter N. Lundblad wrote:
>
> Since the client will never set an expiration date and the lock
> information will only be stored for locks created using the current
> working copy, what's the point in storing an expiration time that will
> never be there?
>

So if I ask my client to tell me about some lock in the repository, 
it's going to deliberately hide that one svn_lock_t field from me?  
Where does this hiding happen?  Does the server not send everything?  
Does the ra_svn/ra_dav library hide something?  Or does libsvn_client 
just not print that one field in svn_lock_t?

Similarly, if I have a locktoken in my wc, I would expect the whole 
svn_lock_t to be cached in .svn/ as well.  Or do we only cache *most* 
of it?

I guess I feel that the client will definitely be manipulating 
svn_lock_t objects, and so it seems like "extra work" for .svn/entries 
to deliberately store only 'some' of the fields, or for various layers 
to worry about screening things out.  It seems like we're creating 
extra complication for no reason.

"Why so difficult?"

An svn_lock_t is a transparent object meant to be used on both sides of 
the network, cached in the client, displayed by the client, presented 
to users.  I so no reason to create extra complexity to hide a field, 
just because the user can't change it.  The user can't change 
lock->creation_date either, right?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12241 - trunk/notes/locking

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 10 Dec 2004, Ben Collins-Sussman wrote:

>
> On Dec 9, 2004, at 8:52 PM, Branko Čibej wrote:
> >>
> > Oy, wait a minute. I thought the expiration date was there only to
> > support generic DAV clients, and that our client should know nothing
> > about it. So why store it in the WC?
> >
>
> The svn_lock_t structure is defined in svn_types.h, available to every
> library on both sides of the network, and is totally transparent.
>
> We agreed that svn clients wouldn't ever have a UI (or API) to *set*
> expiration dates -- that 'svn lock' would always create a non-expiring
> lock.   Only mod_dav_svn would allow generic DAV clients to set
> expiration times.
>
> Still, I don't see why would should "hide" the expiration_time field of
> an svn_lock_t from anyone.  It's still an important piece of

Since the client will never set an expiration date and the lock
information will only be stored for locks created using the current
working copy, what's the point in storing an expiration time that will
never be there?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r12241 - trunk/notes/locking

Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 9, 2004, at 8:52 PM, Branko Čibej wrote:
>>
> Oy, wait a minute. I thought the expiration date was there only to 
> support generic DAV clients, and that our client should know nothing 
> about it. So why store it in the WC?
>

The svn_lock_t structure is defined in svn_types.h, available to every 
library on both sides of the network, and is totally transparent.

We agreed that svn clients wouldn't ever have a UI (or API) to *set* 
expiration dates -- that 'svn lock' would always create a non-expiring 
lock.   Only mod_dav_svn would allow generic DAV clients to set 
expiration times.

Still, I don't see why would should "hide" the expiration_time field of 
an svn_lock_t from anyone.  It's still an important piece of 
information, even if not every client can set the value.  (For example, 
if I run a client command to list all locks in the repository, I would 
know that any locks with non-zero expiration time were created by DAV 
clients.)



>> +               When the commit is finished, if the user didn't 
>> specify
>> +               the --keep-locks option, the client will issue unlock
>> +               commans for the files it has locked.  (### Is it
>> +               cleaner to have a new ra->get_commit_editor2() with a
>> +               flag for this and tlet the server take care of it. It
>> +               will avoid a lot of network round-trips in ra_svn,
>> +               since the server already got the tokens.)
>>
> I think the server should clean up in this case. Apart from avoiding 
> network round trips, it helps to make the whole thing atomic. It would 
> be a bit funny if the commit succeeded, but a subsequent unlock 
> errored out because someone happened to break the lock in the 
> meantime.

Oh jeez.  So we're talking about create svn_fs_commit_txn2() now, which 
takes a "free/keep locks" boolean?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org