Re: Confusing error: "Cannot replace a directory from within"

On 5/9/07, Peter Lundblad <plundblad@google.com> wrote:
> David Glasser writes:
> > The error is generated in libsvn_repos/reporter.c(drive):
> >
> > /* 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"));
> >
> That's an amazing error code! If I remember correctly, this is generated
> when an editor run tries to delete the root of the edit.
> I think the "replace" word comes from someone thinking of a specific case
> when this could happen. But we should definitely be able to have a specific
> error code for this. (That does obviously not fix the problem for older
> servers.(
>
> > The reporter code is invoked for much more than just switch, and this
> > particular error covers more cases than just the issue at hand. I
> > thought about making it return a more specific error based on exactly
> > what s_entry->kind and t_entry->kind was, but I'm not incredibly
>
> Why does it matter exaclty what the kinds are? If the client got a more specific
> error code, it coudl generate a better error message based on what it is
> trying to do.

I'm not sure that every way of generating the current error should be
handled the same by the switch code up top. So just defining a new
error code and checking for it in the switch code might catch some
other cases (I'm not quite sure what).

> > familiar with the reporter code, and catching the invalid input as
> > soon as it occurs seemed to be less invasive.
> >
> Sure, but we know from DAV that ltos of roundtrips aren't exactly cheap;)
> That's why I'm skeptical about adding three without further investigation.

Hmm. switch can be a relatively high-roundtrip command anyway, and I
don't think it's typically invoked very frequently. Adding to that
the issue of older servers, I think it's cleanest to deal with the bad
URLs in the client code like my patch does. We may also want to make
the reporter give a better error than "Cannot replace a directory from
within", but that can be a separate patch.

--dave

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org