Hi All

Mike has suggestion at [1] to make use 'DAV:apply-to-version' to avoid the problematic PROPFIND altogether.

This suggestion works very well.

It is just the ra_neon fix i.e no mod_dav_svn fix

Attached patch implements this fix.

All tests pass with this test(except the known mergeinfo test 8 failure.)


In next few hours I will commit this as it looks fairly obvious unless there are some objections.

Thanks Mike,

With regards
Kamesh Jayachandran

PS
[1] http://mail-archives.apache.org/mod_mbox/subversion-dev/201001.mbox/%3c4b4cc1aa.6000...@collab.net%3e
Make CHECKOUT request on VCC url rather than the baseline url as that avoids
the detection of *out of date* baseline url from the mirror and leave that
to CHECKOUT itself via '<D:apply-to-version/>'.

As CHECKOUT is proxied by the mirror we are not getting hit by the original
issue.

* subversion/libsvn_ra_neon/commit.c
(APPLY_TO_VERSION): New macro.
(do_checkout): Accept 'is_vcc' and make CHECKOUT with '<D:apply-to-version/>'
 if is_vcc is TRUE.
(checkout_resource): Accept 'is_vcc' and cascade.
(commit_delete_entry, commit_add_dir,
 commit_change_dir_prop, commit_add_file,
 commit_open_file, commit_change_file_prop): 
 Call 'checkout_resource' with is_vcc as FALSE.
(apply_revprops): Avoid baseline detecion,
 Call 'checkout_resource' with is_vcc as TRUE and rsrc being that of VCC.

Suggested by: cmpilato
Index: subversion/libsvn_ra_neon/commit.c
===================================================================
--- subversion/libsvn_ra_neon/commit.c  (revision 900682)
+++ subversion/libsvn_ra_neon/commit.c  (working copy)
@@ -48,6 +48,7 @@
 #include "ra_neon.h"
 
 
+#define APPLY_TO_VERSION "<D:apply-to-version/>"
 /*
 ** version_rsrc_t: identify the relevant pieces of a resource on the server
 **
@@ -399,6 +400,7 @@
                                  const char *vsn_url,
                                  svn_boolean_t allow_404,
                                  const char *token,
+                                 svn_boolean_t is_vcc,
                                  int *code,
                                  const char **locn,
                                  apr_pool_t *pool)
@@ -423,7 +425,9 @@
                       "<D:checkout xmlns:D=\"DAV:\">"
                       "<D:activity-set>"
                       "<D:href>%s</D:href>"
-                      "</D:activity-set></D:checkout>", cc->activity_url);
+                      "</D:activity-set>%s</D:checkout>",
+                      cc->activity_url,
+                      is_vcc ? APPLY_TO_VERSION: "");
 
   if (token)
     {
@@ -459,6 +463,7 @@
                                        version_rsrc_t *rsrc,
                                        svn_boolean_t allow_404,
                                        const char *token,
+                                       svn_boolean_t is_vcc,
                                        apr_pool_t *pool)
 {
   int code;
@@ -473,7 +478,8 @@
     }
 
   /* check out the Version Resource */
-  err = do_checkout(cc, rsrc->vsn_url, allow_404, token, &code, &locn, pool);
+  err = do_checkout(cc, rsrc->vsn_url, allow_404, token,
+                    is_vcc, &code, &locn, pool);
 
   /* possibly run the request again, with a re-fetched Version Resource */
   if (err == NULL && allow_404 && code == 404)
@@ -484,7 +490,8 @@
       SVN_ERR(get_version_url(cc, NULL, rsrc, TRUE, pool));
 
       /* do it again, but don't allow a 404 this time */
-      err = do_checkout(cc, rsrc->vsn_url, FALSE, token, &code, &locn, pool);
+      err = do_checkout(cc, rsrc->vsn_url, FALSE, token,
+                        is_vcc, &code, &locn, pool);
     }
 
   /* special-case when conflicts occur */
@@ -711,7 +718,8 @@
     }
 
   /* get the URL to the working collection */
-  SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE, NULL, pool));
+  SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE,
+                            NULL, FALSE, pool));
 
   /* create the URL for the child resource */
   child = svn_path_url_add_component(parent->rsrc->wr_url, name, pool);
