Re: [PATCH] Implicit detection of merge source URL and merge revision range

On Wed, 25 Apr 2007, Paul Burba wrote:

> > -----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...

Yeah, it makes sense -- I understood it yesterday on IRC. Finger and
brain weren't in alignment when I read your email. When I typed
"switched file", I was really thinking of any trigger. But, that's
already handled by the svn_ra_reparent() we're doing in
do_single_file_merge().

Thanks Paul!

- Dan