Greg Stein <gstein@lyra.org> writes:
> > mod_dav_svn should be trapping and ignoring the "item already
> > missing" error that it's getting from the filesystem. Why?
> > Because mod_dav_svn is *merging* on the fly, and in the world of
> > merging, deletion is an idempotent operation. (svn_fs_merge() has
> > the same rule: if you need to delete something, and it's already
> > gone, it just moves on.) This is a pretty easy fix, and I'm
> > almost done with it.
>
> I don't understand how this can be the correct fix. What if a DELETE is
> attempted and the file *truly* isn't there? I see no way to distinguish
> between "deleting *again*" and "deleting *originally*"
That's because there *is* no difference. :-)
> We cannot simply toss out an error message if it accidentally captures the
> second issue.
>
> IMO, we just mark this as an inconvenience until the WC is fixed. If you
> want to "delete, commit, add, commit", then the user must instead do
> "delete, commit, *update*, add, commit". Just tell them to stick an extra
> update in there for the odd chance that they committed between the delete
> and add.
>
> Basically: why the dubious act of ignoring an error, which was originally
> caused by erroneous behavior on the client. And if we have a low-pain
> workaround, then let's simply use that one.
Hmmm, not sure what two cases you're trying to distinguish here...
All this change does is to implement the same merging logic that has
been in tree.c:merge() all along. Remember, we're merging into HEAD
here -- if the request is to delete entry E, and it turns out that
entry E is already gone, than that means the deletion was already done
by someone else. No problem -- not an error, it just means that you
attempting to delete the entry is not a conflict (as opposed to, say,
trying to modify its text). This has always been part of the merge
logic, see this empty `else' clause in tree.c:
else
{
/* It's a double delete, so do nothing.
### kff todo: what about the rename case? */
}
(The rename case is irrelevant to this discussion, don't be misled by
that comment.)
I think Ben has a mail already composed responding to the second half
(below), so I'll sign off here and we'll see what he says...
-K
> > 2. ra_dav is unable to prevent users from committing propchanges on
> > out-of-date dirs. Thus users could end up with working copies
> > containing dirs whose revision numbers are incorrect.
> >
> > ANALYSIS
> >
> > Well, duh. When ra_dav sends PROPPATCH on a directory,
> > mod_dav_svn is always applying it to the "latest" version of the
> > directory. There's a good chance that the change will apply
> > cleanly, and then ra_dav goes and bumps the directory revision --
> > but the whole thing should have failed an out-of-dateness check to
> > begin with.
> >
> > SOLUTION
> >
> > ra_dav can't know if the directory is out-of-date or not; only
> > someone looking at the filesystem can see that. So the change
> > needs to happen in mod_dav_svn.
> >
> > Specifically, the "working" revision of the directory needs to be
> > sent in the PROPPATCH request body. mod_dav_svn can then look at
> > the node-rev-id of the latest version of the directory, see what
> > revision it was *created* in (it's part of the skel), and compare
> > it to the "working" revision from the client. It's an easy
> > out-of-dateness check that requires no undeltifying, and is a
> > foreshadowing of the commit-system of the future. :-)
>
> No. You cannot send the working revision in the body. Blech.
>
> A CHECKOUT is performed before doing the PROPPATCH. That checkout uses the
> version url from when the directory was initially checked out. The system
> should have punted on that initial CHECKOUT. If it didn't, then the question
> is "why?"
>
> Two answers that I can see:
>
> 1) the client had the wrong version url stored
> 2) the server does the comparison wrong
>
> Karl did some work on #1 recently, maybe that fixed this problem? In any
> case, tracking that URL is a precondition to fixing this issue. We need to
> know if it is getting bumped improperly (e.g. the version url must remain
> unchanged w.r.t revision; if the revision doesn't change, neither does the
> url).
>
> For the second, I'd be a bit surprised. It does the check for other
> resources. There is a bit of special casing in the code, with a comment
> about it (although rereading the comment, it isn't perfectly clear why, so
> maybe the comment needs some more tweaking).
>
>
>
> So on the problems, I'd say the first isn't an issue right now, and the
> second needs a bit of investigation before any whacking on it.
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:45 2006