subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Sperling <s...@elego.de>
Subject [PATCH] allow conflict descriptions with NULL repos_relpaths
Date Tue, 12 Aug 2014 18:12:09 GMT
I'd like to always make update/switch target revisions available to
the conflict resolver. We currently don't record this information
for tree conflict victims which are not present in the target revision.
For instance, 'svn info' might show:

Tree conflict: local file edit, incoming file delete or move upon update
  Source  left: (file) ^/trunk/alpha@2
  Source right: (none)

With the patch below, 'svn info' shows the target revision where the
node has been found missing:

Tree conflict: local file edit, incoming file delete or move upon update
  Source  left: (file) ^/trunk/alpha@2
  Source right: (none) @4

One use case I have in mind is a conflict resolver which scans the log
to detect moves heuristically. Without knowing the target revision it
is impossible to know how many revisions need to be scanned. The resolver
is forced to scan up to HEAD which can imply a huge performance hit.

The information might also be needed in other cases, and is already available
whenever the victim exists in the repository at the target revision.
I think not recording this information for missing nodes was a mistake.

There is a slight API change involved. The semantics of
svn_wc_conflict_version_create2() must change such that the conflict victim's
repos relpath is allowed to be NULL if the node kind is 'none'.
Currently the API requires a canonical path in all cases, a contract which
cannot be fulfilled for nodes which don't exist.
The revision number must still be valid in all cases.

Essentially, we're making the interface a bit more liberal in what it accepts,
so it's not a breaking change unless a caller expects to see an error in this
situation. We don't have any such code. Still, should I be cautious and rev
the API for this?

Are there any objections to this plan?

[[[
Record the update/switch target revision for missing tree conflicts
victims in the tree conflict description so the revision can always
be retrieved during conflict resolution.

* subversion/include/svn_wc.h
  (svn_wc_conflict_version_create2): Update docstring.
   A NULL 'repos_relpath' is now valid if 'kind' is svn_node_none.

* subversion/libsvn_wc/conflicts.c
  (conflict__prepend_location, conflict__read_location): Handle NULL
   repos-relpath fields in conflict description.

* subversion/libsvn_wc/update_editor.c
  (complete_conflict): Create conflict versions for paths which don't exist
   so revision number information will be recorded.

* subversion/libsvn_wc/util.c
  (svn_wc_conflict_version_create2): Accept a NULL repos_relpath if the
   node kind is svn_node_none.

* subversion/svn/util.c
  (svn_cl__node_description): Print an empty path if the victim's kind
   is svn_node_none, instead of printing the ^/... placeholder path.
]]] 

Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h	(revision 1617507)
+++ subversion/include/svn_wc.h	(working copy)
@@ -1711,8 +1711,9 @@ typedef struct svn_wc_conflict_version_t
  * @a revision and the @c node_kind to @a kind. Make only shallow
  * copies of the pointer arguments.
  *
- * @a repos_root_url, @a repos_relpath and @a revision must be valid,
- * non-null values. @a repos_uuid should be a valid UUID, but can be
+ * @a repos_root_url, and @a revision must be valid, non-null values.
+ * @a repos_relpath must be a canonical fspath, but can be @c NULL if kind
+ * is @svn_node_none. @a repos_uuid should be a valid UUID, but can be
  * NULL if unknown. @a kind can be any kind, even 'none' or 'unknown'.
  *
  * @since New in 1.8.
