[merge][#109613] don't set bugs property unless bugs are specified

[John Arbash Meinel]

John Arbash Meinel wrote:
> John Arbash Meinel has voted +1.

...

> ah, so maybe that is it. The *reading* code is doing:
>                 key_end = property.find(': ')
>                 assert key_end is not None
> But there is no ': ' because we have ':\n'. And the assert isn't
> trapping what it thinks it is, because key_end would be '-1', not None.
> 
> And that ends up generating
> 
> key = property[:-1].encode('utf-8') # Which should accidentally work
> value = property[-1+2:] # Which is something like 'ugs:'
> 
> So yes, there is a bug for empty properties in the old serializer.
> 


So I have a question about how we should fix this. I see 2 possible fixes:

1) Fix the bundle reader to handle ':\n' properly as a property with an
empty value.

2) Change the writer to emit ": \n" for empty properties.


The nice thing about (2) is that it means bundles generated with a newer
bzr will be readable by the (buggy) older bzr. The downside is that we
then have significant trailing whitespace in our output, which is a bit
ugly (and likely to be damaged by email transports).

(1) makes us compatible with ourselves, and with what older clients
should generate, so I think it should be done regardless.


So do we try to maintain compatibility with a buggy reader? Or do we
just move forward, and allow for the fact that empty properties aren't
read properly by older versions of bzr?

John
=:->