@@ -847,7 +855,8 @@
 
   /* check out the parent resource so that we can create the new collection
      as one of its children. */
-  SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE, NULL, dir_pool));
+  SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE,
+                            NULL, FALSE, dir_pool));
 
   /* create a child object that contains all the resource urls */
   child = apr_pcalloc(dir_pool, sizeof(*child));
@@ -959,7 +968,7 @@
   record_prop_change(dir->pool, dir, name, value);
 
   /* do the CHECKOUT sooner rather than later */
-  SVN_ERR(checkout_resource(dir->cc, dir->rsrc, TRUE, NULL, pool));
+  SVN_ERR(checkout_resource(dir->cc, dir->rsrc, TRUE, NULL, FALSE, pool));
 
   /* Add this path to the valid targets hash. */
   add_valid_target(dir->cc, dir->rsrc->local_path, svn_nonrecursive);
@@ -1000,7 +1009,8 @@
   */
 
   /* Do the parent CHECKOUT first */
-  SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE, NULL, workpool));
+  SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE,
+                            NULL, FALSE, workpool));
 
   /* Construct a file_baton that contains all the resource urls. */
   file = apr_pcalloc(file_pool, sizeof(*file));
@@ -1139,7 +1149,7 @@
 
   /* do the CHECKOUT now. we'll PUT the new file contents later on. */
   SVN_ERR(checkout_resource(parent->cc, file->rsrc, TRUE,
-                            file->token, workpool));
+                            file->token, FALSE, workpool));
 
   /* ### wait for apply_txdelta before doing a PUT. it might arrive a
      ### "long time" from now. certainly after many other operations, so
@@ -1229,7 +1239,8 @@
   record_prop_change(file->pool, file, name, value);
 
   /* do the CHECKOUT sooner rather than later */
-  SVN_ERR(checkout_resource(file->cc, file->rsrc, TRUE, file->token, pool));
+  SVN_ERR(checkout_resource(file->cc, file->rsrc, TRUE,
+                            file->token, FALSE, pool));
 
   /* Add this path to the valid targets hash. */
   add_valid_target(file->cc, file->rsrc->local_path, svn_nonrecursive);
@@ -1365,8 +1376,7 @@
                                     apr_pool_t *pool)
 {
   const char *vcc;
-  const svn_string_t *baseline_url;
-  version_rsrc_t baseline_rsrc = { SVN_INVALID_REVNUM };
+  version_rsrc_t vcc_rsrc = { SVN_INVALID_REVNUM };
   svn_error_t *err = NULL;
   int retry_count = 5;
 
@@ -1376,24 +1386,17 @@
   /* fetch the DAV:version-controlled-configuration from the session's URL */
   SVN_ERR(svn_ra_neon__get_vcc(&vcc, cc->ras, cc->ras->root.path, pool));
 
-  /* ### we should use DAV:apply-to-version on the CHECKOUT so we can skip
-     ### retrieval of the baseline */
 
   do {
 
     svn_error_clear(err);
 
-    /* Get the latest baseline from VCC's DAV:checked-in property.
-       This should give us the HEAD revision of the moment. */
-    SVN_ERR(svn_ra_neon__get_one_prop(&baseline_url, cc->ras,
-                                      vcc, NULL,
-                                      &svn_ra_neon__checked_in_prop, pool));
-    baseline_rsrc.pool = pool;
-    baseline_rsrc.vsn_url = baseline_url->data;
+    vcc_rsrc.pool = pool;
+    vcc_rsrc.vsn_url = vcc;
 
     /* To set the revision properties, we must checkout the latest baseline
        and get back a mutable "working" baseline.  */
-    err = checkout_resource(cc, &baseline_rsrc, FALSE, NULL, pool);
+    err = checkout_resource(cc, &vcc_rsrc, FALSE, NULL, TRUE, pool);
 
     /* There's a small chance of a race condition here, if apache is
        experiencing heavy commit concurrency or if the network has
@@ -1413,7 +1416,7 @@
   if (err)
     return err;
 
-  return svn_ra_neon__do_proppatch(cc->ras, baseline_rsrc.wr_url, 
revprop_table,
+  return svn_ra_neon__do_proppatch(cc->ras, vcc_rsrc.wr_url, revprop_table,
                                    NULL, NULL, pool);
 }
 

Reply via email to