On 9/23/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> On 9/23/05, Philip Martin <philip@codematters.co.uk> wrote:
> > Erik Huelsmann <ehuels@gmail.com> writes:
> > > BTW: which part of the 'remove copyfrom-url and copyfrom-rev' do you
> > > consider non-atomic? Modifying an entry is done with 1 call into
> > > svn_wc__modify_entry.
> >
> > I don't know whether Ivan's change would have introduced an atomic
> > problem, it's quite possible his change will be been OK. If he thinks
> > svn_wc_delete2 is the best place to do then he may be right.
> My opinion that in any case no atomic problems will be indroduced.
> After discussion I can restate my question: Is this logic (about
> cleanuping fields) should applied for all situation when scheduled
> modifed from replaced to deleted or only when entry deleted by
> svn_wc_delete2()?
I think it should always clear those fields: there is no use for those
values, since they can only apply to items which are scheduled for
addition to version control. A 'D'elete state implies the item is
already under version control.
I have the following change sitting in my working copy, which takes
care of Philip's atomicity concerns. It has 1 problem though: I can't
remove the copyfrom_url value from the entry which is being deleted,
since we can't marshall a NULL value into a logfile. That's why those
lines are commented out in the patch below. If we change
svn_wc__modify_entry the way Philip describes, that's solved and some
of the other fields can be removed from the SVN_WC__LOG_MODIFY_ENTRY
tag.
Also, I think that some of the svn_io_check_path() arguments should be
joined with 'parent' to turn them into absolute values. I have only
tested this changed from the $PWD, so that I didn't notice the problem
at first.
The last (commented-out) bit with tmp_entry is obsolete with this
patch; I actually don't think that bit is in my wc anymore, but I
probably sent an old patch to the inbox at work.
I hope this helps into the right direction.
breser: All of the wc-replacements brach may need to have its
md5-checking updated, right? That applies to svn_wc_delete2,
svn_wc_revert2, svn_wc_add(2?) and svn_wc_add_repos_file(). Any other
places where this may go wrong now?
bye,
Erik.
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 16212)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -924,11 +924,107 @@
file, so we split off base_name from the parent path, then fold in
the addition of a delete flag. */
svn_wc_entry_t tmp_entry;
-
+ svn_stringbuf_t *log_accum = svn_stringbuf_create ("", pool);
+ const char *logfile_name = svn_wc__logfile_path (0, pool);
+ apr_file_t *log_file;
+
+ /* Edit the entry to reflect the now deleted state,
+ scratching over values possibly filled by a replace-state */
+ svn_xml_make_open_tag
+ (&log_accum,
+ pool,
+ svn_xml_self_closing,
+ SVN_WC__LOG_MODIFY_ENTRY,
+ SVN_WC__LOG_ATTR_NAME,
+ base_name,
+ SVN_WC__ENTRY_ATTR_SCHEDULE,
+ SVN_WC__ENTRY_VALUE_DELETE,
+ /* SVN_WC__ENTRY_ATTR_COPYFROM_URL,
+ NULL,*/
+ SVN_WC__ENTRY_ATTR_COPYFROM_REV,
+ apr_psprintf (pool, "%ld", SVN_INVALID_REVNUM),
+ SVN_WC__ENTRY_ATTR_COPIED,
+ "false",
+ NULL);
+
+ /* is it a replacement with history? */
+ if (entry->schedule == svn_wc_schedule_replace && entry->copyfrom_url)
+ {
+ const char *text_base =
+ svn_wc__text_base_path (base_name, FALSE, pool);
+ const char *text_revert =
+ svn_wc__text_revert_path (base_name, FALSE, pool);
+ const char *prop_base, *prop_revert;
+ svn_node_kind_t kind = svn_node_unknown;
+
+ SVN_ERR (svn_wc__prop_base_path (&prop_base, base_name,
+ adm_access, FALSE, pool));
+ SVN_ERR (svn_wc__prop_revert_path (&prop_revert, base_name,
+ adm_access, FALSE, pool));
+
+ /* Restore the original text-base */
+ svn_xml_make_open_tag
+ (&log_accum,
+ pool,
+ svn_xml_self_closing,
+ SVN_WC__LOG_MV,
+ SVN_WC__LOG_ATTR_NAME, text_revert,
+ SVN_WC__LOG_ATTR_DEST, text_base,
+ NULL);
+
+ /* Is there a prop-revert to restore to prop-base? */
+ SVN_ERR (svn_io_check_path (prop_revert, &kind, pool));
+ if (kind == svn_node_file)
+ svn_xml_make_open_tag
+ (&log_accum,
+ pool,
+ svn_xml_self_closing,
+ SVN_WC__LOG_MV,
+ SVN_WC__LOG_ATTR_NAME, prop_revert,
+ SVN_WC__LOG_ATTR_DEST, prop_base,
+ NULL);
+ else
+ {
+ /* If not, delete an existing prop_base if there is one. */
+ SVN_ERR (svn_io_check_path (prop_base, &kind, pool));
+ if (kind != svn_node_none)
+ svn_xml_make_open_tag
+ (&log_accum,
+ pool,
+ svn_xml_self_closing,
+ SVN_WC__LOG_RM,
+ SVN_WC__LOG_ATTR_NAME, prop_base);
+ }
+ }
+
+ SVN_ERR (svn_wc__open_adm_file (&log_file,
+ parent, logfile_name,
+ (APR_WRITE | APR_CREATE),
+ pool));
+
+ SVN_ERR_W (svn_io_file_write_full (log_file, log_accum->data,
+ log_accum->len, NULL, pool),
+ apr_psprintf (pool, _("Error writing log for '%s'"),
+ svn_path_local_style (logfile_name, pool)));
+
+ SVN_ERR (svn_wc__close_adm_file (log_file, parent, logfile_name,
+ TRUE, pool));
+
+ SVN_ERR (svn_wc__run_log (adm_access, NULL, pool));
+
+ /*
tmp_entry.schedule = svn_wc_schedule_delete;
+ tmp_entry.copyfrom_url = NULL;
+ tmp_entry.copyfrom_rev = SVN_INVALID_REVNUM;
+ tmp_entry.copied = FALSE;
SVN_ERR (svn_wc__entry_modify (adm_access, base_name, &tmp_entry,
- SVN_WC__ENTRY_MODIFY_SCHEDULE, TRUE,
+ SVN_WC__ENTRY_MODIFY_SCHEDULE |
+ SVN_WC__ENTRY_MODIFY_COPYFROM_URL |
+ SVN_WC__ENTRY_MODIFY_COPYFROM_REV |
+ SVN_WC__ENTRY_MODIFY_COPIED,
+ TRUE,
pool));
+ */
}
/* Report the deletion to the caller. */
Received on Fri Sep 23 09:21:51 2005