You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@finemaltcoding.com> on 2007/08/27 19:49:36 UTC
Re: svn commit: r26339 - in trunk/subversion: libsvn_client libsvn_ra_neon tests/cmdline
On Aug 27, 2007, at 12:12 PM, kameshj@tigris.org wrote:
...
> Fix issue 2889 - mergeinfo REPORT requests are ignoring peg revisions.
>
> Following bug is exposed by the fix to #2889.
> a)To 'get_wc_or_repos_mergeinfo' we pass ra_session which is not
> related
> to Working copy target on which we merge.
>
> * subversion/libsvn_ra_neon/mergeinfo.c
> (svn_ra_neon__get_mergeinfo): Run the merge-info REPORT request on a
> baseline url.
> * subversion/libsvn_client/merge.c
> (do_merge, do_single_file_merge): reparent the ra_session
> corresponding to
> working copy target before calling 'get_wc_or_repos_mergeinfo'.
> * subversion/tests/cmdline/merge_tests.py
> (merge_catches_nonexistent_target): Fix the wrong comment about
> revision
> number.
> (test_list): Remove XFail marker from
> 'merge_fails_if_subtree_is_deleted_on_src'.
...
> --- trunk/subversion/libsvn_client/merge.c (original)
> +++ trunk/subversion/libsvn_client/merge.c Mon Aug 27 12:12:22 2007
> @@ -2100,6 +2100,7 @@
> svn_boolean_t is_root_of_noop_merge = FALSE;
> apr_size_t target_count, merge_target_count;
> apr_pool_t *subpool;
> + svn_boolean_t is_same_repos = FALSE;
>
> ENSURE_VALID_REVISION_KINDS(initial_revision1->kind,
> initial_revision2->kind);
> @@ -2141,13 +2142,26 @@
>
> if (notify_b.same_urls)
> {
> + /* ### TODO: Should we use from_same_repos? I don't like its
> behaviour
> + ### for dry-run case. */
> + const char *src_repos_root;
> + SVN_ERR(svn_ra_get_repos_root(ra_session, &src_repos_root,
> pool));
> + if (svn_path_is_ancestor(src_repos_root, entry->url))
> + is_same_repos = TRUE;
> + }
> + if (notify_b.same_urls && is_same_repos)
> + {
> apr_array_header_t *requested_rangelist;
>
> + /* Reparent ra_session to WC target url. */
> + svn_ra_reparent(ra_session, entry->url, pool);
> SVN_ERR(get_wc_or_repos_mergeinfo(&target_mergeinfo, entry,
> &indirect, FALSE,
> svn_mergeinfo_inherited,
> ra_session,
> target_wcpath, adm_access,
> ctx, pool));
> + /* Reparent ra_session back to initial_URL1. */
> + svn_ra_reparent(ra_session, initial_URL1, pool);
>
> is_rollback = (merge_type == merge_type_rollback);
> SVN_ERR(svn_client__path_relative_to_root(&rel_path,
> initial_URL1, NULL,
...
Kamesh, can you say more about what's objectionable about
from_same_repos? Could is_same_repos and from_same_repos be merged
into a single variable? If not, it would be good to rename
is_same_repos to something which better differentiates itself from
from_same_repos.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26339 - in trunk/subversion: libsvn_client libsvn_ra_neon tests/cmdline
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Aug 28, 2007, at 5:57 AM, Kamesh Jayachandran wrote:
...
> With the current state of things(fix to issue 2889), we need to
> know the whether target and source are in the same repos in all
> cases including --dry-run and --record-only.
>
> Committed the same at r26362.
>
> Thanks for the review.
r26362 and r26363 look good, thanks Kamesh!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26339 - in trunk/subversion: libsvn_client libsvn_ra_neon
tests/cmdline
Posted by Kamesh Jayachandran <ka...@collab.net>.
>>
>> if (notify_b.same_urls)
>> {
>> + /* ### TODO: Should we use from_same_repos? I don't like its
>> behaviour
>> + ### for dry-run case. */
>> + const char *src_repos_root;
>> + SVN_ERR(svn_ra_get_repos_root(ra_session, &src_repos_root,
>> pool));
>> + if (svn_path_is_ancestor(src_repos_root, entry->url))
>> + is_same_repos = TRUE;
>> + }
>>
>> is_rollback = (merge_type == merge_type_rollback);
>> SVN_ERR(svn_client__path_relative_to_root(&rel_path,
>> initial_URL1, NULL,
> ...
>
> Kamesh, can you say more about what's objectionable about
> from_same_repos? Could is_same_repos and from_same_repos be merged
> into a single variable? If not, it would be good to rename
> is_same_repos to something which better differentiates itself from
> from_same_repos.
>
Dan,
'from_same_repos' was giving 'FALSE' for '--dry-run', which I was not
sure why, So put a 'TODO' marker to take care of it.
I started using 'merge_cmd_baton->same_repos' instead of custom code, I
got 7 tests failing, complaining 'merge --dry-run' and 'merge' outputs
are not matching.
The cause of those failures was the optimization introduced at 'r25424'
based on your review comments at
'http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=127292'.
With the current state of things(fix to issue 2889), we need to know the
whether target and source are in the same repos in all cases including
--dry-run and --record-only.
Committed the same at r26362.
Thanks for the review.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org