> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Wednesday, May 23, 2007 5:27 PM
> To: Paul Burba
> Cc: dev@subversion.tigris.org
> Subject: Re: svn commit: r25038 - trunk/subversion/libsvn_client
>
> On Fri, 18 May 2007, Paul Burba wrote:
>
> > > -----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?
>
> Okay, makes sense.
>
> ...
> > 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.
>
> Okay.
>
> >
> > ========================================
> >
> > 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?
>
> If entry->url is NULL, the WC entry is bogus, effectively
> meaning a corrupt WC entries file.
> libsvn_wc/entries.c:read_entry() looks like it'll either fill
> in the appropriate information, or puke up an error.
> So, I think we could remove the "else" block.
Dan,
Ok, I changed this in r25192 to just return SVN_ERR_ENTRY_MISSING_URL if
the entry has no URL (similar to what we do elsewhere in the code).
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 29 23:49:18 2007