Re: [PATCH] v2 #3483 - Extend svn_client_upgrade() to include externals

On Tue, Jan 19, 2010 at 04:01:47PM +0100, Daniel Näslund wrote:
> Ping!
>
> This patch hasn't been reviewed.

I'll leave this to externals/wc-ng experts to commit.

Small review:

> > @@ -69,5 +76,78 @@
> > 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

s/causes/cause/

> > + 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);
> > +
> > + SVN_ERR(svn_wc_parse_externals_description3(&externals_p,
> > + svn_dirent_dirname(path,
> > + iterpool),
> > + external_desc->data, TRUE,
> > + iterpool));

Maybe indent the above like this:

      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,
> > + ctx->cancel_func, ctx->cancel_baton,
> > + ctx->notify_func2, ctx->notify_baton2,
> > + iterpool));
> > + svn_error_clear(err);
> > + }
> > + else if (err)
> > + return svn_error_return(err);
> > + else
> > + ;

You can replace the 'else if (err) ... else ;' with just:

 else
  SVN_ERR(err);

Stefan

> > + }
> > + }
> > +
> > + svn_pool_destroy(iterpool);
> > +
> > return SVN_NO_ERROR;
> > }

-- 
printf("Eh???/n");