You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2018/07/06 14:06:19 UTC

svn commit: r1835254 - in /subversion/trunk/subversion: libsvn_client/client.h libsvn_client/diff.c libsvn_client/diff_local.c tests/cmdline/diff_tests.py

Author: julianfoad
Date: Fri Jul  6 14:06:18 2018
New Revision: 1835254

URL: http://svn.apache.org/viewvc?rev=1835254&view=rev
Log:
Another step in reducing coupling between diff drivers and diff writers.

Let svn_client__arbitrary_nodes_diff() anchor the diff processor API at the
requested target paths always, rather than sometimes there and sometimes at
the parent of one of them.

Move the responsibility for prefixing diff header paths with the first
path's basename in certain cases (when the targets are not both directories)
to the caller, where it is more appropriate since it is a presentation
issue.

* subversion/libsvn_client/client.h,
  subversion/libsvn_client/diff_local.c
  (svn_client__arbitrary_nodes_diff): Always anchor at the requested target
    paths, even when they are not both directories. Remove the (now unused)
    'anchor_at_given_paths' flag and (already unused) 'result_pool' params.

* subversion/libsvn_client/diff.c
  (do_diff): Determine the prefix for diff header paths here, instead.

* subversion/tests/cmdline/diff_tests.py
  (diff_arbitrary_files_and_dirs): Extend this case a little, adding a file
    that is modified and a node that is replaced by a different kind.

Modified:
    subversion/trunk/subversion/libsvn_client/client.h
    subversion/trunk/subversion/libsvn_client/diff.c
    subversion/trunk/subversion/libsvn_client/diff_local.c
    subversion/trunk/subversion/tests/cmdline/diff_tests.py

