Monkey patching private escaping functions breaks with ElementTree 1.3

[Martin Packman]

On Python 2.7 the following tests fail:

    bt.test_bundle.V08BundleTester.test_bundle_unicode_properties
    bt.test_bundle.V09BundleKnit1Tester.test_bundle_unicode_properties
    bt.test_bundle.V09BundleKnit2Tester.test_bundle_unicode_properties
    bt.test_bundle.V4BundleTester.test_bundle_unicode_properties
    bt.test_bundle.V4WeaveBundleTester.test_bundle_unicode_properties

All the assertions involve expecting non-ascii characters, but getting xml escapes instead, like:

    Traceback (most recent call last):
      File ".\bzrlib\tests\test_bundle.py", line 1015, in test_bundle_unicode_properties
        'alpha':u'\u03b1'}, rev.properties)
    AssertionError: not equal:
    a = {'alpha': u'\u03b1', 'branch-nick': 'tree', 'omega': u'\u03a9'}
    b = {'alpha': 'α', 'branch-nick': 'tree', 'omega': 'Ω'}

The specific upstream change that affected this was the update to ElementTree 1.3 so anyone installing that separately will also run into this:
<http://svn.python.org/view?view=rev&revision=78838>

The fault however is in bzrlib.xml_serializer monkey-patching some private ElementTree escaping functions, which have been changed in the new release. Again, there was an intention to avoid this sort of issue when the code was written...
    # performance tuning for elementree's serialiser. This should be
    # sent upstream - RBC 20060523.
    # the functions here are patched into elementtree at runtime.

Is this optimisation still important, or does it only matter for old formats? Either the monkey-patching should be made conditional on ElementTree version, or updated, or just ripped out.

[Martin Packman (gz)]

In the end ripping out the monkey patching was okay as there's a newer escaping function in bzrlib.xml8 anyway.

[Vincent Ladeuil (vila)]

>>>>> Martin [gz] <email address hidden> writes:

    > In the end ripping out the monkey patching was okay as there's a newer
    > escaping function in bzrlib.xml8 anyway.

Do you have a reference for that ?

Because bzr-xmlouput was using bzrlib.xml_serrializer._escape_cdata and
is currently broken.

So it needs to use this newer function or we should keep providing it
even with a deprecation warning.

[Martin Packman (gz)]

Well, that's an underscore prefixed function that was never intended to be publicly exposed, so I feel little guilt about breaking bzr-xmloutput. It's easy to resolve in the short term, just change the import to `from (xml.etree|elementtree).ElementTree import _escape_cdata` instead (with try/except ImportError obviously). The functions at the top of bzrlib.xml8 would also be an option, but I'd prefer if it moved instead to something public and documented.

Looking at the bzr-xmloutput code, the way it is currently creating xml is... not nice, and bug 568941 seems to show it has big problems anyway.