> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Thursday, May 17, 2007 6:13 PM
> To: dev@subversion.tigris.org
> Subject: Re: svn commit: r25038 - trunk/subversion/libsvn_client
>
> This is definitely better than a segfault. However, won't
> this still prevent the 'merge' operation from completing? Do
> we want to handle this specific error in the calling code
> (with notification back to the user that merge info couldn't
> be determined, perhaps)?
Hi Dan,
Let me answer this in two parts, first the case where r25038 returns
SVN_ERR_WC_PATH_NOT_FOUND and then where it returns
SVN_ERR_ENTRY_MISSING_URL.
========================================
SVN_ERR_WC_PATH_NOT_FOUND
Yes, it will prevent the merge operation from completing, but AFAICT
this happens *only* when the merge command targets a missing directory,
that is, given:
svn merge URL DIR_PATH -rX:Y
DIR_PATH itself, not one of it's children, must be missing, and in that
case what else could we possibly do other than error out? For example:
# A directory is missing in my WC
>svn st merge_tests-1
! merge_tests-1\A_COPY\D\H
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1
Properties on 'merge_tests-1\A_COPY\D':
svn:mergeinfo : /A/D:1,4
svn: Directory 'merge_tests-1\A_COPY\D\H' is missing
svn: Directory 'merge_tests-1\A_COPY\D\H' is missing
# What else would be reasonable when merging
# into something that doesn't exist?
>svn merge %URL%/A/D/H merge_tests-1\A_COPY\D\H -r2:6
svn: Path 'merge_tests-1\A_COPY\D\H' is missing
Now if DIR_PATH exists, but one of it's children affected by the merge
is missing, then the merge does complete, albeit with skipped missing
notifications:
>svn merge %URL%/A merge_tests-1\A_COPY -r2:6
Skipped missing target: 'merge_tests-1\A_COPY\D\H\psi'
Skipped missing target: 'merge_tests-1\A_COPY\D\H'
Skipped missing target: 'merge_tests-1\A_COPY\D\H\omega'
Skipped missing target: 'merge_tests-1\A_COPY\D\H'
U merge_tests-1\A_COPY\B\E\beta
Here's why:
get_wc_or_repos_merge_info() is currently called from three places:
DO_SINGLE_FILE_MERGE() and DO_MERGE() - During a merge these are called
directly by svn_client_merge_peg3()/svn_client_merge3() or from within
discover_and_merge_children(). But for discover_and_merge_children()
to "discover" a WC path with merge info, that path obviously can't be
missing, so the only time do_single_file_merge() and do_merge() operate
on a missing TARGET_WCPATH is when called from
svn_client_merge_peg3()/svn_client_merge3() directly.
SVN_CLIENT_GET_MERGEINFO() - Perhaps callers of *this* function might
want to handle the error, but right now it's only caller is
log.c:svn_client__suggest_merge_sources(). Which, as I understand code,
can only be called when we specifiy no merge source path or url and must
derive the source from the cwd. But to do that we must be in the cwd
(so it can't be missing) and the cwd must be versioned or we would have
already errored out with SVN_ERR_WC_NOT_DIRECTORY.
========================================
SVN_ERR_ENTRY_MISSING_URL
Looking at this again:
SVN_ERR(svn_wc__entry_versioned(entry, target_wcpath, adm_access,
FALSE,
pool));
/* If TARGET_WCPATH is a directory we may get an entry with
abrieviated
information from TARGET_WCPATH's parent. This limited entry does
not
have a URL and probably means TARGET_WCPATH is missing or that the
access for TARGET_WCPATH is not in ADM_ACCESS's baton set (i.e.
ADM_ACCESS was opened for TARGET_WCPATH's parent with lock depth ==
0).
Either way we can't get accurate merge info for TARGET_WCPATH. */
if (! (*entry)->url)
{
svn_node_kind_t wckind;
SVN_ERR(svn_io_check_path(target_wcpath, &wckind, pool));
if (wckind == svn_node_none)
{
return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
_("Path '%s' is missing"),
svn_path_local_style(target_wcpath,
pool));
}
else
{
return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
_("Cannot find a URL for '%s'."),
svn_path_local_style(target_wcpath,
pool));
}
}
I'm less sure now about returning the SVN_ERR_ENTRY_MISSING_URL error.
I can't come up with a scenario that would exercise the else block. But
I'm hesitant to change it, since it is a valid reason for
svn_wc__entry_versioned() to succeed but entry-> URL to be NULL. Any
thoughts on this?
========================================
Paul
> On Wed, 16 May 2007, pburba@tigris.org wrote:
>
> > Author: pburba
> > Date: Wed May 16 07:48:05 2007
> > New Revision: 25038
> >
> > Log:
> > Fix segfault in get_wc_or_repos_merge_info() when merging
> to missing target.
> >
> > * subversion/libsvn_client/merge.c
> > (get_wc_or_repos_merge_info): Error out when an
> abbreviated entry for a
> > merge target is found, either the target is missing or we
> have the wrong
> > adm access.
> >
> >
> > Modified:
> > trunk/subversion/libsvn_client/merge.c
> >
> > Modified: trunk/subversion/libsvn_client/merge.c
> > URL:
> >
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/merge.
> > c?pathrev=25038&r1=25037&r2=25038
> >
> ==============================================================
> ================
> > --- trunk/subversion/libsvn_client/merge.c (original)
> > +++ trunk/subversion/libsvn_client/merge.c Wed May 16 07:48:05 2007
> > @@ -1030,6 +1030,31 @@
> > SVN_ERR(svn_wc__entry_versioned(entry, target_wcpath,
> adm_access, FALSE,
> > pool));
> >
> > + /* We may get an entry with abrieviated information from
> TARGET_WCPATH's
> > + parent. This limited entry does not have a URL and
> probably means
> > + TARGET_WCPATH is missing or that the access for
> TARGET_WCPATH is not in
> > + ADM_ACCESS's baton set (i.e. ADM_ACCESS was opened
> for TARGET_WCPATH's
> > + parent with lock depth == 0). Either way we can't
> get accurate merge
> > + info for TARGET_WCPATH. */
> > + if (! (*entry)->url)
> > + {
> > + svn_node_kind_t wckind;
> > +
> > + SVN_ERR(svn_io_check_path(target_wcpath, &wckind, pool));
> > + if (wckind == svn_node_none)
> > + {
> > + return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> > + _("Path '%s' is missing"),
> > +
> svn_path_local_style(target_wcpath, pool));
> > + }
> > + else
> > + {
> > + return
> svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
> > + _("Cannot find a URL
> for '%s'."),
> > +
> svn_path_local_style(target_wcpath, pool));
> > + }
> > + }
> > +
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 18 22:10:04 2007