You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2014/02/05 20:46:55 UTC
svn commit: r1564900 - in /subversion/branches/1.7.x: ./ STATUS
subversion/libsvn_client/diff.c subversion/tests/cmdline/diff_tests.py
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__':