You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/06/18 08:13:22 UTC

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

Author: dannas
Date: Fri Jun 18 06:13:22 2010
New Revision: 955844

URL: http://svn.apache.org/viewvc?rev=955844&view=rev
Log:
Make 'svn patch' able to parse property diffs.

It still can't apply them to the target and it can't make a separation
between deleting the content of a property and deleting the property
itself (must be fixed since we allow properties to exist but be empty).

* subversion/include/svn_diff.h
  (svn_patch_t): Add new field 'property_hunks'.

* subversion/libsvn_client/diff.c
  (display_prop_diffs): Remove localisation from header lines for
    property diff hunks. We need a fixed header to be able to identify
    adds and deletes of properties. If empty properties were not
    allowed, we could just identify the operation from the diff, 
    but that's not the case.

* subversion/libsvn_diff/parse-diff.c
  (parse_hunk_header): Add new parameter 'atat' since we can use
    the function for both parsing text- and property-headers and they 
    use different characters in the headers.
  (parse_prop_name): New.
  (reverse_diff_transformer): Both check for property hunk headers and
    text hunk headers.
  (parse_next_hunk): Add checks for property header lines
    Check for first line of hunk header that contains property name. 
    If one is found, call parse_hunk_header with atat set to '##'.
  (svn_diff__parse_next_patch): Store found text and property hunks in 
    different containers. The property hunks must use a hash table
    since we can have more than one property for each patch target file.

* subversion/tests/libsvn_diff/parse-diff-test.c
  (property_unidiff): New.
  (test_parse_property_diff): New
  (test_funcs): Add new test.


Modified:
    subversion/trunk/subversion/include/svn_diff.h
    subversion/trunk/subversion/libsvn_client/diff.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=955844&r1=955843&r2=955844&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_diff.h (original)
