[MERGE] Implement xml8 serializer

[Ian Clatworthy]

Aaron Bentley wrote:
> Hi all,
> 
> This patch implements the xml8 serializer which will be used by the
> upcoming pack-1.4 format.

Thanks for working on this. I'm not 100% sure I know enough about this
area to give the only review but the code seems straight-forward and I'm
keen to see this land in 1.4.

bb:tweak

This needs a NEW entry. Also, xml8._check_revisions() needs its
docstring updated: "By default no checking is done." is no longer correct.

I also like how you replaced some of the asserts with explicit checks, e.g.

> -        assert elt.tag == 'inventory'
> +        if elt.tag != 'inventory':
> +            raise errors.UnexpectedInventoryFormat('Root tag is %r' % elt.tag)

It would be nice to do this in xml5._unpack_inventory() as well.

> This patch changes the inheritance so that older formats inherit from
> new ones.  This means that the current format is all in one place, and
> we can drop support for old formats easily.  It also makes it easy for
> future formats to use the same numbers for their revision and inventory
> formats.

I'm good with this approach. The one thing that stands out though as a
consequence is that a bunch of "general" escaping functions have moved
from xml5.yyy to xml8.yyy. If and when we were to drop support for xml8
in N years time, we'd arguably be caught doing the same shuffle again.
That's a long way of saying that they arguably have nothing to do with
any particular version and ought to be moved to a more generic module,
e.g. xml_serialiser or xmlescape, say. I'm not saying that's required to
land this, though it does seem as good a time as any to put them in the
"right" place given we're moving them as part of this patch.

Finally, I did a code review of xml_serializer in order to gain enough
context to review this. This bit is either wrong or confusing:

>     def read_inventory(self, f, revision_id=None):
>         try:
>             return self._unpack_inventory(self._read_element(f),
>                 revision_id=None)
>         except ParseError, e:
>             raise errors.UnexpectedInventoryFormat(e)

The call to _unpack_inventory always passes None. If this is deliberate,
it needs a comment. My guess is that it's a bug. Could you please let me
know and perhaps address it in your patch? I'm happy to put through a
patch if you prefer once I know the right answer.

Ian C.