Hi, is this any good?

~Neels
Try to find proper URL information on externals during upgrade. This is only
for same-repos externals. For externals from a different repos, it is not so
easy to find which is the repository root and which is the relpath.

Issue #4016.

* subversion/libsvn_wc/wc_db.c
  (svn_wc__db_upgrade_apply_props): 
    Use svn_wc__resolve_relative_external_url() to fill in the URL parts of
    EXTERNALS rows created during upgrade.

* subversion/include/private/svn_wc_private.h,
* subversion/libsvn_wc/externals.c
  (svn_wc__resolve_relative_external_url): Move here and rename from ...
* subversion/libsvn_client/externals.c
  (resolve_relative_external_url): ... this.
  (uri_scheme): Move along with resolve_relative_external_url, keep as static.
  (handle_external_item_change, svn_client__export_externals): Apply rename.

Index: subversion/include/private/svn_wc_private.h
===================================================================
--- subversion/include/private/svn_wc_private.h (revision 1172371)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -1169,6 +1169,14 @@ svn_wc__node_was_moved_here(const char *
                             apr_pool_t *result_pool,
                             apr_pool_t *scratch_pool);
 
+
+svn_error_t *
+svn_wc__resolve_relative_external_url(const char **resolved_url,
+                                      const svn_wc_external_item2_t *item,
+                                      const char *repos_root_url,
+                                      const char *parent_dir_url,
+                                      apr_pool_t *result_pool,
+                                      apr_pool_t *scratch_pool);
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
Index: subversion/libsvn_client/externals.c
===================================================================
--- subversion/libsvn_client/externals.c        (revision 1172371)
+++ subversion/libsvn_client/externals.c        (working copy)
@@ -27,7 +27,6 @@
 
 /*** Includes. ***/
 
-#include <apr_uri.h>
 #include "svn_wc.h"
 #include "svn_pools.h"
 #include "svn_client.h"
@@ -512,218 +511,6 @@ cleanup:
   return svn_error_trace(err);
 }
 
