[MERGE] don't report malformed bundles as NotABundle

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> Aaron Bentley wrote:
>>>> If we say 'assertRaises', it's a failure if it doesn't raise.
> 
>>> But assertNotRaises could raise an IOError, or BadBundle, or IndexError,
>>> or KeyError, or anything.
> 
> If, in normal use, crlf bundles caused IOError, or IndexError, or
> KeyError, that would be a normal kind of bug.  Raising NotABundle,
> specifically, will cause very very confusing behaviour.  Nothing else
> will cause that.
> 
>>> If you want it to succeed, then just perform the action and don't
>>> assertRaises().
> 
> I want it to either succed or else not raise NotABundle.  I don't want
> to codify that crlf bundles should raise BadBundle, but I can't assert
> that they don't fail.

Well, your assertNotRaises catches all other exceptions. I would
recommend doing:

try:
  do_the_thing
except NotABundle:
  self.fail('do_the_thing can't raise NotABundle')

All other actions are regular bugs that need to be fixed.

> 
>>> I agree that bundles should support \r\n as much as they support \n. In
>>> the loop to find the header of the bundle, we are currently doing:
> 
> That doesn't look like the loop in bundle/serializer/__init__.py to me.
>  Those lines are gone now.
> 
>>> And then in '_read_next_entry()' we can change this line:
>>> line = line[1:-1].decode('utf-8') # Remove the '#' and '\n'
>>> to
>>> line = line[1:].rstrip().decode('utf-8')

Well, then you can change that to "line[1:].rstrip('\r\n')"

I suppose we could have a problem if the message itself used '\r\n' line
endings. Though I believe we strip them out when we read the commit
message. (We read in text mode).

Another possible fix is to check the line ending of the header line. And
then assert that all '#' prefixed lines end with that line ending, and
strip it.

> 
> I don't think so.  We may have trailing whitespace in our text, e.g.:
> 
> # message:
> #   This is a message with a lot of
> #   whitespace in it.

Well, I'm not sure what you wrote, because there are no trailing
whitespaces in the above text.

Probably your mailer stripped them.

> 
>>> The other piece is that I was originally planning on changing from a
>>> custom serializer to using RIO to read/write the information, and just
>>> wrap around the RIO iterator so that we prepend/strip the '# ' for each
>>> line.
> 
> Sure.  With the refactoring done, you certainly have the option of doing
> that.  I think this format is nicer to read than RIO, though.
> 
> Aaron

The only difference I know of, is that RIO puts the first line of a
multiline string on the same line. Whereas this one puts the first line
on the next line. (And uses spaces rather than a tab character).
Is there some other difference that I don't know about?

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060621/3bbc3dda/attachment.pgp