You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Noorul Islam K M <no...@collab.net> on 2011/07/15 14:58:17 UTC

[PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Noorul Islam K M <no...@collab.net> writes:

> From: http://subversion.tigris.org/issues/show_bug.cgi?id=3942
> =======================================================================
> The svnadmin command currently allows you to list and remove locks.  It
> would be useful if you could also create locks.  Suggested syntax would
> be something like:
>
> svnadmin lock REPOS PATH USERNAME COMMENT-FILE TOKEN
>
> Where COMMENT-FILE is handled similar to svnadmin setrevprop TOKEN would
> be optional.  If not provided a lock token would be generated.
>
> The reason I would like this subcommand is that it would make it
> possible to build a system for synchronizing locks between a master and
> slave.  A post-lock hook on master could send this info to slaves and
> they could use it to create the lock on the slaves (with the same lock
> token).  It seems like it would be possible for someone to use this
> command to store lock information along with a dumpfile and recreate
> them after restoring repository from a dump.  That is not my goal, but I
> think it is another possible usage.
>
> Buddied by: cmpilato
> ========================================================================
>
> I would like to work on this enhancement. If someone else is already
> working on this please let me know.
>

As first step I implemented the following syntax.

svnadmin lock REPOS PATH USERNAME COMMENT-FILE

I will add the optional TOKEN argument later. I hope I am progressing in
the right direction.

Log
[[[

Fix issue #3942. Add new sub command 'lock' for 'svnadmin'. The
following syntax is implemented in this patch.

svnadmin lock REPOS PATH USERNAME COMMENT-FILE

The command locks the PATH by USERNAME setting comment from
COMMENT-FILE.

The optional TOKEN argument mentioned in issue tracker will be
implemented in another patch.

* subversion/svnadmin/main.c
  (svn_opt_subcommand_t): New sub command.
  (svn_opt_subcommand_desc2_t): Add description.
  (subcommand_lock): Implement it.

* subversion/tests/cmdline/svnadmin_tests.py
  (lock): New test for 'lock' sub command.
  (test_list): Add new test.

Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]

Thanks and Regards
Noorul


RE: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Julian Foad <ju...@wandisco.com>.
Noorul Islam K M wrote:
> +  SVN_ERR(svn_stringbuf_from_file2(&file_contents, comment_file_name, pool));
> +  comment = file_contents->data;
> +
> +  /* Enforce that the comment be xml-escapable. */
> +  if (comment)

There doesn't seem to be any way that 'comment' could be null in your
current patch, since svn_stringbuf_from_file2() promises to return "a
string".  (But maybe you will make the comment optional in a subsequent
patch.)

> +    {
> +      if (! svn_xml_is_xml_safe(comment, strlen(comment)))

Instead of 'comment' and 'strlen(comment)' you could use
file_contents->data and file_contents->len.  That would also catch the
case where there's a null character in the file.

> +        return svn_error_create
> +          (SVN_ERR_XML_UNESCAPABLE_DATA, NULL,
> +           _("Lock comment contains illegal characters"));
> +    }
> +
> +  SVN_ERR(svn_fs_youngest_rev(&revnum, fs, subpool));  

Don't get the youngest revnum ...

> +  SVN_ERR(svn_utf_cstring_to_utf8(&lock_path_utf8, lock_path, subpool));
> +  
> +  SVN_ERR(svn_repos_fs_lock(&lock, repos, lock_path_utf8,
> +                            NULL,    /* token */
> +                            comment,
> +                            0,       /* is_dav_comment */
> +                            0,       /* No expiration time. */
> +                            revnum,

... because that will only make this call fail if there's a commit
happening at the same time, where otherwise this would succeed.  Just
pass SVN_INVALID_REVNUM.

> +                            FALSE, subpool));


- Julian



RE: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Noorul Islam K M [mailto:noorul@collab.net]
> Sent: vrijdag 15 juli 2011 14:58
> To: Subversion
> Subject: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to
> create a lock
> 
> Noorul Islam K M <no...@collab.net> writes:
> 
> > From: http://subversion.tigris.org/issues/show_bug.cgi?id=3942
> >
> ==========================================================
> =============
> > The svnadmin command currently allows you to list and remove locks.  It
> > would be useful if you could also create locks.  Suggested syntax would
> > be something like:
> >
> > svnadmin lock REPOS PATH USERNAME COMMENT-FILE TOKEN
> >
> > Where COMMENT-FILE is handled similar to svnadmin setrevprop TOKEN
> would
> > be optional.  If not provided a lock token would be generated.
> >
> > The reason I would like this subcommand is that it would make it
> > possible to build a system for synchronizing locks between a master and
> > slave.  A post-lock hook on master could send this info to slaves and
> > they could use it to create the lock on the slaves (with the same lock
> > token).  It seems like it would be possible for someone to use this
> > command to store lock information along with a dumpfile and recreate
> > them after restoring repository from a dump.  That is not my goal, but I
> > think it is another possible usage.
> >
> > Buddied by: cmpilato
> >
> ==========================================================
> ==============
> >
> > I would like to work on this enhancement. If someone else is already
> > working on this please let me know.
> >
> 
> As first step I implemented the following syntax.
> 
> svnadmin lock REPOS PATH USERNAME COMMENT-FILE
> 
> I will add the optional TOKEN argument later. I hope I am progressing in
> the right direction.

For your original usecase you really need the lock token.

If the lock token in the master and slave repository don't match you will
get reports about the lock being stolen.

The username and comment are only informational for users. The only data
that proves you have the lock to Subversion is this lock token.
(Of course hook scripts are more likely to check the username, but
Subversion and Webdav only care about the token)

	Bert


RE: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: vrijdag 15 juli 2011 17:35
> To: Bert Huijben
> Cc: 'Noorul Islam K M'; 'Subversion'; jmountifield@tigris.org
> Subject: Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to
> create a lock
> 
> Where are you quoting that from?  I don't see it on the issue.

http://subversion.tigris.org/issues/show_bug.cgi?id=3457

	Bert


Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by James Mountifield <jm...@collab.net>.

On 15/07/11 16:35, Daniel Shahaf wrote:
> Where are you quoting that from?  I don't see it on the issue.

My comment was against issue 3457 
(http://subversion.tigris.org/issues/show_bug.cgi?id=3457)

>
> jmountifield: svnadmin is not a client; in 'load' it interacts directly
> with the underlying filesystem, while bypassing all the other layers
> (including hooks).  The locks are a first-class object in the
> filesystem, though, rather than some artifact of higher layers.

Yes, that's kind of the point I think I was trying to make, I think ... :)

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Fri, Jul 15, 2011 at 18:35:16 +0300:
> Where are you quoting that from?  I don't see it on the issue.
> 
> jmountifield: svnadmin is not a client; in 'load' it interacts directly

Never mind, I misread you, sorry.

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Where are you quoting that from?  I don't see it on the issue.

jmountifield: svnadmin is not a client; in 'load' it interacts directly
with the underlying filesystem, while bypassing all the other layers
(including hooks).  The locks are a first-class object in the
filesystem, though, rather than some artifact of higher layers.

[ Bert, if you haven't, could you please point to this thread from the
issue where jmountifield's comment is on? Thanks ]

Bert Huijben wrote on Fri, Jul 15, 2011 at 17:20:20 +0200:
> 
> 
> > -----Original Message-----
> > From: C. Michael Pilato [mailto:cmpilato@collab.net]
> > Sent: vrijdag 15 juli 2011 17:04
> > To: Noorul Islam K M
> > Cc: Subversion
> > Subject: Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to
> > create a lock
> > 
> > On 07/15/2011 08:58 AM, Noorul Islam K M wrote:
> > > As first step I implemented the following syntax.
> > >
> > > svnadmin lock REPOS PATH USERNAME COMMENT-FILE
> > >
> > > I will add the optional TOKEN argument later. I hope I am progressing in
> > > the right direction.
> > 
> > I've not reviewed this work, but you might also consider allowing this
> > subcommand to accept --bypass-hooks, too.
> 
> To keep the information on this thread I quote a bit of information that was just added to this issue:
> 
> ------- Additional comments from jmountifield@tigris.org Fri Jul 15 07:01:25 -0700 2011 -------
> 
> Replicating the locks to the slave server, even with the right lock token value,
> still leaves us with some issues in the master/slave configuration. The
> following is based on local testing with svn, version 1.6.12 (r955767).
> 
> Once a slave repository has locks present you cannot keep that slave up to date.
> This is true with both `svnsync sync` and `svnadmin load`.
> 
> With svnsync the reasons are somewhat obvious. The process looks like a normal
> commit, and commits are blocked by locks. So, unless there's some way to change
> the locking behaviour for svnsync specifically, I can't see a workaround here.
> 
> More surprisingly the same is true with `svnadmin load`. This strikes me as
> somewhat odd as these locks are definitely client layer, and svnadmin isn't a
> client, right?. Why should svnadmin load respect the locks?
> 
> When trying to `svnadmin load` into a repository that has a lock on a path
> touched by that load I get the following message:
> 
> mounty@laptop$ svnadmin load /tmp/lock-test < /tmp/lock-test.5.dmp 
> <<< Started new transaction, based on original revision 5
>      * editing path : trunk/foo.c ... done.
> svnadmin: Cannot verify lock on path '/trunk/foo.c'; no username available
> 
> 
> So, to sum up; If we replicate locks to the slave in the scenario described
> above, we effectively make it impossible to keep the slave in sync with the
> master, using the current options available.
> 
> Others have also asked that the svn dump format be able to capture lock tokens.
> In this scenario `svnadmin load` will need to be able to operate around the
> locks to enable loading of incremental dump files, where one or more dump files
> describe a lock token.
> 
> -----------------------------------------------
> 	Bert
> 
> 

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Jul 15, 2011 at 1:17 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 07/15/2011 12:16 PM, Daniel Shahaf wrote:
>> If svnsync presents the lock token that's present on the master, and
>> hopefully the slave checks locks against the master during commit, the
>> commit is going to work isn't it?
>
> I considered this, but the FS code checks not only the lock tokens, but also
> that they match their registered owners.  The problem is that it checks
> against but a single owner -- the authenticated username associated with the
> committing process.
>
> Alternatively, I've considered a mod_dav_svn directive (say,
> "SVNDisableTxnLockCheck on") which could be set on the slave to indicate
> that when the slave calls svn_fs_begin_txn2(), it does so without the
> SVN_FS_TXN_CHECK_LOCKS flag, effectively disabling on-the-fly lock checks.
> I'm not sure if that's sufficient to disable *all* lock checks, though.
> Haven't gotten that deeply into the code yet.

In my case, I will be running svnsync against file:// URL.  So I would
need a command line option ... or something.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Fri, Jul 15, 2011 at 13:17:13 -0400:
> On 07/15/2011 12:16 PM, Daniel Shahaf wrote:
> > If svnsync presents the lock token that's present on the master, and
> > hopefully the slave checks locks against the master during commit, the
> > commit is going to work isn't it?
> 
> I considered this, but the FS code checks not only the lock tokens, but also
> that they match their registered owners.  The problem is that it checks
> against but a single owner -- the authenticated username associated with the
> committing process.
> 
> Alternatively, I've considered a mod_dav_svn directive (say,
> "SVNDisableTxnLockCheck on") which could be set on the slave to indicate
> that when the slave calls svn_fs_begin_txn2(), it does so without the
> SVN_FS_TXN_CHECK_LOCKS flag, effectively disabling on-the-fly lock checks.
> I'm not sure if that's sufficient to disable *all* lock checks, though.
> Haven't gotten that deeply into the code yet.

Two thoughts:

* 'svnadmin load' could learn to not pass the flag you mention,
  in order to address one of James's issues.

* Perhaps we need a boolean configuration option for the FS library
  saying "You are the slave/mirror/replica of something else, and
  higher layers are handling the mirroring"?

  This will enable the FS understand the locks it has are advisory, not
  normative; and will also play to Mark's point of "writing to the
  slave's FS via a non-mod_dav_svn RA layer".

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/15/2011 12:16 PM, Daniel Shahaf wrote:
> If svnsync presents the lock token that's present on the master, and
> hopefully the slave checks locks against the master during commit, the
> commit is going to work isn't it?

I considered this, but the FS code checks not only the lock tokens, but also
that they match their registered owners.  The problem is that it checks
against but a single owner -- the authenticated username associated with the
committing process.

Alternatively, I've considered a mod_dav_svn directive (say,
"SVNDisableTxnLockCheck on") which could be set on the slave to indicate
that when the slave calls svn_fs_begin_txn2(), it does so without the
SVN_FS_TXN_CHECK_LOCKS flag, effectively disabling on-the-fly lock checks.
I'm not sure if that's sufficient to disable *all* lock checks, though.
Haven't gotten that deeply into the code yet.

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


RE: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Mark Phippard [mailto:markphip@gmail.com]
> Sent: vrijdag 15 juli 2011 18:20
> To: Daniel Shahaf
> Cc: Bert Huijben; Noorul Islam K M; Subversion
> Subject: Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to
> create a lock
> 
> On Fri, Jul 15, 2011 at 12:16 PM, Daniel Shahaf <d....@daniel.shahaf.name>
> wrote:
> 
> > If svnsync presents the lock token that's present on the master, and
> > hopefully the slave checks locks against the master during commit, the
> > commit is going to work isn't it?
> 
> Apache proxies commits back to the master.  When you are using svnsync
> to update the replica it is not going through that proxy.  In my case
> it using file://, but in the book it uses a special Apache location
> that does not proxy.  So no, it does not check against the master.
> 
> > Point is: using 'svnadmin lock', despite creating locks on the slave's
> > FS, doesn't change the fact that those locks are *cached* locks, they
> > aren't binding/normative in any way.
> 
> That would be the ideal but it is not true today.  I also do not think
> svnsync even presents a lock token since there is no working copy
> involved to provide the token.

I don't think it is hard to implement to present the lock tokens via svnsync
as we can just retrieve them from the master (or even the slave) via the
existing ra api when syncing, but retrieving the tokens introduces some race
conditions

	Bert


Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Jul 15, 2011 at 12:16 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:

> If svnsync presents the lock token that's present on the master, and
> hopefully the slave checks locks against the master during commit, the
> commit is going to work isn't it?

Apache proxies commits back to the master.  When you are using svnsync
to update the replica it is not going through that proxy.  In my case
it using file://, but in the book it uses a special Apache location
that does not proxy.  So no, it does not check against the master.

> Point is: using 'svnadmin lock', despite creating locks on the slave's
> FS, doesn't change the fact that those locks are *cached* locks, they
> aren't binding/normative in any way.

That would be the ideal but it is not true today.  I also do not think
svnsync even presents a lock token since there is no working copy
involved to provide the token.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Mark Phippard wrote on Fri, Jul 15, 2011 at 12:02:53 -0400:
> On Fri, Jul 15, 2011 at 11:59 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > Mark Phippard wrote on Fri, Jul 15, 2011 at 11:55:07 -0400:
> >> > To keep the information on this thread I quote a bit of information that was just added to this issue:
> >> >
> >> > ------- Additional comments from jmountifield@tigris.org Fri Jul 15 07:01:25 -0700 2011 -------
> >> >
> >> > Replicating the locks to the slave server, even with the right lock token value,
> >> > still leaves us with some issues in the master/slave configuration. The
> >> > following is based on local testing with svn, version 1.6.12 (r955767).
> >> >
> >> > Once a slave repository has locks present you cannot keep that slave up to date.
> >> > This is true with both `svnsync sync` and `svnadmin load`.
> >> >
> >> > With svnsync the reasons are somewhat obvious. The process looks like a normal
> >> > commit, and commits are blocked by locks. So, unless there's some way to change
> >> > the locking behaviour for svnsync specifically, I can't see a workaround here.
> >>
> >> This is good information ... thanks.
> >>
> >> As long as we call post-unlock before post-commit it remains possible
> >> for a replica to have the locks removed before svnsync tries to sync
> >> the commit ... right?
> >
> > But that way you can't make the post-unlock script background itself
> > before it contacts the slaves...
> 
> The solution we use at CollabNet implements a message-queue type
> system.  So post-unlock could queue up a command for all the replicas
> and then post-commit would be queued after it.  All replicas would
> process the commands in sequence.
> 
> Of course it occurs to me that anyone that commits with the
> --keep-locks option would still break things.

If svnsync presents the lock token that's present on the master, and
hopefully the slave checks locks against the master during commit, the
commit is going to work isn't it?

Point is: using 'svnadmin lock', despite creating locks on the slave's
FS, doesn't change the fact that those locks are *cached* locks, they
aren't binding/normative in any way.

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Jul 15, 2011 at 11:59 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Mark Phippard wrote on Fri, Jul 15, 2011 at 11:55:07 -0400:
>> > To keep the information on this thread I quote a bit of information that was just added to this issue:
>> >
>> > ------- Additional comments from jmountifield@tigris.org Fri Jul 15 07:01:25 -0700 2011 -------
>> >
>> > Replicating the locks to the slave server, even with the right lock token value,
>> > still leaves us with some issues in the master/slave configuration. The
>> > following is based on local testing with svn, version 1.6.12 (r955767).
>> >
>> > Once a slave repository has locks present you cannot keep that slave up to date.
>> > This is true with both `svnsync sync` and `svnadmin load`.
>> >
>> > With svnsync the reasons are somewhat obvious. The process looks like a normal
>> > commit, and commits are blocked by locks. So, unless there's some way to change
>> > the locking behaviour for svnsync specifically, I can't see a workaround here.
>>
>> This is good information ... thanks.
>>
>> As long as we call post-unlock before post-commit it remains possible
>> for a replica to have the locks removed before svnsync tries to sync
>> the commit ... right?
>
> But that way you can't make the post-unlock script background itself
> before it contacts the slaves...

The solution we use at CollabNet implements a message-queue type
system.  So post-unlock could queue up a command for all the replicas
and then post-commit would be queued after it.  All replicas would
process the commands in sequence.

Of course it occurs to me that anyone that commits with the
--keep-locks option would still break things.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Mark Phippard wrote on Fri, Jul 15, 2011 at 11:55:07 -0400:
> > To keep the information on this thread I quote a bit of information that was just added to this issue:
> >
> > ------- Additional comments from jmountifield@tigris.org Fri Jul 15 07:01:25 -0700 2011 -------
> >
> > Replicating the locks to the slave server, even with the right lock token value,
> > still leaves us with some issues in the master/slave configuration. The
> > following is based on local testing with svn, version 1.6.12 (r955767).
> >
> > Once a slave repository has locks present you cannot keep that slave up to date.
> > This is true with both `svnsync sync` and `svnadmin load`.
> >
> > With svnsync the reasons are somewhat obvious. The process looks like a normal
> > commit, and commits are blocked by locks. So, unless there's some way to change
> > the locking behaviour for svnsync specifically, I can't see a workaround here.
> 
> This is good information ... thanks.
> 
> As long as we call post-unlock before post-commit it remains possible
> for a replica to have the locks removed before svnsync tries to sync
> the commit ... right?

But that way you can't make the post-unlock script background itself
before it contacts the slaves...


Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Mark Phippard <ma...@gmail.com>.
> To keep the information on this thread I quote a bit of information that was just added to this issue:
>
> ------- Additional comments from jmountifield@tigris.org Fri Jul 15 07:01:25 -0700 2011 -------
>
> Replicating the locks to the slave server, even with the right lock token value,
> still leaves us with some issues in the master/slave configuration. The
> following is based on local testing with svn, version 1.6.12 (r955767).
>
> Once a slave repository has locks present you cannot keep that slave up to date.
> This is true with both `svnsync sync` and `svnadmin load`.
>
> With svnsync the reasons are somewhat obvious. The process looks like a normal
> commit, and commits are blocked by locks. So, unless there's some way to change
> the locking behaviour for svnsync specifically, I can't see a workaround here.

This is good information ... thanks.

As long as we call post-unlock before post-commit it remains possible
for a replica to have the locks removed before svnsync tries to sync
the commit ... right?

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

RE: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: vrijdag 15 juli 2011 17:04
> To: Noorul Islam K M
> Cc: Subversion
> Subject: Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to
> create a lock
> 
> On 07/15/2011 08:58 AM, Noorul Islam K M wrote:
> > As first step I implemented the following syntax.
> >
> > svnadmin lock REPOS PATH USERNAME COMMENT-FILE
> >
> > I will add the optional TOKEN argument later. I hope I am progressing in
> > the right direction.
> 
> I've not reviewed this work, but you might also consider allowing this
> subcommand to accept --bypass-hooks, too.

To keep the information on this thread I quote a bit of information that was just added to this issue:

------- Additional comments from jmountifield@tigris.org Fri Jul 15 07:01:25 -0700 2011 -------

Replicating the locks to the slave server, even with the right lock token value,
still leaves us with some issues in the master/slave configuration. The
following is based on local testing with svn, version 1.6.12 (r955767).

Once a slave repository has locks present you cannot keep that slave up to date.
This is true with both `svnsync sync` and `svnadmin load`.

With svnsync the reasons are somewhat obvious. The process looks like a normal
commit, and commits are blocked by locks. So, unless there's some way to change
the locking behaviour for svnsync specifically, I can't see a workaround here.

More surprisingly the same is true with `svnadmin load`. This strikes me as
somewhat odd as these locks are definitely client layer, and svnadmin isn't a
client, right?. Why should svnadmin load respect the locks?

When trying to `svnadmin load` into a repository that has a lock on a path
touched by that load I get the following message:

mounty@laptop$ svnadmin load /tmp/lock-test < /tmp/lock-test.5.dmp 
<<< Started new transaction, based on original revision 5
     * editing path : trunk/foo.c ... done.
svnadmin: Cannot verify lock on path '/trunk/foo.c'; no username available


So, to sum up; If we replicate locks to the slave in the scenario described
above, we effectively make it impossible to keep the slave in sync with the
master, using the current options available.

Others have also asked that the svn dump format be able to capture lock tokens.
In this scenario `svnadmin load` will need to be able to operate around the
locks to enable loading of incremental dump files, where one or more dump files
describe a lock token.

-----------------------------------------------
	Bert



Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/18/2011 03:11 AM, noorul Islam. Kamal Malmiyoda wrote:
> Bert/Julian/Mike,
> 
> Thank you for your review comments and suggestions.
> 
> Please find attached the updated patch which contains the following
> modifications.

Patch committed in r1228253.  Thanks, Noorul.

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


Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
FTR I'll be happy to review this patch at some point for inclusion in
trunk.  Not right now though.

Noorul Islam K M wrote on Mon, Jul 18, 2011 at 12:41:59 +0530:
> [[[
> Fix issue #3942. Add new sub command 'lock' for 'svnadmin'. The
> following syntax is implemented in this patch.
> 
> svnadmin lock REPOS PATH USERNAME COMMENT-FILE [TOKEN]

Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by Noorul Islam K M <no...@collab.net>.
"C. Michael Pilato" <cm...@collab.net> writes:

> On 07/15/2011 08:58 AM, Noorul Islam K M wrote:
>
>> As first step I implemented the following syntax.
>> 
>> svnadmin lock REPOS PATH USERNAME COMMENT-FILE
>> 
>> I will add the optional TOKEN argument later. I hope I am progressing in
>> the right direction.
>
> I've not reviewed this work, but you might also consider allowing this
> subcommand to accept --bypass-hooks, too.

Bert/Julian/Mike,

Thank you for your review comments and suggestions.

Please find attached the updated patch which contains the following
modifications.

1) Add optional TOKEN argument. So the syntax becomes.

svnadmin lock REPOS PATH USERNAME COMMENT-FILE [TOKEN]

2) Incorporated Julian's review comments

2.a) No longer checking comments xml validity because this is already
done in svn_fs_lock() which is again called by svn_repos_fs_lock()

2.b) Used SVN_INVALID_REVNUM instead of youngest revision.

