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

Michael Nelson (michael.nelson) wrote :

Hi Colin,

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 :).

There is already a work-around that's been CP'd onto Germanium that ensures that the Source index creation of PPAs copes with the incorrectly parsed data.

This branch fixes the actual parsing error so that we won't get any more incorrectly-parsed dsc_binaries etc., ensuring that our custom parser does exactly what apt_pkg.ParseTagFile() would do in the given scenarios that failed.

The problem was not restricted to Binary fields in dsc files, but all values that can span multiple lines in both dsc and changes files. From my manual searching, as well as from the current test-suite coverage, this affected only:
  (1) Indexes generation for PPAs (per the bug) as we do not use apt-ftparchive,
  (2) formatting of text in Queue notification emails.
but there could be places that both the test coverage and I missed.

I've created bug 440014 to completely get rid of our custom parse_tagfile_lines function (replacing it with apt_pkg.ParseTagFile()).

I've also tested this branch on dogfood, details here:

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.

The dsc files with Binary values spanning multiple lines have approximately doubled each week, - mainly from soures that produce many binaries (linux, mono, being added to people's ppas:
24-09-2009 - 12
06-10-2009 - 24
13-10-2009 - 48

Thanks for your time!

« Back to merge proposal