On Fri, Jul 3, 2009 at 21:25, Hyrum K. Wright<hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_client/copy.c Fri Jul 3 12:25:44 2009 (r38328)
>...
> @@ -1992,7 +1996,7 @@ svn_client_copy5(svn_commit_info_t **com
> svn_wc_context_t *wc_ctx;
>
> if (!ctx->wc_ctx)
> - SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, subpool,
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool,
> subpool));
This doesn't seem like a proper change. You need to destroy the
context at the end of the function, so the subpool seems the proper
choice. If you were going to stash it into the client context, then
*maybe* pool would be proper.
>...
> +++ trunk/subversion/libsvn_client/merge.c Fri Jul 3 12:25:44 2009 (r38328)
>...
> @@ -7622,6 +7672,12 @@ svn_client_merge3(const char *source1,
> _("'%s' has no URL"),
> svn_dirent_local_style(source2, pool));
>
> + /* Create the wc context. */
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
> +
> /* Open an admistrative session with the working copy. */
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, target_wcpath,
> ! dry_run, -1, ctx->cancel_func,
> @@ -7766,6 +7822,7 @@ svn_client_merge3(const char *source1,
> record_only, dry_run,
> merge_options,
> &use_sleep, ctx,
> + wc_ctx,
> pool);
> if (err)
> {
> @@ -7802,7 +7859,7 @@ svn_client_merge3(const char *source1,
> ancestral, related, same_repos,
> ignore_ancestry, force, dry_run,
> record_only, FALSE, depth, merge_options,
> - &use_sleep, ctx, pool);
> + &use_sleep, ctx, wc_ctx, pool);
>
> if (use_sleep)
> svn_io_sleep_for_timestamps(target_wcpath, pool);
> @@ -7810,6 +7867,8 @@ svn_client_merge3(const char *source1,
> if (err)
> return svn_error_return(err);
>
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
This could destroy ctx->wc_ctx, couldn't it? iow, Bad.
>...
> @@ -8527,6 +8586,13 @@ svn_client_merge_reintegrate(const char
> static const svn_wc_entry_callbacks2_t walk_callbacks =
> { get_subtree_mergeinfo_walk_cb, get_mergeinfo_error_handler };
> struct get_subtree_mergeinfo_walk_baton wb;
> + svn_wc_context_t *wc_ctx;
> +
> + /* Create the wc context. */
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
>
> /* Open an admistrative session with the working copy. */
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, target_wcpath,
> @@ -8676,7 +8742,7 @@ svn_client_merge_reintegrate(const char
> svn_depth_infinity,
> FALSE, FALSE, FALSE, dry_run,
> merge_options, &use_sleep,
> - ctx, pool);
> + ctx, wc_ctx, pool);
>
> if (use_sleep)
> svn_io_sleep_for_timestamps(target_wcpath, pool);
> @@ -8684,6 +8750,8 @@ svn_client_merge_reintegrate(const char
> if (err)
> return svn_error_return(err);
>
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
Same here.
>...
> @@ -8715,11 +8783,18 @@ svn_client_merge_peg3(const char *source
> svn_boolean_t use_sleep = FALSE;
> svn_error_t *err;
> svn_boolean_t same_repos;
> + svn_wc_context_t *wc_ctx;
>
> /* No ranges to merge? No problem. */
> if (ranges_to_merge->nelts == 0)
> return SVN_NO_ERROR;
>
> + /* Create the wc context. */
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
> +
> /* Open an admistrative session with the working copy. */
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, target_wcpath,
> (! dry_run), -1, ctx->cancel_func,
> @@ -8778,7 +8853,7 @@ svn_client_merge_peg3(const char *source
> err = do_merge(merge_sources, target_wcpath, entry, adm_access,
> TRUE, TRUE, same_repos, ignore_ancestry, force, dry_run,
> record_only, FALSE, depth, merge_options,
> - &use_sleep, ctx, pool);
> + &use_sleep, ctx, wc_ctx, pool);
>
> if (use_sleep)
> svn_io_sleep_for_timestamps(target_wcpath, pool);
> @@ -8786,6 +8861,8 @@ svn_client_merge_peg3(const char *source
> if (err)
> return svn_error_return(err);
>
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
And here.
>...
> +++ trunk/subversion/libsvn_client/mergeinfo.c Fri Jul 3 12:25:44 2009 (r38328)
>...
> @@ -113,17 +118,17 @@ svn_client__record_wc_mergeinfo(const ch
> /* Record the new mergeinfo in the WC. */
> /* ### Later, we'll want behavior more analogous to
> ### svn_client__get_prop_from_wc(). */
> - SVN_ERR(svn_wc_prop_set3(SVN_PROP_MERGEINFO, mergeinfo_str, wcpath,
> - adm_access, TRUE /* skip checks */, NULL, NULL,
> - pool));
> + SVN_ERR(svn_wc_prop_set4(wc_ctx, local_abspath, SVN_PROP_MERGEINFO,
> + mergeinfo_str, TRUE /* skip checks */, NULL, NULL,
> + scratch_pool));
>
> if (ctx->notify_func2)
> {
> ctx->notify_func2(ctx->notify_baton2,
> - svn_wc_create_notify(wcpath,
> + svn_wc_create_notify(local_abspath,
Is this a valid change? To change from relative to absolute paths?
ISTR that the notification callbacks do a lot of path munging to get
"good looking paths", and this could end up changing the "contract"
about the types of paths we feed into the notification system. Maybe a
different notification structure that explicitly contains abspaths?
>...
> @@ -1248,6 +1263,12 @@ svn_client_mergeinfo_log_merged(const ch
> svn_opt_revision_t *real_src_peg_revision;
> apr_hash_index_t *hi;
> svn_revnum_t youngest_rev = SVN_INVALID_REVNUM;
> + svn_wc_context_t *wc_ctx;
> +
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
>
> /* Step 1: Ensure that we have a merge source URL to work with. */
> SVN_ERR(location_from_path_and_rev(&merge_source_url, &real_src_peg_revision,
> @@ -1262,7 +1283,7 @@ svn_client_mergeinfo_log_merged(const ch
> do). */
> /* This get_mergeinfo() call doubles as a mergeinfo capabilities check. */
> SVN_ERR(get_mergeinfo(&tgt_mergeinfo, &repos_root, path_or_url,
> - peg_revision, ctx, pool));
> + peg_revision, ctx, wc_ctx, pool));
> if (! tgt_mergeinfo)
> return SVN_NO_ERROR;
> SVN_ERR(svn_client__get_history_as_mergeinfo(&source_history,
> @@ -1310,10 +1331,14 @@ svn_client_mergeinfo_log_merged(const ch
> using a receiver filter to only allow revisions to pass through
> that are in our rangelist. */
> log_target = svn_path_url_add_component2(repos_root, log_target + 1, pool);
> - return logs_for_mergeinfo_rangelist(log_target, rangelist,
> - discover_changed_paths, revprops,
> - log_receiver, log_receiver_baton,
> - ctx, pool);
> + SVN_ERR(logs_for_mergeinfo_rangelist(log_target, rangelist,
> + discover_changed_paths, revprops,
> + log_receiver, log_receiver_baton,
> + ctx, pool));
> +
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
Looks like another aggressive destruction here.
>...
> @@ -1327,9 +1352,15 @@ svn_client_mergeinfo_get_merged(apr_hash
> const char *repos_root;
> apr_hash_t *full_path_mergeinfo;
> svn_mergeinfo_t mergeinfo;
> + svn_wc_context_t *wc_ctx;
> +
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
>
> SVN_ERR(get_mergeinfo(&mergeinfo, &repos_root, path_or_url,
> - peg_revision, ctx, pool));
> + peg_revision, ctx, wc_ctx, pool));
>
> /* Copy the MERGEINFO hash items into another hash, but change
> the relative paths into full URLs. */
> @@ -1354,6 +1385,8 @@ svn_client_mergeinfo_get_merged(apr_hash
> *mergeinfo_p = full_path_mergeinfo;
> }
>
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
And here.
>...
> @@ -1379,6 +1412,12 @@ svn_client_mergeinfo_log_eligible(const
> svn_revnum_t youngest_rev = SVN_INVALID_REVNUM;
> apr_array_header_t *rangelist;
> const char *log_target = NULL;
> + svn_wc_context_t *wc_ctx;
> +
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
>
> /* Step 1: Ensure that we have a merge source URL to work with. */
> SVN_ERR(location_from_path_and_rev(&merge_source_url, &real_src_peg_revision,
>...
> @@ -1458,10 +1497,14 @@ svn_client_mergeinfo_log_eligible(const
> using a receiver filter to only allow revisions to pass through
> that are in our rangelist. */
> log_target = svn_path_url_add_component2(repos_root, log_target + 1, pool);
> - return logs_for_mergeinfo_rangelist(log_target, rangelist,
> - discover_changed_paths, revprops,
> - log_receiver, log_receiver_baton,
> - ctx, pool);
> + SVN_ERR(logs_for_mergeinfo_rangelist(log_target, rangelist,
> + discover_changed_paths, revprops,
> + log_receiver, log_receiver_baton,
> + ctx, pool));
> +
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
And here.
>...
> @@ -1478,6 +1521,12 @@ svn_client_suggest_merge_sources(apr_arr
> svn_revnum_t copyfrom_rev;
> svn_mergeinfo_t mergeinfo;
> apr_hash_index_t *hi;
> + svn_wc_context_t *wc_ctx;
> +
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
>
> list = apr_array_make(pool, 1, sizeof(const char *));
>
>...
> @@ -1524,6 +1573,8 @@ svn_client_suggest_merge_sources(apr_arr
> }
> }
>
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
Guess what? :-P
>...
> +++ trunk/subversion/libsvn_client/prop_commands.c Fri Jul 3 12:25:44 2009 (r38328)
>...
> @@ -856,6 +854,12 @@ svn_client_propget3(apr_hash_t **props,
> const svn_wc_entry_t *node;
> svn_boolean_t pristine;
> int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> + svn_wc_context_t *wc_ctx;
> +
> + if (!ctx->wc_ctx)
> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool, pool));
> + else
> + wc_ctx = ctx->wc_ctx;
>
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, path_or_url,
> FALSE, adm_lock_level,
> @@ -873,8 +877,10 @@ svn_client_propget3(apr_hash_t **props,
>
> SVN_ERR(svn_client__get_prop_from_wc(*props, propname, path_or_url,
> pristine, node, adm_access,
> - depth, changelists, ctx, pool));
> + depth, changelists, ctx, wc_ctx,
> + pool));
>
> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
Last one.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368158
Received on 2009-07-05 18:55:45 CEST