-/* Return the scheme of @a uri in @a scheme allocated from @a pool.
-   If @a uri does not appear to be a valid URI, then @a scheme will
-   not be updated.  */
-static svn_error_t *
-uri_scheme(const char **scheme, const char *uri, apr_pool_t *pool)
-{
-  apr_size_t i;
-
-  for (i = 0; uri[i] && uri[i] != ':'; ++i)
-    if (uri[i] == '/')
-      goto error;
-
-  if (i > 0 && uri[i] == ':' && uri[i+1] == '/' && uri[i+2] == '/')
-    {
-      *scheme = apr_pstrmemdup(pool, uri, i);
-      return SVN_NO_ERROR;
-    }
-
-error:
-  return svn_error_createf(SVN_ERR_BAD_URL, 0,
-                           _("URL '%s' does not begin with a scheme"),
-                           uri);
-}
-
-/* If the URL for @a item is relative, then using the repository root
-   URL @a repos_root_url and the parent directory URL @parent_dir_url,
-   resolve it into an absolute URL and save it in @a item.
-
-   Regardless if the URL is absolute or not, if there are no errors,
-   the URL in @a item will be canonicalized.
-
-   The following relative URL formats are supported:
-
-     ../    relative to the parent directory of the external
-     ^/     relative to the repository root
-     //     relative to the scheme
-     /      relative to the server's hostname
-
-   The ../ and ^/ relative URLs may use .. to remove path elements up
-   to the server root.
-
-   The external URL should not be canonicalized before calling this function,
-   as otherwise the scheme relative URL '//host/some/path' would have been
-   canonicalized to '/host/some/path' and we would not be able to match on
-   the leading '//'. */
-static svn_error_t *
-resolve_relative_external_url(const char **resolved_url,
-                              const svn_wc_external_item2_t *item,
-                              const char *repos_root_url,
-                              const char *parent_dir_url,
-                              apr_pool_t *result_pool,
-                              apr_pool_t *scratch_pool)
-{
-  const char *url = item->url;
-  apr_uri_t parent_dir_uri;
-  apr_status_t status;
-
-  *resolved_url = item->url;
-
-  /* If the URL is already absolute, there is nothing to do. */
-  if (svn_path_is_url(url))
-    {
-      /* "http://server/path"; */
-      *resolved_url = svn_uri_canonicalize(url, result_pool);
-      return SVN_NO_ERROR;
-    }
-
-  if (url[0] == '/')
-    {
-      /* "/path", "//path", and "///path" */
-      int num_leading_slashes = 1;
-      if (url[1] == '/')
-        {
-          num_leading_slashes++;
-          if (url[2] == '/')
-            num_leading_slashes++;
-        }
-
-      /* "//schema-relative" and in some cases "///schema-relative".
-         This last format is supported on file:// schema relative. */
-      url = apr_pstrcat(scratch_pool,
-                        apr_pstrndup(scratch_pool, url, num_leading_slashes),
-                        svn_relpath_canonicalize(url + num_leading_slashes,
-                                                 scratch_pool),
-                        (char*)NULL);
-    }
-  else
-    {
-      /* "^/path" and "../path" */
-      url = svn_relpath_canonicalize(url, scratch_pool);
-    }
-
-  /* Parse the parent directory URL into its parts. */
-  status = apr_uri_parse(scratch_pool, parent_dir_url, &parent_dir_uri);
-  if (status)
-    return svn_error_createf(SVN_ERR_BAD_URL, 0,
-                             _("Illegal parent directory URL '%s'"),
-                             parent_dir_url);
-
-  /* If the parent directory URL is at the server root, then the URL
-     may have no / after the hostname so apr_uri_parse() will leave
-     the URL's path as NULL. */
-  if (! parent_dir_uri.path)
-    parent_dir_uri.path = apr_pstrmemdup(scratch_pool, "/", 1);
-  parent_dir_uri.query = NULL;
-  parent_dir_uri.fragment = NULL;
-
-  /* Handle URLs relative to the current directory or to the
-     repository root.  The backpaths may only remove path elements,
-     not the hostname.  This allows an external to refer to another
-     repository in the same server relative to the location of this
-     repository, say using SVNParentPath. */
-  if ((0 == strncmp("../", url, 3)) ||
-      (0 == strncmp("^/", url, 2)))
-    {
-      apr_array_header_t *base_components;
-      apr_array_header_t *relative_components;
-      int i;
-
-      /* Decompose either the parent directory's URL path or the
-         repository root's URL path into components.  */
-      if (0 == strncmp("../", url, 3))
-        {
-          base_components = svn_path_decompose(parent_dir_uri.path,
-                                               scratch_pool);
-          relative_components = svn_path_decompose(url, scratch_pool);
-        }
-      else
-        {
-          apr_uri_t repos_root_uri;
-
-          status = apr_uri_parse(scratch_pool, repos_root_url,
-                                 &repos_root_uri);
-          if (status)
-            return svn_error_createf(SVN_ERR_BAD_URL, 0,
-                                     _("Illegal repository root URL '%s'"),
-                                     repos_root_url);
-
-          /* If the repository root URL is at the server root, then
-             the URL may have no / after the hostname so
-             apr_uri_parse() will leave the URL's path as NULL. */
-          if (! repos_root_uri.path)
-            repos_root_uri.path = apr_pstrmemdup(scratch_pool, "/", 1);
-
-          base_components = svn_path_decompose(repos_root_uri.path,
-                                               scratch_pool);
-          relative_components = svn_path_decompose(url + 2, scratch_pool);
-        }
-
-      for (i = 0; i < relative_components->nelts; ++i)
-        {
-          const char *component = APR_ARRAY_IDX(relative_components,
-                                                i,
-                                                const char *);
-          if (0 == strcmp("..", component))
-            {
-              /* Constructing the final absolute URL together with
-                 apr_uri_unparse() requires that the path be absolute,
-                 so only pop a component if the component being popped
-                 is not the component for the root directory. */
-              if (base_components->nelts > 1)
-                apr_array_pop(base_components);
-            }
-          else
-            APR_ARRAY_PUSH(base_components, const char *) = component;
-        }
-
-      parent_dir_uri.path = (char *)svn_path_compose(base_components,
-                                                     scratch_pool);
-      *resolved_url = svn_uri_canonicalize(apr_uri_unparse(scratch_pool,
-                                                           &parent_dir_uri, 0),
-                                       result_pool);
-      return SVN_NO_ERROR;
-    }
-
-  /* The remaining URLs are relative to the either the scheme or
-     server root and can only refer to locations inside that scope, so
-     backpaths are not allowed. */
-  if (svn_path_is_backpath_present(url + 2))
-    return svn_error_createf(SVN_ERR_BAD_URL, 0,
-                             _("The external relative URL '%s' cannot have "
-                               "backpaths, i.e. '..'"),
-                             item->url);
-
-  /* Relative to the scheme: Build a new URL from the parts we know.  */
-  if (0 == strncmp("//", url, 2))
-    {
-      const char *scheme;
-
-      SVN_ERR(uri_scheme(&scheme, repos_root_url, scratch_pool));
-      *resolved_url = svn_uri_canonicalize(apr_pstrcat(scratch_pool, scheme,
-                                                       ":", url, (char *)NULL),
-                                           result_pool);
-      return SVN_NO_ERROR;
-    }
-
-  /* Relative to the server root: Just replace the path portion of the
-     parent's URL.  */
-  if (url[0] == '/')
-    {
-      parent_dir_uri.path = (char *)url;
-      *resolved_url = svn_uri_canonicalize(apr_uri_unparse(scratch_pool,
-                                                           &parent_dir_uri, 0),
-                                           result_pool);
-      return SVN_NO_ERROR;
-    }
-
-  return svn_error_createf(SVN_ERR_BAD_URL, 0,
-                           _("Unrecognized format for the relative external "
-                             "URL '%s'"),
-                           item->url);
-}
 
 static svn_error_t *
 handle_external_item_removal(const struct external_change_baton_t *eb,
@@ -832,7 +619,7 @@ handle_external_item_change(const struct
      iterpool, since the hash table values outlive the iterpool and
      any pointers they have should also outlive the iterpool.  */
 
-  SVN_ERR(resolve_relative_external_url(&new_url,
+  SVN_ERR(svn_wc__resolve_relative_external_url(&new_url,
                                         new_item, eb->repos_root_url,
                                         parent_dir_url,
                                         scratch_pool, scratch_pool));
@@ -1232,7 +1019,7 @@ svn_client__export_externals(apr_hash_t
           item_abspath = svn_dirent_join(local_abspath, item->target_dir,
                                          sub_iterpool);
 
-          SVN_ERR(resolve_relative_external_url(&new_url, item,
+          SVN_ERR(svn_wc__resolve_relative_external_url(&new_url, item,
                                                 repos_root_url, dir_url,
                                                 sub_iterpool, sub_iterpool));
 
Index: subversion/libsvn_wc/externals.c
===================================================================
--- subversion/libsvn_wc/externals.c    (revision 1172371)
+++ subversion/libsvn_wc/externals.c    (working copy)
@@ -30,6 +30,7 @@
 #include <apr_hash.h>
 #include <apr_tables.h>
 #include <apr_general.h>
+#include <apr_uri.h>
 
 #include "svn_dirent_uri.h"
 #include "svn_path.h"
@@ -1248,3 +1249,219 @@ svn_wc__externals_gather_definitions(apr
       return SVN_NO_ERROR;
     }
 }
+
+/* Return the scheme of @a uri in @a scheme allocated from @a pool.
+   If @a uri does not appear to be a valid URI, then @a scheme will
+   not be updated.  */
+static svn_error_t *
+uri_scheme(const char **scheme, const char *uri, apr_pool_t *pool)
+{
+  apr_size_t i;
+
+  for (i = 0; uri[i] && uri[i] != ':'; ++i)
+    if (uri[i] == '/')
+      goto error;
+
+  if (i > 0 && uri[i] == ':' && uri[i+1] == '/' && uri[i+2] == '/')
+    {
+      *scheme = apr_pstrmemdup(pool, uri, i);
+      return SVN_NO_ERROR;
+    }
+
+error:
+  return svn_error_createf(SVN_ERR_BAD_URL, 0,
+                           _("URL '%s' does not begin with a scheme"),
+                           uri);
+}
+
+
+
+/* If the URL for @a item is relative, then using the repository root
+   URL @a repos_root_url and the parent directory URL @parent_dir_url,
+   resolve it into an absolute URL and save it in @a item.
+
+   Regardless if the URL is absolute or not, if there are no errors,
+   the URL in @a item will be canonicalized.
+
+   The following relative URL formats are supported:
+
+     ../    relative to the parent directory of the external
+     ^/     relative to the repository root
+     //     relative to the scheme
+     /      relative to the server's hostname
+
+   The ../ and ^/ relative URLs may use .. to remove path elements up
+   to the server root.
+
+   The external URL should not be canonicalized before calling this function,
+   as otherwise the scheme relative URL '//host/some/path' would have been
+   canonicalized to '/host/some/path' and we would not be able to match on
+   the leading '//'. */
+svn_error_t *
+svn_wc__resolve_relative_external_url(const char **resolved_url,
+                                      const svn_wc_external_item2_t *item,
+                                      const char *repos_root_url,
+                                      const char *parent_dir_url,
+                                      apr_pool_t *result_pool,
+                                      apr_pool_t *scratch_pool)
+{
+  const char *url = item->url;
+  apr_uri_t parent_dir_uri;
+  apr_status_t status;
+
+  *resolved_url = item->url;
+
+  /* If the URL is already absolute, there is nothing to do. */
+  if (svn_path_is_url(url))
+    {
+      /* "http://server/path"; */
+      *resolved_url = svn_uri_canonicalize(url, result_pool);
+      return SVN_NO_ERROR;
+    }
+
+  if (url[0] == '/')
+    {
+      /* "/path", "//path", and "///path" */
+      int num_leading_slashes = 1;
+      if (url[1] == '/')
+        {
+          num_leading_slashes++;
+          if (url[2] == '/')
+            num_leading_slashes++;
+        }
+
+      /* "//schema-relative" and in some cases "///schema-relative".
+         This last format is supported on file:// schema relative. */
+      url = apr_pstrcat(scratch_pool,
+                        apr_pstrndup(scratch_pool, url, num_leading_slashes),
+                        svn_relpath_canonicalize(url + num_leading_slashes,
+                                                 scratch_pool),
+                        (char*)NULL);
+    }
+  else
+    {
+      /* "^/path" and "../path" */
+      url = svn_relpath_canonicalize(url, scratch_pool);
+    }
+
+  /* Parse the parent directory URL into its parts. */
+  status = apr_uri_parse(scratch_pool, parent_dir_url, &parent_dir_uri);
+  if (status)
+    return svn_error_createf(SVN_ERR_BAD_URL, 0,
+                             _("Illegal parent directory URL '%s'"),
+                             parent_dir_url);
+
+  /* If the parent directory URL is at the server root, then the URL
+     may have no / after the hostname so apr_uri_parse() will leave
+     the URL's path as NULL. */
+  if (! parent_dir_uri.path)
+    parent_dir_uri.path = apr_pstrmemdup(scratch_pool, "/", 1);
+  parent_dir_uri.query = NULL;
+  parent_dir_uri.fragment = NULL;
+
+  /* Handle URLs relative to the current directory or to the
+     repository root.  The backpaths may only remove path elements,
+     not the hostname.  This allows an external to refer to another
+     repository in the same server relative to the location of this
+     repository, say using SVNParentPath. */
+  if ((0 == strncmp("../", url, 3)) ||
+      (0 == strncmp("^/", url, 2)))
+    {
+      apr_array_header_t *base_components;
+      apr_array_header_t *relative_components;
+      int i;
+
+      /* Decompose either the parent directory's URL path or the
+         repository root's URL path into components.  */
+      if (0 == strncmp("../", url, 3))
+        {
+          base_components = svn_path_decompose(parent_dir_uri.path,
+                                               scratch_pool);
+          relative_components = svn_path_decompose(url, scratch_pool);
+        }
+      else
+        {
+          apr_uri_t repos_root_uri;
+
+          status = apr_uri_parse(scratch_pool, repos_root_url,
+                                 &repos_root_uri);
+          if (status)
+            return svn_error_createf(SVN_ERR_BAD_URL, 0,
+                                     _("Illegal repository root URL '%s'"),
+                                     repos_root_url);
+
+          /* If the repository root URL is at the server root, then
+             the URL may have no / after the hostname so
+             apr_uri_parse() will leave the URL's path as NULL. */
+          if (! repos_root_uri.path)
+            repos_root_uri.path = apr_pstrmemdup(scratch_pool, "/", 1);
+
+          base_components = svn_path_decompose(repos_root_uri.path,
+                                               scratch_pool);
+          relative_components = svn_path_decompose(url + 2, scratch_pool);
+        }
+
+      for (i = 0; i < relative_components->nelts; ++i)
+        {
+          const char *component = APR_ARRAY_IDX(relative_components,
+                                                i,
+                                                const char *);
+          if (0 == strcmp("..", component))
+            {
+              /* Constructing the final absolute URL together with
+                 apr_uri_unparse() requires that the path be absolute,
+                 so only pop a component if the component being popped
+                 is not the component for the root directory. */
+              if (base_components->nelts > 1)
+                apr_array_pop(base_components);
+            }
+          else
+            APR_ARRAY_PUSH(base_components, const char *) = component;
+        }
+
+      parent_dir_uri.path = (char *)svn_path_compose(base_components,
+                                                     scratch_pool);
+      *resolved_url = svn_uri_canonicalize(apr_uri_unparse(scratch_pool,
+                                                           &parent_dir_uri, 0),
+                                       result_pool);
+      return SVN_NO_ERROR;
+    }
+
+  /* The remaining URLs are relative to the either the scheme or
+     server root and can only refer to locations inside that scope, so
+     backpaths are not allowed. */
+  if (svn_path_is_backpath_present(url + 2))
+    return svn_error_createf(SVN_ERR_BAD_URL, 0,
+                             _("The external relative URL '%s' cannot have "
+                               "backpaths, i.e. '..'"),
+                             item->url);
+
+  /* Relative to the scheme: Build a new URL from the parts we know.  */
+  if (0 == strncmp("//", url, 2))
+    {
+      const char *scheme;
+
+      SVN_ERR(uri_scheme(&scheme, repos_root_url, scratch_pool));
+      *resolved_url = svn_uri_canonicalize(apr_pstrcat(scratch_pool, scheme,
+                                                       ":", url, (char *)NULL),
+                                           result_pool);
+      return SVN_NO_ERROR;
+    }
+
+  /* Relative to the server root: Just replace the path portion of the
+     parent's URL.  */
+  if (url[0] == '/')
+    {
+      parent_dir_uri.path = (char *)url;
+      *resolved_url = svn_uri_canonicalize(apr_uri_unparse(scratch_pool,
+                                                           &parent_dir_uri, 0),
+                                           result_pool);
+      return SVN_NO_ERROR;
+    }
+
+  return svn_error_createf(SVN_ERR_BAD_URL, 0,
+                           _("Unrecognized format for the relative external "
+                             "URL '%s'"),
+                           item->url);
+}
+
Index: subversion/libsvn_wc/wc_db.c
===================================================================
--- subversion/libsvn_wc/wc_db.c        (revision 1172371)
+++ subversion/libsvn_wc/wc_db.c        (working copy)
@@ -10241,6 +10241,7 @@ svn_wc__db_upgrade_apply_props(svn_sqlit
   svn_wc__db_kind_t kind = svn_wc__db_kind_unknown;
   int affected_rows;
 
+
   /* ### working_props: use set_props_txn.
      ### if working_props == NULL, then skip. what if they equal the
      ### pristine props? we should probably do the compare here.
@@ -10378,23 +10379,80 @@ svn_wc__db_upgrade_apply_props(svn_sqlit
         {
           int i;
           apr_array_header_t *ext;
+          const char *node_repos_relpath = "";
+          const char *node_repos_root_url = "";
+          const char *node_url = "";
+          apr_int64_t node_repos_id;
+          const char *local_abspath = svn_dirent_join(dir_abspath,
+                                                      local_relpath,
+                                                      scratch_pool);
+          SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
+                                            STMT_SELECT_BASE_NODE));
+          SVN_ERR(svn_sqlite__bindf(stmt, "is", wc_id, local_relpath));
+          SVN_ERR(svn_sqlite__step(&have_row, stmt));
+          if (have_row)
+            {
+              svn_error_t *err;
+              const char *repos_relpath;
+              err = repos_location_from_columns(&node_repos_id, NULL, 
&repos_relpath,
+                                                stmt, 0, 4, 1, scratch_pool);
+              if (err)
+                svn_error_clear(err);
+              else
+                {
+                  node_repos_relpath = repos_relpath;
+                  SVN_ERR(fetch_repos_info(&node_repos_root_url, NULL, sdb,
+                                           node_repos_id, scratch_pool));
+                  SVN_DBG(("local_relpath=%s url=%s / %s\n",
+                           local_relpath, node_repos_root_url,
+                           node_repos_relpath));
+                }
+            }
+          svn_sqlite__reset(stmt);
+
 
           SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
                                             STMT_INSERT_EXTERNAL_UPGRADE));
 
-          SVN_ERR(svn_wc_parse_externals_description3(
-                            &ext, svn_dirent_join(dir_abspath, local_relpath,
-                                                  scratch_pool),
-                            externals, FALSE, scratch_pool));
+          SVN_ERR(svn_wc_parse_externals_description3(&ext, local_abspath,
+                                                      externals, FALSE,
+                                                      scratch_pool));
           for (i = 0; i < ext->nelts; i++)
             {
               const svn_wc_external_item2_t *item;
               const char *item_relpath;
+              const char *item_resolved_url;
+              const char *item_repos_relpath;
 
               item = APR_ARRAY_IDX(ext, i, const svn_wc_external_item2_t *);
               item_relpath = svn_relpath_join(local_relpath, item->target_dir,
                                               scratch_pool);
 
+              node_url = svn_path_url_add_component2(node_repos_root_url,
+                                                     node_repos_relpath,
+                                                     scratch_pool);
+
+              SVN_ERR(svn_wc__resolve_relative_external_url(&item_resolved_url,
+                                                            item,
+                                                            
node_repos_root_url,
+                                                            node_url,
+                                                            scratch_pool,
+                                                            scratch_pool));
+
+              item_repos_relpath = svn_uri_skip_ancestor(node_repos_root_url,
+                                                         item_resolved_url,
+                                                         scratch_pool);
+
+              if (item_repos_relpath == NULL)
+                {
+                  /* It's from another repository.
+                   * At this point we would have to contact the repository and
+                   * find out its UUID and root. But the same information may
+                   * already be lying around in a checked out external dir.
+                   * ... currently, this is just wrong: */
+                  item_repos_relpath = "";
+                }
+
               SVN_ERR(svn_sqlite__bindf(stmt, "issssis",
                                         wc_id,
                                         item_relpath,
@@ -10402,8 +10460,8 @@ svn_wc__db_upgrade_apply_props(svn_sqlit
                                                             scratch_pool),
                                         "normal",
                                         local_relpath,
-                                        (apr_int64_t)1, /* repos_id */
-                                        "" /* repos_relpath */));
+                                        node_repos_id,
+                                        item_repos_relpath));
 
               SVN_ERR(svn_sqlite__insert(NULL, stmt));
             }

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to