Merge lp:~jameinel/bzr/2.1.0b4-win32-unsupported_encoding_commit_message into lp:bzr

[John A Meinel]

[John A Meinel (jameinel)]

Another small update to get the test suite passing on windows. From the commit message:

test_unsupported_encoding_commit_message no longer applies for Windows.

We now parse a Unicode string directly, so it isn't really possible to pass
a non-ascii string that will fail to decode.

I'm slowly building these up into a separate 'win32 test suite' branch, but I figured it would be a lot easier to review them one-at-a-time.

Then if anything ends up being controversial, it doesn't have to block the rest of them.

[Martin Packman (gz)]

It is possible to give bazaar commit messages in the wrong encoding under windows using subprocess, but as what happens depends on the codepage, it's not very interesting to test. Again, would prefer if this didn't check sys.platform but confirmed that it does turn the failure into a n/a here.

review: Approve

[John A Meinel (jameinel)]

> It is possible to give bazaar commit messages in the wrong encoding under
> windows using subprocess, but as what happens depends on the codepage, it's
> not very interesting to test. Again, would prefer if this didn't check
> sys.platform but confirmed that it does turn the failure into a n/a here.

>>> p = subprocess.call([sys.executable, '-c',
                         'from bzrlib.win32utils import get_unicode_argv;'
                         'print get_unicode_argv()',
    'foo', 'ba\x99'])
[u'foo', u'ba\u2122']

This code is testing that passing something to the process will cause it to
raise an appropriate *error* when decoding the command line arguments. However,
since we now use "get_unicode_argv()" Windows itself is decoding the command
line arguments, and I'm pretty sure all of its codepages support all 8-bit
characters.

I'm not positive to that fact, as I haven't tried different OS installs
(Russian Windows, etc.)

I can say that the meaning of 'ba\x99' will probably depend on the current
codepage, but I haven't found a way for the subprocess to think it has failed
to read its arguments. (A user can't force the arguments in such a way that the
client can tell the arguments aren't meaningful.)

That would be different if you could, for example, set your code-page to a
multi-byte sequence like UTF-8.

However, this *isn't* about getting commit message encodings wrong, this is
about failing to decode command line arguments.

[Martin Packman (gz)]

Right, it's not interesting to test because it's just mangled message (here, malformed multibyte characters get discarded or replaced with a generic character).

=== modified file 'bzrlib/tests/blackbox/test_commit.py'
--- bzrlib/tests/blackbox/test_commit.py	2009-11-03 20:24:25 +0000
+++ bzrlib/tests/blackbox/test_commit.py	2009-11-08 01:20:24 +0000
@@ -25,6 +25,7 @@
     ignores,
     msgeditor,
     osutils,
+    tests,
     )
 from bzrlib.bzrdir import BzrDir
 from bzrlib.tests import (
@@ -263,6 +264,9 @@
         self.run_bzr('commit -m ""', retcode=3)
 
     def test_unsupported_encoding_commit_message(self):
+        if sys.platform == 'win32':
+            raise tests.TestNotApplicable('Win32 parses arguments directly'
+                ' as Unicode, so we can\'t pass invalid non-ascii')
         tree = self.make_branch_and_tree('.')
         self.build_tree_contents([('foo.c', 'int main() {}')])
         tree.add('foo.c')
@@ -273,10 +277,6 @@
         if char is None:
             raise TestSkipped('Cannot find suitable non-ascii character'
                 'for user_encoding (%s)' % osutils.get_user_encoding())
-        # TODO: jam 2009-07-23 This test seems to fail on Windows now. My best
-        #       guess is that the change to use Unicode command lines means
-        #       that we no longer pay any attention to LANG=C when decoding the
-        #       commandline arguments.
         out,err = self.run_bzr_subprocess('commit -m "%s"' % char,
                                           retcode=1,
                                           env_changes={'LANG': 'C'})