Re: [PATCH] Accept --force for switch --relocate was Re: Possible SVN Relocate bug

On 3/14/07, Peter Lundblad <plundblad@google.com> wrote:
> What about changing the validator signature to take an expected repo
> root instead of this root flag. (I guess I didn't think of this scenario
> when implementing this, or I'm missin something now;). Or isn't it possible
> to do svn_ra_get_repos_root on an unreadable root (I hope it is).

Well, the validator function already takes in a 'const char *url'
right now - but I believe the issue is that with this configuration,
you have to call get_repos_root from the URL you have access to - not
the repository root (i.e. entry2.url instead of entry2.repos). Hence,
libsvn_wc has to provide its base so we can call get_repos_root from
that as svn_ra_get_repos_root() *will* fail if you don't have access
to the root. Then, you can confirm that the roots are identical.

That said, is the patch below what you had in mind?

> This would avoid the problem altogether without the user having to run with
> --force.

My hunch is that supporting --force is also a good thing if the user
really really knows that the server is right, but it's currently
unreachable. Yes, it allows them to shoot themselves in the foot, but
that's what --force is for - to let them override our safety checks
which aren't always accurate. So, I'd still like to add --force
irregardless of fixing this issue.

Thanks. -- justin

----
Allow switch --relocate to work against directly unreadable repos roots.
Instead we need to call svn_ra_get_repos_root with the base URL as that is
expected to be readable.
* subversion/include/svn_wc.h
  (svn_wc_relocation_validator3_t): New prototype.
  (svn_wc_relocation_validator2_t): Deprecate.
  (svn_wc_relocate3): New prototype.
  (svn_wc_relocate2): Deprecate.
* subversion/libsvn_wc/relocate.c
  (relocate_entry): Take in svn_wc_relocation_validator3_t and send in new
  args; only do the repos root change if our URL changed too.
  (svn_wc_relocate3): Take in new prototype.
  (compat2_validator, compat_validator): Wrap around validator3_t.
  (svn_wc_relocate2, svn_wc_relocate1): Wrap around relocate3().
* subversion/libsvn_client/relocate.c
  (validator_func): Adjust for prototype change.
  (svn_client_relocate): Call new wc function.
Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h	(revision 23808)
+++ subversion/include/svn_wc.h	(working copy)
@@ -3449,8 +3449,19 @@
  * is the repository root.  Else, it can be an URL inside the repository.
  * @a pool may be used for temporary allocations.
  *
- * @since New in 1.4.
+ * @since New in 1.5.
  */
+typedef svn_error_t *(*svn_wc_relocation_validator3_t)(void *baton,
+                                                       const char *uuid,
+                                                       const char *url,
+                                                       const char *root_url,
+                                                       apr_pool_t *pool);
+
+/** Similar to @c svn_wc_relocation_validator3_t, but without
+ * the @a root_url arguments.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.4 API.
+ */
 typedef svn_error_t *(*svn_wc_relocation_validator2_t)(void *baton,
                                                        const char *uuid,
                                                        const char *url,
@@ -3476,6 +3487,19 @@
  * @a path.
  */
 svn_error_t *
+svn_wc_relocate3(const char *path,
+                 svn_wc_adm_access_t *adm_access,
+                 const char *from,
+                 const char *to,
+                 svn_boolean_t recurse,
+                 svn_wc_relocation_validator3_t validator,
+                 void *validator_baton,
+                 apr_pool_t *pool);
+
+/** Similar to svn_wc_relocate3(), but uses @c svn_wc_relocation_validator2_t.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.4 API. */
+svn_error_t *
 svn_wc_relocate2(const char *path,
                  svn_wc_adm_access_t *adm_access,
                  const char *from,
Index: subversion/libsvn_wc/relocate.c
===================================================================
--- subversion/libsvn_wc/relocate.c	(revision 23808)
+++ subversion/libsvn_wc/relocate.c	(working copy)
@@ -42,7 +42,7 @@
                const svn_wc_entry_t *entry,
                const char *from,
                const char *to,
-               svn_wc_relocation_validator2_t validator,
+               svn_wc_relocation_validator3_t validator,
                void *validator_baton,
                svn_boolean_t do_sync,
                apr_pool_t *pool)
@@ -51,8 +51,17 @@
   apr_uint64_t flags = 0;
   apr_size_t from_len = strlen(from);
-  if (entry->repos)
+  if (entry->url && ! strncmp(entry->url, from, from_len))
     {
+      entry2.url = apr_pstrcat(pool, to, entry->url + from_len, NULL);
+      if (entry->uuid)
+        SVN_ERR(validator(validator_baton, entry->uuid, entry2.url, NULL,
+                          pool));
+      flags |= SVN_WC__ENTRY_MODIFY_URL;
+    }
+
+  if (entry->repos && (flags & SVN_WC__ENTRY_MODIFY_URL))
+    {
       /* We can't relocate beyond the repository root, but the user is allowed
          to specify a redundant part of the fs path in from and to, but only
          if this part is identical in both strings. */
@@ -79,27 +88,18 @@
           entry2.repos = apr_pstrcat(pool, to, entry->repos + from_len, NULL);
           flags |= SVN_WC__ENTRY_MODIFY_REPOS;
           /* Make sure to is really the repository root. */
-          SVN_ERR(validator(validator_baton, entry->uuid, entry2.repos,
-                            TRUE, pool));
+          SVN_ERR(validator(validator_baton, entry->uuid, entry2.url,
+                            entry2.repos, pool));
         }
     }
-  if (entry->url && ! strncmp(entry->url, from, from_len))
-    {
-      entry2.url = apr_pstrcat(pool, to, entry->url + from_len, NULL);
-      if (entry->uuid)
-        SVN_ERR(validator(validator_baton, entry->uuid, entry2.url, FALSE,
-                          pool));
-      flags |= SVN_WC__ENTRY_MODIFY_URL;
-    }
-
   if (entry->copyfrom_url && ! strncmp(entry->copyfrom_url, from, from_len))
     {
       entry2.copyfrom_url = apr_pstrcat(pool, to,
                                         entry->copyfrom_url + from_len, NULL);
       if (entry->uuid)
         SVN_ERR(validator(validator_baton, entry->uuid,
-                          entry2.copyfrom_url, FALSE, pool));
+                          entry2.copyfrom_url, NULL, pool));
       flags |= SVN_WC__ENTRY_MODIFY_COPYFROM_URL;
     }
