Forwarding Stefan's follow-up message...
- Julian
-------- Forwarded Message --------
From: Stefan Sperling <stsp_at_elego.de>
To: Julian Foad <julianfoad_at_btopenworld.com>
Subject: Re: svn_wc_get_tree_conflict
Date: Wed, 17 Sep 2008 02:18:00 +0200
On Wed, Sep 17, 2008 at 12:45:42AM +0200, Stefan Sperling wrote:
> I like this API:
>
> +/** Set @a *tree_conflict to a newly allocated @c svn_wc_conflict_description_t
> + * structure describing the tree conflict state of @a victim_path, or to null
> + * if @a victim_path is not in a state of tree conflict. @a adm_access is the
> + * admin access baton for @a victim_path. Use @a pool for all allocations. */
> +svn_error_t *
> +svn_wc_get_tree_conflict(svn_wc_conflict_description_t **tree_conflict,
> + const char *victim_path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> +
>
> We could call this in the receiver during 'svn info' as well, to find
> out whether the path currently under consideration is a tree conflict
> victim. Correct?
>
> Very nice, just what I was looking for right before our IRC conversation :)
Quick follow-up:
Once the API allows the client library to think in terms of
individual victims, a patch like the one below would make sense, right?
I know you don't want multiple types of conflicts to co-exist,
but for now it matches current behaviour.
Oddly enough, the patch only made a single test fail! :)
[[[
Improve the error reported when a commit fails because of a conflict.
* subversion/libsvn_client/commit_util.c
(bail_if_conflict): New function which returns an appropriate
error for a conflict, or combination of conflicts, at a path.
(harvest_committables): Call bail_if_conflict instead
of reporting the same error for every type of conflict.
* subversion/tests/cmdline/commit_tests.py
(tree_conflicts_block_commit): Adjust expected error output.
]]]
Index: subversion/tests/cmdline/commit_tests.py
===================================================================
--- subversion/tests/cmdline/commit_tests.py (revision 33110)
+++ subversion/tests/cmdline/commit_tests.py (working copy)
@@ -2626,7 +2626,7 @@ def tree_conflicts_block_commit(sbox):
D = os.path.join(wc_dir, 'A', 'D')
G = os.path.join(wc_dir, 'A', 'D', 'G')
- error_re = "remains in conflict"
+ error_re = "is a tree conflict victim"
commit_fails_at_path(wc_dir, wc_dir, error_re)
commit_fails_at_path(A, A, error_re)
commit_fails_at_path(D, D, error_re)
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 33110)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -185,6 +185,47 @@ static svn_wc_entry_callbacks2_t add_tokens_callba
svn_client__default_walker_error_handler
};
+/* Helper function for harvest_committables.
+ *
+ * Return an appropriate error if one or more conflicts occurred at PATH.
+ * A conflict occurred if either TEXT_CONFLICT, PROP_CONFLICT, or
+ * TREE_CONFLICT is true, or if any of those are true at once in
+ * any possible combination.
+ *
+ * Do all allocations in POOL. */
+static svn_error_t *
+bail_if_conflict(svn_boolean_t text_conflict,
+ svn_boolean_t prop_conflict,
+ svn_boolean_t tree_conflict,
+ const char *path,
+ apr_pool_t *pool)
+{
+ const char *reason;
+ svn_string_t *msg;
+
+ if (text_conflict && ! prop_conflict && ! tree_conflict)
+ reason = _("'%s' has text conflicts");
+ else if (! text_conflict && prop_conflict && ! tree_conflict)
+ reason = _("'%s' has property conflicts");
+ else if (! text_conflict && ! prop_conflict && tree_conflict)
+ reason = _("'%s' is a tree conflict victim");
+ else if (text_conflict && prop_conflict && ! tree_conflict)
+ reason = _("'%s' has text and property conflicts");
+ else if (text_conflict && ! prop_conflict && tree_conflict)
+ reason = _("'%s' has text conflicts and is a tree conflict victim");
+ else if (! text_conflict && prop_conflict && tree_conflict)
+ reason = _("'%s' has property conflicts and is a tree conflict victim");
+ else if (text_conflict && prop_conflict && tree_conflict)
+ reason = _("'%s' has text and property conflicts and "
+ "is a tree conflict victim");
+ else /* no conflict */
+ return SVN_NO_ERROR;
+
+ msg = svn_string_createf(pool, reason, svn_path_local_style(path, pool));
+ return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
+ _("Aborting commit: %s"), msg->data);
+}
+
/* Recursively search for commit candidates in (and under) PATH (with
entry ENTRY and ancestry URL), and add those candidates to
COMMITTABLES. If in ADDS_ONLY modes, only new additions are
@@ -346,16 +387,10 @@ harvest_committables(apr_hash_t *committables,
SVN_ERR(svn_wc_conflicted_p2(&tc, &pc, &dummy, p_path, entry, pool));
}
- /* Bail now if any conflicts exist for the ENTRY. */
- if (tc || pc || treec)
- {
- /* Paths in conflict which are not part of our changelist should
- be ignored. */
- if (SVN_WC__CL_MATCH(changelists, entry))
- return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
- _("Aborting commit: '%s' remains in conflict"),
- svn_path_local_style(path, pool));
- }
+ /* Bail now if any conflicts exist for the ENTRY, but only if the
+ entry is part of our changelist. */
+ if (SVN_WC__CL_MATCH(changelists, entry))
+ SVN_ERR(bail_if_conflict(tc, pc, treec, path, pool));
/* If we have our own URL, and we're NOT in COPY_MODE, it wins over
the telescoping one(s). In COPY_MODE, URL will always be the
What do you think? Feel free to go on-list if you want to. I'm just
not posting this patch to dev@ yet because it depends on your unfinished
patch to work correctly. Because right now, directories containing tree
conflict victims will be reported as tree conflict victims themselves.
For example, in the following, 'branch' is of course a directory
(it is the root directory of a working copy of a branch):
stsp_at_jack [svn-sandbox/branch] trunk-power $ svn commit
subversion/libsvn_client/commit.c:911: (apr_err=155015)
svn: Commit failed (details follow):
subversion/libsvn_client/commit_util.c:220: (apr_err=155015)
svn: Aborting commit: '/tmp/svn-sandbox/branch' is a tree conflict victim
But the victim is actually 'branch/alpha' (which was deleted in the
branch prior to a merge from trunk which tried to edit alpha):
stsp_at_jack [svn-sandbox/branch] trunk-power $ grep alpha: .svn/entries
alpha:file:merge:edited:missing
stsp_at_jack [svn-sandbox/branch] trunk-power $ svn info
Path: .
URL: file:///tmp/svn-sandbox/repos/branch
Repository Root: file:///tmp/svn-sandbox/repos
Repository UUID: 1d56f0c2-8420-11dd-a4d9-873576b1ee50
Revision: 2
Node Kind: directory
Schedule: normal
Last Changed Author: stsp
Last Changed Rev: 2
Last Changed Date: 2008-09-16 20:48:56 +0200 (Tue, 16 Sep 2008)
Tree conflicts:
The merge attempted to edit 'alpha'.
'alpha' does not exist locally.
Maybe you renamed it? Or has it been renamed in the history of the branch
you are merging into?
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-17 13:48:12 CEST