You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/02/02 23:04:44 UTC
svn commit: r1441814 - in /subversion/trunk/subversion: svn/ tests/cmdline/
Author: stsp
Date: Sat Feb 2 22:04:44 2013
New Revision: 1441814
URL: http://svn.apache.org/viewvc?rev=1441814&view=rev
Log:
When a binary mime-type is set on a file that looks like a text file,
make the 'svn' client print a warning about potential future problems
with operations such as diff, merge, and blame.
This is only done during local propset for now, because the file needs
to be present on disk to detect its mime-type.
See for related discussion: http://mail-archives.apache.org/mod_mbox/subversion-dev/201301.mbox/%3C20130131185725.GA13721%40ted.stsp.name%3E
* subversion/svn/cl.h
(svn_cl__propset_print_binary_mime_type_warning): Declare.
* subversion/svn/propedit-cmd.c
(svn_cl__propedit): Warn when a binary svn:mime-type property value is
set on a local file that looks like text.
* subversion/svn/propset-cmd.c
(svn_cl__propset): Warn when a binary svn:mime-type property value is
set on a local file that looks like text.
* subversion/svn/util.c
(svn_cl__propset_print_binary_mime_type_warning): New helper function.
* subversion/tests/cmdline/blame_tests.py
(blame_binary): Adjust text expectations.
* subversion/tests/cmdline/commit_tests.py
(commit_one_new_binary_file): Adjust test expectations.
* subversion/tests/cmdline/diff_tests.py
(diff_force, diff_mime_type_changes):
* subversion/tests/cmdline/info_tests.py
(binary_tree_conflict): Adjust test expectations.
* subversion/tests/cmdline/merge_tests.py
(merge_change_to_file_with_executable,
merge_binary_file_with_keywords): Adjust test expectations.
* subversion/tests/cmdline/prop_tests.py
(binary_mime_type_on_text_file_warning): New constant.
(inappropriate_props): Adjust test expectations.
* subversion/tests/cmdline/svnlook_tests.py
(diff_binary): Adjust test expectations.
Modified:
subversion/trunk/subversion/svn/cl.h
subversion/trunk/subversion/svn/propedit-cmd.c
subversion/trunk/subversion/svn/propset-cmd.c
subversion/trunk/subversion/svn/util.c
subversion/trunk/subversion/tests/cmdline/blame_tests.py
subversion/trunk/subversion/tests/cmdline/commit_tests.py
subversion/trunk/subversion/tests/cmdline/diff_tests.py
subversion/trunk/subversion/tests/cmdline/info_tests.py
subversion/trunk/subversion/tests/cmdline/merge_tests.py
subversion/trunk/subversion/tests/cmdline/prop_tests.py
subversion/trunk/subversion/tests/cmdline/svnlook_tests.py
Modified: subversion/trunk/subversion/svn/cl.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cl.h (original)
+++ subversion/trunk/subversion/svn/cl.h Sat Feb 2 22:04:44 2013
@@ -817,6 +817,18 @@ svn_cl__check_related_source_and_target(
svn_client_ctx_t *ctx,
apr_pool_t *pool);
+/* If the user is setting a mime-type to mark one of the TARGETS as binary,
+ * as determined by property name PROPNAME and value PROPVAL, then check
+ * whether Subversion's own binary-file detection recognizes the target as
+ * a binary file. If Subversion doesn't consider the target to be a binary
+ * file, assume the user is making an error and print a warning to inform
+ * the user that some operations might fail on the file in the future. */
+svn_error_t *
+svn_cl__propset_print_binary_mime_type_warning(apr_array_header_t *targets,
+ const char *propname,
+ const svn_string_t *propval,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Modified: subversion/trunk/subversion/svn/propedit-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/propedit-cmd.c?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/propedit-cmd.c (original)
+++ subversion/trunk/subversion/svn/propedit-cmd.c Sat Feb 2 22:04:44 2013
@@ -315,6 +315,10 @@ svn_cl__propedit(apr_getopt_t *os,
sizeof(const char *));
APR_ARRAY_PUSH(targs, const char *) = target;
+
+ SVN_ERR(svn_cl__propset_print_binary_mime_type_warning(
+ targs, pname_utf8, propval, subpool));
+
err = svn_client_propset_local(pname_utf8, edited_propval,
targs, svn_depth_empty,
opt_state->force, NULL,
Modified: subversion/trunk/subversion/svn/propset-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/propset-cmd.c?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/propset-cmd.c (original)
+++ subversion/trunk/subversion/svn/propset-cmd.c Sat Feb 2 22:04:44 2013
@@ -173,6 +173,11 @@ svn_cl__propset(apr_getopt_t *os,
}
}
+ SVN_ERR(svn_cl__propset_print_binary_mime_type_warning(targets,
+ pname_utf8,
+ propval,
+ scratch_pool));
+
SVN_ERR(svn_client_propset_local(pname_utf8, propval, targets,
opt_state->depth, opt_state->force,
opt_state->changelists, ctx,
Modified: subversion/trunk/subversion/svn/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/util.c?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/util.c (original)
+++ subversion/trunk/subversion/svn/util.c Sat Feb 2 22:04:44 2013
@@ -54,8 +54,10 @@
#include "svn_utf.h"
#include "svn_subst.h"
#include "svn_config.h"
+#include "svn_wc.h"
#include "svn_xml.h"
#include "svn_time.h"
+#include "svn_props.h"
#include "svn_private_config.h"
#include "cl.h"
@@ -1053,3 +1055,59 @@ svn_cl__check_related_source_and_target(
}
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_cl__propset_print_binary_mime_type_warning(apr_array_header_t *targets,
+ const char *propname,
+ const svn_string_t *propval,
+ apr_pool_t *scratch_pool)
+{
+ if (strcmp(propname, SVN_PROP_MIME_TYPE) == 0)
+ {
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+ int i;
+
+ for (i = 0; i < targets->nelts; i++)
+ {
+ const char *detected_mimetype;
+ const char *target = APR_ARRAY_IDX(targets, i, const char *);
+ const char *local_abspath;
+ const svn_string_t *canon_propval;
+ svn_node_kind_t node_kind;
+
+ svn_pool_clear(iterpool);
+
+ SVN_ERR(svn_dirent_get_absolute(&local_abspath, target, iterpool));
+ SVN_ERR(svn_io_check_path(local_abspath, &node_kind, iterpool));
+ if (node_kind != svn_node_file)
+ continue;
+
+ SVN_ERR(svn_wc_canonicalize_svn_prop(&canon_propval,
+ propname, propval,
+ local_abspath,
+ svn_node_file,
+ FALSE, NULL, NULL,
+ scratch_pool));
+
+ if (svn_mime_type_is_binary(canon_propval->data))
+ {
+ SVN_ERR(svn_io_detect_mimetype2(&detected_mimetype,
+ local_abspath, NULL,
+ iterpool));
+ if (detected_mimetype == NULL ||
+ !svn_mime_type_is_binary(detected_mimetype))
+ svn_error_clear(svn_cmdline_fprintf(stderr, iterpool,
+ _("svn: warning: '%s' is a binary mime-type but file '%s' "
+ "looks like text; diff, merge, blame, and other "
+ "operations will stop working on this file\n"),
+ canon_propval->data,
+ svn_dirent_local_style(local_abspath, iterpool)));
+
+ }
+ }
+ svn_pool_destroy(iterpool);
+ }
+
+ return SVN_NO_ERROR;
+}
+
Modified: subversion/trunk/subversion/tests/cmdline/blame_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/blame_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/blame_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/blame_tests.py Sat Feb 2 22:04:44 2013
@@ -31,6 +31,8 @@ import os, sys, re
import svntest
from svntest.main import server_has_mergeinfo
+from prop_tests import binary_mime_type_on_text_file_warning
+
# For some basic merge setup used by blame -g tests.
from merge_tests import set_up_branch
@@ -126,7 +128,8 @@ def blame_binary(sbox):
# Then do it again, but this time we set the mimetype to binary.
iota = os.path.join(wc_dir, 'iota')
svntest.main.file_append(iota, "More new contents for iota\n")
- svntest.main.run_svn(None, 'propset', 'svn:mime-type', 'image/jpeg', iota)
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
+ 'propset', 'svn:mime-type', 'image/jpeg', iota)
svntest.main.run_svn(None, 'ci',
'-m', '', iota)
Modified: subversion/trunk/subversion/tests/cmdline/commit_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/commit_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/commit_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/commit_tests.py Sat Feb 2 22:04:44 2013
@@ -31,6 +31,8 @@ import sys, os, re
import svntest
from svntest import wc
+from prop_tests import binary_mime_type_on_text_file_warning
+
# (abbreviation)
Skip = svntest.testcase.Skip_deco
SkipUnless = svntest.testcase.SkipUnless_deco
@@ -213,7 +215,8 @@ def commit_one_new_binary_file(sbox):
expected_status = make_standard_slew_of_changes(wc_dir)
gloo_path = sbox.ospath('A/D/H/gloo')
- svntest.main.run_svn(None, 'propset', 'svn:mime-type',
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
+ 'propset', 'svn:mime-type',
'application/octet-stream', gloo_path)
# Create expected state.
Modified: subversion/trunk/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/diff_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/diff_tests.py Sat Feb 2 22:04:44 2013
@@ -34,6 +34,8 @@ logger = logging.getLogger()
import svntest
from svntest import err
+from prop_tests import binary_mime_type_on_text_file_warning
+
# (abbreviation)
Skip = svntest.testcase.Skip_deco
SkipUnless = svntest.testcase.SkipUnless_deco
@@ -1887,7 +1889,7 @@ def diff_force(sbox):
# Append a line to iota and make it binary.
svntest.main.file_append(iota_path, "new line")
- svntest.main.run_svn(None,
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
'propset', 'svn:mime-type',
'application/octet-stream', iota_path)
@@ -2304,8 +2306,9 @@ def diff_mime_type_changes(sbox):
'diff', '-r', 'BASE:1')
# Mark iota as a binary file in the working copy.
- svntest.actions.run_and_verify_svn(None, None, [],
- 'propset', 'svn:mime-type',
+ svntest.actions.run_and_verify_svn2(None, None,
+ binary_mime_type_on_text_file_warning, 0,
+ 'propset', 'svn:mime-type',
'application/octet-stream', 'iota')
# Check that the earlier diffs against BASE are unaffected by the
Modified: subversion/trunk/subversion/tests/cmdline/info_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/info_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/info_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/info_tests.py Sat Feb 2 22:04:44 2013
@@ -34,6 +34,8 @@ logger = logging.getLogger()
# Our testing module
import svntest
+from prop_tests import binary_mime_type_on_text_file_warning
+
# (abbreviation)
Skip = svntest.testcase.Skip_deco
SkipUnless = svntest.testcase.SkipUnless_deco
@@ -494,7 +496,9 @@ def binary_tree_conflict(sbox):
sbox.build()
wc_dir = sbox.wc_dir
- sbox.simple_propset('svn:mime-type', 'binary/octet-stream', 'iota')
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
+ 'propset', 'svn:mime-type', 'binary/octet-stream',
+ sbox.ospath('iota'))
sbox.simple_commit()
iota = sbox.ospath('iota')
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Sat Feb 2 22:04:44 2013
@@ -32,6 +32,8 @@ import time
import svntest
from svntest import main, wc, verify, actions
+from prop_tests import binary_mime_type_on_text_file_warning
+
# (abbreviation)
Item = wc.StateItem
Skip = svntest.testcase.Skip_deco
@@ -16524,7 +16526,8 @@ def merge_change_to_file_with_executable
beta_path = sbox.ospath('A/B/E/beta')
# Force one of the files to be a binary type
- svntest.actions.run_and_verify_svn(None, None, [],
+ svntest.actions.run_and_verify_svn2(None, None,
+ binary_mime_type_on_text_file_warning, 0,
'propset', 'svn:mime-type',
'application/octet-stream',
alpha_path)
@@ -17915,7 +17918,9 @@ def merge_binary_file_with_keywords(sbox
# make some 'binary' files with keyword expansion enabled
for f in files:
svntest.main.file_append(sbox.ospath(f), "With $Revision: $ keyword.\n")
- sbox.simple_propset('svn:mime-type', 'application/octet-stream', f)
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
+ 'propset', 'svn:mime-type',
+ 'application/octet-stream', sbox.ospath(f))
sbox.simple_propset('svn:keywords', 'Revision', f)
sbox.simple_commit()
Modified: subversion/trunk/subversion/tests/cmdline/prop_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/prop_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/prop_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/prop_tests.py Sat Feb 2 22:04:44 2013
@@ -49,6 +49,10 @@ def is_non_posix_and_non_windows_os():
"""lambda function to skip revprop_change test"""
return (not svntest.main.is_posix_os()) and sys.platform != 'win32'
+# this is global so other test files can use it
+binary_mime_type_on_text_file_warning = \
+ "svn: warning:.*is a binary mime-type but file.*looks like text.*"
+
######################################################################
# Tests
@@ -612,7 +616,9 @@ def inappropriate_props(sbox):
svntest.main.file_append(path, "binary")
sbox.simple_add('binary')
- sbox.simple_propset('svn:mime-type', 'application/octet-stream', 'binary')
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
+ 'propset', 'svn:mime-type', 'application/octet-stream',
+ sbox.ospath('binary'))
svntest.actions.run_and_verify_svn('Illegal target', None,
svntest.verify.AnyOutput,
Modified: subversion/trunk/subversion/tests/cmdline/svnlook_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnlook_tests.py?rev=1441814&r1=1441813&r2=1441814&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnlook_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnlook_tests.py Sat Feb 2 22:04:44 2013
@@ -32,6 +32,7 @@ logger = logging.getLogger()
# Our testing module
import svntest
+from prop_tests import binary_mime_type_on_text_file_warning
# (abbreviation)
Skip = svntest.testcase.Skip_deco
@@ -578,7 +579,8 @@ def diff_binary(sbox):
# Set A/mu to a binary mime-type, tweak its text, and commit.
mu_path = os.path.join(wc_dir, 'A', 'mu')
svntest.main.file_append(mu_path, 'new appended text for mu')
- svntest.main.run_svn(None, 'propset', 'svn:mime-type',
+ svntest.main.run_svn(binary_mime_type_on_text_file_warning,
+ 'propset', 'svn:mime-type',
'application/octet-stream', mu_path)
svntest.main.run_svn(None, 'ci', '-m', 'log msg', mu_path)
Re: svn commit: r1441814 - in /subversion/trunk/subversion: svn/
tests/cmdline/
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 02, 2013 at 11:38:56PM +0100, Bert Huijben wrote:
>From my users I hear that another way this property is introduced is via conversions from other version management systems. Visual SourceSafe (long dead, but still used in a lot of small shops) marks UTF-8 files with a BOM as binary when it does an auto detect.
> (Well what would you guess for a system that wasn’t really updated since that format became popular)
>
> Most conversion tools just copy the binary flag, and there you have this problem on all your historic utf-8 files.
> (Where I worked we had this problem on all .xml files previously stored in sourcesafe).
>
>
> I don't see a lot of users accidentally adding invalid properties themselves.
I agree that automatic creation of such properties is also a problem.
However, it is not the problem that was discussed in the referenced
users@ thread. This really is about the rare case where a novice user
makes a mistake, nothing more.
Re: svn commit: r1441814 - in /subversion/trunk/subversion: svn/ tests/cmdline/
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
As another data point, I have hit this text-as-binary myself just a few
weeks ago when I added a bunch of HTML files to a local repository - so,
it's definitely occurring automatically. I did not have a chance to dig
into why the magic detection failed so miserably... -- justin
On Saturday, February 2, 2013, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: stsp@apache.org <javascript:;> [mailto:stsp@apache.org<javascript:;>
> ]
> > Sent: zaterdag 2 februari 2013 23:05
> > To: commits@subversion.apache.org <javascript:;>
> > Subject: svn commit: r1441814 - in /subversion/trunk/subversion: svn/
> > tests/cmdline/
> >
> > Author: stsp
> > Date: Sat Feb 2 22:04:44 2013
> > New Revision: 1441814
> >
> > URL: http://svn.apache.org/viewvc?rev=1441814&view=rev
> > Log:
> > When a binary mime-type is set on a file that looks like a text file,
> > make the 'svn' client print a warning about potential future problems
> > with operations such as diff, merge, and blame.
> >
> > This is only done during local propset for now, because the file needs
> > to be present on disk to detect its mime-type.
> >
> > See for related discussion: http://mail-
> > archives.apache.org/mod_mbox/subversion-
> > dev/201301.mbox/%3C20130131185725.GA13721%40ted.stsp.name%3E
>
> From my users I hear that another way this property is introduced is via
> conversions from other version management systems. Visual SourceSafe (long
> dead, but still used in a lot of small shops) marks UTF-8 files with a BOM
> as binary when it does an auto detect.
> (Well what would you guess for a system that wasn’t really updated since
> that format became popular)
>
> Most conversion tools just copy the binary flag, and there you have this
> problem on all your historic utf-8 files.
> (Where I worked we had this problem on all .xml files previously stored in
> sourcesafe).
>
>
> I don't see a lot of users accidentally adding invalid properties
> themselves.
>
> Bert
>
>
Re: svn commit: r1441814 - in /subversion/trunk/subversion: svn/
tests/cmdline/
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 02, 2013 at 10:49:50PM +0000, Julian Foad wrote:
> Maybe change the wording to be clear that Subversion *considers* this to be a 'binary' MIME type? Quite likely it is actually a text type that Subversion doesn't know about, and then saying "application/foomatic-plain-text is a binary MIME type" would look stupid.
Yes, good suggestion. Thanks Julian!
Re: svn commit: r1441814 - in /subversion/trunk/subversion: svn/ tests/cmdline/
Posted by Julian Foad <ju...@btopenworld.com>.
Bert Huijben wrote:
>> Author: stsp
>>
>> URL: http://svn.apache.org/viewvc?rev=1441814&view=rev
>> Log:
>> When a binary mime-type is set on a file that looks like a text file,
>> make the 'svn' client print a warning about potential future problems
>> with operations such as diff, merge, and blame.
>>
>> This is only done during local propset for now, because the file needs
>> to be present on disk to detect its mime-type.
Stefan,
Maybe change the wording to be clear that Subversion *considers* this to be a 'binary' MIME type? Quite likely it is actually a text type that Subversion doesn't know about, and then saying "application/foomatic-plain-text is a binary MIME type" would look stupid.
- Julian
RE: svn commit: r1441814 - in /subversion/trunk/subversion: svn/ tests/cmdline/
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: zaterdag 2 februari 2013 23:05
> To: commits@subversion.apache.org
> Subject: svn commit: r1441814 - in /subversion/trunk/subversion: svn/
> tests/cmdline/
>
> Author: stsp
> Date: Sat Feb 2 22:04:44 2013
> New Revision: 1441814
>
> URL: http://svn.apache.org/viewvc?rev=1441814&view=rev
> Log:
> When a binary mime-type is set on a file that looks like a text file,
> make the 'svn' client print a warning about potential future problems
> with operations such as diff, merge, and blame.
>
> This is only done during local propset for now, because the file needs
> to be present on disk to detect its mime-type.
>
> See for related discussion: http://mail-
> archives.apache.org/mod_mbox/subversion-
> dev/201301.mbox/%3C20130131185725.GA13721%40ted.stsp.name%3E
>From my users I hear that another way this property is introduced is via conversions from other version management systems. Visual SourceSafe (long dead, but still used in a lot of small shops) marks UTF-8 files with a BOM as binary when it does an auto detect.
(Well what would you guess for a system that wasn’t really updated since that format became popular)
Most conversion tools just copy the binary flag, and there you have this problem on all your historic utf-8 files.
(Where I worked we had this problem on all .xml files previously stored in sourcesafe).
I don't see a lot of users accidentally adding invalid properties themselves.
Bert