> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Wednesday, April 25, 2007 1:21 AM
> To: Paul Burba
> Cc: Kamesh Jayachandran; dev@subversion.tigris.org
> Subject: Re: [PATCH] Implicit detection of merge source URL
> and merge revision range
>
> Do we also need this in libsvn_client/merge.c:do_single_file_merge()?
Patch automerge.mine.v8 onward reparented the session in
do_single_file_merge(). You suggested this to Kamesh in your review of
v7.
> I recall that you might be able to switch a file to a directory; would
> that function kick in under those circumstances?
The switched path in merge test 48 was a red herring. I'm explaining
the problem poorly, maybe an example is better. Prior to the session
reparenting in do_merge(), Kamesh's patch behaved like this:
r2: svn copy %URL%/A %URL%/A_COPY
r3: commit a change to A/mu
r4: svn copy %URL%/A %URL%/A_COPY_2
svn merge %URL%/A_COPY_2 A_COPY -c3
..\..\..\subversion\libsvn_repos\reporter.c:966: (apr_err=160005)
svn: Cannot replace a directory from within
What goes wrong is:
do_merge() opens a session for '%URL%/A_COPY_2' so
session->priv->fs_path == '/A_COPY_2'
do_merge() calls svn_ra_do_diff3() eventually resulting in a
report_baton_t *b, with b->fs_base == '/A_COPY_2', but for rev 3 we need
'/A' since '/A_COPY_2' doesn't exist in r3.
Eventually in reporter.c:drive() we finally fail:
static svn_error_t *
drive(report_baton_t *b, svn_revnum_t s_rev, path_info_t *info,
apr_pool_t *pool)
{
const char *t_anchor, *s_fullpath;
<snip>
/* Collect information about the source and target nodes. */
s_fullpath = svn_path_join(b->fs_base, b->s_operand, pool);
^^^
'/A_COPY_2'!!!
SVN_ERR(get_source_root(b, &s_root, s_rev));
SVN_ERR(fake_dirent(&s_entry, s_root, s_fullpath, pool));
^^^
NULL
<snip>
/* If the anchor is the operand, the source and target must be dirs.
Check this before opening the root to avoid modifying the wc. */
else if (!*b->s_operand && (!s_entry || s_entry->kind != svn_node_dir
|| t_entry->kind != svn_node_dir))
return svn_error_create(SVN_ERR_FS_PATH_SYNTAX, NULL,
_("Cannot replace a directory from
within"));
^^^^
s_entry == NULL ---> BOOM
Hoepfully the preceeding ramble makes some sense...
> On Tue, 24 Apr 2007, Paul Burba wrote:
>
> > Hi Kamesh,
> >
> > Attached are my tweaks to Dan's version of your patch which
> fixes the
> > problem with merge_test-48 and also allows compilation on Win32 (as
> > previously mentioned in
> > http://svn.haxx.se/dev/archive-2007-04/0676.shtml).
> >
> > The fix for merge test 48 is here, in merge.c:do_merge():
> >
> > const svn_wc_entry_t *entry;
> > int i;
> > svn_boolean_t inherited;
> > + svn_opt_revision_t assumed_initial_revision1,
> > assumed_initial_revision2;
> > apr_size_t target_count, merge_target_count;
> >
> > - ENSURE_VALID_REVISION_KINDS(initial_revision1->kind,
> > - initial_revision2->kind);
> > + /* Establish first RA session to initial_URL1. */
> > + SVN_ERR(svn_client__open_ra_session_internal(&ra_session,
> > initial_URL1, NULL,
> > + NULL, NULL,
> FALSE, TRUE,
> > + ctx, pool));
> > + SVN_ERR(assume_default_rev_range(initial_revision1,
> > + &assumed_initial_revision1,
> > + initial_revision2,
> > + &assumed_initial_revision2,
> > + ra_session,
> > + pool));
> >
> > + ENSURE_VALID_REVISION_KINDS(assumed_initial_revision1.kind,
> > + assumed_initial_revision2.kind);
> > +
> > +
> > /* If we are performing a pegged merge, we need to find
> out what our
> > actual URLs will be. */
> > if (peg_revision->kind != svn_opt_revision_unspecified)
> > @@ -1673,10 +1740,13 @@
> > initial_path2 ?
> initial_path2
> > : initial_URL2,
> > peg_revision,
> > - initial_revision1,
> > - initial_revision2,
> > +
> &assumed_initial_revision1,
> > +
> &assumed_initial_revision2,
> > ctx, pool));
> >
> > + /* Reparent session if actual URL changed. */
> > + if (strcmp(initial_URL1, URL1))
> > + SVN_ERR(svn_ra_reparent(ra_session, URL1, pool));
> > +
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > Merge test 48 failed, not because of anything to do with
> switched paths,
> > but because the initial session opened in do_merge() was
> for the wrong
> > URL for the revision being merged in. Reparenting the
> session if the
> > actual URL was different solved the problem.
> >
> > Anyway, per discussion with Dan in IRC, please go ahead and
> commit this
> > version of the patch.
> >
> > Thanks,
> >
> > Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Apr 25 16:06:38 2007