You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/10/01 15:18:38 UTC

svn commit: r1706240 - in /subversion/trunk/subversion: include/svn_diff.h libsvn_client/patch.c libsvn_diff/parse-diff.c tests/libsvn_diff/parse-diff-test.c

Author: rhuijben
Date: Thu Oct  1 13:18:37 2015
New Revision: 1706240

URL: http://svn.apache.org/viewvc?rev=1706240&view=rev
Log:
In the diff parser: parse the symlink status from the mode flags.

* subversion/include/svn_diff.h
  (svn_patch_t): Rename executable bits to _bits and add
    symlink bit variables.

* subversion/libsvn_client/patch.c
  (contradictory_executability): Update caller.
  (init_patch_target): Don't call contradictory_executability when no
    executable change is described in the mode change. Update caller.
  (apply_one_patch): Update caller.

* subversion/libsvn_diff/parse-diff.c
  (parse_bits_into_executability): Rename to...
  (parse_git_mode_bits): ... this and also parse symlink info.
  (git_old_mode,
   git_new_mode,
   git_new_file,
   git_deleted_file): Update caller.
  (svn_diff_parse_next_patch): Update initialization. Reverse new bits
    when using a reverse diff.

* subversion/tests/libsvn_diff/parse-diff-test.c
  (test_parse_git_diff,
   test_parse_git_tree_and_text_diff): Update caller. Add assertions.

Modified:
    subversion/trunk/subversion/include/svn_diff.h
    subversion/trunk/subversion/libsvn_client/patch.c
    subversion/trunk/subversion/libsvn_diff/parse-diff.c
    subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c

Modified: subversion/trunk/subversion/include/svn_diff.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_diff.h?rev=1706240&r1=1706239&r2=1706240&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_diff.h (original)
+++ subversion/trunk/subversion/include/svn_diff.h Thu Oct  1 13:18:37 2015
@@ -1289,23 +1289,38 @@ typedef struct svn_patch_t {
    * @since New in 1.10. */
   svn_diff_binary_patch_t *binary_patch;
 
-  /** The old and new executability bits, as retrieved from the patch file.
+  /** The old and new executability bits, as retrieved from the patch file, from
+   * the git-like mode headers.
    *
-   * A patch may specify an executability change via @a old_executable_p and
-   * / @a new_executable_p, via a #SVN_PROP_EXECUTABLE propchange hunk, or both
-   * ways; however, if both ways are used, they must specify the same semantic
-   * change.
+   * A patch may specify an executability change via @a old_executable_bit and
+   * @a new_executable_bit, via a #SVN_PROP_EXECUTABLE propchange hunk, or both
+   * ways. It is upto caller how to decide how conflicting information is
+   * handled.
    *
    * #svn_tristate_unknown indicates the patch does not specify the
    * corresponding bit.
    *
    * @since New in 1.10.
    */
-  /* ### This is currently not parsed out of "index" lines (where it
-   * ### serves as an assertion of the executability state, without
-   * ### changing it).  */
-  svn_tristate_t old_executable_p; 
-  svn_tristate_t new_executable_p; 
+  svn_tristate_t old_executable_bit;
+  svn_tristate_t new_executable_bit;
+
+  /** The old and new symlink bits, as retrieved from the patch file, from
+  * the git-like mode headers.
+  *
+  * A patch may specify a symlink change via @a old_executable_bit and
+  * @a new_executable_bit, via a #SVN_PROP_SPECIAL propchange hunk, or both
+  * ways. It is upto caller how to decide how conflicting information is
+  * handled. Most implementations will currently just describe a replacement
+  * of the file though.
+  *
+  * #svn_tristate_unknown indicates the patch does not specify the
+  * corresponding bit.
+  *
+  * @since New in 1.10.
+  */
+  svn_tristate_t old_symlink_bit;
+  svn_tristate_t new_symlink_bit;
 } svn_patch_t;
 
 /** An opaque type representing an open patch file.

Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=1706240&r1=1706239&r2=1706240&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Thu Oct  1 13:18:37 2015
@@ -995,23 +995,23 @@ contradictory_executability(const svn_pa
   switch (target->operation)
     {
       case svn_diff_op_added:
-        return patch->new_executable_p == svn_tristate_false;
+        return patch->new_executable_bit == svn_tristate_false;
 
       case svn_diff_op_deleted:
-        return patch->new_executable_p == svn_tristate_true;
+        return patch->new_executable_bit == svn_tristate_true;
 
       case svn_diff_op_unchanged:
         /* ### Can this happen? */
