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

Re: svn commit: r34247 - in trunk/subversion: include/private libsvn_client libsvn_wc tests/libsvn_wc

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 18 Nov 2008 14:51:08 +0000

neels_at_tigris.org wrote:
> Author: neels
> Date: Mon Nov 17 22:13:52 2008
> New Revision: 34247
>
> Log:
> Fix (probably most of) issue #3320: Commit fails to block on tree-conflict.

Excellent.

> * subversion/libsvn_client/commit_util.c
> (harvest_committables):
> Already check for tree-conflicts on immediate child nodes when harvesting
> a directory, adhering to the requested DEPTH. Thus catch tree-conflicts
> on nodes that don't have an entry (only when recursing; when the target
> is a victim and has no entry, "commit" still gives a different error).
[...]
> Modified: trunk/subversion/libsvn_client/commit_util.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit_util.c?pathrev=34247&r1=34246&r2=34247
> ==============================================================================
> --- trunk/subversion/libsvn_client/commit_util.c Mon Nov 17 17:30:52 2008 (r34246)
> +++ trunk/subversion/libsvn_client/commit_util.c Mon Nov 17 22:13:52 2008 (r34247)
> @@ -343,6 +343,48 @@ harvest_committables(apr_hash_t *committ
> svn_path_local_style(path, pool));
> }
>
> + /* If this is a dir, walk all tree-conflicts recorded on immediate
> + * children. Adhering to the requested depth, abort the commit on a
> + * tree-conflicted child node.
> + * Tree-conflicts information is stored in the victim's immediate parent.
> + * In some cases of an absent tree-conflicted victim, the tree-conflict
> + * information in its parent dir is the only indication that the node
> + * is under version control. This check is necessary for this case.
> + * In all other cases, this simply bails out a little bit earlier.
> + * Note: Tree-conflicts can only be found in "THIS_DIR" entries. */
> + if ((entry->kind == svn_node_dir) && (depth != svn_depth_empty)
> + && (strcmp(entry->name, SVN_WC_ENTRY_THIS_DIR) == 0))
> + {
> + apr_array_header_t *conflicts;
> + const svn_wc_conflict_description_t *conflict;
> + int i;
> +
> + conflicts = apr_array_make(pool, 0,
> + sizeof(svn_wc_conflict_description_t *));
> + SVN_ERR(svn_wc__read_tree_conflicts_from_entry(conflicts, entry,
> + path, pool));
> +
> + for (i = 0; i < conflicts->nelts; i++)
> + {
> + conflict = APR_ARRAY_IDX(conflicts, i,
> + svn_wc_conflict_description_t *);
> +
> + if ((conflict->node_kind == svn_node_dir) &&
> + (depth == svn_depth_files))
> + continue;
> +
> + /* So we've encountered a conflict that is included in DEPTH.
> + * Bail out. */
> +
> + if (SVN_WC__CL_MATCH(changelists, entry))

Here, "entry" is the parent's entry, but we should be checking against
the child's entry (if it has one).

> + return svn_error_createf(
> + SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("Aborting commit: '%s' remains in conflict"),
> + svn_path_local_style(conflict->path, pool));
> + }

The attached uncompiled untested patch is the sort of thing I think it
needs to check changelist memership correctly.

We also should put this chunk of code in a subroutine, so as not to
complicate the reading and understanding of harvest_committables().

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org

Received on 2008-11-18 15:51:27 CET

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.