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

Re: simple tree conflict detection

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2007-11-18 01:15:23 CET

Stephen Butler wrote:
>
> SIMPLE TREE CONFLICT SIGNALING AND PERSISTENCE
>
>
> Issue reference: http://subversion.tigris.org/issues/show_bug.cgi?id=2282
> See also subversion/notes/tree-conflicts/use-cases.txt.

Unfortunately there's a mis-match in use-case numbering.

<http://subversion.tigris.org/issues/show_bug.cgi?id=2282>
(in CMP's comment, July 2007) lists five cases. The issue, and these cases, are
assumed to be relevant to both "svn update" or "svn merge".

<http://svn.collab.net/viewvc/svn/trunk/notes/tree-conflicts/use-cases.txt?view=markup>
(r27454) shows three cases, explicitly relevant to "svn update".

Let me invent references to these and show the correspondence:

[#2282.CMP:1] => [use-cases.txt:<not-shown>]
   "Merging a modification onto a deletion"

[#2282.CMP:2] => [use-cases.txt:up1]
   "Merging a modification onto a rename"

[#2282.CMP:3] => [use-cases.txt:<not-shown>]
   "Merging a deletion onto a modification"

[#2282.CMP:4] => [use-cases.txt:up2]
   "Merging a rename onto a modification"

[#2282.CMP:5] => [use-cases.txt:up3]
   "Merging a rename onto another rename"

> To cut down the complexity of this task, we split the problem of tree
> conflict handling into three parts.
>
> 1. Signaling
> Detect and describe the tree conflict to the user.
> This is use case specific.

I believe this design correctly detects and signals the five use cases
[#2282.CMP:1/2/3/4/5] that we want, and also three other cases, one of which we
do not want but can accept (see below, near the end).

> 2. Persistence
> Disallow commit in the presence of a tree conflict.

I believe this design correctly persists and resolves [#2282.CMP:1/3/4]. In the
cases that involve a WC rename/move [#2282.CMP:2/5], it correctly persists the
conflict as long as the original WC pathname of the victim is included in
commit attempts, and the original WC pathname must be supplied to the
svn_wc__tree_conflict_resolved() function to resolve it.

Have I understood correctly? If so, it looks good.

> 3. Automatic fixes
> Rescue the modifications that are most likely to get lost.
> This is use case specific.
>
> For now we consider only parts 1 and 2.
>
> To signal a tree conflict during an update or merge, we write a human-
> readable description of the tree conflict to a user-visible file
> inside the parent directory in the working copy. The name of this
> file is stored in the conflict_wrk slot in the svn_wc_entry_t
> structure, as suggested by C. Michael Pilato:
>
> IIRC, directories currently only use the property-conflict-file entry
> slot -- maybe the working-conflict-file slot could be used for
> directories in some fashion?
> (http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=126929)
>
> To ensure that the warning persists, svn_wc_conflicted_p() in
> libsvn_wc/questions.c will halt with an SVN_ERR_WC_FOUND_CONFLICT
> error if a new function called svn_wc__is_tree_conflicted() encounters
> a working-conflict-file attached to a directory.
>
> A directory may contain more than one conflict. Each conflict is
> identifiable by the path in the working copy that was deleted or
> modified, which we call the "victim" of the tree conflict.

So you will fully support the cases involving only "delete" and "modify"
operations:

[#2282.CMP:1] => [use-cases.txt:<not-shown>]
   "Merging a modification onto a deletion"

[#2282.CMP:3] => [use-cases.txt:<not-shown>]
   "Merging a deletion onto a modification"

You also expect to partially support at least some cases that involve
rename/move operations, because they include a "delete" as part of the current
implementation.

This definition of how to identify a conflict cannot fully support the
rename/move operation, because when a single "victim" is moved locally it has
two different names/paths in the working copy.

That's OK, I think.

> Running 'svn resolved' on the victim will cause the working-conflict-file
> to be refreshed, removing the victim's conflict description.
>
> To facilitate merging into the 1.5 branch, a requirement desired by
> CollabNet, we avoid changing public 1.5 API at the expense of design
> purity. We modify only internals of libsvn_wc, we don't change its API.
>
> We avoid assuming rename tracking in any form. Use case 5, described in
> the paper attached to issue #2282, requires renames to be detected

That paper would be the file "tree-conflicts-use-cases-svn-1.4.ppt".
Unfortunately I can't read MS PowerPoint files. It might be very helpful if you
could replace it with a more accessible version - text or PDF or PNG for example.

What is use case 5 in the .ppt file?

> properly, which is impossible given how Subversion currently handles
> 'svn move' internally.
>
>
> Here are the data structures we propose for our design.
> We plan on adding a new header file called libsvn_wc/treeconflicts.h
> to declare them, and a corresponding libsvn_wc/treeconflicts.c
> file to implement the functions.
>
> /* Operations that can cause a tree conflict. */
> typedef enum svn_wc__tree_conflict_cause_t
> {
> svn_wc__tree_conflict_cause_update = 0,
> svn_wc__tree_conflict_cause_merge
> } svn_wc__tree_conflict_cause_t;
>
>
> /* This could be merged with svn_wc_conflict_description_t in 1.6. */
> typedef struct svn_wc__tree_conflict_description_t
> {
> /* The path that is being operated on and its node type.
> * For tree conflicts, PATH is always a directory. */
> const char *path;
> svn_node_kind_t node_kind;
>
> /* What operation caused this conflict? */
> svn_wc__tree_conflict_cause_t cause;
>
> /* The path that is the subject of the tree conflict.
> * It may be a file that has been edited in the working
> * copy and deleted in the repository, for example. */
> const char *victim_path;
>
> /* If not NULL, an open working copy access baton to either the
> * path itself (if PATH is a directory), or to the parent
> * directory (if PATH is a file.) */
> svn_wc_adm_access_t *access;
>
> /* The action being attempted on the victim by the update or merge. */
> svn_wc_conflict_action_t their_action;
>
> /* What state in the working copy is causing the conflict? */
> svn_wc_conflict_reason_t reason;
>
> } svn_wc__tree_conflict_description_t;
>
>
> /* Mark the directory at DIR_ENTRY as tree conflicted.

You mean, more precisely, "Add the given CONFLICT to the set of conflicts
currently recorded in the directory DIR." ?

> * The caller has to pass a description of the tree conflict that
> * occurred in CONFLICT. Do all allocations in POOL. */
> svn_error_t*
> svn_wc__mark_tree_conflicted(svn_wc_entry_t dir,
> svn_wc__tree_conflict_description_t
> *conflict,
> apr_pool_t *pool);
>
> /* Set TREE_CONFLICTED to TRUE if the directory DIR_PATH has
> * been marked as tree conflicted.
> *
> * Do all allocations in POOL. */
> svn_error_t*
> svn_wc__is_tree_conflicted(const char *dir_path,
> svn_boolean_t *tree_conflicted.
> apr_pool_t *pool);
>
> /* Return the name of the file containing tree conflict descriptions
> * for all tree conflicts marked in DIR_PATH. */
> const char*
> svn_wc__get_tree_conflict_descs_path(const char *dir_path,
> apr_pool_t *pool);

I wouldn't deliberately expose the file itself, because we are likely to want
to change the method of storing this information later to use something else
other than a file, or to change the format of the file. Instead, provide an API
to get the conflict descriptions (one at a time or all at once).

> /* Mark the tree conflict rooted at directory @a dir_entry
> * with respect to the file specified by @a victim_path as resolved.
> *
> * Do all allocations in POOL. */
> svn_error_t*
> svn_wc__tree_conflict_resolved(svn_wc_entry_t *dir_entry,
> const char* victim_path,
> apr_pool_t *pool);
>
>
> Here are the strings which we want to use to create human readable
> tree conflict descriptions in svn_wc__mark_tree_conflicted():
[...]
I won't worry about the human-readable descriptions yet.

> Currently we plan to detect use cases 1 to 3 at the following points
> in the code:
>
> libsvn_wc/update_editor.c, open_file():
>
> If the file is scheduled for deletion, we have a tree conflict.
> This is use case 1 described in the paper attached to issue #2282

Is this these two?

[#2282.CMP:1] => [use-cases.txt:<not-shown>]
   Merging a modification onto a deletion

[#2282.CMP:2] => [use-cases.txt:up1]
   Merging a modification onto a rename

OK.

> libsvn_wc/update_editor.c, do_entry_deletion():
>
> If we are about to delete a path that has local mods,
> mark the containing directory as tree conflicted.
> This is tree conflict use case 2 as described in the
> paper attached to issue #2282.

Is this these two?

[#2282.CMP:3] => [use-cases.txt:<not-shown>]
   Merging a deletion onto a modification

[#2282.CMP:4] => [use-cases.txt:up2]
   Merging a rename onto a modification

OK.

> If we are about to delete a path that has been scheduled
> for deletion, mark the containing directory as tree conflicted.
> This _could_ be tree conflict use case 3 as described in the
> paper attached to issue #2282. Proper detection would require
> true renames.

Presumably this catches all of these cases:

[#2282.CMP:5] => [use-cases.txt:up3]
   Merging a rename onto another rename
   -> Good to catch it.

[not listed]
   Merging a rename onto a deletion
   -> Good to catch it.

[not listed]
   Merging a deletion onto a rename
   -> Good to catch it.

[not listed]
   Merging a deletion onto a deletion
   -> BAD to catch it - but not very bad.

As Stefan Sperling said in his reply, catching the (deletion + deletion) case
is not what we want, but probably acceptable.

OK.

Please check the correspondence between the three sets of use cases (#2282.CMP,
use-cases.txt, tree-conflicts-use-cases-svn-1.4.ppt).

Thanks.
- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 18 01:15:41 2007

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.