subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bre...@apache.org
Subject svn commit: r1564900 - in /subversion/branches/1.7.x: ./ STATUS subversion/libsvn_client/diff.c subversion/tests/cmdline/diff_tests.py
Date Wed, 05 Feb 2014 19:46:55 GMT
Author: breser
Date: Wed Feb  5 19:46:55 2014
New Revision: 1564900

URL: http://svn.apache.org/r1564900
Log:
Merge the 1.7.x-issue4460 branch:

 * ^/subversion/branches/1.7.x-issue4460
   Fix several issues with diff we broke in 1.7.14
   Justification:
     Regression from 1.7.13
   Notes:
     The guts of the fix are in r1563081 on the branch, the rest are just
     support code for the tests and the tests themselves.
   Votes:
     +1: breser, philip, stefan2

Modified:
    subversion/branches/1.7.x/   (props changed)
    subversion/branches/1.7.x/STATUS
    subversion/branches/1.7.x/subversion/libsvn_client/diff.c
    subversion/branches/1.7.x/subversion/tests/cmdline/diff_tests.py

Propchange: subversion/branches/1.7.x/
------------------------------------------------------------------------------
  Merged /subversion/trunk:r1311747,1561237,1563066-1563068
  Merged /subversion/branches/1.7.x-issue4460:r1563069-1564892

Modified: subversion/branches/1.7.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1564900&r1=1564899&r2=1564900&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Wed Feb  5 19:46:55 2014
@@ -107,13 +107,3 @@ Veto-blocked changes:
 Approved changes:
 =================
 
- * ^/subversion/branches/1.7.x-issue4460
-   Fix several issues with diff we broke in 1.7.14
-   Justification:
-     Regression from 1.7.13
-   Notes:
-     The guts of the fix are in r1563081 on the branch, the rest are just
-     support code for the tests and the tests themselves.
-   Votes:
-     +1: breser, philip, stefan2
-

