Merge lp://qastaging/~wjl/bzr/bug-317644 into lp://qastaging/~bzr/bzr/trunk-old

This fix is a simple and obvious one-liner. It would be great to get this merged.

Seems ok to me; having a test demonstrating it would be better though.

I went to add a test case, to ensure that it really did work correctly, unfortunately I found that it does not.

Part of the problem is that on Windows, python uses UCS-2 internally (16-bit wide characters) while on Linux it generally uses UCS-4 (32-bit wide characters).

As such, len(u'\U0010FFFF') == 2 on Windows and 1 on Linux.

When I started adding simple tests, it turned out that _unicode_re.sub() does very strange things. The test I wanted to add to bzrlib/tests/test_xml.py was simple enough:

+ def test_non_bmp_unicode(self):
+ uni_str = u'\xb5\u0090\U0010abcd'
+ self.assertEqual('µ􊯍',
+ xml8._encode_and_escape(uni_str))

However, the issues with wide characters complicate things.

Specifically, if I just use the change proposed, I get:
UnicodeEncodeError: 'ascii' codec can't encode character u'\udfcd' in position 20: ordinal not in range(128)

Digging into it, I think it is actually a bug in python2.5's Unicode regex _match functionality. In that if you do:

import re
_unicode_re = re.compile(u'[&<>\'\"\u0080-\U0010ffff]')
def print_matches(match):
    val = match.group()
    print repr(val)
    return "&#%d;" % ord(val)

_unicode_re.sub(print_matches, u'\U0010abcd')

It prints out:
u'\udbea'
u'&#56298;\udfcd'

Notice that it *only* matched the first character, and didn't do *any* processing on the second character.

If you change it to:
_unicode_re = re.compile(u'[&<>\'\"\u0080-\uffff]')
_unicode_re.sub(print_matches, u'\U0010abcd')

You get:
u'\udbea'
u'\udfcd'
u'&#56298;&#57293;'

Note, however, that if I run the same tests on Linux I get:
u'\U0010abcd'
'&#1092557;'
versus
# No match
u'\U0010abcd'

My best guess is that we need to check the width of u'\U0010ffff' and respond accordingly. For example:

  if _unicode_re is None:
    if len(u'\U0010ffff') == 2:
      # this platform represents non BMP characters using 2 Py_UNICODE chars, so all
      # individual bytes are from 0 => ffff
      _unicode_re = re.compile(u'[&<>\'\"\u0080-\uffff]')
    else:
      # \U0010ffff is a single Py_UNICODE char, so match it as such
      _unicode_re = re.compile(u'[&<>\'\"\u0080-\U0010ffff]')

And then the test needs to check the validity of both forms:

+ def test_non_bmp_unicode(self):
+ uni_str = u'\U0010abcd'
+ if len(uni_str) == 2:
+ self.assertEqual('&#56298;&#57293;',
+ xml8._encode_and_escape(uni_str))
+ else:
+ self.assertEqual('&#1092557;',
+ xml8._encode_and_escape(uni_str))

However, this also implies that we need to make sure we can translate those
values *back* to their non-normalized form.

Even more worrisome, though, is that now 2 people generating the same identical
inventory will be generating different byte strings (based on whether the
inventory was generated from Windows or Linux.)

This has a fairly serious implication, as it means that line-based deltas won't
always cleanly apply if 2 people have different base texts.

This won't come into play often, because if *I* generate a text, and you *copy*
it from me, things will be fine. However, 'bzr upgrade' (for exam...