Re: Lock non-existent to allow reserving a path
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 philip.mar...@wandisco.com wrote: Thomas Åkesson thomas.akes...@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
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 Å. lock20140224.patch Description: Binary data
Re: Lock non-existent to allow reserving a path
Thomas Åkesson thomas.akes...@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*
Re: Lock non-existent to allow reserving a path
On 02/24/2014 01:56 PM, Philip Martin wrote: Thomas Åkesson thomas.akes...@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
C. Michael Pilato cmpil...@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
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
Re: Lock non-existent to allow reserving a path
Philip Martin philip.mar...@wandisco.com writes: Ben Reser b...@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
On 24 feb 2014, at 20:36, Ben Reser b...@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
AW: Lock non-existent to allow reserving a path
Hi, Von: Ben Reser [mailto:b...@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.scha...@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