+++ subversion/trunk/subversion/include/svn_diff.h Fri Jun 18 06:13:22 2010
@@ -869,6 +869,11 @@ typedef struct svn_patch_t {
   apr_array_header_t *hunks;
 
   /**
+   * A hash table containing an array of svn_hunk_t object for each property
+   * parsed from the patch. The property names act as keys.  */
+  apr_hash_t *property_hunks;
+
+  /**
    * Represents the operation performed on the file. */
   svn_diff_operation_kind_t operation;
 } svn_patch_t;

Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=955844&r1=955843&r2=955844&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Fri Jun 18 06:13:22 2010
@@ -248,11 +248,11 @@ display_prop_diffs(const apr_array_heade
         continue;
 
       if (! original_value)
-        action = _("Added");
+        action = "Added";
       else if (! propchange->value)
-        action = _("Deleted");
+        action = "Deleted";
       else
-        action = _("Modified");
+        action = "Modified";
       SVN_ERR(file_printf_from_utf8(file, encoding, "%s: %s%s", action,
                                     propchange->name, APR_EOL_STR));
 

Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=955844&r1=955843&r2=955844&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
+++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Fri Jun 18 06:13:22 2010
@@ -100,14 +100,14 @@ parse_range(svn_linenum_t *start, svn_li
 }
 
 /* Try to parse a hunk header in string HEADER, putting parsed information
- * into HUNK. Return TRUE if the header parsed correctly.
+ * into HUNK. Return TRUE if the header parsed correctly. ATAT is the
+ * character string used to delimit the hunk header.
  * If REVERSE is TRUE, invert the hunk header while parsing it.
  * Do all allocations in POOL. */
 static svn_boolean_t
 parse_hunk_header(const char *header, svn_hunk_t *hunk,
-                  svn_boolean_t reverse, apr_pool_t *pool)
+                  const char *atat, svn_boolean_t reverse, apr_pool_t *pool)
 {
-  static const char * const atat = "@@";
   const char *p;
   svn_stringbuf_t *range;
 
@@ -233,7 +233,7 @@ reverse_diff_transformer(svn_stringbuf_t
   /* ### Pass the already parsed hunk via the baton?
    * ### Maybe we should really make svn_stream_readline() a proper stream
    * ### method and override it instead of adding special callbacks? */
-  if (parse_hunk_header(line, &hunk, FALSE, scratch_pool))
+  if (parse_hunk_header(line, &hunk, "@@", FALSE, scratch_pool))
     {
       *buf = svn_stringbuf_createf(result_pool,
                                    "@@ -%lu,%lu +%lu,%lu @@",
@@ -242,6 +242,15 @@ reverse_diff_transformer(svn_stringbuf_t
                                    hunk.original_start,
                                    hunk.original_length);
     }
+  else if (parse_hunk_header(line, &hunk, "##", FALSE, scratch_pool))
+    {
+      *buf = svn_stringbuf_createf(result_pool,
+                                   "## -%lu,%lu +%lu,%lu ##",
+                                   hunk.modified_start,
+                                   hunk.modified_length,
+                                   hunk.original_start,
+                                   hunk.original_length);
+    }
   else if (line[0] == '+')
     {
       *buf = svn_stringbuf_create(line, result_pool);
@@ -258,14 +267,31 @@ reverse_diff_transformer(svn_stringbuf_t
   return SVN_NO_ERROR;
 }
 
+/* Parse PROP_NAME from HEADER as the part after the INDICATOR line. */
+static svn_error_t *
+parse_prop_name(const char **prop_name, const char *header, 
+                const char *indicator, apr_pool_t *result_pool)
+{
+  SVN_ERR(svn_utf_cstring_to_utf8(prop_name,
+                                  header + strlen(indicator),
+                                  result_pool));
+
+  /* ### Are we guarenteed that there are no trailing or leading
+   * ### whitespaces in the name? */
+
+  return SVN_NO_ERROR;
+}
+
 /* Return the next *HUNK from a PATCH, using STREAM to read data
- * from the patch file. If no hunk can be found, set *HUNK to NULL.
- * If REVERSE is TRUE, invert the hunk while parsing it. If
- * IGNORE_WHiTESPACES is TRUE, let lines without leading spaces be
- * recognized as context lines.  Allocate results in RESULT_POOL.  Use
- * SCRATCH_POOL for all other allocations. */
+ * from the patch file. If no hunk can be found, set *HUNK to NULL. If we
+ * have a property hunk, PROP_NAME will be set. If we have a text hunk,
+ * PROP_NAME will be NULL. If REVERSE is TRUE, invert the hunk while
+ * parsing it. If IGNORE_WHiTESPACES is TRUE, let lines without leading
+ * spaces be recognized as context lines.  Allocate results in RESULT_POOL.
+ * Use SCRATCH_POOL for all other allocations. */
 static svn_error_t *
 parse_next_hunk(svn_hunk_t **hunk,
+                const char **prop_name,
                 svn_patch_t *patch,
                 svn_stream_t *stream,
                 svn_boolean_t reverse,
@@ -274,7 +300,8 @@ parse_next_hunk(svn_hunk_t **hunk,
                 apr_pool_t *scratch_pool)
 {
   static const char * const minus = "--- ";
-  static const char * const atat = "@@";
+  static const char * const text_atat = "@@";
+  static const char * const prop_atat = "##";
   svn_stringbuf_t *line;
   svn_boolean_t eof, in_hunk, hunk_seen;
   apr_off_t pos, last_line;
@@ -289,6 +316,12 @@ parse_next_hunk(svn_hunk_t **hunk,
   svn_boolean_t changed_line_seen;
   apr_pool_t *iterpool;
 
+  /* We only set this if we have a property hunk. 
+   * ### prop_name acts as both a state flag inside this function and a
+   * ### qualifier to discriminate between props and text hunks. Is that
+   * ### kind of overloading ok? */
+  *prop_name = NULL;
+
   if (apr_file_eof(patch->patch_file) == APR_EOF)
     {
       /* No more hunks here. */
@@ -407,16 +440,44 @@ parse_next_hunk(svn_hunk_t **hunk,
         }
       else
         {
-          if (starts_with(line->data, atat))
+          if (starts_with(line->data, text_atat))
             {
               /* Looks like we have a hunk header, try to rip it apart. */
-              in_hunk = parse_hunk_header(line->data, *hunk, reverse,
-                                          iterpool);
+              in_hunk = parse_hunk_header(line->data, *hunk, text_atat,
+                                          reverse, iterpool);
               if (in_hunk)
                 {
                   original_lines = (*hunk)->original_length;
                   modified_lines = (*hunk)->modified_length;
+                  *prop_name = NULL;
                 }
+              }
+          else if (starts_with(line->data, prop_atat) && *prop_name)
+            {
+              /* Looks like we have a property hunk header, try to rip it
+               * apart. */
+              in_hunk = parse_hunk_header(line->data, *hunk, prop_atat,
+                                          reverse, iterpool);
+              if (in_hunk)
+                {
+                  original_lines = (*hunk)->original_length;
+                  modified_lines = (*hunk)->modified_length;
+                }
+            }
+          else if (starts_with(line->data, "Added: "))
+            {
+              SVN_ERR(parse_prop_name(prop_name, line->data, "Added: ",
+                                      result_pool));
+            }
+          else if (starts_with(line->data, "Deleted: "))
+            {
+              SVN_ERR(parse_prop_name(prop_name, line->data, "Deleted: ",
+                                      result_pool));
+            }
+          else if (starts_with(line->data, "Modified: "))
+            {
+              SVN_ERR(parse_prop_name(prop_name, line->data, "Modified: ",
+                                      result_pool));
             }
           else if (starts_with(line->data, minus))
             /* This could be a header of another patch. Bail out. */
@@ -532,6 +593,8 @@ svn_diff_parse_next_patch(svn_patch_t **
   svn_stream_t *stream;
   svn_boolean_t eof, in_header;
   svn_hunk_t *hunk;
+  const char *prop_name;
+
   apr_pool_t *iterpool;
 
   if (apr_file_eof(patch_file) == APR_EOF)
@@ -624,14 +687,33 @@ svn_diff_parse_next_patch(svn_patch_t **
     {
       /* Parse hunks. */
       (*patch)->hunks = apr_array_make(result_pool, 10, sizeof(svn_hunk_t *));
+      (*patch)->property_hunks = apr_hash_make(result_pool);
       do
         {
           svn_pool_clear(iterpool);
 
-          SVN_ERR(parse_next_hunk(&hunk, *patch, stream, reverse,
-                                  ignore_whitespace, result_pool, iterpool));
-          if (hunk)
+          SVN_ERR(parse_next_hunk(&hunk, &prop_name, *patch, stream,
+                                  reverse, ignore_whitespace,
+                                  result_pool, iterpool));
+          if (hunk && prop_name)
+            {
+              apr_array_header_t *hunks;
+
+              hunks = apr_hash_get((*patch)->property_hunks, prop_name,
+                                    APR_HASH_KEY_STRING);
+              if (! hunks)
+                {
+                  hunks = apr_array_make(result_pool, 1, 
+                                          sizeof(svn_hunk_t *));
+                  apr_hash_set((*patch)->property_hunks, prop_name,
+                                       APR_HASH_KEY_STRING, hunks);
+                }
+
+              APR_ARRAY_PUSH(hunks, svn_hunk_t *) = hunk;
+            }
+          else if (hunk)
             APR_ARRAY_PUSH((*patch)->hunks, svn_hunk_t *) = hunk;
+
         }
       while (hunk);
     }

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=955844&r1=955843&r2=955844&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c Fri Jun 18 06:13:22 2010
@@ -86,6 +86,31 @@ static const char *git_unidiff =
   "new file mode 100644"                                                NL
   ""                                                                    NL;
 
+  static const char *property_unidiff =
+  "Index: iota"                                                         NL
+  "===================================================================" NL
+  "--- iota"                                                            NL
+  "+++ iota"                                                            NL
+  ""                                                                    NL
+  "Property changes on: iota"                                           NL
+  "___________________________________________________________________" NL
+  "Deleted: prop_del"                                                   NL
+  "## -1 +0,0 ##"                                                       NL
+  "-value"                                                              NL
+  ""                                                                    NL
+  "Property changes on: iota"                                           NL
+  "___________________________________________________________________" NL
+  "Added: prop_add"                                                     NL
+  "## -0,0 +1 ##"                                                       NL
+  "+value"                                                              NL
+  ""                                                                    NL
+  "Property changes on: iota"                                           NL
+  "___________________________________________________________________" NL
+  "Modified: prop_mod"                                                  NL
+  "## -1 +1 ##"                                                         NL
+  "-value"                                                              NL
+  "+new value"                                                          NL;
+
 static svn_error_t *
 test_parse_unidiff(apr_pool_t *pool)
 {
@@ -336,6 +361,110 @@ test_parse_git_diff(apr_pool_t *pool)
   return SVN_NO_ERROR;
 }
 
+/* Tests to parse a diff with three property changes, one is added, one is
+ * modified and one is deleted. */
+static svn_error_t *
+test_parse_property_diff(apr_pool_t *pool)
+{
+  apr_file_t *patch_file;
+  apr_status_t status;
+  apr_size_t len;
+  svn_patch_t *patch;
+  svn_stringbuf_t *buf;
+  svn_boolean_t eof;
+  svn_hunk_t *hunk;
+  apr_off_t pos;
+  svn_stream_t *original_text;
+  svn_stream_t *modified_text;
+  apr_array_header_t *hunks;
+  const char *fname = "test_parse_property_diff.patch";
+
+  /* Create a patch file. */
+  status = apr_file_open(&patch_file, fname,
+                        (APR_READ | APR_WRITE | APR_CREATE | APR_TRUNCATE |
+                         APR_DELONCLOSE), APR_OS_DEFAULT, pool);
+  if (status != APR_SUCCESS)
+    return svn_error_createf(SVN_ERR_TEST_FAILED, NULL, "Cannot open '%s'",
+                             fname);
+  /* Write to the file */
+  len = strlen(property_unidiff);
+  status = apr_file_write_full(patch_file, property_unidiff, len, &len);
+  if (status || len != strlen(property_unidiff))
+    return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+                             "Cannot write to '%s'", fname);
+  /* Reset file pointer. */
+  pos = 0;
+  SVN_ERR(svn_io_file_seek(patch_file, APR_SET, &pos, pool));
+
+  SVN_ERR(svn_diff_parse_next_patch(&patch, patch_file, 
+                                    FALSE, /* reverse */
+                                    FALSE, /* ignore_whitespace */ 
+                                    pool, pool));
+  SVN_TEST_ASSERT(patch);
+  SVN_TEST_ASSERT(! strcmp(patch->old_filename, "iota"));
+  SVN_TEST_ASSERT(! strcmp(patch->new_filename, "iota"));
+  SVN_TEST_ASSERT(patch->hunks->nelts == 0);
+  /* SVN_TEST_ASSERT(patch->operation == svn_diff_op_deleted); */
+  SVN_TEST_ASSERT(apr_hash_count(patch->property_hunks) == 3);
+
+  /* Check the added property */
+  hunks = apr_hash_get(patch->property_hunks, "prop_add", APR_HASH_KEY_STRING);
+  SVN_TEST_ASSERT(hunks->nelts == 1);
+  hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *);
+  original_text = hunk->original_text;
+  modified_text = hunk->modified_text;
+
+  SVN_ERR(svn_stream_readline(original_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(eof);
+  SVN_TEST_ASSERT(buf->len == 0);
+
+  SVN_ERR(svn_stream_readline(modified_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(! eof);
+  SVN_TEST_ASSERT(! strcmp(buf->data, "value"));
+  SVN_ERR(svn_stream_readline(modified_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(eof);
+  SVN_TEST_ASSERT(buf->len == 0);
+
+  /* Check the deleted property */
+  hunks = apr_hash_get(patch->property_hunks, "prop_del", APR_HASH_KEY_STRING);
+  SVN_TEST_ASSERT(hunks->nelts == 1);
+  hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *);
+  original_text = hunk->original_text;
+  modified_text = hunk->modified_text;
+  SVN_ERR(svn_stream_readline(original_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(! eof);
+  SVN_TEST_ASSERT(! strcmp(buf->data, "value"));
+  SVN_ERR(svn_stream_readline(original_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(eof);
+  SVN_TEST_ASSERT(buf->len == 0);
+
+  SVN_ERR(svn_stream_readline(modified_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(eof);
+  SVN_TEST_ASSERT(buf->len == 0);
+
+  /* Check the modified property */
+  hunks = apr_hash_get(patch->property_hunks, "prop_mod", APR_HASH_KEY_STRING);
+  SVN_TEST_ASSERT(hunks->nelts == 1);
+  hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *);
+  original_text = hunk->original_text;
+  modified_text = hunk->modified_text;
+  SVN_ERR(svn_stream_readline(original_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(! eof);
+  SVN_TEST_ASSERT(! strcmp(buf->data, "value"));
+  SVN_ERR(svn_stream_readline(original_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(eof);
+  SVN_TEST_ASSERT(buf->len == 0);
+
+  SVN_ERR(svn_stream_readline(modified_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(! eof);
+  SVN_TEST_ASSERT(! strcmp(buf->data, "new value"));
+  SVN_ERR(svn_stream_readline(modified_text, &buf, NL, &eof, pool));
+  SVN_TEST_ASSERT(eof);
+  SVN_TEST_ASSERT(buf->len == 0);
+
+  return SVN_NO_ERROR;
+}
+
 /* ========================================================================== */
 
 struct svn_test_descriptor_t test_funcs[] =
@@ -345,5 +474,7 @@ struct svn_test_descriptor_t test_funcs[
                    "test unidiff parsing"),
     SVN_TEST_XFAIL2(test_parse_git_diff,
                     "test git unidiff parsing"),
+    SVN_TEST_PASS2(test_parse_property_diff,
+                   "test property unidiff parsing"),
     SVN_TEST_NULL
   };