launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01067
[Merge] lp:~jelmer/launchpad/635591-sync-source-unicode into lp:launchpad/devel
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/635591-sync-source-unicode into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
#635591 Regression: syncing packages with UTF-8 changelogs fails
https://bugs.launchpad.net/bugs/635591
This branch fixes the ability to sync sources that contain a changelog entry with non-ASCII UTF8 characters.
I've moved generate_changes() so that it would be possible to write unit tests for it.
tests: ./bin/test lp.soyuz.scripts.test_sync_source
--
https://code.launchpad.net/~jelmer/launchpad/635591-sync-source-unicode/+merge/35637
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/635591-sync-source-unicode into lp:launchpad/devel.
=== modified file 'lib/lp/soyuz/scripts/ftpmaster.py'
--- lib/lp/soyuz/scripts/ftpmaster.py 2010-08-24 15:29:01 +0000
+++ lib/lp/soyuz/scripts/ftpmaster.py 2010-09-16 09:34:47 +0000
@@ -20,11 +20,13 @@
]
import commands
+from debian.deb822 import Changes
import hashlib
import os
import stat
import sys
import tempfile
+import time
import apt_pkg
from zope.component import getUtility
@@ -1416,3 +1418,59 @@
# Collect extra debug messages from chroot_manager.
for debug_message in chroot_manager._messages:
self.logger.debug(debug_message)
+
+
+def generate_changes(dsc, dsc_files, suite, changelog, urgency, closes,
+ lp_closes, section, priority, description,
+ files_from_librarian, requested_by, origin):
+ """Generate a Changes object.
+
+ :param dsc: A `Dsc` instance for the related source package.
+ :param suite: Distribution name
+ :param changelog: Relevant changelog data
+ :param urgency: Urgency string (low, medium, high, etc)
+ :param closes: Sequence of Debian bug numbers (as strings) fixed by
+ this upload.
+ :param section: Debian section
+ :param priority: Package priority
+ """
+
+ # XXX cprov 2007-07-03:
+ # Changed-By can be extracted from most-recent changelog footer,
+ # but do we care?
+ # XXX James Troup 2006-01-30:
+ # 'Closes' but could be gotten from changelog, but we don't use them?
+
+ changes = Changes()
+ changes["Origin"] = "%s/%s" % (origin["name"], origin["suite"])
+ changes["Format"] = "1.7"
+ changes["Date"] = time.strftime("%a, %d %b %Y %H:%M:%S %z")
+ changes["Source"] = dsc["source"]
+ changes["Binary"] = dsc["binary"]
+ changes["Architecture"] = "source"
+ changes["Version"] = dsc["version"]
+ changes["Distribution"] = suite
+ changes["Urgency"] = urgency
+ changes["Maintainer"] = dsc["maintainer"]
+ changes["Changed-By"] = requested_by
+ if description:
+ changes["Description"] = "\n %s" % description
+ if closes:
+ changes["Closes"] = " ".join(closes)
+ if lp_closes:
+ changes["Launchpad-bugs-fixed"] = " ".join(lp_closes)
+ changes["Changes"] = "\n%s" % changelog
+ files = []
+ for filename in dsc_files:
+ if filename in files_from_librarian:
+ continue
+ files.append({"md5sum": dsc_files[filename]["md5sum"],
+ "size": dsc_files[filename]["size"],
+ "section": section,
+ "priority": priority,
+ "name": filename,
+ })
+
+ changes["Files"] = files
+ # Strip trailing newline
+ return changes
=== modified file 'lib/lp/soyuz/scripts/tests/test_sync_source.py'
--- lib/lp/soyuz/scripts/tests/test_sync_source.py 2010-08-27 12:21:25 +0000
+++ lib/lp/soyuz/scripts/tests/test_sync_source.py 2010-09-16 09:34:47 +0000
@@ -1,10 +1,14 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""SyncSource facilities tests."""
__metaclass__ = type
+from debian.deb822 import (
+ Deb822Dict,
+ Dsc,
+ )
import os
import shutil
import subprocess
@@ -29,6 +33,7 @@
from lp.soyuz.scripts.ftpmaster import (
SyncSource,
SyncSourceError,
+ generate_changes,
)
@@ -416,5 +421,85 @@
os.unlink(expected_changesfile)
+class TestGenerateChanges(TestCase):
+ """Test generate_changes()."""
+
+ def getBaseDsc(self):
+ """Create a basic Dsc object for use with generate_changes()."""
+ dsc = Dsc()
+ dsc["source"] = "mysrcpkg"
+ dsc["binary"] = "mybinpkg"
+ dsc["version"] = "4.2"
+ dsc["maintainer"] = "Maintainer <maintainer@xxxxxxxxxx>"
+ return dsc
+
+ def getBaseOrigin(self):
+ """Create a basic Origin dict for use with generate_changes()."""
+ origin = Deb822Dict()
+ origin["Name"] = "Debian"
+ origin["Suite"] = "sid"
+ return origin
+
+ def generateChanges(self, dsc=None, dsc_files=None, suite="maverick",
+ changelog=None, urgency="low", closes=None,
+ lp_closes=None, section="net", priority="extra",
+ description=None, files_from_librarian=[],
+ requested_by="Somebody <somebody@xxxxxxxxxx>",
+ origin=None):
+ if dsc is None:
+ dsc = self.getBaseDsc()
+ if dsc_files is None:
+ dsc_files = []
+ if origin is None:
+ origin = self.getBaseOrigin()
+ if changelog is None:
+ changelog = 'changelog entry'
+ return generate_changes(
+ dsc=dsc, dsc_files=dsc_files, suite=suite, changelog=changelog,
+ urgency=urgency, closes=closes, lp_closes=lp_closes,
+ section=section, priority=priority, description=description,
+ files_from_librarian=files_from_librarian,
+ requested_by=requested_by, origin=origin)
+
+ def test_minimum_fields(self):
+ # The right (minimum) set of fields are set by generate_changes().
+ changes = self.generateChanges()
+ self.assertEquals("1.7", changes["Format"])
+ self.assertEquals("mysrcpkg", changes["Source"])
+ self.assertEquals("mybinpkg", changes["Binary"])
+ self.assertEquals("source", changes["Architecture"])
+ self.assertEquals("4.2", changes["Version"])
+ self.assertEquals("maverick", changes["Distribution"])
+ self.assertEquals("low", changes["Urgency"])
+ self.assertEquals("\nchangelog entry", changes["Changes"])
+ self.assertEquals(
+ "Maintainer <maintainer@xxxxxxxxxx>", changes["Maintainer"])
+ self.assertFalse("Description" in changes)
+ self.assertFalse("Closes" in changes)
+ self.assertFalse("Launchpad-bugs-fixed" in changes)
+ self.assertEquals([], changes["Files"])
+
+ def test_closes(self):
+ # Closes gets set if any Debian bugs to close were specified.
+ changes = self.generateChanges(closes=["1232", "4323"])
+ self.assertEquals("1232 4323", changes["Closes"])
+ self.assertFalse("Launchpad-bugs-fixed" in changes)
+
+ def test_lp_closes(self):
+ # Launchpad-Bugs-Fixed gets set if any Launchpad bugs to close were
+ # specified.
+ changes = self.generateChanges(lp_closes=["987987"])
+ self.assertEquals("987987", changes["Launchpad-Bugs-Fixed"])
+
+ def test_utf8_changelog(self):
+ # A changelog entry with non-ASCII UTF-8 characters is serialized in
+ # Changes properly.
+ changes = self.generateChanges(
+ changelog="* Updated French translation by J\xc3\xa9lmer.")
+ contents = changes.dump(encoding="utf-8").encode("utf-8")
+ self.assertTrue(
+ contents.find("Updated French translation by J\xc3\xa9lmer."))
+
+
def test_suite():
return TestLoader().loadTestsFromName(__name__)
=== modified file 'scripts/ftpmaster-tools/sync-source.py'
--- scripts/ftpmaster-tools/sync-source.py 2010-08-27 12:21:25 +0000
+++ scripts/ftpmaster-tools/sync-source.py 2010-09-16 09:34:47 +0000
@@ -18,16 +18,7 @@
import apt_pkg
import commands
-try:
- from debian.deb822 import Dsc
-except ImportError:
- # In older versions of python-debian the main package was named
- # debian_bundle
- # XXX: Remove this when an up to date version of python-debian lands in
- # the PPA or Ubuntu. Maverick will be the first release that has an
- # up to date version of python-debian.
- from debian_bundle.deb822 import Dsc
-
+from debian.deb822 import Dsc
import errno
import optparse
import os
@@ -36,7 +27,6 @@
import stat
import string
import tempfile
-import time
import urllib
import dak_utils
@@ -45,16 +35,28 @@
from zope.component import getUtility
from contrib.glock import GlobalLock
-from canonical.database.sqlbase import sqlvalues, cursor
+from canonical.database.sqlbase import (
+ cursor,
+ sqlvalues,
+ )
from canonical.launchpad.interfaces import (
- IDistributionSet, IPersonSet)
+ IDistributionSet,
+ IPersonSet,
+ )
from canonical.launchpad.scripts import (
- execute_zcml_for_scripts, logger, logger_options)
+ execute_zcml_for_scripts,
+ logger,
+ logger_options,
+ )
from canonical.librarian.client import LibrarianClient
from canonical.lp import initZopeless
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.soyuz.enums import PackagePublishingStatus
-from lp.soyuz.scripts.ftpmaster import SyncSource, SyncSourceError
+from lp.soyuz.scripts.ftpmaster import (
+ generate_changes,
+ SyncSource,
+ SyncSourceError,
+ )
reject_message = ""
@@ -81,7 +83,7 @@
return md5sum
-def reject (str, prefix="Rejected: "):
+def reject(str, prefix="Rejected: "):
global reject_message
if str:
reject_message += prefix + str + "\n"
@@ -139,53 +141,6 @@
os.unlink(temp_filename)
-def generate_changes(dsc, dsc_files, suite, changelog, urgency, closes,
- lp_closes, section, priority, description,
- files_from_librarian, requested_by, origin):
- """Generate a .changes as a string"""
-
- # XXX cprov 2007-07-03:
- # Changed-By can be extracted from most-recent changelog footer,
- # but do we care?
- # XXX James Troup 2006-01-30:
- # 'Closes' but could be gotten from changelog, but we don't use them?
-
- changes = ""
- changes += "Origin: %s/%s\n" % (origin["name"], origin["suite"])
- changes += "Format: 1.7\n"
- changes += "Date: %s\n" % (time.strftime("%a, %d %b %Y %H:%M:%S %z"))
- changes += "Source: %s\n" % (dsc["source"])
- changes += "Binary: %s\n" % (dsc["binary"])
- changes += "Architecture: source\n"
- changes += "Version: %s\n"% (dsc["version"])
- # XXX: James Troup 2006-01-30:
- # 'suite' forced to string to avoid unicode-vs-str grudge match
- changes += "Distribution: %s\n" % (str(suite))
- changes += "Urgency: %s\n" % (urgency)
- changes += "Maintainer: %s\n" % (dsc["maintainer"])
- changes += "Changed-By: %s\n" % (requested_by)
- if description:
- changes += "Description: \n"
- changes += " %s\n" % (description)
- if closes:
- changes += "Closes: %s\n" % (" ".join(closes))
- if lp_closes:
- changes += "Launchpad-bugs-fixed: %s\n" % (" ".join(lp_closes))
- changes += "Changes: \n"
- changes += changelog
- changes += "Files: \n"
- for filename in dsc_files:
- if filename in files_from_librarian:
- continue
- changes += " %s %s %s %s %s\n" % (dsc_files[filename]["md5sum"],
- dsc_files[filename]["size"],
- section, priority, filename)
- # Strip trailing newline
- changes = changes[:-1]
-
- return changes
-
-
def parse_changelog(changelog_filename, previous_version):
if not os.path.exists(changelog_filename):
dak_utils.fubar("debian/changelog not found in extracted source.")
@@ -500,9 +455,10 @@
dsc["source"], dak_utils.re_no_epoch.sub('', dsc["version"]))
filehandle = open(output_filename, 'w')
- # XXX cprov 2007-07-03: The Soyuz .changes parser requires the extra '\n'
- filehandle.write(changes+'\n')
- filehandle.close()
+ try:
+ changes.dump(filehandle, encoding="utf-8")
+ finally:
+ filehandle.close()
def read_current_source(distro_series, valid_component=None, arguments=None):