You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Fergus Slorach <fe...@methodics.com> on 2014/03/18 22:18:45 UTC
Behaviour of 'svn lock' in a read-only workspace
Hi,
In Subversion 1.6 'svn lock <file>' in a read-only workspace fails as expected
and no lock is created either in the workspace or in the repository.
In Svn 1.7/1.8 'svn lock <file>' still fails, but it creates a repository-side
lock (see below). Is this a regression or was the change intentional?
> svn st -u abc
Status against revision: 1633
> svn lock abc
svn: E200031: sqlite[S8]: attempt to write a readonly database
svn: E200031: Additional errors:
svn: E200031: sqlite[S8]: attempt to write a readonly database
> svn st -u abc
O 1633 abc
Status against revision: 1633
All the best,
fergus
Re: Behaviour of 'svn lock' in a read-only workspace
Posted by Philip Martin <ph...@wandisco.com>.
Ben Reser <be...@reser.org> writes:
> Index: subversion/libsvn_client/locking_commands.c
> ===================================================================
> --- subversion/libsvn_client/locking_commands.c (revision 1579078)
> +++ subversion/libsvn_client/locking_commands.c (working copy)
> @@ -294,6 +294,12 @@ organize_lock_targets(const char **common_parent_u
> _("No common parent found, unable to operate "
> "on disjoint arguments"));
>
> + /* Make sure the working copy is writable before modifying the
> + * repository otherwise we'll have a lock token with no place to put it
> + * or won't be able to remove the local lock token. */
> + SVN_ERR(svn_wc__acquire_write_lock(NULL, wc_ctx, common_dirent, FALSE,
> + result_pool, scratch_pool));
> +
> /* Get the URL for each target (which also serves to verify that
> the dirent targets are sane). */
> target_urls = apr_array_make(scratch_pool, rel_targets->nelts,
> @@ -504,6 +510,9 @@ svn_client_lock(const apr_array_header_t *targets,
> SVN_ERR(svn_ra_lock(ra_session, path_revs, comment,
> steal_lock, store_locks_callback, &cb, pool));
>
> + if (base_dir)
> + SVN_ERR(svn_wc__release_write_lock(ctx->wc_ctx, base_dir, pool));
> +
> return SVN_NO_ERROR;
> }
>
> @@ -549,6 +558,9 @@ svn_client_unlock(const apr_array_header_t *target
> SVN_ERR(svn_ra_unlock(ra_session, path_tokens, break_lock,
> store_locks_callback, &cb, pool));
>
> + if (base_dir)
> + SVN_ERR(svn_wc__release_write_lock(ctx->wc_ctx, base_dir, pool));
> +
> return SVN_NO_ERROR;
> }
>
Is that is going to leave locks behind if an error occurs? We have
svn_wc__call_with_write_lock and SVN_WC__CALL_WITH_WRITE_LOCK that might
avoid that problem. However multiple working copy support, something
1.6 didn't have, makes it all more tricky.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: Behaviour of 'svn lock' in a read-only workspace
Posted by Ben Reser <be...@reser.org>.
On 3/18/14, 2:18 PM, Fergus Slorach wrote:
> In Subversion 1.6 'svn lock <file>' in a read-only workspace fails as expected
> and no lock is created either in the workspace or in the repository.
>
> In Svn 1.7/1.8 'svn lock <file>' still fails, but it creates a repository-side
> lock (see below). Is this a regression or was the change intentional?
>
>> svn st -u abc
> Status against revision: 1633
>> svn lock abc
> svn: E200031: sqlite[S8]: attempt to write a readonly database
> svn: E200031: Additional errors:
> svn: E200031: sqlite[S8]: attempt to write a readonly database
>> svn st -u abc
> O 1633 abc
> Status against revision: 1633
I don't think it was deliberate, it's just a consequence of how locking works
and how WC-NG works.
We always open the SQLite database in write mode, and SQLite doesn't error out
if it doesn't have write access it just gives us the database in read-only
mode. This pushes the failure out until we actually try to write.
In the case of locking we don't try to write anything to the wc database until
we have a successful lock back from the server (actually we do it in callbacks
for each lock made). Thus the failure happens after the lock has already been
made on the server side.
With WCv1 we'd take out a write lock on the working copy before we'd execute
the lock on the server side. The relevant call in the 1.6.x code is the
svn_wc_adm_probe_open3() call in organize_lock_targets() in
subversion/libsvn_client/locking_commands.c (note it passes TRUE for if the
open should take out a write lock). This was rewritten for WC-NG in r879836.
I suspect Hyrum just didn't consider this error scenario when he rewrote this code.
I took a stab at fixing this but haven't finished yet because it needs to
account for multiple working copies (the common_dirent doesn't have to be a
working copy) and needs appropriate cleanups so that the write locks are
cleaned up when there are recoverable failures (e.g. failures from the server
side). This at least solves the issue, while breaking a bunch of other things
per the limitations I mentioned above. I post it below in case someone else
gets around to finishing this before I do.
[[[
Index: subversion/libsvn_client/locking_commands.c
===================================================================
--- subversion/libsvn_client/locking_commands.c (revision 1579078)
+++ subversion/libsvn_client/locking_commands.c (working copy)
@@ -294,6 +294,12 @@ organize_lock_targets(const char **common_parent_u
_("No common parent found, unable to operate "
"on disjoint arguments"));
+ /* Make sure the working copy is writable before modifying the
+ * repository otherwise we'll have a lock token with no place to put it
+ * or won't be able to remove the local lock token. */
+ SVN_ERR(svn_wc__acquire_write_lock(NULL, wc_ctx, common_dirent, FALSE,
+ result_pool, scratch_pool));
+
/* Get the URL for each target (which also serves to verify that
the dirent targets are sane). */
target_urls = apr_array_make(scratch_pool, rel_targets->nelts,
@@ -504,6 +510,9 @@ svn_client_lock(const apr_array_header_t *targets,
SVN_ERR(svn_ra_lock(ra_session, path_revs, comment,
steal_lock, store_locks_callback, &cb, pool));
+ if (base_dir)
+ SVN_ERR(svn_wc__release_write_lock(ctx->wc_ctx, base_dir, pool));
+
return SVN_NO_ERROR;
}
@@ -549,6 +558,9 @@ svn_client_unlock(const apr_array_header_t *target
SVN_ERR(svn_ra_unlock(ra_session, path_tokens, break_lock,
store_locks_callback, &cb, pool));
+ if (base_dir)
+ SVN_ERR(svn_wc__release_write_lock(ctx->wc_ctx, base_dir, pool));
+
return SVN_NO_ERROR;
}
]]]