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

Re: svn commit: r1344833 - in /subversion/trunk/subversion: libsvn_client/client.h libsvn_client/merge.c libsvn_client/ra.c libsvn_client/switch.c tests/libsvn_client/client-test.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 1 Jun 2012 14:56:04 +0100 (BST)

rhuijben_at_apache.org wrote:

> URL: http://svn.apache.org/viewvc?rev=1344833&view=rev
> Log:
> Reuse an existing ra session when fetching the common ancestor of two urls
> from the merge and switch handling.
>
> * subversion/libsvn_client/client.h
>   (svn_client__get_youngest_common_ancestor): Allow passing a session.

Nice improvement.  We should go one step further and *require* passing a pool ...

> * subversion/libsvn_client/merge.c
>   (merge_locked,
>   calculate_left_hand_side,
>   find_reintegrate_merge,
>   find_symmetric_merge): Pass existing sessions.
>
> * subversion/libsvn_client/ra.c
>   (svn_client__get_youngest_common_ancestor): Only open a session if the user
>     didn't pass one.
>   (svn_client__youngest_common_ancestor): Update caller.
>
> * subversion/libsvn_client/switch.c
>   (switch_internal): Pass session.
>
> * subversion/tests/libsvn_client/client-test.c
>   (test_youngest_common_ancestor): Pass NULL for session.

... because the only caller that doesn't now pass a pool is this test (and it's better if a test uses the same code path as the real code).

- Julian

