so here's a slightly nicer patch, no functional change. Thanks for the review, Julian!
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. We would have to add another valid repository id to the database, so can't just hack something up.) 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 1173575) +++ subversion/include/private/svn_wc_private.h (working copy) @@ -1169,6 +1169,35 @@ svn_wc__node_was_moved_here(const char * apr_pool_t *result_pool, apr_pool_t *scratch_pool); +/* 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 *resolved_url. + + Regardless if the URL is absolute or not, if there are no errors, + the URL returned in @a *resolved_url 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); + #ifdef __cplusplus } #endif /* __cplusplus */ Index: subversion/libsvn_client/externals.c =================================================================== --- subversion/libsvn_client/externals.c (revision 1173575) +++ subversion/libsvn_client/externals.c (working copy) @@ -512,219 +512,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 *resolved_url. - - Regardless if the URL is absolute or not, if there are no errors, - the URL returned in @a *resolved_url 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 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, const char *defining_abspath, @@ -832,10 +619,10 @@ 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, - new_item, eb->repos_root_url, - parent_dir_url, - scratch_pool, scratch_pool)); + SVN_ERR(svn_wc__resolve_relative_external_url(&new_url, + new_item, eb->repos_root_url, + parent_dir_url, + scratch_pool, scratch_pool)); /* If the external is being checked out, exported or updated, determine if the external is a file or directory. */ @@ -1232,9 +1019,10 @@ 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, - repos_root_url, dir_url, - sub_iterpool, sub_iterpool)); + SVN_ERR(svn_wc__resolve_relative_external_url(&new_url, item, + repos_root_url, + dir_url, sub_iterpool, + sub_iterpool)); /* The target dir might have multiple components. Guarantee the path leading down to the last component. */ Index: subversion/libsvn_wc/externals.c =================================================================== --- subversion/libsvn_wc/externals.c (revision 1173575) +++ 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,196 @@ 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); +} + +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 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 1173575) +++ 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,96 @@ 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 = ""; + apr_int64_t node_repos_id; + const char *node_url = NULL; + const char *local_abspath = svn_dirent_join(dir_abspath, + local_relpath, + scratch_pool); + + /* Find out the current node's URL: repos root and relpath. I think + * the REPOS_ID will always be 1, but fetch it while at it. (Not the + * external, but the node where svn:externals is defined.) */ + 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)); + node_url = svn_path_url_add_component2(node_repos_root_url, + node_repos_relpath, + scratch_pool); + } + } + 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; + apr_int64_t item_repos_id; + const char *item_repos_relpath = NULL; item = APR_ARRAY_IDX(ext, i, const svn_wc_external_item2_t *); item_relpath = svn_relpath_join(local_relpath, item->target_dir, scratch_pool); + /* If we've found the defining node's URL (I think this will + * always be the case, but let's be conservative), try to figure + * out the external's URL. */ + if (node_url) + { + 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) + { + /* The external is from another repository (or we haven't + * found the defining node's URL). At this point we would + * have to contact the repository of the external 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: put trivial values into + * the database. :( */ + item_repos_id = 1; + item_repos_relpath = ""; + } + else + { + /* An external from the same repository. Same repos id. */ + item_repos_id = node_repos_id; + } + SVN_ERR(svn_sqlite__bindf(stmt, "issssis", wc_id, item_relpath, @@ -10402,8 +10476,8 @@ svn_wc__db_upgrade_apply_props(svn_sqlit scratch_pool), "normal", local_relpath, - (apr_int64_t)1, /* repos_id */ - "" /* repos_relpath */)); + item_repos_id, + item_repos_relpath)); SVN_ERR(svn_sqlite__insert(NULL, stmt)); }
signature.asc
Description: OpenPGP digital signature