-        return (patch->old_executable_p != svn_tristate_unknown
-                && patch->new_executable_p != svn_tristate_unknown
-                && patch->old_executable_p != patch->new_executable_p);
+        return (patch->old_executable_bit != svn_tristate_unknown
+                && patch->new_executable_bit != svn_tristate_unknown
+                && patch->old_executable_bit != patch->new_executable_bit);
 
       case svn_diff_op_modified:
         /* Can't happen: the property should only ever be added or deleted,
          * but never modified from one valid value to another. */
-        return (patch->old_executable_p != svn_tristate_unknown
-                && patch->new_executable_p != svn_tristate_unknown
-                && patch->old_executable_p == patch->new_executable_p);
+        return (patch->old_executable_bit != svn_tristate_unknown
+                && patch->new_executable_bit != svn_tristate_unknown
+                && patch->old_executable_bit == patch->new_executable_bit);
 
       default:
         /* Can't happen: the proppatch parser never generates other values. */
@@ -1256,7 +1256,8 @@ init_patch_target(patch_target_t **patch
            * an svn:executable property patch. */
           prop_executable_target = svn_hash_gets(target->prop_targets,
                                                  SVN_PROP_EXECUTABLE);
-          if (patch->new_executable_p != svn_tristate_unknown
+          if (patch->new_executable_bit != svn_tristate_unknown
+              && patch->new_executable_bit != patch->old_executable_bit
               && prop_executable_target)
             {
               if (contradictory_executability(patch, prop_executable_target))
@@ -1280,19 +1281,19 @@ init_patch_target(patch_target_t **patch
                    */
                 }
             }
-          else if (patch->new_executable_p != svn_tristate_unknown
+          else if (patch->new_executable_bit != svn_tristate_unknown
                    && !prop_executable_target)
             {
               svn_diff_operation_kind_t operation;
               svn_boolean_t nothing_to_do = FALSE;
               prop_patch_target_t *prop_target;
 
-              if (patch->old_executable_p == patch->new_executable_p)
+              if (patch->old_executable_bit == patch->new_executable_bit)
                 {
                     /* Noop change. */
                     operation = svn_diff_op_unchanged;
                 }
-              else switch (patch->old_executable_p)
+              else switch (patch->old_executable_bit)
                 {
                   case svn_tristate_false:
                     /* Made executable. */
@@ -1305,7 +1306,7 @@ init_patch_target(patch_target_t **patch
                     break;
 
                   case svn_tristate_unknown:
-                    if (patch->new_executable_p == svn_tristate_true)
+                    if (patch->new_executable_bit == svn_tristate_true)
                       /* New, executable file. */
                       operation = svn_diff_op_added;
                     else
@@ -2663,8 +2664,8 @@ apply_one_patch(patch_target_t **patch_t
     }
 
   /* Match implied property hunks. */
-  if (patch->new_executable_p != svn_tristate_unknown
-      && patch->new_executable_p != patch->old_executable_p
+  if (patch->new_executable_bit != svn_tristate_unknown
+      && patch->new_executable_bit != patch->old_executable_bit
       && svn_hash_gets(target->prop_targets, SVN_PROP_EXECUTABLE)
       && !svn_hash_gets(patch->prop_patches, SVN_PROP_EXECUTABLE))
     {
@@ -2673,7 +2674,7 @@ apply_one_patch(patch_target_t **patch_t
       prop_patch_target_t *prop_target = svn_hash_gets(target->prop_targets,
                                                        SVN_PROP_EXECUTABLE);
 
-      if (patch->new_executable_p == svn_tristate_true)
+      if (patch->new_executable_bit == svn_tristate_true)
         SVN_ERR(svn_diff_hunk__create_adds_single_line(
                                             &hunk,
                                             SVN_PROP_EXECUTABLE_VALUE,

Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=1706240&r1=1706239&r2=1706240&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
+++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Thu Oct  1 13:18:37 2015
@@ -1600,10 +1600,11 @@ git_plus(enum parse_state *new_state, ch
 }
 
 /* Helper for git_old_mode() and git_new_mode().  Translate the git
- * file mode MODE_STR into a binary "executable?" notion EXECUTABLE_P. */
+ * file mode MODE_STR into a binary "executable?" and "symlink?" state. */
 static svn_error_t *
-parse_bits_into_executability(svn_tristate_t *executable_p,
-                              const char *mode_str)
+parse_git_mode_bits(svn_tristate_t *executable_p,
+                    svn_tristate_t *symlink_p,
+                    const char *mode_str)
 {
   apr_uint64_t mode;
   SVN_ERR(svn_cstring_strtoui64(&mode, mode_str,
@@ -1639,6 +1640,24 @@ parse_bits_into_executability(svn_trista
         break;
     }
 
+  switch (mode & 0170000 /* S_IFMT */)
+    {
+      case 0120000: /* S_IFLNK */
+        *symlink_p = svn_tristate_true;
+        break;
+
+      case 0100000: /* S_IFREG */
+      case 0040000: /* S_IFDIR */
+        *symlink_p = svn_tristate_false;
+        break;
+
+      default:
+        /* Ignore unknown values.
+           (Including those generated by Subversion <= 1.9) */
+        *symlink_p = svn_tristate_unknown;
+        break;
+    }
+
   return SVN_NO_ERROR;
 }
 
@@ -1647,12 +1666,13 @@ static svn_error_t *
 git_old_mode(enum parse_state *new_state, char *line, svn_patch_t *patch,
              apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
-  SVN_ERR(parse_bits_into_executability(&patch->old_executable_p,
-                                        line + STRLEN_LITERAL("old mode ")));
+  SVN_ERR(parse_git_mode_bits(&patch->old_executable_bit,
+                              &patch->old_symlink_bit,
+                              line + STRLEN_LITERAL("old mode ")));
 
 #ifdef SVN_DEBUG
   /* If this assert trips, the "old mode" is neither ...644 nor ...755 . */
-  SVN_ERR_ASSERT(patch->old_executable_p != svn_tristate_unknown);
+  SVN_ERR_ASSERT(patch->old_executable_bit != svn_tristate_unknown);
 #endif
 
   *new_state = state_old_mode_seen;
@@ -1664,12 +1684,13 @@ static svn_error_t *
 git_new_mode(enum parse_state *new_state, char *line, svn_patch_t *patch,
              apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
-  SVN_ERR(parse_bits_into_executability(&patch->new_executable_p,
-                                        line + STRLEN_LITERAL("new mode ")));
+  SVN_ERR(parse_git_mode_bits(&patch->new_executable_bit,
+                              &patch->new_symlink_bit,
+                              line + STRLEN_LITERAL("new mode ")));
 
 #ifdef SVN_DEBUG
   /* If this assert trips, the "old mode" is neither ...644 nor ...755 . */
-  SVN_ERR_ASSERT(patch->new_executable_p != svn_tristate_unknown);
+  SVN_ERR_ASSERT(patch->new_executable_bit != svn_tristate_unknown);
 #endif
 
   /* Don't touch patch->operation. */
@@ -1738,9 +1759,9 @@ static svn_error_t *
 git_new_file(enum parse_state *new_state, char *line, svn_patch_t *patch,
              apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
-  SVN_ERR(
-    parse_bits_into_executability(&patch->new_executable_p,
-                                  line + STRLEN_LITERAL("new file mode ")));
+  SVN_ERR(parse_git_mode_bits(&patch->new_executable_bit,
+                              &patch->new_symlink_bit,
+                              line + STRLEN_LITERAL("new file mode ")));
 
   patch->operation = svn_diff_op_added;
 
@@ -1755,9 +1776,9 @@ static svn_error_t *
 git_deleted_file(enum parse_state *new_state, char *line, svn_patch_t *patch,
                  apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
-  SVN_ERR(
-    parse_bits_into_executability(&patch->old_executable_p,
-                                  line + STRLEN_LITERAL("deleted file mode ")));
+  SVN_ERR(parse_git_mode_bits(&patch->old_executable_bit,
+                              &patch->old_symlink_bit,
+                              line + STRLEN_LITERAL("deleted file mode ")));
 
   patch->operation = svn_diff_op_deleted;
 
@@ -2069,8 +2090,10 @@ svn_diff_parse_next_patch(svn_patch_t **
     }
 
   patch = apr_pcalloc(result_pool, sizeof(*patch));
-  patch->old_executable_p = svn_tristate_unknown;
-  patch->new_executable_p = svn_tristate_unknown;
+  patch->old_executable_bit = svn_tristate_unknown;
+  patch->new_executable_bit = svn_tristate_unknown;
+  patch->old_symlink_bit = svn_tristate_unknown;
+  patch->new_symlink_bit = svn_tristate_unknown;
 
   pos = patch_file->next_patch_offset;
   SVN_ERR(svn_io_file_seek(patch_file->apr_file, APR_SET, &pos, scratch_pool));
@@ -2180,9 +2203,13 @@ svn_diff_parse_next_patch(svn_patch_t **
             break; /* Stays modify */
         }
 
-      ts_tmp = patch->old_executable_p;
-      patch->old_executable_p = patch->new_executable_p;
-      patch->new_executable_p = ts_tmp;
+      ts_tmp = patch->old_executable_bit;
+      patch->old_executable_bit = patch->new_executable_bit;
+      patch->new_executable_bit = ts_tmp;
+
+      ts_tmp = patch->old_symlink_bit;
+      patch->old_symlink_bit = patch->new_symlink_bit;
+      patch->new_symlink_bit = ts_tmp;
     }
 
   if (patch->old_filename == NULL || patch->new_filename == NULL)

Modified: subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c?rev=1706240&r1=1706239&r2=1706240&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c Thu Oct  1 13:18:37 2015
@@ -471,8 +471,10 @@ test_parse_git_diff(apr_pool_t *pool)
   SVN_TEST_STRING_ASSERT(patch->new_filename, "A/C/gamma");
   SVN_TEST_ASSERT(patch->operation == svn_diff_op_modified);
   SVN_TEST_ASSERT(patch->hunks->nelts == 1);
-  SVN_TEST_ASSERT(patch->old_executable_p = svn_tristate_false);
-  SVN_TEST_ASSERT(patch->new_executable_p = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->old_executable_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_executable_bit = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->old_symlink_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_symlink_bit = svn_tristate_false);
 
   hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_diff_hunk_t *);
 
@@ -508,8 +510,10 @@ test_parse_git_diff(apr_pool_t *pool)
   SVN_TEST_STRING_ASSERT(patch->new_filename, "new");
   SVN_TEST_ASSERT(patch->operation == svn_diff_op_added);
   SVN_TEST_ASSERT(patch->hunks->nelts == 0);
-  SVN_TEST_ASSERT(patch->old_executable_p = svn_tristate_unknown);
-  SVN_TEST_ASSERT(patch->new_executable_p = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->old_executable_bit = svn_tristate_unknown);
+  SVN_TEST_ASSERT(patch->new_executable_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->old_symlink_bit = svn_tristate_unknown);
+  SVN_TEST_ASSERT(patch->new_symlink_bit = svn_tristate_false);
 
   SVN_ERR(svn_diff_close_patch_file(patch_file, pool));
 
@@ -535,8 +539,10 @@ test_parse_git_tree_and_text_diff(apr_po
   SVN_TEST_ASSERT(patch);
   SVN_TEST_STRING_ASSERT(patch->old_filename, "iota");
   SVN_TEST_STRING_ASSERT(patch->new_filename, "iota.copied");
-  SVN_TEST_ASSERT(patch->old_executable_p = svn_tristate_false);
-  SVN_TEST_ASSERT(patch->new_executable_p = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->old_executable_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_executable_bit = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->old_symlink_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_symlink_bit = svn_tristate_false);
   SVN_TEST_ASSERT(patch->operation == svn_diff_op_copied);
   SVN_TEST_ASSERT(patch->hunks->nelts == 1);
 
@@ -559,8 +565,10 @@ test_parse_git_tree_and_text_diff(apr_po
   SVN_TEST_ASSERT(patch);
   SVN_TEST_STRING_ASSERT(patch->old_filename, "A/mu");
   SVN_TEST_STRING_ASSERT(patch->new_filename, "A/mu.moved");
-  SVN_TEST_ASSERT(patch->old_executable_p = svn_tristate_false);
-  SVN_TEST_ASSERT(patch->new_executable_p = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->old_executable_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_executable_bit = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->old_symlink_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_symlink_bit = svn_tristate_false);
   SVN_TEST_ASSERT(patch->operation == svn_diff_op_moved);
   SVN_TEST_ASSERT(patch->hunks->nelts == 1);
 
@@ -604,8 +612,10 @@ test_parse_git_tree_and_text_diff(apr_po
   SVN_TEST_STRING_ASSERT(patch->new_filename, "/dev/null");
   SVN_TEST_ASSERT(patch->operation == svn_diff_op_deleted);
   SVN_TEST_ASSERT(patch->hunks->nelts == 1);
-  SVN_TEST_ASSERT(patch->old_executable_p = svn_tristate_true);
-  SVN_TEST_ASSERT(patch->new_executable_p = svn_tristate_unknown);
+  SVN_TEST_ASSERT(patch->old_executable_bit = svn_tristate_true);
+  SVN_TEST_ASSERT(patch->new_executable_bit = svn_tristate_unknown);
+  SVN_TEST_ASSERT(patch->old_symlink_bit = svn_tristate_false);
+  SVN_TEST_ASSERT(patch->new_symlink_bit = svn_tristate_unknown);
 
   hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_diff_hunk_t *);