Index: subversion/libsvn_wc/conflicts.c
===================================================================
--- subversion/libsvn_wc/conflicts.c	(revision 1617507)
+++ subversion/libsvn_wc/conflicts.c	(working copy)
@@ -117,8 +117,11 @@ conflict__prepend_location(svn_skel_t *skel,
 
   svn_skel__prepend_int(location->peg_rev, loc, result_pool);
 
-  svn_skel__prepend_str(apr_pstrdup(result_pool, location->path_in_repos), loc,
-                        result_pool);
+  if (!location->path_in_repos) /* can be NULL if non-existent */
+    svn_skel__prepend(svn_skel__make_empty_list(result_pool), loc);
+  else
+    svn_skel__prepend_str(apr_pstrdup(result_pool, location->path_in_repos), loc,
+                          result_pool);
 
   if (!location->repos_uuid) /* Can theoretically be NULL */
     svn_skel__prepend(svn_skel__make_empty_list(result_pool), loc);
@@ -168,7 +171,10 @@ conflict__read_location(svn_wc_conflict_version_t
     repos_uuid = NULL;
   c = c->next;
 
-  repos_relpath = apr_pstrmemdup(result_pool, c->data, c->len);
+  if (c->is_atom)
+    repos_relpath = apr_pstrmemdup(result_pool, c->data, c->len);
+  else
+    repos_relpath = NULL;
   c = c->next;
 
   SVN_ERR(svn_skel__parse_int(&v, c, scratch_pool));
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c	(revision 1617507)
+++ subversion/libsvn_wc/update_editor.c	(working copy)
@@ -840,7 +840,9 @@ complete_conflict(svn_skel_t *conflict,
   if (is_complete)
     return SVN_NO_ERROR; /* Already completed */
 
-  if (old_repos_relpath)
+  if (old_repos_relpath == NULL)
+    local_kind = svn_node_none;
+  if (SVN_IS_VALID_REVNUM(old_revision))
     original_version = svn_wc_conflict_version_create2(eb->repos_root,
                                                        eb->repos_uuid,
                                                        old_repos_relpath,
@@ -850,15 +852,14 @@ complete_conflict(svn_skel_t *conflict,
   else
     original_version = NULL;
 
-  if (new_repos_relpath)
-    target_version = svn_wc_conflict_version_create2(eb->repos_root,
-                                                        eb->repos_uuid,
-                                                        new_repos_relpath,
-                                                        *eb->target_revision,
-                                                        target_kind,
-                                                        result_pool);
-  else
-    target_version = NULL;
+  if (new_repos_relpath == NULL)
+    target_kind = svn_node_none;
+  target_version = svn_wc_conflict_version_create2(eb->repos_root,
+                                                   eb->repos_uuid,
+                                                   new_repos_relpath,
+                                                   *eb->target_revision,
+                                                   target_kind,
+                                                   result_pool);
 
   if (eb->switch_repos_relpath)
     SVN_ERR(svn_wc__conflict_skel_set_op_switch(conflict,
Index: subversion/libsvn_wc/util.c
===================================================================
--- subversion/libsvn_wc/util.c	(revision 1617507)
+++ subversion/libsvn_wc/util.c	(working copy)
@@ -296,11 +296,15 @@ svn_wc_conflict_version_create2(const char *repos_
 
   version = apr_pcalloc(result_pool, sizeof(*version));
 
-    SVN_ERR_ASSERT_NO_RETURN(svn_uri_is_canonical(repos_url, result_pool)
-                             && svn_relpath_is_canonical(repos_relpath)
-                             && SVN_IS_VALID_REVNUM(revision)
-                             /* ### repos_uuid can be NULL :( */);
+  if (repos_relpath)
+    SVN_ERR_ASSERT_NO_RETURN(svn_relpath_is_canonical(repos_relpath));
+  else
+    SVN_ERR_ASSERT_NO_RETURN(kind == svn_node_none);
 
+  SVN_ERR_ASSERT_NO_RETURN(svn_uri_is_canonical(repos_url, result_pool)
+                           && SVN_IS_VALID_REVNUM(revision)
+                           /* ### repos_uuid can be NULL :( */);
+
   version->repos_url = repos_url;
   version->peg_rev = revision;
   version->path_in_repos = repos_relpath;
Index: subversion/svn/util.c
===================================================================
--- subversion/svn/util.c	(revision 1617507)
+++ subversion/svn/util.c	(working copy)
@@ -923,6 +923,11 @@ svn_cl__node_description(const svn_wc_conflict_ver
 
   if (node->path_in_repos)
     path_str = node->path_in_repos;
+  else if (node->node_kind == svn_node_none)
+    {
+      root_str = "";
+      path_str = "";
+    }
 
   return apr_psprintf(pool, "(%s) %s@%ld",
                       svn_cl__node_kind_str_human_readable(node->node_kind),



Mime
View raw message