← Back to team overview

launchpad-reviewers team mailing list archive

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