> Modified:
>     subversion/trunk/subversion/libsvn_client/client.h
>     subversion/trunk/subversion/libsvn_client/merge.c
>     subversion/trunk/subversion/libsvn_client/ra.c
>     subversion/trunk/subversion/libsvn_client/switch.c
>     subversion/trunk/subversion/tests/libsvn_client/client-test.c
>
> Modified: subversion/trunk/subversion/libsvn_client/client.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=1344833&r1=1344832&r2=1344833&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/client.h (original)
> +++ subversion/trunk/subversion/libsvn_client/client.h Thu May 31 17:52:24 2012
> @@ -198,6 +198,9 @@ svn_client__repos_location_segments(apr_
>     LOC1 and LOC2.  If the locations have no common ancestor (including if
>     they don't have the same repository root URL), set *ANCESTOR_P to NULL.
>
> +  If SESSION is not NULL, use it for retrieving the common ancestor instead
> +  of creating a new session.
> +
>     Use the authentication baton cached in CTX to authenticate against
>     the repository.  Use POOL for all allocations.
>
> @@ -207,6 +210,7 @@ svn_error_t *
> svn_client__get_youngest_common_ancestor(svn_client__pathrev_t **ancestor_p,
>                                           const svn_client__pathrev_t *loc1,
>                                           const svn_client__pathrev_t *loc2,
> +                                        svn_ra_session_t *session,
>                                           svn_client_ctx_t *ctx,
>                                           apr_pool_t *result_pool,
>                                           apr_pool_t *scratch_pool);
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1344833&r1=1344832&r2=1344833&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu May 31 17:52:24 2012
> @@ -9550,7 +9550,8 @@ merge_locked(const char *source1,
>   /* Unless we're ignoring ancestry, see if the two sources are related. 
> */
>   if (! ignore_ancestry)
>     SVN_ERR(svn_client__get_youngest_common_ancestor(
> -              &yca, source1_loc, source2_loc, ctx, scratch_pool,
> scratch_pool));
> +                    &yca, source1_loc, source2_loc, ra_session1, ctx,
> +                    scratch_pool, scratch_pool));
>
>   /* Check for a youngest common ancestor.  If we have one, we'll be
>       doing merge tracking.
> @@ -10468,7 +10469,8 @@ calculate_left_hand_side(svn_client__pat
>       actually related, we can't reintegrate if they are not.  Also
>       get an initial value for the YCA revision number. */
>   SVN_ERR(svn_client__get_youngest_common_ancestor(
> -            &yc_ancestor, source_loc, &target->loc, ctx, iterpool,
> iterpool));
> +              &yc_ancestor, source_loc, &target->loc,
> target_ra_session, ctx,
> +              iterpool, iterpool));
>   if (! yc_ancestor)
>     return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
>                               _("'%s@%ld' must be ancestrally
> related to "
> @@ -10633,7 +10635,7 @@ find_reintegrate_merge(merge_source_t **
>     SVN_ERR(svn_ra_reparent(target_ra_session, source.loc1->url,
> scratch_pool));
>
>   SVN_ERR(svn_client__get_youngest_common_ancestor(
> -            &yc_ancestor, source.loc2, source.loc1,
> +            &yc_ancestor, source.loc2, source.loc1, target_ra_session,
>             ctx, scratch_pool, scratch_pool));
>
>   /* The source side of a reintegrate merge is not 'ancestral', except
> in
> @@ -11443,7 +11445,7 @@ find_symmetric_merge(svn_client__pathrev
>             s_t->target_ra_session, ctx, scratch_pool));
>
>   SVN_ERR(svn_client__get_youngest_common_ancestor(
> -            &s_t->yca, s_t->source, &s_t->target->loc,
> +            &s_t->yca, s_t->source, &s_t->target->loc,
> s_t->source_ra_session,
>             ctx, result_pool, result_pool));
>
>   /* Find the latest revision of A synced to B and the latest
>
> Modified: subversion/trunk/subversion/libsvn_client/ra.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/ra.c?rev=1344833&r1=1344832&r2=1344833&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/ra.c (original)
> +++ subversion/trunk/subversion/libsvn_client/ra.c Thu May 31 17:52:24 2012
> @@ -843,12 +843,12 @@ svn_error_t *
> svn_client__get_youngest_common_ancestor(svn_client__pathrev_t **ancestor_p,
>                                           const svn_client__pathrev_t *loc1,
>                                           const svn_client__pathrev_t *loc2,
> +                                        svn_ra_session_t *session,
>                                           svn_client_ctx_t *ctx,
>                                           apr_pool_t *result_pool,
>                                           apr_pool_t *scratch_pool)
> {
> -  apr_pool_t *sesspool = svn_pool_create(scratch_pool);
> -  svn_ra_session_t *session;
> +  apr_pool_t *sesspool = NULL;
>   apr_hash_t *history1, *history2;
>   apr_hash_index_t *hi;
>   svn_revnum_t yc_revision = SVN_INVALID_REVNUM;
> @@ -863,7 +863,11 @@ svn_client__get_youngest_common_ancestor
>     }
>
>   /* Open an RA session for the two locations. */
> -  SVN_ERR(svn_client_open_ra_session(&session, loc1->url, ctx,
> sesspool));
> +  if (session == NULL)
> +    {
> +      sesspool = svn_pool_create(scratch_pool);
> +      SVN_ERR(svn_client_open_ra_session(&session, loc1->url, ctx,
> sesspool));
> +    }
>
>   /* We're going to cheat and use history-as-mergeinfo because it
>       saves us a bunch of annoying custom data comparisons and such. */
> @@ -879,8 +883,9 @@ svn_client__get_youngest_common_ancestor
>                                                 SVN_INVALID_REVNUM,
>                                                 SVN_INVALID_REVNUM,
>                                                 session, ctx, scratch_pool));
> -  /* Close the source and target sessions. */
> -  svn_pool_destroy(sesspool);
> +  /* Close the ra session if we opened one. */
> +  if (sesspool)
> +    svn_pool_destroy(sesspool);
>
>   /* Loop through the first location's history, check for overlapping
>       paths and ranges in the second location's history, and
> @@ -959,7 +964,7 @@ svn_client__youngest_common_ancestor(con
>                               ctx, scratch_pool));
>
>   SVN_ERR(svn_client__get_youngest_common_ancestor(
> -            &ancestor, loc1, loc2, ctx, result_pool, scratch_pool));
> +            &ancestor, loc1, loc2, session, ctx, result_pool,
> scratch_pool));
>
>   if (ancestor)
>     {
>
> Modified: subversion/trunk/subversion/libsvn_client/switch.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/switch.c?rev=1344833&r1=1344832&r2=1344833&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/switch.c (original)
> +++ subversion/trunk/subversion/libsvn_client/switch.c Thu May 31 17:52:24 2012
> @@ -208,7 +208,8 @@ switch_internal(svn_revnum_t *result_rev
>           /* ### It would be nice if this function could reuse the existing
>               ra session instead of opening two for its own use. */
>           SVN_ERR(svn_client__get_youngest_common_ancestor(
> -                  &yca, switch_loc, target_base_loc, ctx, pool, pool));
> +                  &yca, switch_loc, target_base_loc, ra_session, ctx,
> +                  pool, pool));
>         }
>       if (! yca)
>         return svn_error_createf(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
>
> Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/client-test.c?rev=1344833&r1=1344832&r2=1344833&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_client/client-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Thu May 31
> 17:52:24 2012
> @@ -689,7 +689,7 @@ test_youngest_common_ancestor(const svn_
>               repos_url, repos_uuid, 2, "iota", pool),
>             svn_client__pathrev_create_with_relpath(
>               repos_url, repos_uuid, 2, "A/iota", pool),
> -            ctx, pool, pool));
> +            NULL, ctx, pool, pool));
>   SVN_TEST_STRING_ASSERT(svn_client__pathrev_relpath(yc_ancestor, pool),
>                           "iota");
>   SVN_TEST_ASSERT(yc_ancestor->rev == 1);
> @@ -713,7 +713,7 @@ test_youngest_common_ancestor(const svn_
>               repos_url, repos_uuid, 0, "", pool),
>             svn_client__pathrev_create_with_relpath(
>               repos_url, repos_uuid, 3, "A/ROOT", pool),
> -            ctx, pool, pool));
> +            NULL, ctx, pool, pool));
>   SVN_TEST_STRING_ASSERT(svn_client__pathrev_relpath(yc_ancestor, pool),
> "");
>   SVN_TEST_ASSERT(yc_ancestor->rev == 0);
>
Received on 2012-06-01 15:56:41 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.