On Wed, Mar 09, 2005 at 02:22:06PM +0000, Philip Martin wrote:
> I think there is an atomic problem here.
>
> The code is going to copy the source text-base onto the destination
> text-base, and then call svn_wc_add which will modify the checksum.
> If the client gets killed in the window between copying the text-base
> and modifying the checksum then the working copy is broken, the
> schedule delete file will have the wrong text-base. (It can be fixed
> by copying the .svn-revert back to .svn-base.) Also, if the user
> attempts to repeat the aborted copy in the broken working copy the
> bogus text-base will overwrite the correct one stored in .svn-revert.
>
> Obviously such bugs are bad, on the other hand, I don't think the
> existing wc code always gets atomic operations right either.
Yup you're right.
First of all when we do an add (with or without history) we don't set a
checksum on the file. I'm not sure this is a such a great idea but
that's the way it works now. It makes sense for adds without history
because there is no base so there's nothing to checksum. But for adds
with history then the checksum should be set for the base, but it isn't.
So the problem is actually worse than you thought becaues the wc
wouldn't know it was broken.
Ultimately, the problem here is that svn_wc_add is being called from the
copy code execution path. Add knows nothing about the copy so copy is
having to slip admin files in before the add gets run. Since we don't
currently support doing adds with histories over removed files in the wc
we don't run into this right now.
What we need is a private function that does what svn_wc_add does but
understands how to handle installing the admin files for us. Then the
copy operation can use it. That private function would copy the admin
files to their tmp locations and then write a log that modifies entries
and renames the files into their proper location.
To avoid code duplication the private function would need to handle the
normal svn_wc_add cases so svn_wc_add can just be the public stub.
This allows an interrupted operation to be completed by cleanup. If a
user tried to repeat the operation after an interrupted operation then
the wc would be locked and they'd be told to run cleanup...
Do you agree that this would resolve the problem?
--
Ben Reser <ben@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 9 20:42:17 2005