Merge lp:~michael.nelson/launchpad/436182-newlines-in-sources-fix-without-workaround into lp:launchpad

Proposed by Michael Nelson
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~michael.nelson/launchpad/436182-newlines-in-sources-fix-without-workaround
Merge into: lp:launchpad
Diff against target: 537 lines
9 files modified
lib/canonical/launchpad/emailtemplates/upload-accepted.txt (+1/-0)
lib/canonical/launchpad/emailtemplates/upload-announcement.txt (+1/-0)
lib/canonical/launchpad/emailtemplates/upload-new.txt (+1/-0)
lib/canonical/launchpad/emailtemplates/upload-rejection.txt (+1/-0)
lib/lp/archiveuploader/changesfile.py (+15/-1)
lib/lp/archiveuploader/tagfiles.py (+57/-22)
lib/lp/archiveuploader/tests/data/test436182_0.1_source.changes (+23/-0)
lib/lp/archiveuploader/tests/test_tagfiles.py (+101/-37)
lib/lp/soyuz/model/queue.py (+15/-7)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/436182-newlines-in-sources-fix-without-workaround
Reviewer Review Type Date Requested Status
Colin Watson Approve
Gavin Panella (community) Approve
Review via email: mp+12988@code.launchpad.net
To post a comment you must log in.
Michael Nelson (michael.nelson) wrote :
Download full text (4.0 KiB)

= Summary =
This branch fixes a soyuz parsing error when a debian source control
file is uploaded.

The error is described in bug 436182 (although, somewhat confusedly...
as I was discovering the issues as I went).

To re-cap, soyuz has a function for parsing tag files (such as debian
source control files - dsc, or changes files), which should adhere to

http://www.debian.org/doc/debian-policy/ch-controlfields.html

but it doesn't in a number of cases, in particular one case which
started occurring recently - fields containing values that span multiple
lines.

The original issue that is noted in bug 436182 is that, when a value for
a field spans multiple lines, our parser was leaving a trailing '\n' on
the parsed value - which caused the Source index to be created with a
*blank* line (as shown in the original snippet in the bug description).
This is completely incorrect and causes any reparsing of this file by
standard libraries (such as apt_pkg.ParseTagFile) to fail.

While creating a workaround for this a second issue was discovered: our
parser removes the required space/tab from subsequent lines of a value
that spans multiple lines. According to the policy:

"Many fields' values may span several lines; in this case each
continuation line must start with a space or a tab."

This issue can also be seen in the example snippet in the bug
description. The effect that a Source generated from this incorrectly
parsed data will be reparsed by apt_pgk.ParseTagFile which will ignore
the subsequent lines that are not preceded with space or tab.

== Proposed fix ==

I've already landed a work-around that will handle the corrupt data in
the database (of which there are 27 as of 2009-10-06) when generating
the index files (see
http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/revision/9606
and
http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/revision/9654
) and will get this cherry-picked.

This branch itself ensures that our parse_tagfiles function parses the
three different types of multi-line values available in tag files (a
new-line-delimited value, a value spanning multiple lines and a
description) with exactly the same result as apt_pkg.ParseTagFile().
Part of the reason for this is to make it easier to later remove the
function and replace it with apt_pkg.ParseTagFile() - if that's an
option (I need to talk with Colin or someone who knows the history
regarding why we've created our own parsing function).

== Pre-implementation notes ==

== Implementation details ==

== Tests ==

bin/test -vv -t Testtagfiles -t TestTagFileDebianPolicyCompat

== Demo and Q/A ==

I'll be QA'ing this on DF.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Wow - ok, so I guess this file hasn't been touched since our python
style guide came into effect ;).

I've fixed all the lint issues below.

Linting changed files:
  lib/lp/archiveuploader/tagfiles.py
  lib/lp/archiveuploader/tests/test_tagfiles.py
  lib/lp/archiveuploader/tests/data/test436182_0.1_source.changes

== Pyflakes notices ==

lib/lp/archiveuploader/tests/test_tagfiles.py
    10: 'sys'...

Read more...

Gavin Panella (allenap) wrote :
Download full text (10.0 KiB)

Hi Michael,

I looked at the diff (revisions 9647 and 9648) and there's nothing to
comment on there, so the diff I've reviewed doesn't contain those
revisions.

My only comments below are almost trivial, so r=me. It was a very good
idea to compare to apt_pkg. Also, parse_tagfile_lines is a ugly pig of
a function!

Gavin.

