You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/12/25 12:28:27 UTC
svn commit: r1553367 - in /subversion/trunk/subversion:
libsvn_ra_serf/commit.c libsvn_ra_serf/ra_serf.h libsvn_ra_serf/serf.c
tests/cmdline/prop_tests.py
Author: rhuijben
Date: Wed Dec 25 11:28:26 2013
New Revision: 1553367
URL: http://svn.apache.org/r1553367
Log:
Resolve issue #3086 "mod-dav-svn ignores pre-revprop-change failure on
revprop delete" by using the atomic revpropchange mode for revision
property deletes.
A followup patch will tweak svnsync to avoid the additional request needed
for the sync properties.
* subversion/libsvn_ra_serf/commit.c
(svn_ra_serf__change_rev_prop): Make sure we have the old property value when
deleting a property on an atomic revprop capable server to allow seeing
errors.
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__rev_prop): Make public within library.
* subversion/libsvn_ra_serf/serf.c
(includes): Add svn_props.h.
(svn_ra_serf__rev_proplist): Rename to ...
(serf__rev_proplist): ... and make the list of properties to obtain an
argument.
(svn_ra_serf__rev_proplist): New function, wrapping serf__rev_proplist.
(svn_ra_serf__rev_prop): Optimize the retrieval of "svn:*" properties by
passing the DAV name to serf__rev_proplist.
* subversion/tests/cmdline/prop_tests.py
(revprop_change): Remove XFail marker.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/commit.c
subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
subversion/trunk/subversion/libsvn_ra_serf/serf.c
subversion/trunk/subversion/tests/cmdline/prop_tests.py
Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1553367&r1=1553366&r2=1553367&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Dec 25 11:28:26 2013
@@ -2348,16 +2348,36 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
const char *proppatch_target;
const char *ns;
svn_error_t *err;
+ svn_string_t *tmp_old_value;
+ svn_boolean_t atomic_capable = FALSE;
+
+ if (old_value_p || !value)
+ SVN_ERR(svn_ra_serf__has_capability(ra_session, &atomic_capable,
+ SVN_RA_CAPABILITY_ATOMIC_REVPROPS,
+ pool));
if (old_value_p)
{
- svn_boolean_t capable;
- SVN_ERR(svn_ra_serf__has_capability(ra_session, &capable,
- SVN_RA_CAPABILITY_ATOMIC_REVPROPS,
- pool));
-
/* How did you get past the same check in svn_ra_change_rev_prop2()? */
- SVN_ERR_ASSERT(capable);
+ SVN_ERR_ASSERT(atomic_capable);
+ }
+ else if (! value && atomic_capable)
+ {
+ /* mod_dav_svn doesn't report a failure when a property delete fails. The
+ atomic revprop change behavior is a nice workaround, to allow getting
+ access to the error anyway.
+
+ Somehow the mod_dav maintainers think that returning an error from
+ mod_dav's property delete is an RFC violation.
+ See https://issues.apache.org/bugzilla/show_bug.cgi?id=53525 */
+
+ SVN_ERR(svn_ra_serf__rev_prop(ra_session, rev, name, &tmp_old_value,
+ pool));
+
+ if (!tmp_old_value)
+ return SVN_NO_ERROR; /* Nothing to delete */
+
+ old_value_p = &tmp_old_value;
}
commit = apr_pcalloc(pool, sizeof(*commit));
@@ -2399,12 +2419,12 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
proppatch_ctx->pool = pool;
proppatch_ctx->commit = commit;
proppatch_ctx->path = proppatch_target;
- proppatch_ctx->changed_props = apr_hash_make(proppatch_ctx->pool);
- proppatch_ctx->removed_props = apr_hash_make(proppatch_ctx->pool);
+ proppatch_ctx->changed_props = apr_hash_make(pool);
+ proppatch_ctx->removed_props = apr_hash_make(pool);
if (old_value_p)
{
- proppatch_ctx->previous_changed_props = apr_hash_make(proppatch_ctx->pool);
- proppatch_ctx->previous_removed_props = apr_hash_make(proppatch_ctx->pool);
+ proppatch_ctx->previous_changed_props = apr_hash_make(pool);
+ proppatch_ctx->previous_removed_props = apr_hash_make(pool);
}
proppatch_ctx->base_revision = SVN_INVALID_REVNUM;
@@ -2412,35 +2432,34 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
{
svn_ra_serf__set_prop(proppatch_ctx->previous_changed_props,
proppatch_ctx->path,
- ns, name, *old_value_p, proppatch_ctx->pool);
+ ns, name, *old_value_p, pool);
}
else if (old_value_p)
{
- svn_string_t *dummy_value = svn_string_create_empty(proppatch_ctx->pool);
+ svn_string_t *dummy_value = svn_string_create_empty(pool);
svn_ra_serf__set_prop(proppatch_ctx->previous_removed_props,
proppatch_ctx->path,
- ns, name, dummy_value, proppatch_ctx->pool);
+ ns, name, dummy_value, pool);
}
if (value)
{
svn_ra_serf__set_prop(proppatch_ctx->changed_props, proppatch_ctx->path,
- ns, name, value, proppatch_ctx->pool);
+ ns, name, value, pool);
}
else
{
- value = svn_string_create_empty(proppatch_ctx->pool);
+ value = svn_string_create_empty(pool);
svn_ra_serf__set_prop(proppatch_ctx->removed_props, proppatch_ctx->path,
- ns, name, value, proppatch_ctx->pool);
+ ns, name, value, pool);
}
- err = proppatch_resource(proppatch_ctx, commit, proppatch_ctx->pool);
+ err = proppatch_resource(proppatch_ctx, commit, pool);
if (err)
return
- svn_error_create
- (SVN_ERR_RA_DAV_REQUEST_FAILED, err,
+ svn_error_create(SVN_ERR_RA_DAV_PROPPATCH_FAILED, err,
_("DAV request failed; it's possible that the repository's "
"pre-revprop-change hook either failed or is non-existent"));
Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1553367&r1=1553366&r2=1553367&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Wed Dec 25 11:28:26 2013
@@ -1501,6 +1501,14 @@ svn_ra_serf__reparent(svn_ra_session_t *
const char *url,
apr_pool_t *pool);
+/* Implements svn_ra__vtable_t.rev_prop(). */
+svn_error_t *
+svn_ra_serf__rev_prop(svn_ra_session_t *session,
+ svn_revnum_t rev,
+ const char *name,
+ svn_string_t **value,
+ apr_pool_t *pool);
+
/* Implements svn_ra__vtable_t.get_log(). */
svn_error_t *
svn_ra_serf__get_log(svn_ra_session_t *session,
Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/serf.c?rev=1553367&r1=1553366&r2=1553367&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Wed Dec 25 11:28:26 2013
@@ -40,6 +40,7 @@
#include "svn_dirent_uri.h"
#include "svn_hash.h"
#include "svn_path.h"
+#include "svn_props.h"
#include "svn_time.h"
#include "svn_version.h"
@@ -850,12 +851,13 @@ svn_ra_serf__get_latest_revnum(svn_ra_se
latest_revnum, session, pool));
}
-/* Implements svn_ra__vtable_t.rev_proplist(). */
+/* Implementation of svn_ra_serf__rev_proplist(). */
static svn_error_t *
-svn_ra_serf__rev_proplist(svn_ra_session_t *ra_session,
- svn_revnum_t rev,
- apr_hash_t **ret_props,
- apr_pool_t *pool)
+serf__rev_proplist(svn_ra_session_t *ra_session,
+ svn_revnum_t rev,
+ const svn_ra_serf__dav_props_t *fetch_props,
+ apr_hash_t **ret_props,
+ apr_pool_t *pool)
{
svn_ra_serf__session_t *session = ra_session->priv;
apr_hash_t *props;
@@ -879,7 +881,7 @@ svn_ra_serf__rev_proplist(svn_ra_session
/* ### fix: fetch hash of *just* the PATH@REV props. no nested hash. */
SVN_ERR(svn_ra_serf__retrieve_props(&props, session, session->conns[0],
- propfind_path, rev, "0", all_props,
+ propfind_path, rev, "0", fetch_props,
pool, pool));
SVN_ERR(svn_ra_serf__select_revprops(ret_props, propfind_path, rev, props,
@@ -888,8 +890,21 @@ svn_ra_serf__rev_proplist(svn_ra_session
return SVN_NO_ERROR;
}
-/* Implements svn_ra__vtable_t.rev_prop(). */
+/* Implements svn_ra__vtable_t.rev_proplist(). */
static svn_error_t *
+svn_ra_serf__rev_proplist(svn_ra_session_t *ra_session,
+ svn_revnum_t rev,
+ apr_hash_t **ret_props,
+ apr_pool_t *pool)
+{
+ return svn_error_trace(
+ serf__rev_proplist(ra_session, rev, all_props,
+ ret_props, pool));
+}
+
+
+/* Implements svn_ra__vtable_t.rev_prop(). */
+svn_error_t *
svn_ra_serf__rev_prop(svn_ra_session_t *session,
svn_revnum_t rev,
const char *name,
@@ -897,8 +912,25 @@ svn_ra_serf__rev_prop(svn_ra_session_t *
apr_pool_t *pool)
{
apr_hash_t *props;
+ svn_ra_serf__dav_props_t specific_props[2];
+ const svn_ra_serf__dav_props_t *fetch_props = all_props;
+
+ /* The DAV propfind doesn't allow property fetches for any property name
+ as there is no defined way to quote values. If we are just fetching a
+ "svn:property" we can safely do this. In other cases we just fetch all
+ revision properties and filter the right one out */
+ if (strncmp(name, SVN_PROP_PREFIX, sizeof(SVN_PROP_PREFIX)-1) == 0
+ && !strchr(name + sizeof(SVN_PROP_PREFIX)-1, ':'))
+ {
+ specific_props[0].namespace = SVN_DAV_PROP_NS_SVN;
+ specific_props[0].name = name + sizeof(SVN_PROP_PREFIX)-1;
+ specific_props[1].namespace = NULL;
+ specific_props[1].name = NULL;
+
+ fetch_props = specific_props;
+ }
- SVN_ERR(svn_ra_serf__rev_proplist(session, rev, &props, pool));
+ SVN_ERR(serf__rev_proplist(session, rev, fetch_props, &props, pool));
*value = svn_hash_gets(props, name);
Modified: subversion/trunk/subversion/tests/cmdline/prop_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/prop_tests.py?rev=1553367&r1=1553366&r2=1553367&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/prop_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/prop_tests.py Wed Dec 25 11:28:26 2013
@@ -808,7 +808,6 @@ def copy_inherits_special_props(sbox):
# non-Posix platforms, we won't have to skip here:
@Skip(is_non_posix_and_non_windows_os)
@Issue(3086)
-@XFail(svntest.main.is_ra_type_dav)
def revprop_change(sbox):
"set, get, and delete a revprop change"