Modified: subversion/branches/1.7.x/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_client/diff.c?rev=1564900&r1=1564899&r2=1564900&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_client/diff.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_client/diff.c Wed Feb  5 19:46:55 2014
@@ -1892,6 +1892,7 @@ diff_repos_repos_added_or_deleted_file(c
   const char *file_abspath;
   svn_stream_t *content;
   apr_hash_t *prop_hash;
+  svn_string_t *mimetype;
 
   SVN_ERR(svn_stream_open_unique(&content, &file_abspath, NULL,
                                  svn_io_file_del_on_pool_cleanup,
@@ -1900,13 +1901,13 @@ diff_repos_repos_added_or_deleted_file(c
                           &prop_hash, scratch_pool));
   SVN_ERR(svn_stream_close(content));
 
+  mimetype = apr_hash_get(prop_hash, SVN_PROP_MIME_TYPE, APR_HASH_KEY_STRING);
+
   if (show_deletion)
     {
       SVN_ERR(callbacks->file_deleted(NULL, NULL,
                                       target, file_abspath, empty_file,
-                                      apr_hash_get(prop_hash,
-                                                   SVN_PROP_MIME_TYPE,
-                                                   APR_HASH_KEY_STRING),
+                                      mimetype ? mimetype->data : NULL,
                                       NULL,
                                       make_regular_props_hash(
                                         prop_hash, scratch_pool, scratch_pool),
@@ -1917,8 +1918,7 @@ diff_repos_repos_added_or_deleted_file(c
       SVN_ERR(callbacks->file_added(NULL, NULL, NULL,
                                     target, empty_file, file_abspath,
                                     rev1, rev2, NULL,
-                                    apr_hash_get(prop_hash, SVN_PROP_MIME_TYPE,
-                                                 APR_HASH_KEY_STRING),
+                                    mimetype ? mimetype->data : NULL,
                                     NULL, SVN_INVALID_REVNUM,
                                     make_regular_props_array(prop_hash,
                                                              scratch_pool,
@@ -2243,6 +2243,7 @@ diff_repos_wc_file_target(const char *ta
   apr_hash_t *file1_props = NULL;
   apr_hash_t *file2_props;
   svn_boolean_t is_copy = FALSE;
+  svn_string_t *mimetype1, *mimetype2;
 
   /* Get content and props of file 1 (the remote file). */
   SVN_ERR(svn_stream_open_unique(&file1_content, &file1_abspath, NULL,
@@ -2292,6 +2293,7 @@ diff_repos_wc_file_target(const char *ta
     {
       apr_hash_t *keywords = NULL;
       svn_string_t *keywords_prop;
+      svn_string_t *eol_prop;
       svn_subst_eol_style_t eol_style;
       const char *eol_str;
 
@@ -2299,17 +2301,17 @@ diff_repos_wc_file_target(const char *ta
                                 scratch_pool, scratch_pool));
 
       /* We might have to create a normalised version of the working file. */
+      eol_prop = apr_hash_get(file2_props, SVN_PROP_EOL_STYLE,
+                              APR_HASH_KEY_STRING);
       svn_subst_eol_style_from_value(&eol_style, &eol_str,
-                                     apr_hash_get(file2_props,
-                                                  SVN_PROP_EOL_STYLE,
-                                                  APR_HASH_KEY_STRING));
+                                     eol_prop ? eol_prop->data : NULL);
       keywords_prop = apr_hash_get(file2_props, SVN_PROP_KEYWORDS,
                                    APR_HASH_KEY_STRING);
       if (keywords_prop)
         SVN_ERR(svn_subst_build_keywords2(&keywords, keywords_prop->data,
                                           NULL, NULL, 0, NULL,
                                           scratch_pool));
-      if (svn_subst_translation_required(eol_style, SVN_SUBST_NATIVE_EOL_STR,
+      if (svn_subst_translation_required(eol_style, eol_str,
                                          keywords, FALSE, TRUE))
         {
           svn_stream_t *working_content;
@@ -2323,7 +2325,7 @@ diff_repos_wc_file_target(const char *ta
                                          svn_io_file_del_on_pool_cleanup,
                                          scratch_pool, scratch_pool));
           normalized_content = svn_subst_stream_translated(
-                                 file2_content, SVN_SUBST_NATIVE_EOL_STR,
+                                 file2_content, eol_str,
                                  TRUE, keywords, FALSE, scratch_pool);
           SVN_ERR(svn_stream_copy3(working_content, normalized_content,
                                    ctx->cancel_func, ctx->cancel_baton,
@@ -2331,42 +2333,46 @@ diff_repos_wc_file_target(const char *ta
         }
     }
 
+  mimetype1 = file1_props ? apr_hash_get(file1_props, SVN_PROP_MIME_TYPE,
+                                         APR_HASH_KEY_STRING)
+                          : NULL;
+  mimetype2 = apr_hash_get(file2_props, SVN_PROP_MIME_TYPE,
+                           APR_HASH_KEY_STRING);
+
   if (kind1 == svn_node_file && !(show_copies_as_adds && is_copy))
     {
+      apr_array_header_t *propchanges;
+
       SVN_ERR(callbacks->file_opened(NULL, NULL, target,
                                      reverse ? SVN_INVALID_REVNUM : rev,
                                      callback_baton, scratch_pool));
 
       if (reverse)
-        SVN_ERR(callbacks->file_changed(NULL, NULL, NULL, target,
-                                        file2_abspath, file1_abspath,
-                                        SVN_INVALID_REVNUM, rev,
-                                        apr_hash_get(file2_props,
-                                                     SVN_PROP_MIME_TYPE,
-                                                     APR_HASH_KEY_STRING),
-                                        apr_hash_get(file1_props,
-                                                     SVN_PROP_MIME_TYPE,
-                                                     APR_HASH_KEY_STRING),
-                                        make_regular_props_array(
-                                          file1_props, scratch_pool,
-                                          scratch_pool),
-                                        file2_props,
-                                        callback_baton, scratch_pool));
+        {
+          SVN_ERR(svn_prop_diffs(&propchanges, file1_props, file2_props,
+                                 scratch_pool));
+
+          SVN_ERR(callbacks->file_changed(NULL, NULL, NULL, target,
+                                          file2_abspath, file1_abspath,
+                                          SVN_INVALID_REVNUM, rev,
+                                          mimetype2 ? mimetype2->data : NULL,
+                                          mimetype1 ? mimetype1->data : NULL,
+                                          propchanges, file2_props,
+                                          callback_baton, scratch_pool));
+        }
       else
-        SVN_ERR(callbacks->file_changed(NULL, NULL, NULL, target,
-                                        file1_abspath, file2_abspath,
-                                        rev, SVN_INVALID_REVNUM,
-                                        apr_hash_get(file1_props,
-                                                     SVN_PROP_MIME_TYPE,
-                                                     APR_HASH_KEY_STRING),
-                                        apr_hash_get(file2_props,
-                                                     SVN_PROP_MIME_TYPE,
-                                                     APR_HASH_KEY_STRING),
-                                        make_regular_props_array(
-                                          file2_props, scratch_pool,
-                                          scratch_pool),
-                                        file1_props,
-                                        callback_baton, scratch_pool));
+        {
+          SVN_ERR(svn_prop_diffs(&propchanges, file2_props, file1_props,
+                                 scratch_pool));
+
+          SVN_ERR(callbacks->file_changed(NULL, NULL, NULL, target,
+                                          file1_abspath, file2_abspath,
+                                          rev, SVN_INVALID_REVNUM,
+                                          mimetype1 ? mimetype1->data : NULL,
+                                          mimetype2 ? mimetype2->data : NULL,
+                                          propchanges, file1_props,
+                                          callback_baton, scratch_pool));
+        }
     }
   else
     {
@@ -2374,9 +2380,7 @@ diff_repos_wc_file_target(const char *ta
         {
           SVN_ERR(callbacks->file_deleted(NULL, NULL,
                                           target, file2_abspath, file1_abspath,
-                                          apr_hash_get(file2_props,
-                                                       SVN_PROP_MIME_TYPE,
-                                                       APR_HASH_KEY_STRING),
+                                          mimetype2 ? mimetype2->data : NULL,
                                           NULL,
                                           make_regular_props_hash(
                                             file2_props, scratch_pool,
@@ -2389,9 +2393,7 @@ diff_repos_wc_file_target(const char *ta
                                         file1_abspath, file2_abspath,
                                         rev, SVN_INVALID_REVNUM,
                                         NULL,
-                                        apr_hash_get(file2_props,
-                                                     SVN_PROP_MIME_TYPE,
-                                                     APR_HASH_KEY_STRING),
+                                        mimetype2 ? mimetype2->data : NULL,
                                         NULL, SVN_INVALID_REVNUM,
                                         make_regular_props_array(
                                           file2_props, scratch_pool,

Modified: subversion/branches/1.7.x/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/tests/cmdline/diff_tests.py?rev=1564900&r1=1564899&r2=1564900&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/branches/1.7.x/subversion/tests/cmdline/diff_tests.py Wed Feb  5 19:46:55 2014
@@ -45,16 +45,39 @@ Item = svntest.wc.StateItem
 ######################################################################
 # Generate expected output
 
-def make_diff_header(path, old_tag, new_tag):
+def is_absolute_url(target):
+  return (target.startswith('file://')
+          or target.startswith('http://')
+          or target.startswith('https://')
+          or target.startswith('svn://')
+          or target.startswith('svn+ssh://'))
+
+def make_diff_header(path, old_tag, new_tag, src_label=None, dst_label=None):
   """Generate the expected diff header for file PATH, with its old and new
-  versions described in parentheses by OLD_TAG and NEW_TAG. Return the header
-  as an array of newline-terminated strings."""
+  versions described in parentheses by OLD_TAG and NEW_TAG. SRC_LABEL and
+  DST_LABEL are paths or urls that are added to the diff labels if we're
+  diffing against the repository or diffing two arbitrary paths.
+  Return the header as an array of newline-terminated strings."""
+  if src_label:
+    src_label = src_label.replace('\\', '/')
+    if not is_absolute_url(src_label):
+      src_label = '.../' + src_label
+    src_label = '\t(' + src_label + ')'
+  else:
+    src_label = ''
+  if dst_label:
+    dst_label = dst_label.replace('\\', '/')
+    if not is_absolute_url(dst_label):
+      dst_label = '.../' + dst_label
+    dst_label = '\t(' + dst_label + ')'
+  else:
+    dst_label = ''
   path_as_shown = path.replace('\\', '/')
   return [
     "Index: " + path_as_shown + "\n",
     "===================================================================\n",
-    "--- " + path_as_shown + "\t(" + old_tag + ")\n",
-    "+++ " + path_as_shown + "\t(" + new_tag + ")\n",
+    "--- " + path_as_shown + src_label + "\t(" + old_tag + ")\n",
+    "+++ " + path_as_shown + dst_label + "\t(" + new_tag + ")\n",
     ]
 
 def make_no_diff_deleted_header(path, old_tag, new_tag):
@@ -3867,6 +3890,122 @@ def diff_deleted_url(sbox):
                                      'diff', '-c2',
                                      sbox.repo_url + '/A/D/H')
 
+@Issue(4460)
+def diff_repo_wc_file_props(sbox):
+  "diff repo to wc file target with props"
+  sbox.build()
+  iota = sbox.ospath('iota')
+
+  # add a mime-type and a line to iota to test the binary check
+  sbox.simple_propset('svn:mime-type', 'text/plain', 'iota')
+  sbox.simple_append('iota','second line\n')
+
+  # test that we get the line and the property add
+  expected_output = make_diff_header(iota, 'revision 1', 'working copy') + \
+                    [ '@@ -1 +1,2 @@\n',
+                      " This is the file 'iota'.\n",
+                      "+second line\n", ] + \
+                    make_diff_prop_header(iota) + \
+                    make_diff_prop_added('svn:mime-type', 'text/plain')
+  svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                     'diff', '-r1', iota)
+
+  # reverse the diff, should get a property delete and line delete
+  # skip actually testing the output since apparently 1.7 is busted
+  # this isn't related to issue #4460, older versions of 1.7 had the issue
+  # as well
+  #expected_output = make_diff_header(iota, 'working copy', 'revision 1') + \
+  #                  [ '@@ -1,2 +1 @@\n',
+  #                    " This is the file 'iota'.\n",
+  #                    "-second line\n", ] + \
+  #                  make_diff_prop_header(iota) + \
+  #                  make_diff_prop_deleted('svn:mime-type', 'text/plain')
+  expected_output = None
+  svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                     'diff', '--old', iota,
+                                     '--new', iota + '@1')
+
+  # copy iota to test with --show-copies as adds
+  sbox.simple_copy('iota', 'iota_copy')
+  iota_copy = sbox.ospath('iota_copy')
+
+  # test that we get all lines as added and the property added
+  # TODO: We only test that this test doesn't error out because of Issue #4464
+  # if and when that issue is fixed this test should check output
+  svntest.actions.run_and_verify_svn(None, None, [], 'diff',
+                                     '--show-copies-as-adds', '-r1', iota_copy)
+
+  # reverse the diff, should get all lines as a delete and no property
+  # TODO: We only test that this test doesn't error out because of Issue #4464
+  # if and when that issue is fixed this test should check output
+  svntest.actions.run_and_verify_svn(None, None, [], 'diff',
+                                     '--show-copies-as-adds',
+                                     '--old', iota_copy,
+                                     '--new', iota + '@1')
+
+  # revert and commit with the eol-style of LF and then update so
+  # that we can see a change on either windows or *nix.
+  sbox.simple_revert('iota', 'iota_copy')
+  sbox.simple_propset('svn:eol-style', 'LF', 'iota')
+  sbox.simple_commit() #r2
+  sbox.simple_update()
+
+  # now that we have a LF file on disk switch to CRLF
+  sbox.simple_propset('svn:eol-style', 'CRLF', 'iota')
+
+  # test that not only the property but also the file changes
+  # i.e. that the line endings substitution works
+  if svntest.main.is_os_windows():
+    # test suite normalizes crlf output into just lf on Windows.
+    # so we have to assume it worked because there is an add and
+    # remove line with the same content.  Fortunately, it does't
+    # do this on *nix so we can be pretty sure that it works right.
+    # TODO: Provide a way to handle this better
+    crlf = '\n'
+  else:
+    crlf = '\r\n'
+  expected_output = make_diff_header(iota, 'revision 1', 'working copy') + \
+                    [ '@@ -1 +1 @@\n',
+                      "-This is the file 'iota'.\n",
+                      "+This is the file 'iota'." + crlf ] + \
+                    make_diff_prop_header(iota) + \
+                    make_diff_prop_added('svn:eol-style', 'CRLF')
+
+  svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                     'diff', '-r1', iota)
+
+
+@Issue(4460)
+def diff_repo_repo_added_file_mime_type(sbox):
+    "diff repo to repo added file with mime-type"
+    sbox.build()
+    wc_dir = sbox.wc_dir
+    newfile = sbox.ospath('newfile')
+
+    # add a file with a mime-type
+    sbox.simple_append('newfile', "This is the file 'newfile'.\n")
+    sbox.simple_add('newfile')
+    sbox.simple_propset('svn:mime-type', 'text/plain', 'newfile')
+    sbox.simple_commit() # r2
+
+    # try to diff across the addition
+    expected_output = make_diff_header(newfile, 'revision 1', 'revision 2') + \
+                      [ '@@ -0,0 +1 @@\n',
+                        "+This is the file 'newfile'.\n" ] + \
+                      make_diff_prop_header(newfile) + \
+                      make_diff_prop_added('svn:mime-type', 'text/plain')
+
+    svntest.actions.run_and_verify_svn(None, expected_output, [], 'diff',
+                                       '-r1:2', newfile)
+
+    # reverse the diff to diff across a deletion
+    # Note no property delete is printed when whole file is deleted
+    expected_output = make_diff_header(newfile, 'revision 2', 'revision 1') + \
+                      [ '@@ -1, +0,0 @@\n',
+                        "-This is the file 'newfile'.\n" ]
+    svntest.actions.run_and_verify_svn(None, None, [], 'diff',
+                                       '-r2:1', newfile)
+
 ########################################################################
 #Run the tests
 
@@ -3935,6 +4074,8 @@ test_list = [ None,
               no_spurious_conflict,
               diff_deleted_url,
               diff_git_format_wc_wc_dir_mv,
+              diff_repo_wc_file_props,
+              diff_repo_repo_added_file_mime_type,
               ]
 
 if __name__ == '__main__':



Mime
View raw message