> === modified file 'lib/lp/archiveuploader/tagfiles.py'
> --- lib/lp/archiveuploader/tagfiles.py 2009-06-24 23:33:29 +0000
> +++ lib/lp/archiveuploader/tagfiles.py 2009-10-06 12:09:42 +0000
> @@ -64,8 +64,8 @@
> """This exception is raised if parse_changes encounters nastiness"""
> pass
>
> -re_single_line_field = re.compile(r"^(\S*)\s*:\s*(.*)");
> -re_multi_line_field = re.compile(r"^\s(.*)");
> +re_single_line_field = re.compile(r"^(\S*)\s*:\s*(.*)")
> +re_multi_line_field = re.compile(r"^(\s.*)")
>
>
> def parse_tagfile_lines(lines, dsc_whitespace_rules=0, allow_unsigned=False,
> @@ -103,7 +103,8 @@
>
> num_of_lines = len(indexed_lines.keys())
> index = 0
> - first = -1
> + first_value_for_newline_delimited_field = False
> + more_values_can_follow = False
> while index < num_of_lines:
> index += 1
> line = indexed_lines[index]
> @@ -145,21 +146,47 @@
> if slf:
> field = slf.groups()[0].lower()
> changes[field] = slf.groups()[1]
> - first = 1
> +
> + # If there is no value on this line, we assume this is
> + # the first line of a multiline field, such as the 'files'
> + # field.
> + if changes[field] == '':
> + first_value_for_newline_delimited_field = True
> +
> + # Either way, more values for this field could follow
> + # on the next line.
> + more_values_can_follow = True
> continue
> if line.rstrip() == " .":
> - changes[field] += '\n'
> + changes[field] += '\n' + line
> continue
> mlf = re_multi_line_field.match(line)
> if mlf:
> - if first == -1:
> + if more_values_can_follow is False:
> raise TagFileParseError("%s: could not parse .changes file "
> "line %d: '%s'\n [Multi-line field continuing on from nothing?]"
> % (filename, index,line))
> - if first == 1 and changes[field] != "":
> - changes[field] += '\n'
> - first = 0
> - changes[field] += mlf.groups()[0] + '\n'
> +
> + # XXX Michael Nelson 20091001 bug=440014
> + # Is there any reason why we're not simply using
> + # apt_pkg.ParseTagFile instead of this looong function.
> + # If we can get rid of this code that is trying to mimic
> + # what ParseTagFile does out of the box, it would be a good
> + # thing.
> +
> + # The first value for a newline delimited field, such as
> + # the 'files' field, has its leading spaces stripped. Other
> + # fields (such as a 'binary' field spanning multiple lines)
> + # should *not* be l-stripped of their leading spaces otherwise
> + ...

review: Approve
Michael Nelson (michael.nelson) wrote :
Download full text (5.3 KiB)

On Wed, 2009-10-07 at 14:51 +0000, Gavin Panella wrote:
> Review: Approve
> Hi Michael,
>
> I looked at the diff (revisions 9647 and 9648) and there's nothing to
> comment on there, so the diff I've reviewed doesn't contain those
> revisions.
>
> My only comments below are almost trivial, so r=me. It was a very good
> idea to compare to apt_pkg. Also, parse_tagfile_lines is a ugly pig of
> a function!

Thanks Gavin - yeah, I'm keen to get rid of parse_tagfile_lines in the
future if possible.

>
> Gavin.
>

[snip]

> > === modified file 'lib/lp/archiveuploader/tests/test_tagfiles.py'
> > --- lib/lp/archiveuploader/tests/test_tagfiles.py 2009-06-24 23:33:29 +0000
> > +++ lib/lp/archiveuploader/tests/test_tagfiles.py 2009-10-07 11:10:35 +0000
> > @@ -5,6 +5,7 @@
> >
> > # arch-tag: 52e0c871-49a3-4186-beb8-9817d02d5465
> >
> > +import apt_pkg
> > import unittest
> > import sys
> > import shutil
> > @@ -106,19 +107,110 @@
> > tf = parse_tagfile(datadir("changes-with-exploit-bottom"))
> > self.assertRaises(KeyError, tf.__getitem__, "you")
> >
> > +
> > +class TestTagFileDebianPolicyCompat(unittest.TestCase):
> > +
> > + def setUp(self):
> > + """Parse the test file using apt_pkg for comparison."""
> > +
> > + tagfile_path = datadir("test436182_0.1_source.changes")
> > + tagfile = open(tagfile_path)
> > + self.apt_pkg_parsed_version = apt_pkg.ParseTagFile(tagfile)
> > + self.apt_pkg_parsed_version.Step()
> > +
> > + # Is it just because this is a very old file that test-related
> > + # things are imported locally?
> > + from lp.archiveuploader.tagfiles import parse_tagfile
> > + self.parse_tagfile_version = parse_tagfile(
> > + tagfile_path, allow_unsigned = True)
> > +
> > + def test_parse_tagfile_with_multiline_values(self):
> > + """parse_tagfile should not leave trailing '\n' on multiline values.
> > +
> > + This is a regression test for bug 436182.
> > + Previously we,
> > + 1. Stripped leading space/tab from subsequent lines of multiline
> > + values, and
> > + 2. appended a trailing '\n' to the end of the value.
> > + """
> > +
> > + expected_text = (
> > + 'test75874, anotherbinary,\n'
> > + ' andanother, andonemore,\n'
> > + '\tlastone')
> > +
> > + self.assertEqual(
> > + expected_text,
> > + self.apt_pkg_parsed_version.Section['Binary'])
> > +
> > + self.assertEqual(
> > + expected_text,
> > + self.parse_tagfile_version['binary'])
> > +
> > + def test_parse_tagfile_with_newline_delimited_field(self):
> > + """parse_tagfile should not leave leading or tailing '\n' when
> > + parsing newline delimited fields.
> > +
> > + Newline-delimited fields should be parsed to match
> > + apt_pkg.ParseTageFile.
> > +
> > + Note: in the past, our parse_tagfile function left the leading
> > + '\n' in the parsed value, whereas it should not have.
> > +
> > + XXX check where this value is used and ensure it won't be affected
> > + (ie. skipp...

Read more...

Michael Nelson (michael.nelson) wrote :

Just adding a incremental that fixes test breakages - ensuring that we
always format the changes before displaying it in emails (so it appears
exactly as it used to in emails - without the '\n .' blank line token or
initial spaces.

Julian has already reviewed the incremental and I've put it through ec2
test so it's ready for testing on dogfood.

--
Michael

=== modified file 'lib/canonical/launchpad/emailtemplates/upload-accepted.txt'
--- lib/canonical/launchpad/emailtemplates/upload-accepted.txt 2008-09-18 11:08:16 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-accepted.txt 2009-10-12 09:21:07 +0000
@@ -1,4 +1,5 @@
1%(CHANGESFILE)s1%(CHANGESFILE)s
2
2%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s3%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s
3%(SPR_URL)s4%(SPR_URL)s
45
56
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-announcement.txt'
--- lib/canonical/launchpad/emailtemplates/upload-announcement.txt 2008-09-18 11:08:16 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-announcement.txt 2009-10-12 09:27:30 +0000
@@ -1,3 +1,4 @@
1%(CHANGESFILE)s1%(CHANGESFILE)s
2
2%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s3%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s
3%(SPR_URL)s4%(SPR_URL)s
45
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-new.txt'
--- lib/canonical/launchpad/emailtemplates/upload-new.txt 2007-07-03 11:15:10 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-new.txt 2009-10-12 09:17:07 +0000
@@ -2,6 +2,7 @@
22
3%(CHANGESFILE)s3%(CHANGESFILE)s
44
5
5Your package contains new components which requires manual editing of6Your package contains new components which requires manual editing of
6the override file. It is ok otherwise, so please be patient. New7the override file. It is ok otherwise, so please be patient. New
7packages are usually added to the overrides about once a week.8packages are usually added to the overrides about once a week.
89
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-rejection.txt'
--- lib/canonical/launchpad/emailtemplates/upload-rejection.txt 2008-09-26 07:34:37 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-rejection.txt 2009-10-12 09:34:50 +0000
@@ -2,6 +2,7 @@
2%(SUMMARY)s2%(SUMMARY)s
33
4%(CHANGESFILE)s4%(CHANGESFILE)s
5
5%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s6%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s
67
7===8===
89
=== modified file 'lib/lp/archiveuploader/changesfile.py'
--- lib/lp/archiveuploader/changesfile.py 2009-06-24 23:33:29 +0000
+++ lib/lp/archiveuploader/changesfile.py 2009-10-12 08:33:56 +0000
@@ -307,10 +307,24 @@
307 """Return changesfile claimed version."""307 """Return changesfile claimed version."""
308 return self._dict['version']308 return self._dict['version']
309309
310 @classmethod
311 def formatChangesComment(cls, comment):
312 """A class utility method for formatting changes for display."""
313
314 # Return the display version of the comment using the
315 # debian policy rules. First replacing the blank line
316 # indicator '\n .' and then stripping one space from each
317 # successive line.
318 comment = comment.replace('\n .', '\n')
319 comment = comment.replace('\n ', '\n')
320 return comment
321
310 @property322 @property
311 def changes_comment(self):323 def changes_comment(self):
312 """Return changesfile 'change' comment."""324 """Return changesfile 'change' comment."""
313 return self._dict['changes']325 comment = self._dict['changes']
326
327 return self.formatChangesComment(comment)
314328
315 @property329 @property
316 def date(self):330 def date(self):
317331
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2009-10-01 07:05:22 +0000
+++ lib/lp/soyuz/model/queue.py 2009-10-12 07:53:03 +0000
@@ -33,6 +33,7 @@
33from lp.archivepublisher.config import getPubConfig33from lp.archivepublisher.config import getPubConfig
34from lp.archivepublisher.customupload import CustomUploadError34from lp.archivepublisher.customupload import CustomUploadError
35from lp.archivepublisher.utils import get_ppa_reference35from lp.archivepublisher.utils import get_ppa_reference
36from lp.archiveuploader.changesfile import ChangesFile
36from lp.archiveuploader.tagfiles import parse_tagfile_lines37from lp.archiveuploader.tagfiles import parse_tagfile_lines
37from lp.archiveuploader.utils import safe_fix_maintainer38from lp.archiveuploader.utils import safe_fix_maintainer
38from lp.buildmaster.pas import BuildDaemonPackagesArchSpecific39from lp.buildmaster.pas import BuildDaemonPackagesArchSpecific
@@ -737,14 +738,16 @@
737 """PPA rejected message."""738 """PPA rejected message."""
738 template = get_email_template('ppa-upload-rejection.txt')739 template = get_email_template('ppa-upload-rejection.txt')
739 SUMMARY = sanitize_string(summary_text)740 SUMMARY = sanitize_string(summary_text)
740 CHANGESFILE = sanitize_string("".join(changes_lines))741 CHANGESFILE = sanitize_string(
742 ChangesFile.formatChangesComment("".join(changes_lines)))
741 USERS_ADDRESS = config.launchpad.users_address743 USERS_ADDRESS = config.launchpad.users_address
742744
743 class RejectedMessage:745 class RejectedMessage:
744 """Rejected message."""746 """Rejected message."""
745 template = get_email_template('upload-rejection.txt')747 template = get_email_template('upload-rejection.txt')
746 SUMMARY = sanitize_string(summary_text)748 SUMMARY = sanitize_string(summary_text)
747 CHANGESFILE = sanitize_string(changes['changes'])749 CHANGESFILE = sanitize_string(
750 ChangesFile.formatChangesComment(changes['changes']))
748 CHANGEDBY = ''751 CHANGEDBY = ''
749 ORIGIN = ''752 ORIGIN = ''
750 SIGNER = ''753 SIGNER = ''
@@ -821,7 +824,8 @@
821824
822 STATUS = "New"825 STATUS = "New"
823 SUMMARY = summarystring826 SUMMARY = summarystring
824 CHANGESFILE = sanitize_string(changes['changes'])827 CHANGESFILE = sanitize_string(
828 ChangesFile.formatChangesComment(changes['changes']))
825 DISTRO = self.distroseries.distribution.title829 DISTRO = self.distroseries.distribution.title
826 if announce_list:830 if announce_list:
827 ANNOUNCE = 'Announcing to %s' % announce_list831 ANNOUNCE = 'Announcing to %s' % announce_list
@@ -835,7 +839,8 @@
835 STATUS = "Waiting for approval"839 STATUS = "Waiting for approval"
836 SUMMARY = summarystring + (840 SUMMARY = summarystring + (
837 "\nThis upload awaits approval by a distro manager\n")841 "\nThis upload awaits approval by a distro manager\n")
838 CHANGESFILE = sanitize_string(changes['changes'])842 CHANGESFILE = sanitize_string(
843 ChangesFile.formatChangesComment(changes['changes']))
839 DISTRO = self.distroseries.distribution.title844 DISTRO = self.distroseries.distribution.title
840 if announce_list:845 if announce_list:
841 ANNOUNCE = 'Announcing to %s' % announce_list846 ANNOUNCE = 'Announcing to %s' % announce_list
@@ -853,7 +858,8 @@
853858
854 STATUS = "Accepted"859 STATUS = "Accepted"
855 SUMMARY = summarystring860 SUMMARY = summarystring
856 CHANGESFILE = sanitize_string(changes['changes'])861 CHANGESFILE = sanitize_string(
862 ChangesFile.formatChangesComment(changes['changes']))
857 DISTRO = self.distroseries.distribution.title863 DISTRO = self.distroseries.distribution.title
858 if announce_list:864 if announce_list:
859 ANNOUNCE = 'Announcing to %s' % announce_list865 ANNOUNCE = 'Announcing to %s' % announce_list
@@ -871,14 +877,16 @@
871877
872 STATUS = "Accepted"878 STATUS = "Accepted"
873 SUMMARY = summarystring879 SUMMARY = summarystring
874 CHANGESFILE = guess_encoding("".join(changes_lines))880 CHANGESFILE = guess_encoding(
881 ChangesFile.formatChangesComment("".join(changes_lines)))
875882
876 class AnnouncementMessage:883 class AnnouncementMessage:
877 template = get_email_template('upload-announcement.txt')884 template = get_email_template('upload-announcement.txt')
878885
879 STATUS = "Accepted"886 STATUS = "Accepted"
880 SUMMARY = summarystring887 SUMMARY = summarystring
881 CHANGESFILE = sanitize_string(changes['changes'])888 CHANGESFILE = sanitize_string(
889 ChangesFile.formatChangesComment(changes['changes']))
882 CHANGEDBY = ''890 CHANGEDBY = ''
883 ORIGIN = ''891 ORIGIN = ''
884 SIGNER = ''892 SIGNER = ''
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: http://pastebin.ubuntu.com/292254/

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, oo.org) being added to people's ppas:
24-09-2009 - 12
06-10-2009 - 24
13-10-2009 - 48

Thanks for your time!

Colin Watson (cjwatson) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-accepted.txt'
--- lib/canonical/launchpad/emailtemplates/upload-accepted.txt 2008-09-18 11:08:16 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-accepted.txt 2009-10-15 13:21:16 +0000
@@ -1,4 +1,5 @@
1%(CHANGESFILE)s1%(CHANGESFILE)s
2
2%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s3%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s
3%(SPR_URL)s4%(SPR_URL)s
45
56
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-announcement.txt'
--- lib/canonical/launchpad/emailtemplates/upload-announcement.txt 2008-09-18 11:08:16 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-announcement.txt 2009-10-15 13:21:16 +0000
@@ -1,3 +1,4 @@
1%(CHANGESFILE)s1%(CHANGESFILE)s
2
2%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s3%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s
3%(SPR_URL)s4%(SPR_URL)s
45
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-new.txt'
--- lib/canonical/launchpad/emailtemplates/upload-new.txt 2007-07-03 11:15:10 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-new.txt 2009-10-15 13:21:16 +0000
@@ -2,6 +2,7 @@
22
3%(CHANGESFILE)s3%(CHANGESFILE)s
44
5
5Your package contains new components which requires manual editing of6Your package contains new components which requires manual editing of
6the override file. It is ok otherwise, so please be patient. New7the override file. It is ok otherwise, so please be patient. New
7packages are usually added to the overrides about once a week.8packages are usually added to the overrides about once a week.
89
=== modified file 'lib/canonical/launchpad/emailtemplates/upload-rejection.txt'
--- lib/canonical/launchpad/emailtemplates/upload-rejection.txt 2008-09-26 07:34:37 +0000
+++ lib/canonical/launchpad/emailtemplates/upload-rejection.txt 2009-10-15 13:21:16 +0000
@@ -2,6 +2,7 @@
2%(SUMMARY)s2%(SUMMARY)s
33
4%(CHANGESFILE)s4%(CHANGESFILE)s
5
5%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s6%(DATE)s%(CHANGEDBY)s%(MAINTAINER)s%(SIGNER)s%(ORIGIN)s
67
7===8===
89
=== modified file 'lib/lp/archiveuploader/changesfile.py'
--- lib/lp/archiveuploader/changesfile.py 2009-06-24 23:33:29 +0000
+++ lib/lp/archiveuploader/changesfile.py 2009-10-15 13:21:16 +0000
@@ -307,10 +307,24 @@
307 """Return changesfile claimed version."""307 """Return changesfile claimed version."""
308 return self._dict['version']308 return self._dict['version']
309309
310 @classmethod
311 def formatChangesComment(cls, comment):
312 """A class utility method for formatting changes for display."""
313
314 # Return the display version of the comment using the
315 # debian policy rules. First replacing the blank line
316 # indicator '\n .' and then stripping one space from each
317 # successive line.
318 comment = comment.replace('\n .', '\n')
319 comment = comment.replace('\n ', '\n')
320 return comment
321
310 @property322 @property
311 def changes_comment(self):323 def changes_comment(self):
312 """Return changesfile 'change' comment."""324 """Return changesfile 'change' comment."""
313 return self._dict['changes']325 comment = self._dict['changes']
326
327 return self.formatChangesComment(comment)
314328
315 @property329 @property
316 def date(self):330 def date(self):
317331
=== modified file 'lib/lp/archiveuploader/tagfiles.py'
--- lib/lp/archiveuploader/tagfiles.py 2009-06-24 23:33:29 +0000
+++ lib/lp/archiveuploader/tagfiles.py 2009-10-15 13:21:16 +0000
@@ -53,19 +53,19 @@
53 """Expose a dicty has_key"""53 """Expose a dicty has_key"""
54 return key in self.stanza.keys()54 return key in self.stanza.keys()
5555
56 """Enables (foo in bar) functionality"""56 # Enables (foo in bar) functionality.
57 __contains__ = has_key57 __contains__ = has_key
5858
59 def items(self):59 def items(self):
60 """Allows for k,v in foo.items()"""60 """Allows for k,v in foo.items()"""
61 return [ (k,self.stanza[k]) for k in self.stanza.keys() ]61 return [ (k, self.stanza[k]) for k in self.stanza.keys() ]
6262
63class TagFileParseError(Exception):63class TagFileParseError(Exception):
64 """This exception is raised if parse_changes encounters nastiness"""64 """This exception is raised if parse_changes encounters nastiness"""
65 pass65 pass
6666
67re_single_line_field = re.compile(r"^(\S*)\s*:\s*(.*)");67re_single_line_field = re.compile(r"^(\S*)\s*:\s*(.*)")
68re_multi_line_field = re.compile(r"^\s(.*)");68re_multi_line_field = re.compile(r"^(\s.*)")
6969
7070
71def parse_tagfile_lines(lines, dsc_whitespace_rules=0, allow_unsigned=False,71def parse_tagfile_lines(lines, dsc_whitespace_rules=0, allow_unsigned=False,
@@ -103,7 +103,8 @@
103103
104 num_of_lines = len(indexed_lines.keys())104 num_of_lines = len(indexed_lines.keys())
105 index = 0105 index = 0
106 first = -1106 first_value_for_newline_delimited_field = False
107 more_values_can_follow = False
107 while index < num_of_lines:108 while index < num_of_lines:
108 index += 1109 index += 1
109 line = indexed_lines[index]110 line = indexed_lines[index]
@@ -116,11 +117,15 @@
116 if dsc_whitespace_rules:117 if dsc_whitespace_rules:
117 index += 1118 index += 1
118 if index > num_of_lines:119 if index > num_of_lines:
119 raise TagFileParseError("%s: invalid .dsc file at line %d" % (filename, index))120 raise TagFileParseError(
121 "%s: invalid .dsc file at line %d" % (
122 filename, index))
120 line = indexed_lines[index]123 line = indexed_lines[index]
121 if not line.startswith("-----BEGIN PGP SIGNATURE"):124 if not line.startswith("-----BEGIN PGP SIGNATURE"):
122 raise TagFileParseError("%s: invalid .dsc file at line %d -- "125 raise TagFileParseError(
123 "expected PGP signature; got '%s'" % (filename, index,line))126 "%s: invalid .dsc file at line %d -- "
127 "expected PGP signature; got '%s'" % (
128 filename, index,line))
124 inside_signature = 0129 inside_signature = 0
125 break130 break
126 else:131 else:
@@ -128,8 +133,9 @@
128 if line.startswith("-----BEGIN PGP SIGNATURE"):133 if line.startswith("-----BEGIN PGP SIGNATURE"):
129 break134 break
130135
131 # If we're at the start of a signed section, then consume the signature136 # If we're at the start of a signed section, then consume the
132 # information, and remember that we're inside the signed data.137 # signature information, and remember that we're inside the signed
138 # data.
133 if line.startswith("-----BEGIN PGP SIGNED MESSAGE"):139 if line.startswith("-----BEGIN PGP SIGNED MESSAGE"):
134 inside_signature = 1140 inside_signature = 1
135 if dsc_whitespace_rules:141 if dsc_whitespace_rules:
@@ -145,31 +151,60 @@
145 if slf:151 if slf:
146 field = slf.groups()[0].lower()152 field = slf.groups()[0].lower()
147 changes[field] = slf.groups()[1]153 changes[field] = slf.groups()[1]
148 first = 1154
155 # If there is no value on this line, we assume this is
156 # the first line of a multiline field, such as the 'files'
157 # field.
158 if changes[field] == '':
159 first_value_for_newline_delimited_field = True
160
161 # Either way, more values for this field could follow
162 # on the next line.
163 more_values_can_follow = True
149 continue164 continue
150 if line.rstrip() == " .":165 if line.rstrip() == " .":
151 changes[field] += '\n'166 changes[field] += '\n' + line
152 continue167 continue
153 mlf = re_multi_line_field.match(line)168 mlf = re_multi_line_field.match(line)
154 if mlf:169 if mlf:
155 if first == -1:170 if more_values_can_follow is False:
156 raise TagFileParseError("%s: could not parse .changes file "171 raise TagFileParseError(
157 "line %d: '%s'\n [Multi-line field continuing on from nothing?]"172 "%s: could not parse .changes file line %d: '%s'\n"
158 % (filename, index,line))173 " [Multi-line field continuing on from nothing?]" % (
159 if first == 1 and changes[field] != "":174 filename, index,line))
160 changes[field] += '\n'175
161 first = 0176 # XXX Michael Nelson 20091001 bug=440014
162 changes[field] += mlf.groups()[0] + '\n'177 # Is there any reason why we're not simply using
178 # apt_pkg.ParseTagFile instead of this looong function.
179 # If we can get rid of this code that is trying to mimic
180 # what ParseTagFile does out of the box, it would be a good
181 # thing.
182
183 # The first value for a newline delimited field, such as
184 # the 'files' field, has its leading spaces stripped. Other
185 # fields (such as a 'binary' field spanning multiple lines)
186 # should *not* be l-stripped of their leading spaces otherwise
187 # they will be re-parsed incorrectly by apt_get.ParseTagFiles()
188 # (for example, from a Source index).
189 value = mlf.groups()[0]
190 if first_value_for_newline_delimited_field:
191 changes[field] = value.lstrip()
192 else:
193 changes[field] += '\n' + value
194
195 first_value_for_newline_delimited_field = False
163 continue196 continue
164 error += line197 error += line
165198
166 if dsc_whitespace_rules and inside_signature:199 if dsc_whitespace_rules and inside_signature:
167 raise TagFileParseError("%s: invalid .dsc format at line %d" % (filename, index))200 raise TagFileParseError(
201 "%s: invalid .dsc format at line %d" % (filename, index))
168202
169 changes["filecontents"] = "".join(lines)203 changes["filecontents"] = "".join(lines)
170204
171 if error:205 if error:
172 raise TagFileParseError("%s: unable to parse .changes file: %s" % (filename, error))206 raise TagFileParseError(
207 "%s: unable to parse .changes file: %s" % (filename, error))
173208
174 return changes209 return changes
175210
176211
=== added file 'lib/lp/archiveuploader/tests/data/test436182_0.1_source.changes'
--- lib/lp/archiveuploader/tests/data/test436182_0.1_source.changes 1970-01-01 00:00:00 +0000
+++ lib/lp/archiveuploader/tests/data/test436182_0.1_source.changes 2009-10-15 13:21:16 +0000
@@ -0,0 +1,23 @@
1Format: 1.0
2Date: Fri, 15 Dec 2006 12:02:42 +0000
3Source: test75874
4Binary: test75874 anotherbinary
5 andanother andonemore
6 lastone
7Architecture: all
8Version: 0.1
9Maintainer: Colin Watson <cjwatson@ubuntu.com>
10Description: Here's the single-line synopsis.
11 Then there is the extended description which can
12 span multiple lines, and even include blank-lines like this:
13 .
14 There you go. If a line starts with two or more spaces,
15 it will be displayed verbatim. Like this one:
16 Example verbatim line.
17 Another verbatim line.
18 OK, back to normal.
19Files:
20 f26bb9b29b1108e53139da3584a4dc92 1511 test75874_0.1.tar.gz
21 29c955ff520cea32ab3e0316306d0ac1 393742 pmount_0.9.7.orig.tar.gz
22 91a8f46d372c406fadcb57c6ff7016f3 5302 pmount_0.9.7-2ubuntu2.diff.gz
23
024
=== modified file 'lib/lp/archiveuploader/tests/test_tagfiles.py'
--- lib/lp/archiveuploader/tests/test_tagfiles.py 2009-06-24 23:33:29 +0000
+++ lib/lp/archiveuploader/tests/test_tagfiles.py 2009-10-15 13:21:16 +0000
@@ -5,23 +5,18 @@
55
6# arch-tag: 52e0c871-49a3-4186-beb8-9817d02d54656# arch-tag: 52e0c871-49a3-4186-beb8-9817d02d5465
77
8import apt_pkg
8import unittest9import unittest
9import sys
10import shutil
11from lp.archiveuploader.tests import datadir10from lp.archiveuploader.tests import datadir
1211
12from lp.archiveuploader.tagfiles import (
13 parse_tagfile, TagFile, TagFileParseError)
14
13class Testtagfiles(unittest.TestCase):15class Testtagfiles(unittest.TestCase):
1416
15 def testImport(self):
16 """lp.archiveuploader.tagfiles should be importable"""
17 from lp.archiveuploader.tagfiles import TagFile
18 from lp.archiveuploader.tagfiles import TagFileParseError
19 from lp.archiveuploader.tagfiles import parse_tagfile
20
21 def testTagFileOnSingular(self):17 def testTagFileOnSingular(self):
22 """lp.archiveuploader.tagfiles.TagFile should parse a singular stanza18 """lp.archiveuploader.tagfiles.TagFile should parse a singular stanza
23 """19 """
24 from lp.archiveuploader.tagfiles import TagFile
25 f = TagFile(file(datadir("singular-stanza"), "r"))20 f = TagFile(file(datadir("singular-stanza"), "r"))
26 seenone = False21 seenone = False
27 for stanza in f:22 for stanza in f:
@@ -32,8 +27,7 @@
32 self.assertEquals("FooBar" in stanza, False)27 self.assertEquals("FooBar" in stanza, False)
3328
34 def testTagFileOnSeveral(self):29 def testTagFileOnSeveral(self):
35 """lp.archiveuploader.tagfiles.TagFile should parse multiple stanzas"""30 """TagFile should parse multiple stanzas."""
36 from lp.archiveuploader.tagfiles import TagFile
37 f = TagFile(file(datadir("multiple-stanzas"), "r"))31 f = TagFile(file(datadir("multiple-stanzas"), "r"))
38 seen = 032 seen = 0
39 for stanza in f:33 for stanza in f:
@@ -47,15 +41,12 @@
47 """lp.archiveuploader.tagfiles.parse_tagfile should work on a good41 """lp.archiveuploader.tagfiles.parse_tagfile should work on a good
48 changes file42 changes file
49 """43 """
50 from lp.archiveuploader.tagfiles import parse_tagfile
51 p = parse_tagfile(datadir("good-signed-changes"))44 p = parse_tagfile(datadir("good-signed-changes"))
5245
53 def testCheckParseBadChangesRaises(self):46 def testCheckParseBadChangesRaises(self):
54 """lp.archiveuploader.tagfiles.parse_chantges should raise47 """lp.archiveuploader.tagfiles.parse_chantges should raise
55 TagFileParseError on failure48 TagFileParseError on failure
56 """49 """
57 from lp.archiveuploader.tagfiles import parse_tagfile
58 from lp.archiveuploader.tagfiles import TagFileParseError
59 self.assertRaises(TagFileParseError,50 self.assertRaises(TagFileParseError,
60 parse_tagfile, datadir("badformat-changes"), 1)51 parse_tagfile, datadir("badformat-changes"), 1)
6152
@@ -63,8 +54,6 @@
63 """lp.archiveuploader.tagfiles.parse_chantges should raise54 """lp.archiveuploader.tagfiles.parse_chantges should raise
64 TagFileParseError on empty55 TagFileParseError on empty
65 """56 """
66 from lp.archiveuploader.tagfiles import parse_tagfile
67 from lp.archiveuploader.tagfiles import TagFileParseError
68 self.assertRaises(TagFileParseError,57 self.assertRaises(TagFileParseError,
69 parse_tagfile, datadir("empty-file"), 1)58 parse_tagfile, datadir("empty-file"), 1)
7059
@@ -72,16 +61,12 @@
72 """lp.archiveuploader.tagfiles.parse_chantges should raise61 """lp.archiveuploader.tagfiles.parse_chantges should raise
73 TagFileParseError on malformed signatures62 TagFileParseError on malformed signatures
74 """63 """
75 from lp.archiveuploader.tagfiles import parse_tagfile
76 from lp.archiveuploader.tagfiles import TagFileParseError
77 self.assertRaises(TagFileParseError,64 self.assertRaises(TagFileParseError,
78 parse_tagfile, datadir("malformed-sig-changes"), 1)65 parse_tagfile, datadir("malformed-sig-changes"), 1)
7966
80 def testCheckParseMalformedMultilineRaises(self):67 def testCheckParseMalformedMultilineRaises(self):
81 """lp.archiveuploader.tagfiles.parse_chantges should raise68 """lp.archiveuploader.tagfiles.parse_chantges should raise
82 TagFileParseError on malformed continuation lines"""69 TagFileParseError on malformed continuation lines"""
83 from lp.archiveuploader.tagfiles import parse_tagfile
84 from lp.archiveuploader.tagfiles import TagFileParseError
85 self.assertRaises(TagFileParseError,70 self.assertRaises(TagFileParseError,
86 parse_tagfile, datadir("bad-multiline-changes"), 1)71 parse_tagfile, datadir("bad-multiline-changes"), 1)
8772
@@ -89,8 +74,6 @@
89 """lp.archiveuploader.tagfiles.parse_chantges should raise74 """lp.archiveuploader.tagfiles.parse_chantges should raise
90 TagFileParseError on unterminated signatures75 TagFileParseError on unterminated signatures
91 """76 """
92 from lp.archiveuploader.tagfiles import parse_tagfile
93 from lp.archiveuploader.tagfiles import TagFileParseError
94 self.assertRaises(TagFileParseError,77 self.assertRaises(TagFileParseError,
95 parse_tagfile,78 parse_tagfile,
96 datadir("unterminated-sig-changes"),79 datadir("unterminated-sig-changes"),
@@ -100,25 +83,106 @@
100 """lp.archiveuploader.tagfiles.parse_tagfile should not be vulnerable83 """lp.archiveuploader.tagfiles.parse_tagfile should not be vulnerable
101 to tags outside of the signed portion84 to tags outside of the signed portion
102 """85 """
103 from lp.archiveuploader.tagfiles import parse_tagfile
104 tf = parse_tagfile(datadir("changes-with-exploit-top"))86 tf = parse_tagfile(datadir("changes-with-exploit-top"))
105 self.assertRaises(KeyError, tf.__getitem__, "you")87 self.assertRaises(KeyError, tf.__getitem__, "you")
106 tf = parse_tagfile(datadir("changes-with-exploit-bottom"))88 tf = parse_tagfile(datadir("changes-with-exploit-bottom"))
107 self.assertRaises(KeyError, tf.__getitem__, "you")89 self.assertRaises(KeyError, tf.__getitem__, "you")
10890
91
92class TestTagFileDebianPolicyCompat(unittest.TestCase):
93
94 def setUp(self):
95 """Parse the test file using apt_pkg for comparison."""
96
97 tagfile_path = datadir("test436182_0.1_source.changes")
98 tagfile = open(tagfile_path)
99 self.apt_pkg_parsed_version = apt_pkg.ParseTagFile(tagfile)
100 self.apt_pkg_parsed_version.Step()
101
102 self.parse_tagfile_version = parse_tagfile(
103 tagfile_path, allow_unsigned = True)
104
105 def test_parse_tagfile_with_multiline_values(self):
106 """parse_tagfile should not leave trailing '\n' on multiline values.
107
108 This is a regression test for bug 436182.
109 Previously we,
110 1. Stripped leading space/tab from subsequent lines of multiline
111 values, and
112 2. appended a trailing '\n' to the end of the value.
113 """
114
115 expected_text = (
116 'test75874 anotherbinary\n'
117 ' andanother andonemore\n'
118 '\tlastone')
119
120 self.assertEqual(
121 expected_text,
122 self.apt_pkg_parsed_version.Section['Binary'])
123
124 self.assertEqual(
125 expected_text,
126 self.parse_tagfile_version['binary'])
127
128 def test_parse_tagfile_with_newline_delimited_field(self):
129 """parse_tagfile should not leave leading or tailing '\n' when
130 parsing newline delimited fields.
131
132 Newline-delimited fields should be parsed to match
133 apt_pkg.ParseTageFile.
134
135 Note: in the past, our parse_tagfile function left the leading
136 '\n' in the parsed value, whereas it should not have.
137
138 For an example,
139 see http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Files
140 """
141
142 expected_text = (
143 'f26bb9b29b1108e53139da3584a4dc92 1511 test75874_0.1.tar.gz\n '
144 '29c955ff520cea32ab3e0316306d0ac1 393742 '
145 'pmount_0.9.7.orig.tar.gz\n'
146 ' 91a8f46d372c406fadcb57c6ff7016f3 5302 '
147 'pmount_0.9.7-2ubuntu2.diff.gz')
148
149 self.assertEqual(
150 expected_text,
151 self.apt_pkg_parsed_version.Section['Files'])
152
153 self.assertEqual(
154 expected_text,
155 self.parse_tagfile_version['files'])
156
157 def test_parse_description_field(self):
158 """Apt-pkg preserves the blank-line indicator and does not strip
159 leading spaces.
160
161 See http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Description
162 """
163 expected_text = (
164 "Here's the single-line synopsis.\n"
165 " Then there is the extended description which can\n"
166 " span multiple lines, and even include blank-lines like this:\n"
167 " .\n"
168 " There you go. If a line starts with two or more spaces,\n"
169 " it will be displayed verbatim. Like this one:\n"
170 " Example verbatim line.\n"
171 " Another verbatim line.\n"
172 " OK, back to normal.")
173
174 self.assertEqual(
175 expected_text,
176 self.apt_pkg_parsed_version.Section['Description'])
177
178 # In the past our parse_tagfile function replaced blank-line
179 # indicators in the description (' .\n') with new lines ('\n'),
180 # but it is now compatible with ParseTagFiles (and ready to be
181 # replaced by ParseTagFiles).
182 self.assertEqual(
183 expected_text,
184 self.parse_tagfile_version['description'])
185
109def test_suite():186def test_suite():
110 suite = unittest.TestSuite()187 return unittest.TestLoader().loadTestsFromName(__name__)
111 loader = unittest.TestLoader()
112 suite.addTest(loader.loadTestsFromTestCase(Testtagfiles))
113 return suite
114
115def main(argv):
116 suite = test_suite()
117 runner = unittest.TextTestRunner(verbosity = 2)
118 if not runner.run(suite).wasSuccessful():
119 return 1
120 return 0
121
122if __name__ == '__main__':
123 sys.exit(main(sys.argv))
124188
125189
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2009-10-09 11:13:30 +0000
+++ lib/lp/soyuz/model/queue.py 2009-10-15 13:21:16 +0000
@@ -33,6 +33,7 @@
33from lp.archivepublisher.config import getPubConfig33from lp.archivepublisher.config import getPubConfig
34from lp.archivepublisher.customupload import CustomUploadError34from lp.archivepublisher.customupload import CustomUploadError
35from lp.archivepublisher.utils import get_ppa_reference35from lp.archivepublisher.utils import get_ppa_reference
36from lp.archiveuploader.changesfile import ChangesFile
36from lp.archiveuploader.tagfiles import parse_tagfile_lines37from lp.archiveuploader.tagfiles import parse_tagfile_lines
37from lp.archiveuploader.utils import safe_fix_maintainer38from lp.archiveuploader.utils import safe_fix_maintainer
38from lp.buildmaster.pas import BuildDaemonPackagesArchSpecific39from lp.buildmaster.pas import BuildDaemonPackagesArchSpecific
@@ -747,14 +748,16 @@
747 """PPA rejected message."""748 """PPA rejected message."""
748 template = get_email_template('ppa-upload-rejection.txt')749 template = get_email_template('ppa-upload-rejection.txt')
749 SUMMARY = sanitize_string(summary_text)750 SUMMARY = sanitize_string(summary_text)
750 CHANGESFILE = sanitize_string("".join(changes_lines))751 CHANGESFILE = sanitize_string(
752 ChangesFile.formatChangesComment("".join(changes_lines)))
751 USERS_ADDRESS = config.launchpad.users_address753 USERS_ADDRESS = config.launchpad.users_address
752754
753 class RejectedMessage:755 class RejectedMessage:
754 """Rejected message."""756 """Rejected message."""
755 template = get_email_template('upload-rejection.txt')757 template = get_email_template('upload-rejection.txt')
756 SUMMARY = sanitize_string(summary_text)758 SUMMARY = sanitize_string(summary_text)
757 CHANGESFILE = sanitize_string(changes['changes'])759 CHANGESFILE = sanitize_string(
760 ChangesFile.formatChangesComment(changes['changes']))
758 CHANGEDBY = ''761 CHANGEDBY = ''
759 ORIGIN = ''762 ORIGIN = ''
760 SIGNER = ''763 SIGNER = ''
@@ -831,7 +834,8 @@
831834
832 STATUS = "New"835 STATUS = "New"
833 SUMMARY = summarystring836 SUMMARY = summarystring
834 CHANGESFILE = sanitize_string(changes['changes'])837 CHANGESFILE = sanitize_string(
838 ChangesFile.formatChangesComment(changes['changes']))
835 DISTRO = self.distroseries.distribution.title839 DISTRO = self.distroseries.distribution.title
836 if announce_list:840 if announce_list:
837 ANNOUNCE = 'Announcing to %s' % announce_list841 ANNOUNCE = 'Announcing to %s' % announce_list
@@ -845,7 +849,8 @@
845 STATUS = "Waiting for approval"849 STATUS = "Waiting for approval"
846 SUMMARY = summarystring + (850 SUMMARY = summarystring + (
847 "\nThis upload awaits approval by a distro manager\n")851 "\nThis upload awaits approval by a distro manager\n")
848 CHANGESFILE = sanitize_string(changes['changes'])852 CHANGESFILE = sanitize_string(
853 ChangesFile.formatChangesComment(changes['changes']))
849 DISTRO = self.distroseries.distribution.title854 DISTRO = self.distroseries.distribution.title
850 if announce_list:855 if announce_list:
851 ANNOUNCE = 'Announcing to %s' % announce_list856 ANNOUNCE = 'Announcing to %s' % announce_list
@@ -863,7 +868,8 @@
863868
864 STATUS = "Accepted"869 STATUS = "Accepted"
865 SUMMARY = summarystring870 SUMMARY = summarystring
866 CHANGESFILE = sanitize_string(changes['changes'])871 CHANGESFILE = sanitize_string(
872 ChangesFile.formatChangesComment(changes['changes']))
867 DISTRO = self.distroseries.distribution.title873 DISTRO = self.distroseries.distribution.title
868 if announce_list:874 if announce_list:
869 ANNOUNCE = 'Announcing to %s' % announce_list875 ANNOUNCE = 'Announcing to %s' % announce_list
@@ -881,14 +887,16 @@
881887
882 STATUS = "Accepted"888 STATUS = "Accepted"
883 SUMMARY = summarystring889 SUMMARY = summarystring
884 CHANGESFILE = guess_encoding("".join(changes_lines))890 CHANGESFILE = guess_encoding(
891 ChangesFile.formatChangesComment("".join(changes_lines)))
885892
886 class AnnouncementMessage:893 class AnnouncementMessage:
887 template = get_email_template('upload-announcement.txt')894 template = get_email_template('upload-announcement.txt')
888895
889 STATUS = "Accepted"896 STATUS = "Accepted"
890 SUMMARY = summarystring897 SUMMARY = summarystring
891 CHANGESFILE = sanitize_string(changes['changes'])898 CHANGESFILE = sanitize_string(
899 ChangesFile.formatChangesComment(changes['changes']))
892 CHANGEDBY = ''900 CHANGEDBY = ''
893 ORIGIN = ''901 ORIGIN = ''
894 SIGNER = ''902 SIGNER = ''