[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1345158 - /subversion/trunk/subversion/libsvn_client/merge.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 1 Jun 2012 15:05:47 +0100 (BST)

rhuijben_at_apache.org wrote:

> URL: http://svn.apache.org/viewvc?rev=1345158&view=rev
> Log:
> Avoid creating another ra session in most code paths in the merge code. At the
> same time fix accidentally opening the ra session with a non read-only working
> copy.
>
> This patch relies on the ra.c fix in r1345143, as before this patch we
> sometimes opened a working copy that would try to update the dav cache.
>
> * subversion/libsvn_client/merge.c
>   (svn_client_open_ra_session): Use svn_client_open_ra_session() as we only

Should say "(ensure_ra_session_url):".

>     use the standard options.
>   (do_merge): Accept a ra_session to help avoiding creating another session.

Please update the doc string!

>   (merge_cousins_and_supplement_mergeinfo,
>   merge_locked): Pass session.
>   (merge_peg_locked): Pass session. Use session pool as scratch pool for local
>     work.
>   (do_symmetric_merge_locked): Pass NULL for session.

We can make this simpler by requiring a non-null session parameter, and simply creating one in this caller.  (This caller already creates a session in the other branch of the 'if', so we can just move that outside the 'if'.)

- Julian

> Modified:
>     subversion/trunk/subversion/libsvn_client/merge.c
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1345158&r1=1345157&r2=1345158&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Fri Jun  1 13:16:03 2012
> @@ -8856,7 +8856,7 @@ do_directory_merge(svn_mergeinfo_catalog
>   * repository, or by allocating a new *RA_SESSION in POOL.
>   * (RA_SESSION itself cannot be null, of course.)
>   *
> - * CTX is used as for svn_client__open_ra_session_internal().
> + * CTX is used as for svn_client_open_ra_session().
>   */
> static svn_error_t *
> ensure_ra_session_url(svn_ra_session_t **ra_session,
> @@ -8876,8 +8876,7 @@ ensure_ra_session_url(svn_ra_session_t *
>   if (! *ra_session || (err && err->apr_err ==
> SVN_ERR_RA_ILLEGAL_URL))
>     {
>       svn_error_clear(err);
> -      err = svn_client__open_ra_session_internal(ra_session, NULL, url, NULL,
> -                                                NULL, FALSE, TRUE, ctx, pool);
> +      err = svn_client_open_ra_session(ra_session, url, ctx, pool);
>     }
>   SVN_ERR(err);
>
> @@ -8944,6 +8943,7 @@ do_merge(apr_hash_t **modified_subtrees,
>           svn_mergeinfo_catalog_t result_catalog,
>           const apr_array_header_t *merge_sources,
>           const merge_target_t *target,
> +        svn_ra_session_t *src_session,
>           svn_boolean_t sources_related,
>           svn_boolean_t same_repos,
>           svn_boolean_t ignore_ancestry,
> @@ -8967,6 +8967,7 @@ do_merge(apr_hash_t **modified_subtrees,
>   int i;
>   svn_boolean_t checked_mergeinfo_capability = FALSE;
>   svn_ra_session_t *ra_session1 = NULL, *ra_session2 = NULL;
> +  const char *old_src_session_url = NULL;
>   apr_pool_t *iterpool;
>
>   SVN_ERR_ASSERT(svn_dirent_is_absolute(target->abspath));
> @@ -9059,6 +9060,13 @@ do_merge(apr_hash_t **modified_subtrees,
>   notify_baton.merge_b = &merge_cmd_baton;
>   notify_baton.pool = result_pool;
>
> +  if (src_session)
> +    {
> +      SVN_ERR(svn_ra_get_session_url(src_session, &old_src_session_url,
> +                                    scratch_pool));
> +      ra_session1 = src_session;
> +    }
> +
>   for (i = 0; i < merge_sources->nelts; i++)
>     {
>       merge_source_t *source =
> @@ -9162,6 +9170,9 @@ do_merge(apr_hash_t **modified_subtrees,
>   /* Let everyone know we're finished here. */
>   notify_merge_completed(target->abspath, ctx, iterpool);
>
> +  if (src_session)
> +    SVN_ERR(svn_ra_reparent(src_session, old_src_session_url, iterpool));
> +
>   svn_pool_destroy(iterpool);
>   return SVN_NO_ERROR;
> }
> @@ -9233,7 +9244,7 @@ merge_cousins_and_supplement_mergeinfo(c
>       modified_subtrees = apr_hash_make(scratch_pool);
>       APR_ARRAY_PUSH(faux_sources, const merge_source_t *) = source;
>       SVN_ERR(do_merge(&modified_subtrees, NULL, faux_sources, target,
> -                      TRUE, same_repos,
> +                      URL1_ra_session, TRUE, same_repos,
>                         ignore_ancestry, force, dry_run, FALSE, NULL, TRUE,
>                         FALSE, depth, merge_options, use_sleep, ctx,
>                         scratch_pool, subpool));
> @@ -9265,14 +9276,14 @@ merge_cousins_and_supplement_mergeinfo(c
>       notify_mergeinfo_recording(target->abspath, NULL, ctx, scratch_pool);
>       svn_pool_clear(subpool);
>       SVN_ERR(do_merge(NULL, add_result_catalog, add_sources, target,
> -                      TRUE, same_repos,
> +                      URL1_ra_session, TRUE, same_repos,
>                         ignore_ancestry, force, dry_run, TRUE,
>                         modified_subtrees, TRUE,
>                         TRUE, depth, merge_options, use_sleep, ctx,
>                         scratch_pool, subpool));
>       svn_pool_clear(subpool);
>       SVN_ERR(do_merge(NULL, remove_result_catalog, remove_sources, target,
> -                      TRUE, same_repos,
> +                      URL1_ra_session, TRUE, same_repos,
>                         ignore_ancestry, force, dry_run, TRUE,
>                         modified_subtrees, TRUE,
>                         TRUE, depth, merge_options, use_sleep, ctx,
> @@ -9641,7 +9652,7 @@ merge_locked(const char *source1,
>     }
>
>   err = do_merge(NULL, NULL, merge_sources, target,
> -                related, same_repos,
> +                ra_session1, related, same_repos,
>                   ignore_ancestry, force, dry_run,
>                   record_only, NULL, FALSE, FALSE, depth, merge_options,
>                   &use_sleep, ctx, scratch_pool, scratch_pool);
> @@ -10730,10 +10741,9 @@ open_reintegrate_source_and_target(svn_r
>   SVN_ERR(open_target_wc(&target, target_abspath,
>                           FALSE, FALSE, FALSE,
>                           ctx, scratch_pool, scratch_pool));
> -  SVN_ERR(svn_client__open_ra_session_internal(target_ra_session_p, NULL,
> -                                              target->loc.url,
> -                                              NULL, NULL, FALSE, FALSE,
> -                                              ctx, scratch_pool));
> +  SVN_ERR(svn_client_open_ra_session(target_ra_session_p,
> +                                    target->loc.url,
> +                                    ctx, scratch_pool));
>   if (! target->loc.url)
>     return svn_error_createf(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
>                               _("Can't reintegrate into '%s'
> because it is "
> @@ -10926,42 +10936,41 @@ merge_peg_locked(const char *source_path
>
>   SVN_ERR_ASSERT(svn_dirent_is_absolute(target_abspath));
>
> +  /* Create a short lived session pool */
> +  sesspool = svn_pool_create(scratch_pool);
> +
>   SVN_ERR(open_target_wc(&target, target_abspath,
>                           allow_mixed_rev, TRUE, TRUE,
> -                        ctx, scratch_pool, scratch_pool));
> +                        ctx, sesspool, sesspool));
>
>   /* Open an RA session to our source URL, and determine its root URL. */
> -  sesspool = svn_pool_create(scratch_pool);
>   SVN_ERR(open_source_session(&source_loc, &ra_session,
>                               source_path_or_url, source_peg_revision,
> -                              ctx, sesspool, scratch_pool));
> +                              ctx, sesspool, sesspool));
>
>   /* Normalize our merge sources. */
>   SVN_ERR(normalize_merge_sources(&merge_sources, source_path_or_url,
>                                   source_loc,
>                                   ranges_to_merge, ra_session, ctx,
> -                                  scratch_pool, scratch_pool));
> +                                  sesspool, sesspool));
>
>   /* Check for same_repos. */
>   same_repos = is_same_repos(&target->loc, source_loc, TRUE /*
> strict_urls */);
>
> -  /* We're done with our little RA session. */
> -  svn_pool_destroy(sesspool);
> -
>   /* Do the real merge!  (We say with confidence that our merge
>       sources are both ancestral and related.) */
> -  err = do_merge(NULL, NULL, merge_sources, target,
> +  err = do_merge(NULL, NULL, merge_sources, target, ra_session,
>                   TRUE, same_repos, ignore_ancestry, force, dry_run,
>                   record_only, NULL, FALSE, FALSE, depth, merge_options,
> -                &use_sleep, ctx, scratch_pool, scratch_pool);
> +                &use_sleep, ctx, sesspool, sesspool);
>
>   if (use_sleep)
> -    svn_io_sleep_for_timestamps(target_abspath, scratch_pool);
> +    svn_io_sleep_for_timestamps(target_abspath, sesspool);
>
> -  if (err)
> -    return svn_error_trace(err);
> +  /* We're done with our RA session. */
> +  svn_pool_destroy(sesspool);
>
> -  return SVN_NO_ERROR;
> +  return svn_error_trace(err);
> }
>
> svn_error_t *
> @@ -11596,7 +11605,7 @@ do_symmetric_merge_locked(const svn_clie
>       merge_sources = apr_array_make(scratch_pool, 1, sizeof(merge_source_t
> *));
>       APR_ARRAY_PUSH(merge_sources, const merge_source_t *) = &source;
>
> -      err = do_merge(NULL, NULL, merge_sources, target,
> +      err = do_merge(NULL, NULL, merge_sources, target, NULL,
>                       TRUE /*related*/,
>                       TRUE /*same_repos*/, ignore_ancestry, force, dry_run,
>                       record_only, NULL, FALSE, FALSE, depth, merge_options,
>
Received on 2012-06-01 16:06:22 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.