Modified: subversion/trunk/subversion/libsvn_client/client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=1835254&r1=1835253&r2=1835254&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/client.h (original)
+++ subversion/trunk/subversion/libsvn_client/client.h Fri Jul  6 14:06:18 2018
@@ -1120,35 +1120,16 @@ svn_client__resolve_conflicts(svn_boolea
  *
  * No copy or move information is reported to the diff processor.
  *
- * If ANCHOR_AT_GIVEN_PATHS is true:
- *
- *   Anchor the DIFF_PROCESSOR at the requested diff targets
- *   (LEFT_ABSPATH, RIGHT_ABSPATH).
- *
- * If ANCHOR_AT_GIVEN_PATHS is false:
- *
- *   If both LEFT_ABSPATH and RIGHT_ABSPATH are directories on disk:
- *
- *     Anchor the DIFF_PROCESSOR at the requested diff targets
- *     (LEFT_ABSPATH, RIGHT_ABSPATH).
- *
- *   else:
- *
- *     Anchor the DIFF_PROCESSOR at the parent of LEFT_ABSPATH
- *     (so the paths all start with a basename(LEFT_ABSPATH) component).
- *
- * As any children reached by recursion are matched by name, a diff
- * processor relpath applies equally to both sides of the diff, except
- * for its first component in the latter case above.
+ * Anchor the DIFF_PROCESSOR at the requested diff targets (LEFT_ABSPATH,
+ * RIGHT_ABSPATH). As any children reached by recursion are matched by
+ * name, a diff processor relpath applies equally to both sides of the diff.
  */
 svn_error_t *
-svn_client__arbitrary_nodes_diff(svn_boolean_t anchor_at_given_paths,
-                                 const char *left_abspath,
+svn_client__arbitrary_nodes_diff(const char *left_abspath,
                                  const char *right_abspath,
                                  svn_depth_t depth,
                                  const svn_diff_tree_processor_t *diff_processor,
                                  svn_client_ctx_t *ctx,
-                                 apr_pool_t *result_pool,
                                  apr_pool_t *scratch_pool);
 
 

Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1835254&r1=1835253&r2=1835254&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Fri Jul  6 14:06:18 2018
@@ -2426,17 +2426,29 @@ do_diff(diff_driver_info_t *ddi,
 
               if (ddi)
                 {
+                  svn_node_kind_t kind1, kind2;
+
+                  SVN_ERR(svn_io_check_resolved_path(abspath1, &kind1,
+                                                     scratch_pool));
+                  SVN_ERR(svn_io_check_resolved_path(abspath2, &kind2,
+                                                     scratch_pool));
+                  if (kind1 == svn_node_dir && kind2 == svn_node_dir)
+                    {
+                      ddi->anchor = "";
+                    }
+                  else
+                    {
+                      ddi->anchor = svn_dirent_basename(abspath1, NULL);
+                    }
                   ddi->orig_path_1 = path_or_url1;
                   ddi->orig_path_2 = path_or_url2;
                 }
 
               /* Ignores changelists, ignore_ancestry */
-              SVN_ERR(svn_client__arbitrary_nodes_diff(!ddi,
-                                                       abspath1, abspath2,
+              SVN_ERR(svn_client__arbitrary_nodes_diff(abspath1, abspath2,
                                                        depth,
                                                        diff_processor,
-                                                       ctx,
-                                                       result_pool, scratch_pool));
+                                                       ctx, scratch_pool));
             }
           else
             {

Modified: subversion/trunk/subversion/libsvn_client/diff_local.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff_local.c?rev=1835254&r1=1835253&r2=1835254&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff_local.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff_local.c Fri Jul  6 14:06:18 2018
@@ -647,19 +647,17 @@ do_dir_diff(const char *left_abspath,
 }
 
 svn_error_t *
-svn_client__arbitrary_nodes_diff(svn_boolean_t anchor_at_given_paths,
-                                 const char *left_abspath,
+svn_client__arbitrary_nodes_diff(const char *left_abspath,
                                  const char *right_abspath,
                                  svn_depth_t depth,
                                  const svn_diff_tree_processor_t *diff_processor,
                                  svn_client_ctx_t *ctx,
-                                 apr_pool_t *result_pool,
                                  apr_pool_t *scratch_pool)
 {
   svn_node_kind_t left_kind;
   svn_node_kind_t right_kind;
-  const char *left_root_abspath;
-  const char *right_root_abspath;
+  const char *left_root_abspath = left_abspath;
+  const char *right_root_abspath = right_abspath;
   svn_boolean_t left_before_right = TRUE; /* Future argument? */
 
   if (depth == svn_depth_unknown)
@@ -670,26 +668,6 @@ svn_client__arbitrary_nodes_diff(svn_boo
 
   if (left_kind == svn_node_dir && right_kind == svn_node_dir)
     {
-      left_root_abspath = left_abspath;
-      right_root_abspath = right_abspath;
-    }
-  else
-    {
-      const char *left_target;
-
-      svn_dirent_split(&left_root_abspath, &left_target, left_abspath,
-                       scratch_pool);
-      right_root_abspath = svn_dirent_dirname(right_abspath, scratch_pool);
-
-      if (anchor_at_given_paths)
-        {
-          diff_processor = svn_diff__tree_processor_filter_create(
-                             diff_processor, left_target, scratch_pool);
-        }
-    }
-
-  if (left_kind == svn_node_dir && right_kind == svn_node_dir)
-    {
       SVN_ERR(do_dir_diff(left_abspath, right_abspath,
                           left_root_abspath, right_root_abspath,
                           FALSE, FALSE, left_before_right,
@@ -707,79 +685,48 @@ svn_client__arbitrary_nodes_diff(svn_boo
   else if (left_kind == svn_node_file || left_kind == svn_node_dir
            || right_kind == svn_node_file || right_kind == svn_node_dir)
     {
-      void *dir_baton;
-      svn_boolean_t skip = FALSE;
-      svn_boolean_t skip_children = FALSE;
-      svn_diff_source_t *left_src;
-      svn_diff_source_t *right_src;
-
-      left_src = svn_diff__source_create(SVN_INVALID_REVNUM, scratch_pool);
-      right_src = svn_diff__source_create(SVN_INVALID_REVNUM, scratch_pool);
-
-      /* The root is replaced... */
-      /* Report delete and/or add */
-
-      SVN_ERR(diff_processor->dir_opened(&dir_baton, &skip, &skip_children, "",
-                                         left_src,
-                                         right_src,
-                                         NULL /* copyfrom_src */,
-                                         NULL,
-                                         diff_processor,
-                                         scratch_pool, scratch_pool));
-
-      if (skip)
-        return SVN_NO_ERROR;
-      else if (!skip_children)
+      /* The root is added/deleted/replaced. Report delete and/or add. */
+      if (left_before_right)
         {
-          if (left_before_right)
-            {
-              if (left_kind == svn_node_file)
-                SVN_ERR(do_file_diff(left_abspath, right_abspath,
-                                     left_root_abspath, right_root_abspath,
-                                     TRUE, FALSE, NULL /* parent_baton */,
-                                     diff_processor, ctx, scratch_pool));
-              else if (left_kind == svn_node_dir)
-                SVN_ERR(do_dir_diff(left_abspath, right_abspath,
-                                    left_root_abspath, right_root_abspath,
-                                    TRUE, FALSE, left_before_right,
-                                    depth, NULL /* parent_baton */,
-                                    diff_processor, ctx, scratch_pool));
-            }
-
-          if (right_kind == svn_node_file)
+          if (left_kind == svn_node_file)
             SVN_ERR(do_file_diff(left_abspath, right_abspath,
                                  left_root_abspath, right_root_abspath,
-                                 FALSE, TRUE, NULL /* parent_baton */,
+                                 TRUE, FALSE, NULL /* parent_baton */,
                                  diff_processor, ctx, scratch_pool));
-          else if (right_kind == svn_node_dir)
+          else if (left_kind == svn_node_dir)
             SVN_ERR(do_dir_diff(left_abspath, right_abspath,
                                 left_root_abspath, right_root_abspath,
-                                FALSE, TRUE,  left_before_right,
+                                TRUE, FALSE, left_before_right,
                                 depth, NULL /* parent_baton */,
                                 diff_processor, ctx, scratch_pool));
-
-          if (! left_before_right)
-            {
-              if (left_kind == svn_node_file)
-                SVN_ERR(do_file_diff(left_abspath, right_abspath,
-                                     left_root_abspath, right_root_abspath,
-                                     TRUE, FALSE, NULL /* parent_baton */,
-                                     diff_processor, ctx, scratch_pool));
-              else if (left_kind == svn_node_dir)
-                SVN_ERR(do_dir_diff(left_abspath, right_abspath,
-                                    left_root_abspath, right_root_abspath,
-                                    TRUE, FALSE,  left_before_right,
-                                    depth, NULL /* parent_baton */,
-                                    diff_processor, ctx, scratch_pool));
-            }
         }
 
-      SVN_ERR(diff_processor->dir_closed("",
-                                         left_src,
-                                         right_src,
-                                         dir_baton,
-                                         diff_processor,
-                                         scratch_pool));
+      if (right_kind == svn_node_file)
+        SVN_ERR(do_file_diff(left_abspath, right_abspath,
+                             left_root_abspath, right_root_abspath,
+                             FALSE, TRUE, NULL /* parent_baton */,
+                             diff_processor, ctx, scratch_pool));
+      else if (right_kind == svn_node_dir)
+        SVN_ERR(do_dir_diff(left_abspath, right_abspath,
+                            left_root_abspath, right_root_abspath,
+                            FALSE, TRUE, left_before_right,
+                            depth, NULL /* parent_baton */,
+                            diff_processor, ctx, scratch_pool));
+
+      if (! left_before_right)
+        {
+          if (left_kind == svn_node_file)
+            SVN_ERR(do_file_diff(left_abspath, right_abspath,
+                                 left_root_abspath, right_root_abspath,
+                                 TRUE, FALSE, NULL /* parent_baton */,
+                                 diff_processor, ctx, scratch_pool));
+          else if (left_kind == svn_node_dir)
+            SVN_ERR(do_dir_diff(left_abspath, right_abspath,
+                                left_root_abspath, right_root_abspath,
+                                TRUE, FALSE, left_before_right,
+                                depth, NULL /* parent_baton */,
+                                diff_processor, ctx, scratch_pool));
+        }
     }
   else
     return svn_error_createf(SVN_ERR_NODE_UNEXPECTED_KIND, NULL,

Modified: subversion/trunk/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/diff_tests.py?rev=1835254&r1=1835253&r2=1835254&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/diff_tests.py Fri Jul  6 14:06:18 2018
@@ -3855,7 +3855,7 @@ def diff_arbitrary_files_and_dirs(sbox):
   sbox.build()
   wc_dir = sbox.wc_dir
 
-  # diff iota with A/mu
+  # diff files (iota with A/mu)
   expected_output = make_diff_header("iota", "working copy", "working copy",
                                      "iota", "A/mu") + [
                       "@@ -1 +1 @@\n",
@@ -3866,7 +3866,11 @@ def diff_arbitrary_files_and_dirs(sbox):
                                      'diff', '--old', sbox.ospath('iota'),
                                      '--new', sbox.ospath('A/mu'))
 
-  # diff A/B/E with A/D
+  # diff dirs (A/B/E with A/D)
+  # .../gamma is to show as replaced; .../beta is to show as modified
+  sbox.simple_mkdir('A/B/E/gamma')
+  sbox.simple_propset('p', 'v', 'A/B/E/gamma')
+  sbox.simple_add_text("This is a different beta file.\n", 'A/D/beta')
   expected_output = make_diff_header("G/pi", "nonexistent", "working copy",
                                      "B/E", "D") + [
                       "@@ -0,0 +1 @@\n",
@@ -3896,11 +3900,16 @@ def diff_arbitrary_files_and_dirs(sbox):
                       "@@ -1 +0,0 @@\n",
                       "-This is the file 'alpha'.\n"
                     ] + make_diff_header("beta", "working copy",
-                                         "nonexistent", "B/E", "D") + [
-                      "@@ -1 +0,0 @@\n",
-                      "-This is the file 'beta'.\n"
-                    ] + make_diff_header("gamma", "nonexistent",
                                          "working copy", "B/E", "D") + [
+                      "@@ -1 +1 @@\n",
+                      "-This is the file 'beta'.\n",
+                      "+This is a different beta file.\n"
+                    ] + make_diff_header("gamma", "working copy",
+                                           "nonexistent", "B/E", "D") \
+                      + make_diff_prop_header("gamma") \
+                      + make_diff_prop_deleted("p", "v") \
+                      + make_diff_header("gamma", "nonexistent",
+                                       "working copy", "B/E", "D") + [
                       "@@ -0,0 +1 @@\n",
                       "+This is the file 'gamma'.\n"
                     ]