@@ -110,12 +110,12 @@
 }
 svn_error_t *
-svn_wc_relocate2(const char *path,
+svn_wc_relocate3(const char *path,
                  svn_wc_adm_access_t *adm_access,
                  const char *from,
                  const char *to,
                  svn_boolean_t recurse,
-                 svn_wc_relocation_validator2_t validator,
+                 svn_wc_relocation_validator3_t validator,
                  void *validator_baton,
                  apr_pool_t *pool)
 {
@@ -171,7 +171,7 @@
             continue;
           SVN_ERR(svn_wc_adm_retrieve(&subdir_access, adm_access,
                                       subdir, subpool));
-          SVN_ERR(svn_wc_relocate2(subdir, subdir_access, from, to,
+          SVN_ERR(svn_wc_relocate3(subdir, subdir_access, from, to,
                                    recurse, validator,
                                    validator_baton, subpool));
         }
@@ -187,17 +187,38 @@
 }
 /* Compatibility baton and wrapper. */
+struct compat2_baton {
+  svn_wc_relocation_validator2_t validator;
+  void *baton;
+};
+
+/* Compatibility baton and wrapper. */
 struct compat_baton {
   svn_wc_relocation_validator_t validator;
   void *baton;
 };
-/* This implements svn_wc_relocate_validator2_t. */
+/* This implements svn_wc_relocate_validator3_t. */
 static svn_error_t *
+compat2_validator(void *baton,
+                  const char *uuid,
+                  const char *url,
+                  const char *root_url,
+                  apr_pool_t *pool)
+{
+  struct compat2_baton *cb = baton;
+  /* The old callback type doesn't set root_url. */
+  return cb->validator(cb->baton, uuid,
+                       (root_url ? root_url : url), (root_url ? TRUE : FALSE),
+                       pool);
+}
+
+/* This implements svn_wc_relocate_validator3_t. */
+static svn_error_t *
 compat_validator(void *baton,
                  const char *uuid,
                  const char *url,
-                 svn_boolean_t root,
+                 const char *root_url,
                  apr_pool_t *pool)
 {
   struct compat_baton *cb = baton;
@@ -208,6 +229,25 @@
 }
 svn_error_t *
+svn_wc_relocate2(const char *path,
+                 svn_wc_adm_access_t *adm_access,
+                 const char *from,
+                 const char *to,
+                 svn_boolean_t recurse,
+                 svn_wc_relocation_validator2_t validator,
+                 void *validator_baton,
+                 apr_pool_t *pool)
+{
+  struct compat2_baton cb;
+
+  cb.validator = validator;
+  cb.baton = validator_baton;
+
+  return svn_wc_relocate3(path, adm_access, from, to, recurse,
+                          compat_validator, &cb, pool);
+}
+
+svn_error_t *
 svn_wc_relocate(const char *path,
                 svn_wc_adm_access_t *adm_access,
                 const char *from,
@@ -222,6 +262,6 @@
   cb.validator = validator;
   cb.baton = validator_baton;
-  return svn_wc_relocate2(path, adm_access, from, to, recurse,
+  return svn_wc_relocate3(path, adm_access, from, to, recurse,
                           compat_validator, &cb, pool);
 }
Index: subversion/libsvn_client/relocate.c
===================================================================
--- subversion/libsvn_client/relocate.c	(revision 23808)
+++ subversion/libsvn_client/relocate.c	(working copy)
@@ -55,7 +55,7 @@
 validator_func(void *baton,
                const char *uuid,
                const char *url,
-               svn_boolean_t root,
+               const char *root_url,
                apr_pool_t *pool)
 {
   struct validator_baton_t *b = baton;
@@ -95,8 +95,8 @@
     }
   /* Make sure the url is a repository root if desired. */
-  if (root
-      && strcmp(url, url_uuid->root) != 0)
+  if (root_url
+      && strcmp(root_url, url_uuid->root) != 0)
     return svn_error_createf(SVN_ERR_CLIENT_INVALID_RELOCATION, NULL,
                              _("'%s' is not the root of the repository"),
                              url);
@@ -133,7 +133,7 @@
   vb.path = path;
   vb.url_uuids = apr_array_make(pool, 1, sizeof(struct url_uuid_t));
   vb.pool = pool;
-  SVN_ERR(svn_wc_relocate2(path, adm_access, from, to,
+  SVN_ERR(svn_wc_relocate3(path, adm_access, from, to,
                            recurse, validator_func, &vb, pool));
   /* All done.  Clean up, and move on out. */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org