[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r13312 - branches/wc-replacements/subversion/libsvn_wc

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-03-09 15:22:06 CET

breser@tigris.org writes:

> Author: breser
> Date: Tue Mar 8 15:03:29 2005
> New Revision: 13312

This is good, I like the idea.

> --- branches/wc-replacements/subversion/libsvn_wc/copy.c (original)
> +++ branches/wc-replacements/subversion/libsvn_wc/copy.c Tue Mar 8 15:03:29 2005
> @@ -137,6 +137,7 @@
> svn_node_kind_t dst_kind;
> const svn_wc_entry_t *src_entry, *dst_entry;
> svn_boolean_t special;
> + svn_boolean_t replace = FALSE;
>
> /* The 'dst_path' is simply dst_parent/dst_basename */
> const char *dst_path
> @@ -157,10 +158,7 @@
> if (dst_entry && dst_entry->kind == svn_node_file)
> {
> if (dst_entry->schedule == svn_wc_schedule_delete)
> - return svn_error_createf (SVN_ERR_ENTRY_EXISTS, NULL,
> - _("'%s' is scheduled for deletion; it must"
> - " be committed before being overwritten"),
> - svn_path_local_style (dst_path, pool));
> + replace = TRUE;
> else
> return svn_error_createf (SVN_ERR_ENTRY_EXISTS, NULL,
> _("There is already a versioned item '%s'"),
> @@ -198,6 +196,17 @@
> TRUE, /* expand */
> TRUE, /* special */
> pool));
> +
> + /* If we're replacing the file then we need to save the destination files
> + * text base before replacing it (see comments below for why it gets
> + * replaced). This allows us to revert the entire change. */
> + if (replace)
> + {
> + const char *src_txtb = svn_wc__text_base_path (dst_path, FALSE, pool);
> + const char *dst_rvrt = svn_wc__text_revert_path (dst_path, FALSE, pool);
> +
> + SVN_ERR (svn_io_copy_file (src_txtb, dst_rvrt, TRUE, pool));
> + }
>
> /* Copy the pristine text-base over. Why? Because it's the *only*
> way we can detect any upcoming local mods on the copy.

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.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 9 15:23:28 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.