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

Re: svn commit: r34158 - in trunk/subversion: libsvn_wc tests/cmdline tests/cmdline/svntest

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 04 Dec 2008 18:35:42 +0000

On Wed, 2008-11-12 at 09:08 -0800, sbutler_at_tigris.org wrote:
> Author: sbutler
> Date: Wed Nov 12 09:08:35 2008
> New Revision: 34158
>
> Log:
> Fix update/switch to skip all tree conflict victims and their
> descendants. Skip both newly-discovered and existing tree conflicts.
>
> Create only one tree conflict at the root of each conflicted tree, not
> a tree conflict in every directory.
>
> Notify the user just once about any given tree conflict if the target
> contains a tree conflict, is itself tree-conflicted, or is inside a
> tree conflict.
>
> Tweak test expectations to reflect these new features. Add tests that
> run update/switch in trees that already contain tree conflict victims.
> Add tests that chdir into conflicted trees and attempt to update/switch.
>
>
> * subversion/libsvn_wc/update_editor.c
> (edit_baton): Add field current_tree_conflict.
> (make_editor): Initialize new field.
> (dir_baton): Remove unused field tree_conflicted.
> (make_dir_baton): Track field removal.
> (open_root): TODO comment, for now.
> (check_tree_conflict): Thin out the comments a bit. Remove useless
> checks for entry validity. Remove the redundant (delete:obstructed)
> and (delete:missing) conflicts.
> (already_in_a_tree_conflict): New function.
> (do_entry_deletion
> add_directory,
> open_directory,
> add_file,
> open_file): Skip the operation if there is an existing or new tree
> conflict. Notify the user once per tree conflict.
> (close_directory): Unset current_tree_conflict when returning to
> the victim dir. Don't notify about tree conflicts.
> (complete_directory): Do nothing if inside a tree conflict.
[...]

> Modified: trunk/subversion/libsvn_wc/update_editor.c
[...]
> @@ -1531,31 +1598,73 @@ do_entry_deletion(struct edit_baton *eb,
> return SVN_NO_ERROR;
> }
>
> - SVN_ERR(check_tree_conflict(&tree_conflict, eb, log_item, full_path, entry,
> - adm_access, svn_wc_conflict_action_delete, pool));
> + /* Is an ancestor-dir (already visited by this edit) a tree conflict
> + victim? If so, skip without notification. */
> + if (eb->current_tree_conflict)
> + {
> + SVN_ERR_ASSERT(svn_path_is_ancestor(eb->current_tree_conflict,
> + full_path));
> +
> + remember_skipped_path(eb, full_path);
> +
> + return SVN_NO_ERROR;
> + }
> +
> + /* Is this path, or an ancestor-dir NOT visited by this edit, already
> + marked as a tree conflict victim? */
> + SVN_ERR(already_in_a_tree_conflict(&victim_path, full_path, eb->cancel_func,
> + eb->cancel_baton, pool));
> +
> + /* Is this path the victim of a newly-discovered tree conflict? */
> + tree_conflict = NULL;
> + if (victim_path == NULL)
> + SVN_ERR(check_tree_conflict(&tree_conflict, eb, log_item, full_path,
> + entry, adm_access,
> + svn_wc_conflict_action_delete, pool));
>
> - /* If this raised a tree-conflict, record. Continue deleting the entry. */
> - if (tree_conflict)
> + if (tree_conflict != NULL)
> {
> + /* Run the log immediately, so that the tree conflict is recorded. */
> SVN_ERR(svn_wc__write_log(adm_access, *log_number, log_item, pool));
> + SVN_ERR(svn_wc__run_log(adm_access, NULL, pool));
> + *log_number = 0;
> + }
>
> - /* If the thing being deleted is the *target* of this update, then
> - we may need to recreate an entry, so that the parent can give
> - accurate reports about itself in future. */
> - if (strcmp(path, eb->target) == 0)
> - {
> - svn_wc_entry_t tmp_entry;
> + if (victim_path != NULL || tree_conflict != NULL)
> + {
> + remember_skipped_path(eb, full_path);
> +
> + /* ### TODO: Also print victim_path in the skip msg. */
> + if (eb->notify_func)
> + (*eb->notify_func)(eb->notify_baton,
> + svn_wc_create_notify(full_path,
> + (tree_conflict != NULL)
> + ? svn_wc_notify_tree_conflict
> + : svn_wc_notify_skip,
> + pool),
> + pool);
>
> - tmp_entry.revision = *(eb->target_revision);
> - tmp_entry.kind =
> - (entry->kind == svn_node_file) ? svn_node_file : svn_node_dir;
> + return SVN_NO_ERROR;
> + }
>
> - SVN_ERR(svn_wc__loggy_entry_modify(&log_item, adm_access,
> - full_path, &tmp_entry,
> - SVN_WC__ENTRY_MODIFY_REVISION
> - | SVN_WC__ENTRY_MODIFY_KIND,
> - pool));
> - }
> + SVN_ERR(svn_wc__write_log(adm_access, *log_number, log_item, pool));
> +
> + /* If the thing being deleted is the *target* of this update, then
> + we may need to recreate an entry, so that the parent can give
> + accurate reports about itself in future. */
> + if (strcmp(path, eb->target) == 0)
> + {
> + svn_wc_entry_t tmp_entry;
> +
> + tmp_entry.revision = *(eb->target_revision);
> + tmp_entry.kind =
> + (entry->kind == svn_node_file) ? svn_node_file : svn_node_dir;
> +
> + SVN_ERR(svn_wc__loggy_entry_modify(&log_item, adm_access,
> + full_path, &tmp_entry,
> + SVN_WC__ENTRY_MODIFY_REVISION
> + | SVN_WC__ENTRY_MODIFY_KIND,
> + pool));
> }

There seems to be a problem here. This block of 17 lines was moved out
of the "if (victim_path || tree_conflict) ... return" block just above.
It is repeated a few lines further down, so it now executes twice in the
main code path instead of once in the "if" block and once in the main
path.

The "SVN_ERR(svn_wc__write_log(...));" just above it was added here, and
that doesn't look necessary either, as it also is repeated below.

I am removing both.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=979763
Received on 2008-12-04 19:36:10 CET

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