Code review comment for lp:~michael.nelson/launchpad/436182-newlines-in-sources-fix-without-workaround

> I'm adding you as a reviewer as apparently you've been around long enough to
> have a better understanding of the code in question here (our custom
> parse_tagfile_lines function), or at least know who else might :).

Thanks for asking me to review. The algorithmic changes look OK to me. I'm not sure I know any better than you why we have our own version of this function, though. It's been around since the very first version of Soyuz was committed (see r542, although it was called parse_changes back then). Even then, apt_pkg.ParseTagFile was used in the very same file, so it clearly wasn't that Daniel didn't know about it; although the TagFile class has apparently never been used outside tests.

Reasons I can think of why it might have needed to be a separate function:

  * no other way to enforce dsc_whitespace_rules?
  * problems with case-insensitivity at the time? (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=384182)
  * apt_pkg.ParseTagFile seems to really want a real file, not e.g. StringIO

> So if you approve of this branch landing, when would you prefer to be landed?
> We could
> 1. get it cherry-pick it now and so have some time to be condident that there
> are no unforseen problems, or
> 2. land it in devel without a CP, but then it will be rolled out in the
> beginning of Nov - probably not ideal, or
> 3. wait until LP 3.1.10 has been rolled out, and Karmic is released, and
> commit it to devel then (perhaps CPing it). Safer - but then more corrupt data
> in the database.

Given that it's much more likely to affect PPAs than the main archive, and as you say the amount of corrupt data is increasing, I'd be happy for this to be cherry-picked ASAP.