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

RE: svn commit: r25233 - in trunk/subversion: libsvn_client tests/cmdline

From: Paul Burba <pburba_at_collab.net>
Date: 2007-06-01 18:19:56 CEST

> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Thursday, May 31, 2007 1:36 PM
> To: dev@subversion.tigris.org
> Cc: pburba@tigris.org
> Subject: Re: svn commit: r25233 - in trunk/subversion:
> libsvn_client tests/cmdline
>
> Nice fix, Paul!
>
> In r25235, I refactored the duplicate code used to determine
> into a helper function.

Thanks Dan, my code duplication detector was busted :-\

> Along the way, I realized that if
> record_only is set, that we don't actually care about the
> value of same_repos.

I don't think I follow, are you saying a merge without --record-only
from a different repos won't update mergeinfo, but with --record-only it
will?

> It would be more efficient to either a)
> not calculate same_repos when record_only is true, or b)
> delay calculation until same_repos is actually needed
> (sticking any necessary contextual data in the baton).
> Is this minor efficiency gain worth the additional complexity
> from (a) or (b)? I lean towards "no", but thought I'd
> mention it anyways..
 
> On Thu, 31 May 2007, pburba@tigris.org wrote:
>
> > Author: pburba
> > Date: Thu May 31 09:06:00 2007
> > New Revision: 25233
> >
> > Log:
> > Fix issue #2788, Don't update merge info when merging from
> another repos.
> >
> > * subversion/libsvn_client/merge.c
> > (merge_cmd_baton): Add new field same_repos.
> > (do_merge, do_single_file_merge): Don't update WC merge
> info unless
> > same_repos flag is set.
> > (svn_client_merge3, svn_client_merge_peg3): Initialize
> new same_repos field
> > in the merge command baton.
> >
> > * subversion/tests/cmdline/merge_tests.py
> > (test_list): Remove XFail from
> diff_repos_does_not_update_mergeinfo.
> >
> > Modified:
> > trunk/subversion/libsvn_client/merge.c
> > trunk/subversion/tests/cmdline/merge_tests.py
> >
> > Modified: trunk/subversion/libsvn_client/merge.c
> > URL:
> >
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/merge.
> > c?pathrev=25233&r1=25232&r2=25233
> >
> ==============================================================
> ================
> > --- trunk/subversion/libsvn_client/merge.c (original)
> > +++ trunk/subversion/libsvn_client/merge.c Thu May 31 09:06:00 2007
> ...
> > if (merge_b->record_only)
> > {
> > - if (merge_b->dry_run)
> > + if (merge_b->dry_run || !merge_b->same_repos)
> > {
> > return SVN_NO_ERROR;
> > }
> > @@ -2009,7 +2012,7 @@
> >
> > if (notify_b.same_urls)
> > {
> > - if (!merge_b->dry_run)
> > + if (!merge_b->dry_run && merge_b->same_repos)
> > {
> > /* Update the WC merge info here to account
> for our new
> > merges, minus any unresolved conflicts
> and skips. */
> > @@ -2223,7 +2226,7 @@
> > merge for the specified range. */
> > if (merge_b->record_only)
> > {
> > - if (merge_b->dry_run)
> > + if (merge_b->dry_run || !merge_b->same_repos)
> > {
> > return SVN_NO_ERROR;
> > }
> > @@ -2328,7 +2331,7 @@
> >
> > if (notify_b.same_urls)
> > {
> > - if (!merge_b->dry_run)
> > + if (!merge_b->dry_run && merge_b->same_repos)
> > {
> > /* Update the WC merge info here to account
> for our new
> > merges, minus any unresolved conflicts
> and skips. */
> > @@ -2555,6 +2558,23 @@
> > merge_cmd_baton.add_necessitated_merge = FALSE;
> > merge_cmd_baton.dry_run_deletions = (dry_run ?
> apr_hash_make(pool) : NULL);
> > merge_cmd_baton.ctx = ctx;
> > +
> > + /* Remember if merge source is a different repos from
> the target so we
> > + don't set merge info. */
> > + {
> > + svn_ra_session_t *ra_session;
> > + const char *source_root;
> > +
> > + /* No need to check URL2 since if it's from a
> different repository
> > + than URL1, then the whole merge will fail anyway. */
> > +
> SVN_ERR(svn_client__open_ra_session_internal(&ra_session, URL1, NULL,
> > + NULL, NULL, FALSE,
> > + TRUE, ctx, pool));
> > + SVN_ERR(svn_ra_get_repos_root(ra_session, &source_root, pool));
> > + merge_cmd_baton.same_repos =
> > + svn_path_is_ancestor(source_root, entry->repos) ?
> TRUE : FALSE;
> > + }
> > +
> > merge_cmd_baton.pool = pool;
> >
> > /* Set up the diff3 command, so various callers don't
> have to. */
> > @@ -2786,6 +2806,21 @@
> > merge_cmd_baton.add_necessitated_merge = FALSE;
> > merge_cmd_baton.dry_run_deletions = (dry_run ?
> apr_hash_make(pool) : NULL);
> > merge_cmd_baton.ctx = ctx;
> > +
> > + /* Remember if merge source is a different repos from
> the target so we
> > + don't set merge info. */
> > + {
> > + svn_ra_session_t *ra_session;
> > + const char *source_root;
> > +
> > +
> SVN_ERR(svn_client__open_ra_session_internal(&ra_session, URL, NULL,
> > + NULL, NULL, FALSE,
> > + TRUE, ctx, pool));
> > + SVN_ERR(svn_ra_get_repos_root(ra_session, &source_root, pool));
> > + merge_cmd_baton.same_repos =
> > + svn_path_is_ancestor(source_root, entry->repos) ?
> TRUE : FALSE;
> > + }
> > +
> ...
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 1 18:20:36 2007

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.