Re: simple tree conflict detection

Some comments inline.

Stephen Butler wrote:
>
> Hi folks,
>
> we understand that most of you are busy getting 1.5 ready for prime
> time, and that right now is not the time for discussing new features
> for Subversion.
>
> Folks at CollabNet have asked us to continue investigating tree
> conflicts anyway due to customer interest in the subject.
>
> We hope that at least a few of you can afford the time to take a look
> at this design proposal. It does not try to provide an ideal solution
> to the use cases we submitted earlier. We just want a mechanism to
> protect users from the most common errors.
>
> We tried to keep it short!
>
> Regards,
> Steve Butler
> Stefan Sperling
>
>
> -*- text -*-
>
> 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.
>
> 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.
>
> 2. Persistence
> Disallow commit in the presence of a tree conflict.
>
> 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.
>
> Running 'svn resolved' on the victim will cause the working-conflict-file
> to be refreshed, removing the victim's conflict description.

Suggest saying "removed" instead of "refreshed" here. That *is* what you
mean, right?

> 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
> 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.
> * 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);
>
> /* 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():
>
> struct svn_wc__tree_conflict_human_readable_phrases
> {
> const char *update_delete;
> const char *update_edit;
> const char *merge_delete;
> const char *merge_edit;
> const char *we_deleted;
> const char *we_edited;
> const char *does_not_exist;
> };
>
> static struct svn_wc__tree_conflict_human_readable_phrases phrases = {
> "The update wants to delete the file '%s'\n"
> "(possibly as part of a rename operation)"\n,
> "The update wants to edit the file '%s'\n",
> "The merge wants to delete the file '%s'\n"
> "(possibly as part of a rename operation)"\n,
> "The merge wants to edit the file '%s'\n",
> "but you have deleted it locally. Maybe you renamed it?\n",
> "but you have edited it locally.\n",
> "but it does not exist locally. Maybe you renamed it?\n"
> };

I'm not sure if this sort of make-a-sentence-from-parts stuff is permissable
under our localization guidelines. But I suspect that you don't require
that these strings be treated as sentence fragments anyway -- if they each
accept the '%s' formatting token, they could each be full sentences:

   The update wants to delete the file 'foo'.
   You have deleted 'foo' locally. Maybe you renamed it?

> 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
>
> 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.
>
> 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.

Looks good so far, Stephen.

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand