You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Thomas Åkesson <th...@simonsoft.se> on 2014/02/24 17:30:33 UTC
Lock non-existent to allow reserving a path
Hi,
We would like to enhance the locking in Subversion to support use cases where the user needs to ensure that added files will be possible to commit. There are a couple of use cases:
1. Adding files that are dependencies to other files, where the reference mechanism is cumbersome. Simple cases include XML files that reference graphics or XIncludes (can be difficult to refactor after a conflict, especially for non-technical users). More complex cases include file formats that are not open and where references are difficult to manage, e.g. MS Word, InDesign, certain modeling tools.
2. There is a standardized naming convention based on series. The next file of a certain type should take the next available number. Then there is a race condition btw different users; how quickly can they finish work and commit after determining the next number and adding to WC.
One mapping towards svn usage could be:
svn up
touch model0018.xml #or create the file with the software.
svn add model0018.xml
svn lock model0018.xml
# invest effort in the new file and potentially link to it from other files.
svn commit
3. When there is other software support above svn, it also makes sense to reserve a path via:
svn lock http://.../model0018.xml
Then performs a programmatic commit that ensures that the lock token is supplied (or svn unlock URL if aborting).
Current status:
Svn does not allow locking non-existent paths. It is blocked both in libsvn_fs_base/libsvn_fs_fs as well as in mod_dav_svn. In the same areas of the code in fs comments say:
"While our locking implementation easily supports the locking of
nonexistent paths, we deliberately choose not to allow such madness."
Given that we now present valid use-cases, it is reassuring that the authors of the locking code said it "easily supports the locking of nonexistent paths".
Locking an added file fails using the command line client.
In 1.6: svn: warning: W160042: LOCK request on '/svn/repo2/newfile.txt' failed: 405 Method Not Allowed
In 1.8: svn: E155010: The node '/Users/.../newfile.txt' was not found.
Breakdown of changes:
1. Make mod_dav_svn and fs allow locking of non-existent paths. This part is mandatory and I am attaching a rough patch that demonstrates the feasibility. With this change, the use-case 3 can be supported.
This part is the most important for us, to enable our software to avoid race conditions btw users.
It can be discussed whether it should be possible to configure the server to disallow these locks. I don't think they do much harm and configurability makes testing more complex.
2. There are already functional tools to manage these locks (svnadmin lslocks/rmlocks/unlock and svn unlock URL). Are any improvements required? E.g. making svn lslocks available (remotely).
3. Make the Working Copy support svn lock after svn add. This also requires taking care of (or blocking) some combinations with move and switch.
The implementation of these parts might be dependent on whether there is wider interest in these use-cases.
Proposal:
We would like to develop a patch for this functionality, tests first. I believe we can do tests, libsvn_fs and mod_dav_svn changes. However, we might need some assistance with the changes to Working copy / command line client.
The attached patch contains test coverage for the server side changes. There is one test for WC (currently marked XFail) and a list of ideas for additional WC tests. The areas I feel are most complex are related to move and switch. I personally don't think there is a strong need to support the combination of lock nonexistent with move/switch (just ensure we block those combinations).
There is also a question of how mod_dav_svn should handle "null-locks" when communicating with generic Webdav clients. The WebDAV specification initially proposed support for null-locks but in later revisions moved away from them. I propose to keep the current zero-size-file concept for non-svn clients. There is already a distinction btw svn and non-svn clients in that code, I just tweaked it.
Thanks,
Thomas Å.
AW: Lock non-existent to allow reserving a path
Posted by Markus Schaber <m....@codesys.com>.
Hi,
Von: Ben Reser [mailto:ben@reser.org]
> On 2/24/14, 8:30 AM, Thomas Åkesson wrote:
> > We would like to enhance the locking in Subversion to support use cases
> where the user needs to ensure that added files will be possible to commit.
> There are a couple of use cases:
> >
> > 1. Adding files that are dependencies to other files, where the reference
> mechanism is cumbersome. Simple cases include XML files that reference
> graphics or XIncludes (can be difficult to refactor after a conflict,
> especially for non-technical users). More complex cases include file formats
> that are not open and where references are difficult to manage, e.g. MS Word,
> InDesign, certain modeling tools.
> >
> > 2. There is a standardized naming convention based on series. The next file
> of a certain type should take the next available number. Then there is a race
> condition btw different users; how quickly can they finish work and commit
> after determining the next number and adding to WC.
> >
> > One mapping towards svn usage could be:
> > svn up
> > touch model0018.xml #or create the file with the software.
> > svn add model0018.xml
> > svn lock model0018.xml
> > # invest effort in the new file and potentially link to it from other files.
> > svn commit
>
> Why does doing the following where the empty file is always immediately
> created not work for these use cases:
> svn up
> touch model0018.xml
> svn add model0018.xml
> svn ci -m'Create model0018.xml'
> svn lock model0018.xml
>
> Worst case senario for this seems to be the following.
One obvious difference is that you actually have an empty file committed for
some timeframe, which may lead to other problems (like CI-Systems failing, or
maybe even pre-commit hooks which try to perform consistency checks...)
Best regards
Markus Schaber
CODESYS® a trademark of 3S-Smart Software Solutions GmbH
Inspiring Automation Solutions
3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50
E-Mail: m.schaber@codesys.com | Web: http://www.codesys.com | CODESYS store: http://store.codesys.com
CODESYS forum: http://forum.codesys.com
Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915
Re: Lock non-existent to allow reserving a path
Posted by Thomas Åkesson <th...@simonsoft.se>.
> On 24 feb 2014, at 20:36, Ben Reser <be...@reser.org> wrote:
>
>> On 2/24/14, 8:30 AM, Thomas Åkesson wrote:
>> We would like to enhance the locking in Subversion to support use cases where the user needs to ensure that added files will be possible to commit. There are a couple of use cases:
>>
>> 1. Adding files that are dependencies to other files, where the reference mechanism is cumbersome. Simple cases include XML files that reference graphics or XIncludes (can be difficult to refactor after a conflict, especially for non-technical users). More complex cases include file formats that are not open and where references are difficult to manage, e.g. MS Word, InDesign, certain modeling tools.
>>
>> 2. There is a standardized naming convention based on series. The next file of a certain type should take the next available number. Then there is a race condition btw different users; how quickly can they finish work and commit after determining the next number and adding to WC.
>>
>> One mapping towards svn usage could be:
>> svn up
>> touch model0018.xml #or create the file with the software.
>> svn add model0018.xml
>> svn lock model0018.xml
>> # invest effort in the new file and potentially link to it from other files.
>> svn commit
>
> Why does doing the following where the empty file is always immediately created
> not work for these use cases:
> svn up
> touch model0018.xml
> svn add model0018.xml
> svn ci -m'Create model0018.xml'
> svn lock model0018.xml
>
> Worst case senario for this seems to be the following.
>
> Alice and Bob are both about to make a new model file. They select the next
> appropriate filename model0018.xml. They both touch and add the file and then
> try to check it in. Alice succeeds, Bob gets a message telling him his commit
> is out of date because Alice won the race. Bob then repeats the steps from the
> top (including selecting the now next available filename).
>
> I'm guessing the answer comes down to user education difficulties.
No. I simplified the use cases, too much obviously.
Let me try again:
svn up
for several hours:
svn add a few files
svn lock those files
work, including linking to those files
revise a bit
review and perhaps svn unlock and svn rm some added files
svn commit
>
>> 3. When there is other software support above svn, it also makes sense to reserve a path via:
>> svn lock http://.../model0018.xml
>> Then performs a programmatic commit that ensures that the lock token is supplied (or svn unlock URL if aborting).
>
> Or maybe I'm missing something since I don't think handling this would be
> difficult in software.
It becomes complicated when:
- we might be talking about a few hundred files, and we would prefer not to commit them one by one (many reasons not to)
- it is desirable to allow the user to review the end result before commit
- I was not only talking about automated selection of file names, could be combinations of manual and series.
- again, links btw the files
Without a locking mechanism, we would basically need a layer of temporary naming and linking that is resolved when attempting to commit, and retried until a commit succeeds without conflict. If we don't have an open file format that we can manipulate, well forget it.
>
>> Current status:
>>
>> Svn does not allow locking non-existent paths. It is blocked both in libsvn_fs_base/libsvn_fs_fs as well as in mod_dav_svn. In the same areas of the code in fs comments say:
>> "While our locking implementation easily supports the locking of
>> nonexistent paths, we deliberately choose not to allow such madness."
>>
>> Given that we now present valid use-cases, it is reassuring that the authors of the locking code said it "easily supports the locking of nonexistent paths".
>>
>> Locking an added file fails using the command line client.
>>
>> In 1.6: svn: warning: W160042: LOCK request on '/svn/repo2/newfile.txt' failed: 405 Method Not Allowed
>>
>> In 1.8: svn: E155010: The node '/Users/.../newfile.txt' was not found.
>
> Yes the existing implementations make this easy to do. But your suggested
> implementation is not the right path to go down.
>
>> There is also a question of how mod_dav_svn should handle "null-locks" when communicating with generic Webdav clients. The WebDAV specification initially proposed support for null-locks but in later revisions moved away from them. I propose to keep the current zero-size-file concept for non-svn clients. There is already a distinction btw svn and non-svn clients in that code, I just tweaked it.
>
> Yes and RFC 4918 killed lock-null resources off for good reason. The whole
> thing was overly complicated, requiring a separate DB on the server side to
> track the non-existent paths it should pretend to exist. Even worse if you
> wanted to support DeltaV you had to keep track of which lock-null resources
> existed when forever. However, if you just created the empty file then all of
> this went away and there was nothing special to handle.
>
> So you can't do what you propose and be RFC4918 compliant. The RFC says that
> locks of non-existent files MUST create the file and removing the lock MUST not
> remove the file.
Well, that is why I propose to stay compliant when talking to a generic webdav client. Lock-null would only be allowed with svn clients. There are other areas where svn clients are deviating from Webdav RFCs, right?
> If we really want to make the UI appear to support locking non-existent files
> we can. There are some issues to work through but I think making the backend
> support locking non-existent files is the wrong approach.
>
> I see two approaches to doing this:
>
> 1) Server supports creating the empty files for the client with the LOCK
> request like we do with auto-versioning clients. This presents problems with
> how the working copy finds out about the new empty file. Since otherwise the
> client will give an out of date error on commit and then the update will have a
> conflict. If we just change the server then old clients talking with such
> servers will suddenly have this problem. Which seems undesirable.
> 2) Have the client create, schedule and commit the new empty file and then
> issue the lock. Essentially doing the steps I presented above for the user
> automatically. This presents the following problems:
>
> a) How does the client know it needs to create the empty file before the lock?
> I'd guess the answer here is that we check if the path exists and try to
> create it. If the wc has the path as already versioned and committed, then
> just try to create the lock (so we fail if you're trying to lock something
> someone has deleted).
>
> b) Does the race condition between two clients trying to lock the same path
> present a problem? Consider that one client is slightly behind another such
> that it runs the check path just as the first client has finished creating the
> file and then the two LOCK requests race. I'm guessing we're not going to be
> happy if the client that created the new file loses that race? For one thing
> the commit message from creating the file might not make sense. I'm guessing
> this could be solved with extending the commit to support a way to lock a file
> iff the commit succeeds. Only newer servers would support this and we could
> decide to allow newer clients with older servers to lock non-existent files
> with the above mentioned known limitations or simply not allow it at all.
>
> It seems to me that 2) is the better approach to me.
None of these approaches address the use-cases when generalised, and these approaches can be accomplished with currently released svn. I probably over-simplified the use cases, sorry about that. I think use-case 1 does to some degree describe the complexity we would like to address.
Regards,
Thomas Å.
Re: Lock non-existent to allow reserving a path
Posted by Ben Reser <be...@reser.org>.
On 2/24/14, 8:30 AM, Thomas Åkesson wrote:
> We would like to enhance the locking in Subversion to support use cases where the user needs to ensure that added files will be possible to commit. There are a couple of use cases:
>
> 1. Adding files that are dependencies to other files, where the reference mechanism is cumbersome. Simple cases include XML files that reference graphics or XIncludes (can be difficult to refactor after a conflict, especially for non-technical users). More complex cases include file formats that are not open and where references are difficult to manage, e.g. MS Word, InDesign, certain modeling tools.
>
> 2. There is a standardized naming convention based on series. The next file of a certain type should take the next available number. Then there is a race condition btw different users; how quickly can they finish work and commit after determining the next number and adding to WC.
>
> One mapping towards svn usage could be:
> svn up
> touch model0018.xml #or create the file with the software.
> svn add model0018.xml
> svn lock model0018.xml
> # invest effort in the new file and potentially link to it from other files.
> svn commit
Why does doing the following where the empty file is always immediately created
not work for these use cases:
svn up
touch model0018.xml
svn add model0018.xml
svn ci -m'Create model0018.xml'
svn lock model0018.xml
Worst case senario for this seems to be the following.
Alice and Bob are both about to make a new model file. They select the next
appropriate filename model0018.xml. They both touch and add the file and then
try to check it in. Alice succeeds, Bob gets a message telling him his commit
is out of date because Alice won the race. Bob then repeats the steps from the
top (including selecting the now next available filename).
I'm guessing the answer comes down to user education difficulties.
> 3. When there is other software support above svn, it also makes sense to reserve a path via:
> svn lock http://.../model0018.xml
> Then performs a programmatic commit that ensures that the lock token is supplied (or svn unlock URL if aborting).
Or maybe I'm missing something since I don't think handling this would be
difficult in software.
> Current status:
>
> Svn does not allow locking non-existent paths. It is blocked both in libsvn_fs_base/libsvn_fs_fs as well as in mod_dav_svn. In the same areas of the code in fs comments say:
> "While our locking implementation easily supports the locking of
> nonexistent paths, we deliberately choose not to allow such madness."
>
> Given that we now present valid use-cases, it is reassuring that the authors of the locking code said it "easily supports the locking of nonexistent paths".
>
> Locking an added file fails using the command line client.
>
> In 1.6: svn: warning: W160042: LOCK request on '/svn/repo2/newfile.txt' failed: 405 Method Not Allowed
>
> In 1.8: svn: E155010: The node '/Users/.../newfile.txt' was not found.
Yes the existing implementations make this easy to do. But your suggested
implementation is not the right path to go down.
> There is also a question of how mod_dav_svn should handle "null-locks" when communicating with generic Webdav clients. The WebDAV specification initially proposed support for null-locks but in later revisions moved away from them. I propose to keep the current zero-size-file concept for non-svn clients. There is already a distinction btw svn and non-svn clients in that code, I just tweaked it.
Yes and RFC 4918 killed lock-null resources off for good reason. The whole
thing was overly complicated, requiring a separate DB on the server side to
track the non-existent paths it should pretend to exist. Even worse if you
wanted to support DeltaV you had to keep track of which lock-null resources
existed when forever. However, if you just created the empty file then all of
this went away and there was nothing special to handle.
So you can't do what you propose and be RFC4918 compliant. The RFC says that
locks of non-existent files MUST create the file and removing the lock MUST not
remove the file.
If we really want to make the UI appear to support locking non-existent files
we can. There are some issues to work through but I think making the backend
support locking non-existent files is the wrong approach.
I see two approaches to doing this:
1) Server supports creating the empty files for the client with the LOCK
request like we do with auto-versioning clients. This presents problems with
how the working copy finds out about the new empty file. Since otherwise the
client will give an out of date error on commit and then the update will have a
conflict. If we just change the server then old clients talking with such
servers will suddenly have this problem. Which seems undesirable.
2) Have the client create, schedule and commit the new empty file and then
issue the lock. Essentially doing the steps I presented above for the user
automatically. This presents the following problems:
a) How does the client know it needs to create the empty file before the lock?
I'd guess the answer here is that we check if the path exists and try to
create it. If the wc has the path as already versioned and committed, then
just try to create the lock (so we fail if you're trying to lock something
someone has deleted).
b) Does the race condition between two clients trying to lock the same path
present a problem? Consider that one client is slightly behind another such
that it runs the check path just as the first client has finished creating the
file and then the two LOCK requests race. I'm guessing we're not going to be
happy if the client that created the new file loses that race? For one thing
the commit message from creating the file might not make sense. I'm guessing
this could be solved with extending the commit to support a way to lock a file
iff the commit succeeds. Only newer servers would support this and we could
decide to allow newer clients with older servers to lock non-existent files
with the above mentioned known limitations or simply not allow it at all.
It seems to me that 2) is the better approach to me.
Re: Lock non-existent to allow reserving a path
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Ben Reser <be...@reser.org> writes:
>
>> On 2/24/14, 11:10 AM, Philip Martin wrote:
>>> It's hard to fix. Commit and unlock are separate filesystem operations
>>> and the server can always die, or fail the unlock, after the commit. I
>>> suppose a new filesystem might have a commit-and-unlock operation but
>>> how could FSFS solve it? We might be able to make FSFS ignore the lock
>>> if the file has been deleted, but that just postpones the problem until
>>> another commit recreates the file.
>>
>> So remove the locks as part of creating a new file. Even if the commit fails
>> for whatever reason and you remove locks that's not a problem. You were
>> ignoring those locks anyway.
>
> Yes, that might work, we would probably need to check added directories
> as well as added files. We would need to do it during the final stage
> of the commit when converting the transaction to a revision. We could
> also check when adding a node to a transaction but that is not
> sufficient because even if no no lock exists at that point a file could
> get added/locked/deleted before the transaction completes.
We must do part of this already, since these phantom locks cause commits
to fail.
We can't simply fail all adds when a lock exists since we need to be
make sure replace works:
svn lock wc/f
svn rm wc/f
touch wc/f
svn add wc/f
svn ci wc
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: Lock non-existent to allow reserving a path
Posted by Philip Martin <ph...@wandisco.com>.
Ben Reser <be...@reser.org> writes:
> On 2/24/14, 11:10 AM, Philip Martin wrote:
>> It's hard to fix. Commit and unlock are separate filesystem operations
>> and the server can always die, or fail the unlock, after the commit. I
>> suppose a new filesystem might have a commit-and-unlock operation but
>> how could FSFS solve it? We might be able to make FSFS ignore the lock
>> if the file has been deleted, but that just postpones the problem until
>> another commit recreates the file.
>
> So remove the locks as part of creating a new file. Even if the commit fails
> for whatever reason and you remove locks that's not a problem. You were
> ignoring those locks anyway.
Yes, that might work, we would probably need to check added directories
as well as added files. We would need to do it during the final stage
of the commit when converting the transaction to a revision. We could
also check when adding a node to a transaction but that is not
sufficient because even if no no lock exists at that point a file could
get added/locked/deleted before the transaction completes.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: Lock non-existent to allow reserving a path
Posted by Ben Reser <be...@reser.org>.
On 2/24/14, 11:10 AM, Philip Martin wrote:
> It's hard to fix. Commit and unlock are separate filesystem operations
> and the server can always die, or fail the unlock, after the commit. I
> suppose a new filesystem might have a commit-and-unlock operation but
> how could FSFS solve it? We might be able to make FSFS ignore the lock
> if the file has been deleted, but that just postpones the problem until
> another commit recreates the file.
So remove the locks as part of creating a new file. Even if the commit fails
for whatever reason and you remove locks that's not a problem. You were
ignoring those locks anyway.
Re: Lock non-existent to allow reserving a path
Posted by Philip Martin <ph...@wandisco.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> On 02/24/2014 01:56 PM, Philip Martin wrote:
>> There is a way to create such locks at present: checkout, lock a
>> file, delete the file or parent directory, commit with --no-unlock.
>> We have a regression test for this:
>> lock_tests.py:deleted_path_lock. (Possibly this behaviour could be
>> considered a bug, perhaps 'commit --no-unlock' should remove the
>> locks on deleted files, but implementing that would be hard.)
>
> That behavior *is* a bug that someone must have decided post-facto
> might
> be useful. But hey, I'm not pointing fingers. That approach has
> brought us such awesome features in the past as "file externals".
>
> Oh.
>
> Wait.
It's hard to fix. Commit and unlock are separate filesystem operations
and the server can always die, or fail the unlock, after the commit. I
suppose a new filesystem might have a commit-and-unlock operation but
how could FSFS solve it? We might be able to make FSFS ignore the lock
if the file has been deleted, but that just postpones the problem until
another commit recreates the file.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: Lock non-existent to allow reserving a path
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/24/2014 01:56 PM, Philip Martin wrote:
> Thomas Åkesson <th...@simonsoft.se> writes:
>
>> Svn does not allow locking non-existent paths. It is blocked both in
>> libsvn_fs_base/libsvn_fs_fs as well as in mod_dav_svn. In the same
>> areas of the code in fs comments say:
>> "While our locking implementation easily supports the locking of
>> nonexistent paths, we deliberately choose not to allow such madness."
>>
>> Given that we now present valid use-cases, it is reassuring that the
>> authors of the locking code said it "easily supports the locking of
>> nonexistent paths".
> There is a way to create such locks at present: checkout, lock a file,
> delete the file or parent directory, commit with --no-unlock. We have a
> regression test for this: lock_tests.py:deleted_path_lock. (Possibly
> this behaviour could be considered a bug, perhaps 'commit --no-unlock'
> should remove the locks on deleted files, but implementing that would be
> hard.)
That behavior *is* a bug that someone must have decided post-facto might
be useful. But hey, I'm not pointing fingers. That approach has
brought us such awesome features in the past as "file externals".
Oh.
Wait.
Re: Lock non-existent to allow reserving a path
Posted by Thomas Åkesson <th...@simonsoft.se>.
Thanks Philip for sharing your insight into the lock mechanisms.
Sorry about the delay, wanted to find time to investigate.
On 24 feb 2014, at 19:56, Philip Martin <ph...@wandisco.com> wrote:
> Thomas Åkesson <th...@simonsoft.se> writes:
>
>> Svn does not allow locking non-existent paths. It is blocked both in
>> libsvn_fs_base/libsvn_fs_fs as well as in mod_dav_svn. In the same
>> areas of the code in fs comments say:
>> "While our locking implementation easily supports the locking of
>> nonexistent paths, we deliberately choose not to allow such madness."
>>
>> Given that we now present valid use-cases, it is reassuring that the
>> authors of the locking code said it "easily supports the locking of
>> nonexistent paths".
>
> There is a way to create such locks at present: checkout, lock a file,
> delete the file or parent directory, commit with --no-unlock. We have a
> regression test for this: lock_tests.py:deleted_path_lock. (Possibly
> this behaviour could be considered a bug, perhaps 'commit --no-unlock'
> should remove the locks on deleted files, but implementing that would be
> hard.)
As discussed else-thread, should likely be considered a bug. It is not useful for these use cases since it requires the file to already exist.
>
>> Breakdown of changes:
>>
>> 1. Make mod_dav_svn and fs allow locking of non-existent paths. This
>> part is mandatory and I am attaching a rough patch that demonstrates
>> the feasibility. With this change, the use-case 3 can be supported.
>>
>> This part is the most important for us, to enable our software to
>> avoid race conditions btw users.
>>
>> It can be discussed whether it should be possible to configure the
>> server to disallow these locks. I don't think they do much harm and
>> configurability makes testing more complex.
>>
>> 2. There are already functional tools to manage these locks (svnadmin
>> lslocks/rmlocks/unlock and svn unlock URL). Are any improvements
>> required? E.g. making svn lslocks available (remotely).
>>
>> 3. Make the Working Copy support svn lock after svn add. This also
>> requires taking care of (or blocking) some combinations with move and
>> switch.
>
> The working copy is likely to be the hard bit.
>
> Probably we would want to block operations that undo the add: revert,
> move, delete, etc. when the lock is present. Attempting to move/remove
> the lock automatically would make these non-network operations into
> network operations. I'm not sure exactly what behaviour would be
> acceptable here. Do we want to prevent people reverting without network
> access?
Yes, I think we would want to block a good number of operations. If using locks on added files one might very well have to unlock before performing certain operations.
I just noticed that revert does not have a switch to release the locks. Actually, when reverting it is very easy to accidentally leave locks behind. It might make sense to never allow a revert of locked files without specifying either "--unlock" or "--no-unlock".
>
> Another tricky bit is how are update/status going to interact with these
> locks? I think we would want the behaviour to be similar to the current
> locks. So update will report if a lock gets broken/stolen and will
> remove the lock from the working copy. I think this requires
> update/status to start reporting added files to the server so that the
> server can report on the locks. That looks as if it will require
> protocol changes.
Interesting point. I agree we would want a consistent behaviour.
Perhaps it would be an acceptable performance degradation to get a separate lock report when having locked added files in WC.
>
> It gets even more tricky with directories:
>
> svn mkdir wc1/A
> touch wc1/A/f
> svn add wc1/A/f
> svn lock wc1/A/f
>
> svn mkdir wc2/A
>
> The lock wc1/A/f will prevent the commit of wc2/A, or at least I think
> it will.
Yes it does. Verified with the patch in my first post.
This code is in fs_make_dir(..) in tree.c:
/* Check (recursively) to see if some lock is 'reserving' a path at
that location, or even some child-path; if so, check that we can
use it. */
if (root->txn_flags & SVN_FS_TXN_CHECK_LOCKS)
SVN_ERR(svn_fs_fs__allow_locked_operation(path, root->fs, TRUE, FALSE,
pool));
I can't see any reason to make a recursive check for locks when doing mkdir (does not matter now, when locking non-existent is not allowed). Should be a simple change. An mkdir does not carry the same implications as a move or rmdir (they must of course do recursive check).
Copy is more complicated.
svn mkdir wc1/A
touch wc1/A/f
svn add wc1/A/f
svn lock wc1/A/f
svn cp wc2/B wc2/A
Copy also checks locks recursively for to_path. Not sure if that matters now (when locking non-existent is not allowed).
If allowing locking non-existent I personally think it would be fine to fail that copy if there are locks below (no change to current code). Otherwise we could investigate if the locks actually conflict with nodes in the copy-source.
> Should status on wc2 show the lock? If wc2 doesn't have an
> added 'A' then the lock is of no interest, but when 'A' is added then
> the lock should be reported.
Yes, preferably. Is 'svn stat -u' doing a report similar to update? No separate lock report? More or less the same challenges as removing broken locks during update.
Do we see any other reasons to start reporting added files during an update? I have been thinking about the move improvements but not yet identified any situation.
I have identified a situation related to sparse Working Copies where reporting the added file would improve the user experience. When adding a file to a dir, which due to depth settings doesn't contain all files, a subsequent update does not trigger a conflict. The commit does. Reporting the added file could reveal the conflict earlier.
I would be interested in finishing the patch with the scope of allowing locking non-existent paths using URL and enhancing svn import such that the locks can be used for creating new files in the repository (add switch to send in lock tokens). I will also investigate the feasibility of reporting these locks in 'svn stat -u'. Support in the Working Copy can be a separate task awaiting more interest in those use cases.
Thoughts?
/Thomas Å.
Re: Lock non-existent to allow reserving a path
Posted by Philip Martin <ph...@wandisco.com>.
Thomas Åkesson <th...@simonsoft.se> writes:
> Svn does not allow locking non-existent paths. It is blocked both in
> libsvn_fs_base/libsvn_fs_fs as well as in mod_dav_svn. In the same
> areas of the code in fs comments say:
> "While our locking implementation easily supports the locking of
> nonexistent paths, we deliberately choose not to allow such madness."
>
> Given that we now present valid use-cases, it is reassuring that the
> authors of the locking code said it "easily supports the locking of
> nonexistent paths".
There is a way to create such locks at present: checkout, lock a file,
delete the file or parent directory, commit with --no-unlock. We have a
regression test for this: lock_tests.py:deleted_path_lock. (Possibly
this behaviour could be considered a bug, perhaps 'commit --no-unlock'
should remove the locks on deleted files, but implementing that would be
hard.)
> Breakdown of changes:
>
> 1. Make mod_dav_svn and fs allow locking of non-existent paths. This
> part is mandatory and I am attaching a rough patch that demonstrates
> the feasibility. With this change, the use-case 3 can be supported.
>
> This part is the most important for us, to enable our software to
> avoid race conditions btw users.
>
> It can be discussed whether it should be possible to configure the
> server to disallow these locks. I don't think they do much harm and
> configurability makes testing more complex.
>
> 2. There are already functional tools to manage these locks (svnadmin
> lslocks/rmlocks/unlock and svn unlock URL). Are any improvements
> required? E.g. making svn lslocks available (remotely).
>
> 3. Make the Working Copy support svn lock after svn add. This also
> requires taking care of (or blocking) some combinations with move and
> switch.
The working copy is likely to be the hard bit.
Probably we would want to block operations that undo the add: revert,
move, delete, etc. when the lock is present. Attempting to move/remove
the lock automatically would make these non-network operations into
network operations. I'm not sure exactly what behaviour would be
acceptable here. Do we want to prevent people reverting without network
access?
Another tricky bit is how are update/status going to interact with these
locks? I think we would want the behaviour to be similar to the current
locks. So update will report if a lock gets broken/stolen and will
remove the lock from the working copy. I think this requires
update/status to start reporting added files to the server so that the
server can report on the locks. That looks as if it will require
protocol changes.
It gets even more tricky with directories:
svn mkdir wc1/A
touch wc1/A/f
svn add wc1/A/f
svn lock wc1/A/f
svn mkdir wc2/A
The lock wc1/A/f will prevent the commit of wc2/A, or at least I think
it will. Should status on wc2 show the lock? If wc2 doesn't have an
added 'A' then the lock is of no interest, but when 'A' is added then
the lock should be reported.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*