2.c) I am not sure whether the comment should be optional.

3) Implemented --bypass-hooks

Log
[[[
Fix issue #3942. Add new sub command 'lock' for 'svnadmin'. The
following syntax is implemented in this patch.

svnadmin lock REPOS PATH USERNAME COMMENT-FILE [TOKEN]

The command locks the PATH by USERNAME setting comment from
COMMENT-FILE. If the optional TOKEN is given then it is used as lock
token. If --bypass-hooks options is passed then bypass the repository
hook system.

* subversion/svnadmin/main.c
  (svn_opt_subcommand_t): New sub command.
  (svn_opt_subcommand_desc2_t): Add description.
  (subcommand_lock): Implement it.

* subversion/tests/cmdline/svntest/main.py
  (get_pre_lock_hook_path): Return the path of the pre-lock hook script.

* subversion/tests/cmdline/svnadmin_tests.py
  (lock): New test for 'lock' sub command.
  (test_list): Add new test.

]]]

Thanks and Regards
Noorul


Re: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/15/2011 08:58 AM, Noorul Islam K M wrote:
> As first step I implemented the following syntax.
> 
> svnadmin lock REPOS PATH USERNAME COMMENT-FILE
> 
> I will add the optional TOKEN argument later. I hope I am progressing in
> the right direction.

I've not reviewed this work, but you might also consider allowing this
subcommand to accept --bypass-hooks, too.

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