On Sun, 2010-03-07, dannas_at_apache.org wrote:
> Author: dannas
> Date: Sun Mar 7 21:21:58 2010
> New Revision: 920118
>
> URL: http://svn.apache.org/viewvc?rev=920118&view=rev
> Log:
> Fix issue #3483 - extend svn_client_upgrade() to include externals. I've
> done the externals upgrading after wc upgrade is finished. In that way
> no errors in the externals will affect the wc.
Hi Daniel. A few post-commit review comments.
> * subversion/libsvn_client/cleanup.c
> (svn_client_upgrade): Get all svn:externals. We need the target_dir so
> we have to parse the description. For each target_dir we call
> svn_wc_upgrade() which will recursively upgrade the external.
>
> * subversion/tests/cmdline/upgrade_tests.py
> (upgrade_with_externals): New. Checks the format of a 1.6 wc upgraded
> to wc-ng.
> (test_list): Add upgrade with_externals.
>
> * subversion/tests/cmdline/upgrade_tests_data/upgrade_with_extenals.tar.bz2
> (...): New. An 1.6 wc with the same structure as those used in
> externals_tests.py.
>
> Approved by: rhuijben
>
> Added:
> subversion/trunk/subversion/tests/cmdline/upgrade_tests_data/upgrade_with_externals.tar.bz2 (with props)
> Modified:
> subversion/trunk/subversion/libsvn_client/cleanup.c
> subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/cleanup.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/cleanup.c?rev=920118&r1=920117&r2=920118&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/cleanup.c (original)
> +++ subversion/trunk/subversion/libsvn_client/cleanup.c Sun Mar 7 21:21:58 2010
> @@ -34,8 +34,11 @@
> #include "svn_dirent_uri.h"
> #include "svn_pools.h"
> #include "client.h"
> +#include "svn_pools.h"
Included twice now. Bert already included it, just above, a few days
before :-)
> +#include "svn_props.h"
>
> #include "svn_private_config.h"
> +#include "private/svn_wc_private.h"
>
>
> /*** Code. ***/
> @@ -110,6 +113,10 @@
> apr_pool_t *scratch_pool)
> {
> const char *local_abspath;
> + apr_hash_t *externals;
> + apr_hash_index_t *hi;
> + apr_pool_t *iterpool;
> + svn_opt_revision_t rev = {svn_opt_revision_unspecified, {0}};
> struct repos_info_baton info_baton;
> info_baton.pool = scratch_pool;
> info_baton.ctx = ctx;
> @@ -123,5 +130,79 @@
> ctx->notify_func2, ctx->notify_baton2,
> scratch_pool));
>
> + /* Now it's time to upgrade the externals too. We do it after the wc
> + upgrade to avoid that errors in the externals causes the wc upgrade to
> + fail. Thanks to caching the performance penalty of walking the wc a
> + second time shouldn't be too severe */
> + SVN_ERR(svn_client_propget3(&externals, SVN_PROP_EXTERNALS, path, &rev,
> + &rev, NULL, svn_depth_infinity, NULL, ctx,
> + scratch_pool));
> +
> + iterpool = svn_pool_create(scratch_pool);
> +
> + for (hi = apr_hash_first(scratch_pool, externals); hi;
> + hi = apr_hash_next(hi))
> + {
> + const char *key;
> + int i;
> + apr_ssize_t klen;
> + svn_string_t *external_desc;
> + apr_array_header_t *externals_p;
> +
> + svn_pool_clear(iterpool);
> + externals_p = apr_array_make(iterpool, 1,
> + sizeof(svn_wc_external_item2_t*));
> +
> + apr_hash_this(hi, (void*)&key, &klen, NULL);
> +
> + external_desc = apr_hash_get(externals, key, klen);
apr_hash_this() can give you the current item's key and value, so you
don't need to look up the value separately: you can just
apr_hash_this(hi, &key, NULL, &value). However, I discourage use of
apr_hash_this() as it requires (void *) pointers, as you have seen.
A neater solution enables the key and value to be assigned straight in
to variables of the appropriate type:
for (hi = ...)
{
const char *external_path = svn_apr_hash_index_key(hi);
svn_string_t *external_desc = svn_apr_hash_index_value(hi);
...
}
And by calling variable 'external_path' rather than 'key' you can
describe the data it holds. (I see you have an 'external_path' in the
inner loop too, so you might want to choose a different name or
eliminate the inner one.)
> + SVN_ERR(svn_wc_parse_externals_description3(&externals_p,
> + svn_dirent_dirname(path,
> + iterpool),
> + external_desc->data, TRUE,
> + iterpool));
> + for (i = 0; i < externals_p->nelts; i++)
> + {
> + svn_wc_external_item2_t *item;
> + const char *external_abspath;
> + const char *external_path;
> + svn_node_kind_t kind;
> + svn_error_t *err;
> +
> + item = APR_ARRAY_IDX(externals_p, i, svn_wc_external_item2_t*);
> +
> + /* The key is the path to the dir the svn:externals was set on */
> + external_path = svn_dirent_join(key, item->target_dir, iterpool);
> +
> + SVN_ERR(svn_dirent_get_absolute(&external_abspath, external_path,
> + iterpool));
> +
> + /* This is hack. We can only send dirs to svn_wc_upgrade(). This
> + way we will get an exception saying that the wc must be
> + upgraded if it's a dir. If it's a file then the lookup is done
> + in an adm_dir belonging to the real wc and since that was
> + updated before the externals no error is returned. */
> + err = svn_wc__node_get_kind(&kind, ctx->wc_ctx, external_abspath,
> + FALSE, iterpool);
> +
> + if (err && err->apr_err == SVN_ERR_WC_UPGRADE_REQUIRED)
> + {
> + SVN_ERR(svn_wc_upgrade(ctx->wc_ctx, external_abspath,
> + fetch_repos_info, &info_baton,
> + ctx->cancel_func, ctx->cancel_baton,
> + ctx->notify_func2, ctx->notify_baton2,
> + iterpool));
> + svn_error_clear(err);
> + }
[...]
- Julian
Received on 2010-03-09 15:20:29 CET