Merge lp:~jelmer/launchpad/sync-tbz2 into lp:launchpad/db-devel

[Jelmer Vernooij]

Fixes support for syncing packages in the the version 3 source format.

[Jelmer Vernooij (jelmer)]

Fixes support for syncing packages in the the version 3 source format (bug 225151).

This patch adds support for .orig.tar.bz2 files as well as multiple component orig tarballs in the script that syncs source packages from Debian. These new file formats are used by the new Debian source package format, v3.0.

This branch also refactors sync-source.py to use code from lp.soyuz.scripts.ftpmaster (well unit tested) rather its local copy (no tests). It also removes duplicate code for parsing dsc files from lp.soyuz.scripts.dak_utils in favor of the Dsc parser in debian_bundle.deb822.

[Guilherme Salgado (salgado)]

Hi Jelmer,

I have just a few comments/questions below, but this looks good. And
thanks a lot for getting rid of the untested/duplicated code.

 review approve code

On Thu, 2010-02-04 at 16:24 +0000, Jelmer Vernooij wrote:

> === modified file 'lib/canonical/launchpad/scripts/logger.py'
> --- lib/canonical/launchpad/scripts/logger.py 2009-10-16 18:07:01 +0000
> +++ lib/canonical/launchpad/scripts/logger.py 2010-02-04 16:24:23 +0000
[...]
> @@ -847,19 +854,14 @@
> Return the fetched filename if it was present in Librarian or None
> if it wasn't.
> """
> - # XXX cprov 2007-01-10 bug=78683: Looking for files within ubuntu
> - # only. It doesn't affect the usual sync-source procedure. However
> - # it needs to be revisited for derivation, we probably need
> - # to pass the target distribution in order to make proper lookups.
> - ubuntu = getUtility(IDistributionSet)['ubuntu']

Does this mean the bug above will be fixed too?

> try:
> - libraryfilealias = ubuntu.getFileByName(
> + libraryfilealias = self.todistro.getFileByName(
> filename, source=True, binary=False)
> except NotFoundError:
> return None
>
> - self.debug(
> - "\t%s: already in distro - downloading from librarian" %
> + self.logger.info(
> + "%s: already in distro - downloading from librarian" %
> filename)
>
> output_file = open(filename, 'w')
> @@ -871,23 +873,23 @@
> """Try to fetch files from Librarian.
>
> It raises SyncSourceError if anything else then an
> - 'orig.tar.gz' was found in Librarian.
> - Return a boolean indicating whether or not the 'orig.tar.gz' is
> - required in the upload.
> + orig tarball was found in Librarian.
> + Return the names of the files retrieved from the librarian.
> """
> - orig_filename = None
> + retrieved = []
> for filename in self.files.keys():
> if not self.fetchFileFromLibrarian(filename):
> continue
> + file_type = determine_source_file_type(filename)
> # set the return code if an orig was, in fact,
> # fetched from Librarian
> - if filename.endswith("orig.tar.gz"):
> - orig_filename = filename
> - else:
> + if not file_type in (SourcePackageFileType.ORIG_TARBALL,
> + SourcePackageFileType.COMPONENT_ORIG_TARBALL):

The indentation above is a bit odd; the two elements of the tuple should
be lined up.

> raise SyncSourceError(
> - 'Oops, only orig.tar.gz can be retrieved from librarian')
> + 'Oops, only orig tarball can be retrieved from librarian.')
> + retrieved.append(filename)
>
> - return orig_filename
> + return retrieved
>
> def fetchSyncFiles(self):
> """Fetch files from the original sync source.

>
> === modified file 'scripts/ftpmaster-tools/sync-source.py'
> --- scripts/ftpmaster-tools/sync-source.py 2010-02...

review: Approve (code)

[Jelmer Vernooij (jelmer)]

Hi Guilherme,

Thanks for the review.

On Fri, 2010-02-05 at 17:48 +0000, Guilherme Salgado wrote:
> On Thu, 2010-02-04 at 16:24 +0000, Jelmer Vernooij wrote:
> > === modified file 'lib/canonical/launchpad/scripts/logger.py'
> > --- lib/canonical/launchpad/scripts/logger.py 2009-10-16 18:07:01 +0000
> > +++ lib/canonical/launchpad/scripts/logger.py 2010-02-04 16:24:23 +0000
> [...]
> > @@ -847,19 +854,14 @@
> > Return the fetched filename if it was present in Librarian or None
> > if it wasn't.
> > """
> > - # XXX cprov 2007-01-10 bug=78683: Looking for files within ubuntu
> > - # only. It doesn't affect the usual sync-source procedure. However
> > - # it needs to be revisited for derivation, we probably need
> > - # to pass the target distribution in order to make proper lookups.
> > - ubuntu = getUtility(IDistributionSet)['ubuntu']
> Does this mean the bug above will be fixed too?
Partially - bug 78683 is about sync-source only working with Ubuntu, and
this change gets us further in that direction but there are still some
places where we hardcode 'ubuntu' in scripts/ftp-master/sync-source.py.

I've addressed the style issue you've raised and I'll ec2 land after I
finish testing on dogfood.

Cheers,

Jelmer

=== modified file 'lib/canonical/launchpad/scripts/logger.py'
--- lib/canonical/launchpad/scripts/logger.py	2009-10-16 18:07:01 +0000
+++ lib/canonical/launchpad/scripts/logger.py	2010-02-11 16:55:27 +0000
@@ -5,7 +5,7 @@
 
 """Logging setup for scripts.
 
-Don't import from this module. Import it from canonical.scripts.
+Don't import from this module. Import it from canonical.launchpad.scripts.
 
 Parts of this may be moved into canonical.launchpad somewhere if it is
 to be used for non-script stuff.

=== modified file 'lib/lp/soyuz/scripts/ftpmaster.py'
--- lib/lp/soyuz/scripts/ftpmaster.py	2009-12-13 11:55:40 +0000
+++ lib/lp/soyuz/scripts/ftpmaster.py	2010-02-11 16:55:27 +0000
@@ -36,11 +36,14 @@
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.launchpad.webapp.interfaces import NotFoundError
+from lp.archiveuploader.utils import (
+    determine_source_file_type)
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import (
     PackagePublishingPocket, pocketsuffix)
+from lp.registry.interfaces.sourcepackage import SourcePackageFileType
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.binarypackagerelease import IBinaryPackageReleaseSet
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
@@ -803,12 +806,14 @@
         pocket_chroot.chroot.open()
         copy_and_close(pocket_chroot.chroot, local_file)
 
+
 class SyncSourceError(Exception):
     """Raised when an critical error occurs inside SyncSource.
 
     The entire procedure should be aborted in order to avoid unknown problems.
     """
 
+
 class SyncSource:
     """Sync Source procedure helper class.
 
@@ -818,20 +823,22 @@
     'aptMD5Sum' is provided as a classmethod during the integration time.
     """
 
-    def __init__(self, files, origin, debug, downloader):
+    def __init__(self, files, origin, logger, downloader, todistro):
         """Store local context.
 
         files: a dictionary where the keys are the filename and the
                value another dictionary with the file informations.
         origin: a dictionary similar to 'files' but where the values
                 contain information for download files to be synchronized
-        debug: a debug function, 'debug(message)'
+        logger: a logger
         downloader: a callable that fetchs URLs, 'downloader(url, destination)'
+        todistro: target distribution object
         """
         self.files = files
         self.origin = origin
-        self.debug = debug
+        self.logger = logger
         self.downloader = downloader
+        self.todistro = todistro
 
     @classmethod
     def generateMD5Sum(self, filename):
@@ -847,19 +854,14 @@
         Return the fetched filename if it was present in Librarian or None
         if it wasn't.
         """
-        # XXX cprov 2007-01-10 bug=78683: Looking for files within ubuntu
-        # only. It doesn't affect the usual sync-source procedure. However
-        # it needs to be revisited for derivation, we probably need
-        # to pass the target distribution in order to make proper lookups.
-        ubuntu = getUtility(IDistributionSet)['ubuntu']
         try:
-            libraryfilealias = ubuntu.getFileByName(
+            libraryfilealias = self.todistro.getFileByName(
                 filename, source=True, binary=False)
         except NotFoundError:
             return None
 
-        self.debug(
-            "\t%s: already in distro - downloading from librarian" %
+        self.logger.info(
+            "%s: already in distro - downloading from librarian" %
             filename)
 
         output_file = open(filename, 'w')
@@ -871,23 +873,23 @@
         """Try to fetch files from Librarian.
 
         It raises SyncSourceError if anything else then an
-        'orig.tar.gz' was found in Librarian.
-        Return a boolean indicating whether or not the 'orig.tar.gz' is
-        required in the upload.
+        orig tarball was found in Librarian.
+        Return the names of the files retrieved from the librarian.
         """
-        orig_filename = None
+        retrieved = []
         for filename in self.files.keys():
             if not self.fetchFileFromLibrarian(filename):
                 continue
+            file_type = determine_source_file_type(filename)
             # set the return code if an orig was, in fact,
             # fetched from Librarian
-            if filename.endswith("orig.tar.gz"):
-                orig_filename = filename
-            else:
+            if not file_type in (SourcePackageFileType.ORIG_TARBALL,
+                                 SourcePackageFileType.COMPONENT_ORIG_TARBALL):
                 raise SyncSourceError(
-                    'Oops, only orig.tar.gz can be retrieved from librarian')
+                    'Oops, only orig tarball can be retrieved from librarian.')
+            retrieved.append(filename)
 
-        return orig_filename
+        return retrieved
 
     def fetchSyncFiles(self):
         """Fetch files from the original sync source.
@@ -896,21 +898,19 @@
         """
         dsc_filename = None
         for filename in self.files.keys():
+            file_type = determine_source_file_type(filename)
+            if file_type == SourcePackageFileType.DSC:
+                dsc_filename = filename
             if os.path.exists(filename):
+                self.logger.info("  - <%s: cached>" % (filename))
                 continue
-            self.debug(
+            self.logger.info(
                 "  - <%s: downloading from %s>" %
                 (filename, self.origin["url"]))
             download_f = ("%s%s" % (self.origin["url"],
                                     self.files[filename]["remote filename"]))
             sys.stdout.flush()
             self.downloader(download_f, filename)
-            # only set the dsc_filename if the DSC was really downloaded.
-            # this loop usually includes the other files for the upload,
-            # DIFF and ORIG.
-            if filename.endswith(".dsc"):
-                dsc_filename = filename
-
         return dsc_filename
 
     def checkDownloadedFiles(self):

=== modified file 'lib/lp/soyuz/scripts/tests/sync_source_home/Debian_incoming_main_Sources'
--- lib/lp/soyuz/scripts/tests/sync_source_home/Debian_incoming_main_Sources	2008-02-14 12:38:50 +0000
+++ lib/lp/soyuz/scripts/tests/sync_source_home/Debian_incoming_main_Sources	2010-02-11 16:55:27 +0000
@@ -10,3 +10,31 @@
  fc1464e5985b962a042d5354452f361d 164 bar_1.0.orig.tar.gz
  a259bf88aedcdeded4e5d94ad4af3b4a 610 bar_1.0-1.diff.gz
 
+Format: 3.0 (quilt)
+Package: sample1
+Binary: sample1
+Architecture: all
+Version: 1.0-1
+Maintainer: Raphael Hertzog <[e-mail]>
+Standards-Version: 3.8.1
+Checksums-Sha1: 
+ 724e3cdad5f4af13383e5d71f525465add1ce897 1741 sample1_1.0-1.dsc
+ 296cb89762668772c3ae4799bd9ce7973f4154b2 171 sample1_1.0.orig-component1.tar.bz2
+ ccb198be27c065880696cbeb6fb5aebf190c8544 157 sample1_1.0.orig-component2.tar.lzma
+ ea3c0c6ee23410a9c454d9b35681a80460343456 201 sample1_1.0.orig-component3.tar.gz
+ c62b2496d3ff0c64dcf6488c95e1d5cbc9f5f30d 201 sample1_1.0.orig.tar.gz
+ b3c92ffaabc2e3e95e5f74e1de94351fddcb065c 1327 sample1_1.0-1.debian.tar.gz
+Checksums-Sha256: 
+ d5390b56d3d5ea301bb565487eded8a6739d29ea5d680389f35b87b30f846412 1741 sample1_1.0-1.dsc
+ a0871b73922a228ef8c1f4c1a7e1aef94d8a59d919218242655b7faae352d5c9 171 sample1_1.0.orig-component1.tar.bz2
+ 39963ed90d70e918a6cae8d73c81f42dcb7f0dc283a4715f032a1e28b98d1a25 157 sample1_1.0.orig-component2.tar.lzma
+ dc174acae810faf5f9a679e042f480905622fe4a854510dad6e200dbe6381c8d 201 sample1_1.0.orig-component3.tar.gz
+ 9c0f0b42f6283614b204a47600cb9b0d8b19dfa4cb43c69e6a0a40f3d1064e0e 201 sample1_1.0.orig.tar.gz
+ 6864ceeb315ad7b6a59636a0251525cbd8e9b76c6e934f3746271d4ebce5da3c 1327 sample1_1.0-1.debian.tar.gz
+Files: 
+ aea108574786aa670dc5ef26dc225be2 1741 sample1_1.0-1.dsc
+ 8a90ee77cf3c68c32bdb5c55d40d06f4 171 sample1_1.0.orig-component1.tar.bz2
+ 3535d4766df47e531372802a0c2ddb82 157 sample1_1.0.orig-component2.tar.lzma
+ 673b42066e8e858208b5d0c7a006c5fb 201 sample1_1.0.orig-component3.tar.gz
+ 9047a93bf12a7c749045e9bc9ff9ee99 201 sample1_1.0.orig.tar.gz
+ 9404bc99cdb06876a988d4c2262c073e 1327 sample1_1.0-1.debian.tar.gz

=== added file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.debian.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.debian.tar.gz	1970-01-01 00:00:00 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.debian.tar.gz	2010-02-11 16:55:27 +0000 differ
=== added file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.dsc'
--- lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.dsc	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.dsc	2010-02-11 16:55:27 +0000
@@ -0,0 +1,37 @@
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA1
+
+Format: 3.0 (quilt)
+Source: sample1
+Binary: sample1
+Architecture: all
+Version: 1.0-1
+Maintainer: Raphael Hertzog <[e-mail]>
+Standards-Version: 3.8.1
+Build-Depends: quilt, debhelper (>= 7)
+Checksums-Sha1: 
+ 296cb89762668772c3ae4799bd9ce7973f4154b2 171 sample1_1.0.orig-component1.tar.bz2
+ ccb198be27c065880696cbeb6fb5aebf190c8544 157 sample1_1.0.orig-component2.tar.lzma
+ ea3c0c6ee23410a9c454d9b35681a80460343456 201 sample1_1.0.orig-component3.tar.gz
+ c62b2496d3ff0c64dcf6488c95e1d5cbc9f5f30d 201 sample1_1.0.orig.tar.gz
+ b3c92ffaabc2e3e95e5f74e1de94351fddcb065c 1327 sample1_1.0-1.debian.tar.gz
+Checksums-Sha256: 
+ a0871b73922a228ef8c1f4c1a7e1aef94d8a59d919218242655b7faae352d5c9 171 sample1_1.0.orig-component1.tar.bz2
+ 39963ed90d70e918a6cae8d73c81f42dcb7f0dc283a4715f032a1e28b98d1a25 157 sample1_1.0.orig-component2.tar.lzma
+ dc174acae810faf5f9a679e042f480905622fe4a854510dad6e200dbe6381c8d 201 sample1_1.0.orig-component3.tar.gz
+ 9c0f0b42f6283614b204a47600cb9b0d8b19dfa4cb43c69e6a0a40f3d1064e0e 201 sample1_1.0.orig.tar.gz
+ 6864ceeb315ad7b6a59636a0251525cbd8e9b76c6e934f3746271d4ebce5da3c 1327 sample1_1.0-1.debian.tar.gz
+Files: 
+ 8a90ee77cf3c68c32bdb5c55d40d06f4 171 sample1_1.0.orig-component1.tar.bz2
+ 3535d4766df47e531372802a0c2ddb82 157 sample1_1.0.orig-component2.tar.lzma
+ 673b42066e8e858208b5d0c7a006c5fb 201 sample1_1.0.orig-component3.tar.gz
+ 9047a93bf12a7c749045e9bc9ff9ee99 201 sample1_1.0.orig.tar.gz
+ 9404bc99cdb06876a988d4c2262c073e 1327 sample1_1.0-1.debian.tar.gz
+
+-----BEGIN PGP SIGNATURE-----
+Version: GnuPG v1.4.10 (GNU/Linux)
+
+iEYEARECAAYFAktq7/kACgkQPa9Uoh7vUnahEQCgmRTupXtrHegzFOb5jQMOCSXS
+/p4AoIroJtmjVryFpWqAABZTFGjeWo85
+=vwh8
+-----END PGP SIGNATURE-----

=== added file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component1.tar.bz2'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component1.tar.bz2	1970-01-01 00:00:00 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component1.tar.bz2	2010-02-11 16:55:27 +0000 differ
=== added file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component2.tar.lzma'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component2.tar.lzma	1970-01-01 00:00:00 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component2.tar.lzma	2010-02-11 16:55:27 +0000 differ
=== added file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component3.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component3.tar.gz	1970-01-01 00:00:00 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component3.tar.gz	2010-02-11 16:55:27 +0000 differ
=== added file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig.tar.gz	1970-01-01 00:00:00 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig.tar.gz	2010-02-11 16:55:27 +0000 differ
=== modified file 'lib/lp/soyuz/scripts/tests/test_sync_source.py'
--- lib/lp/soyuz/scripts/tests/test_sync_source.py	2009-06-25 04:06:00 +0000
+++ lib/lp/soyuz/scripts/tests/test_sync_source.py	2010-02-11 16:55:27 +0000
@@ -1,7 +1,7 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""ChrootManager facilities tests."""
+"""SyncSource facilities tests."""
 
 __metaclass__ = type
 
@@ -12,11 +12,15 @@
 import tempfile
 from unittest import TestCase, TestLoader
 
-from lp.archiveuploader.tagfiles import parse_tagfile
+from zope.component import getUtility
+
 from canonical.config import config
+from canonical.launchpad.scripts import BufferLogger
 from canonical.librarian.ftests.harness import (
     fillLibrarianFile, cleanupLibrarianFiles)
 from canonical.testing import LaunchpadZopelessLayer
+from lp.archiveuploader.tagfiles import parse_tagfile
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.soyuz.scripts.ftpmaster import (
     SyncSource, SyncSourceError)
 
@@ -37,7 +41,7 @@
         self._home = os.path.abspath('')
         self._jail = tempfile.mkdtemp()
         os.chdir(self._jail)
-        self.messages = []
+        self.logger = BufferLogger()
         self.downloads = []
 
     def tearDown(self):
@@ -54,9 +58,9 @@
         """Return a list of files present in jail."""
         return os.listdir(self._jail)
 
-    def local_debug(self, message):
-        """Store debug messages for future inspection."""
-        self.messages.append(message)
+    def get_messages(self):
+        """Retrieve the messages sent using the logger."""
+        return self.logger.buffer.getvalue().splitlines()
 
     def local_downloader(self, url, filename):
         """Store download requests for future inspections."""
@@ -72,8 +76,9 @@
         them later.
         """
         sync_source = SyncSource(
-            files=files, origin=origin, debug=self.local_debug,
-            downloader=self.local_downloader)
+            files=files, origin=origin, logger=self.logger,
+            downloader=self.local_downloader,
+            todistro=getUtility(IDistributionSet)['ubuntu'])
         return sync_source
 
     def testInstantiate(self):
@@ -86,8 +91,8 @@
         self.assertEqual(sync_source.files, files)
         self.assertEqual(sync_source.origin, origin)
 
-        sync_source.debug('opa')
-        self.assertEqual(self.messages, ['opa'])
+        sync_source.logger.debug('opa')
+        self.assertEqual(self.get_messages(), ['DEBUG: opa'])
 
         sync_source.downloader('somewhere', 'foo')
         self.assertEqual(self.downloads, [('somewhere', 'foo')])
@@ -159,26 +164,26 @@
         test_file is skipped.
         """
         files = {
-            'foo.diff.gz': {'remote filename': 'xxx'},
-            'foo.dsc': {'remote filename': 'yyy'},
-            'foo.orig.gz': {'remote filename': 'zzz'},
+            'foo_0.1.diff.gz': {'remote filename': 'xxx'},
+            'foo_0.1.dsc': {'remote filename': 'yyy'},
+            'foo_0.1.orig.gz': {'remote filename': 'zzz'},
             }
         origin = {'url': 'http://somewhere/'}
 
         sync_source = self._getSyncSource(files, origin)
 
-        test_file = open('foo.diff.gz', 'w')
+        test_file = open('foo_0.1.diff.gz', 'w')
         test_file.write('nahhh')
         test_file.close()
 
         dsc_filename = sync_source.fetchSyncFiles()
 
-        self.assertEqual(dsc_filename, 'foo.dsc')
+        self.assertEqual(dsc_filename, 'foo_0.1.dsc')
 
         self.assertEqual(
             self.downloads,
-            [('http://somewhere/yyy', 'foo.dsc'),
-             ('http://somewhere/zzz', 'foo.orig.gz')])
+            [('http://somewhere/zzz', 'foo_0.1.orig.gz'),
+             ('http://somewhere/yyy', 'foo_0.1.dsc')])
 
         for filename in files.keys():
             self.assertTrue(os.path.exists(filename))
@@ -196,13 +201,13 @@
         origin = {}
         sync_source = self._getSyncSource(files, origin)
 
-        orig_filename = sync_source.fetchLibrarianFiles()
+        librarian_files = sync_source.fetchLibrarianFiles()
 
-        self.assertEqual(orig_filename, 'netapplet_1.0.0.orig.tar.gz')
+        self.assertEqual(librarian_files, ['netapplet_1.0.0.orig.tar.gz'])
         self.assertEqual(self._listFiles(), ['netapplet_1.0.0.orig.tar.gz'])
         self.assertEqual(
-            self.messages,
-            ['\tnetapplet_1.0.0.orig.tar.gz: already in distro '
+            self.get_messages(),
+            ['INFO: netapplet_1.0.0.orig.tar.gz: already in distro '
              '- downloading from librarian'])
 
     def testFetchLibrarianFilesGotDuplicatedDSC(self):
@@ -224,8 +229,8 @@
             sync_source.fetchLibrarianFiles)
 
         self.assertEqual(
-            self.messages,
-            ['\tfoobar-1.0.dsc: already in distro '
+            self.get_messages(),
+            ['INFO: foobar-1.0.dsc: already in distro '
              '- downloading from librarian'])
         self.assertEqual(self._listFiles(), ['foobar-1.0.dsc'])
 
@@ -261,7 +266,7 @@
         stdout, stderr = process.communicate()
         return (process.returncode, stdout, stderr)
 
-    def testSyncSourceRun(self):
+    def testSyncSourceRunV1(self):
         """Try a simple sync-source.py run.
 
         It will run in a special tree prepared to cope with sync-source
@@ -288,15 +293,17 @@
         self.assertEqual(
             err.splitlines(),
             ['W: Could not find blacklist file on '
-                  '/srv/launchpad.net/dak/sync-blacklist.txt'])
+             '/srv/launchpad.net/dak/sync-blacklist.txt',
+             'INFO      - <bar_1.0-1.diff.gz: cached>',
+             'INFO      - <bar_1.0.orig.tar.gz: cached>',
+             'INFO      - <bar_1.0-1.dsc: cached>',
+             ])
         self.assertEqual(
             out.splitlines(),
             ['Getting binaries for hoary...',
              '[Updating] bar (0 [Ubuntu] < 1.0-1 [Debian])',
              ' * Trying to add bar...',
-             '  - <bar_1.0-1.diff.gz: cached>',
-             '  - <bar_1.0.orig.tar.gz: cached>',
-             '  - <bar_1.0-1.dsc: cached>'])
+             ])
 
         expected_changesfile = 'bar_1.0-1_source.changes'
         self.assertTrue(
@@ -331,6 +338,76 @@
 
         os.unlink(expected_changesfile)
 
+    def testSyncSourceRunV3(self):
+        """Try a simple sync-source.py run with a version 3 source format 
+        package.
+
+        It will run in a special tree prepared to cope with sync-source
+        requirements (see `setUp`). It contains a usable archive index
+        named as '$distribution_$suite_$component_Sources' and the
+        'etherwake' source files.
+
+        Check that:
+         * return code is ZERO,
+         * check standard error and standard output,
+         * check if the expected changesfile was generated,
+         * parse and inspect the changesfile using the archiveuploader
+           component (the same approach adopted by Soyuz).
+         * delete the changesfile.
+        """
+        returncode, out, err = self.runSyncSource(
+            extra_args=['-b', 'cprov', '-D', 'debian', '-C', 'main',
+                        '-S', 'incoming', 'sample1'])
+
+        self.assertEqual(
+            0, returncode, "\nScript Failed:%s\nStdout:\n%s\nStderr\n%s\n"
+            % (returncode, out, err))
+
+        self.assertEqual(
+            err.splitlines(),
+            ['W: Could not find blacklist file on '
+             '/srv/launchpad.net/dak/sync-blacklist.txt', 
+             'INFO      - <sample1_1.0.orig-component3.tar.gz: cached>',
+             'INFO      - <sample1_1.0-1.dsc: cached>',
+             'INFO      - <sample1_1.0-1.debian.tar.gz: cached>',
+             'INFO      - <sample1_1.0.orig-component1.tar.bz2: cached>',
+             'INFO      - <sample1_1.0.orig-component2.tar.lzma: cached>',
+             'INFO      - <sample1_1.0.orig.tar.gz: cached>'])
+        self.assertEqual(
+            out.splitlines(),
+            ['Getting binaries for hoary...',
+             '[Updating] sample1 (0 [Ubuntu] < 1.0-1 [Debian])',
+             ' * Trying to add sample1...',
+             ])
+
+        expected_changesfile = 'sample1_1.0-1_source.changes'
+        self.assertTrue(
+            os.path.exists(expected_changesfile),
+            "Couldn't find %s." % expected_changesfile)
+
+        # Parse the generated unsigned changesfile.
+        parsed_changes = parse_tagfile(
+            expected_changesfile, allow_unsigned=True)
+
+        # It refers to the right source/version.
+        self.assertEqual(parsed_changes['source'], 'sample1')
+        self.assertEqual(parsed_changes['version'], '1.0-1')
+
+        # It includes the correct 'origin' and 'target' information.
+        self.assertEqual(parsed_changes['origin'], 'Debian/incoming')
+        self.assertEqual(parsed_changes['distribution'], 'hoary')
+
+        # And finally, 'maintainer' role was preserved and 'changed-by'
+        # role was assigned as specified in the sync-source command-line.
+        self.assertEqual(
+            parsed_changes['maintainer'],
+            'Raphael Hertzog <[e-mail]>')
+        self.assertEqual(
+            parsed_changes['changed-by'],
+            'Celso Providelo <[e-mail]>')
+
+        os.unlink(expected_changesfile)
+
 
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)

=== modified file 'scripts/ftpmaster-tools/dak_utils.py'
--- scripts/ftpmaster-tools/dak_utils.py	2009-11-18 02:58:23 +0000
+++ scripts/ftpmaster-tools/dak_utils.py	2010-02-11 16:55:27 +0000
@@ -34,156 +34,6 @@
 re_no_epoch = re.compile(r"^\d+\:")
 re_extract_src_version = re.compile(r"(\S+)\s*\((.*)\)")
 
-changes_parse_error_exc = "Can't parse line in .changes file"
-invalid_dsc_format_exc = "Invalid .dsc file"
-nk_format_exc = "Unknown Format: in .changes file";
-no_files_exc = "No Files: field in .dsc or .changes file.";
-
-################################################################################
-
-def parse_changes(filename, signing_rules=0):
-    """Parses a changes file and returns a dictionary where each field is a
-key.  The mandatory first argument is the filename of the .changes
-file.
-
-signing_rules is an optional argument:
-
- o If signing_rules == -1, no signature is required.
- o If signing_rules == 0 (the default), a signature is required.
- o If signing_rules == 1, it turns on the same strict format checking
-   as dpkg-source.
-
-The rules for (signing_rules == 1)-mode are:
-
-  o The PGP header consists of "-----BEGIN PGP SIGNED MESSAGE-----"
-    followed by any PGP header data and must end with a blank line.
-
-  o The data section must end with a blank line and must be followed by
-    "-----BEGIN PGP SIGNATURE-----".
-"""
-
-    error = ""
-    changes = {}
-
-    changes_in = open(filename)
-    lines = changes_in.readlines()
-
-    if not lines:
-	raise changes_parse_error_exc, "[Empty changes file]"
-
-    # Reindex by line number so we can easily verify the format of
-    # .dsc files...
-    index = 0
-    indexed_lines = {}
-    for line in lines:
-        index += 1
-        indexed_lines[index] = line[:-1]
-
-    inside_signature = 0
-
-    num_of_lines = len(indexed_lines.keys())
-    index = 0
-    first = -1
-    while index < num_of_lines:
-        index += 1
-        line = indexed_lines[index]
-        if line == "":
-            if signing_rules == 1:
-                index += 1
-                if index > num_of_lines:
-                    raise invalid_dsc_format_exc, index
-                line = indexed_lines[index]
-                if not line.startswith("-----BEGIN PGP SIGNATURE"):
-                    raise invalid_dsc_format_exc, index
-                inside_signature = 0
-                break
-            else:
-                continue
-        if line.startswith("-----BEGIN PGP SIGNATURE"):
-            break
-        if line.startswith("-----BEGIN PGP SIGNED MESSAGE"):
-            inside_signature = 1
-            if signing_rules == 1:
-                while index < num_of_lines and line != "":
-                    index += 1
-                    line = indexed_lines[index]
-            continue
-        # If we're not inside the signed data, don't process anything
-        if signing_rules >= 0 and not inside_signature:
-            continue
-        slf = re_single_line_field.match(line)
-        if slf:
-            field = slf.groups()[0].lower()
-            changes[field] = slf.groups()[1]
-	    first = 1
-            continue
-        if line == " .":
-            changes[field] += '\n'
-            continue
-        mlf = re_multi_line_field.match(line)
-        if mlf:
-            if first == -1:
-                raise changes_parse_error_exc, "'%s'\n [Multi-line field continuing on from nothing?]" % (line)
-            if first == 1 and changes[field] != "":
-                changes[field] += '\n'
-            first = 0
-	    changes[field] += mlf.groups()[0] + '\n'
-            continue
-	error += line
-
-    if signing_rules == 1 and inside_signature:
-        raise invalid_dsc_format_exc, index
-
-    changes_in.close()
-    changes["filecontents"] = "".join(lines)
-
-    if error:
-	raise changes_parse_error_exc, error
-
-    return changes
-
-################################################################################
-
-def build_file_list(changes, is_a_dsc=0):
-    files = {};
-
-    # Make sure we have a Files: field to parse...
-    if not changes.has_key("files"):
-	raise no_files_exc;
-
-    # Make sure we recognise the format of the Files: field
-    format = changes.get("format", "");
-    if format != "":
-	format = float(format.split()[0]);
-    if not is_a_dsc and (format < 1.5 or format > 2.0):
-	raise nk_format_exc, format;
-
-    # Parse each entry/line:
-    for i in changes["files"].split('\n'):
-        if not i:
-            break;
-        s = i.split();
-        section = priority = "";
-        try:
-            if is_a_dsc:
-                (md5, size, name) = s;
-            else:
-                (md5, size, section, priority, name) = s;
-        except ValueError:
-            raise changes_parse_error_exc, i;
-
-        if section == "":
-            section = "-";
-        if priority == "":
-            priority = "-";
-
-        (section, component) = extract_component_from_section(section);
-
-        files[name] = Dict(md5sum=md5, size=size, section=section,
-                           priority=priority, component=component);
-
-    return files
-
 ################################################################################
 
 def fubar(msg, exit_code=1):
@@ -309,9 +159,9 @@
 ################################################################################
 
 def join_with_commas_and(list):
-	if len(list) == 0: return "nothing";
-	if len(list) == 1: return list[0];
-	return ", ".join(list[:-1]) + " and " + list[-1];
+    if len(list) == 0: return "nothing";
+    if len(list) == 1: return list[0];
+    return ", ".join(list[:-1]) + " and " + list[-1];
 
 ################################################################################
 

=== modified file 'scripts/ftpmaster-tools/sync-source.py'
--- scripts/ftpmaster-tools/sync-source.py	2010-02-02 22:34:39 +0000
+++ scripts/ftpmaster-tools/sync-source.py	2010-02-11 16:55:27 +0000
@@ -18,6 +18,7 @@
 
 import apt_pkg
 import commands
+from debian_bundle.deb822 import Dsc
 import errno
 import optparse
 import os
@@ -25,7 +26,6 @@
 import shutil
 import stat
 import string
-import sys
 import tempfile
 import time
 import urllib
@@ -44,6 +44,7 @@
 from canonical.librarian.client import LibrarianClient
 from canonical.lp import initZopeless
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.soyuz.scripts.ftpmaster import SyncSource, SyncSourceError
 
 
 reject_message = ""
@@ -130,7 +131,7 @@
 
 def generate_changes(dsc, dsc_files, suite, changelog, urgency, closes,
                      lp_closes, section, priority, description,
-                     have_orig_tar_gz, requested_by, origin):
+                     files_from_librarian, requested_by, origin):
     """Generate a .changes as a string"""
 
     # XXX cprov 2007-07-03:
@@ -164,7 +165,7 @@
     changes += changelog
     changes += "Files: \n"
     for filename in dsc_files:
-        if filename.endswith(".orig.tar.gz") and have_orig_tar_gz:
+        if filename in files_from_librarian:
             continue
         changes += " %s %s %s %s %s\n" % (dsc_files[filename]["md5sum"],
                                           dsc_files[filename]["size"],
@@ -361,13 +362,91 @@
                 source, source_component, binary, current_version,
                 current_component)
 
+def split_gpg_and_payload(sequence):
+    """Return a (gpg_pre, payload, gpg_post) tuple
+
+    Each element of the returned tuple is a list of lines (with trailing
+    whitespace stripped).
+    """
+    # XXX JRV 20100211: Copied from deb822.py in python-debian. When 
+    # Launchpad switches to Lucid this copy should be removed.
+    # bug=520508
+
+    gpg_pre_lines = []
+    lines = []
+    gpg_post_lines = []
+    state = 'SAFE'
+    gpgre = re.compile(r'^-----(?P<action>BEGIN|END) PGP (?P<what>[^-]+)-----$')
+    blank_line = re.compile('^$')
+    first_line = True
+
+    for line in sequence:
+        line = line.strip('\r\n')
+
+        # skip initial blank lines, if any
+        if first_line:
+            if blank_line.match(line):
+                continue
+            else:
+                first_line = False
+
+        m = gpgre.match(line)
+
+        if not m:
+            if state == 'SAFE':
+                if not blank_line.match(line):
+                    lines.append(line)
+                else:
+                    if not gpg_pre_lines:
+                        # There's no gpg signature, so we should stop at
+                        # this blank line
+                        break
+            elif state == 'SIGNED MESSAGE':
+                if blank_line.match(line):
+                    state = 'SAFE'
+                else:
+                    gpg_pre_lines.append(line)
+            elif state == 'SIGNATURE':
+                gpg_post_lines.append(line)
+        else:
+            if m.group('action') == 'BEGIN':
+                state = m.group('what')
+            elif m.group('action') == 'END':
+                gpg_post_lines.append(line)
+                break
+            if not blank_line.match(line):
+                if not lines:
+                    gpg_pre_lines.append(line)
+                else:
+                    gpg_post_lines.append(line)
+
+    if len(lines):
+        return (gpg_pre_lines, lines, gpg_post_lines)
+    else:
+        raise EOFError('only blank lines found in input')
+
 
 def import_dsc(dsc_filename, suite, previous_version, signing_rules,
-               have_orig_tar_gz, requested_by, origin, current_sources,
+               files_from_librarian, requested_by, origin, current_sources,
                current_binaries):
-    dsc = dak_utils.parse_changes(dsc_filename, signing_rules)
-    dsc_files = dak_utils.build_file_list(dsc, is_a_dsc=1)\
-
+    dsc_file = open(dsc_filename, 'r')
+    dsc = Dsc(dsc_file)
+
+    if signing_rules.startswith("must be signed"):
+        dsc_file.seek(0)
+        # XXX JRV 20100211: When Launchpad starts depending on Lucid,
+        # use dsc.split_gpg_and_payload() instead. 
+        # bug=520508
+        (gpg_pre, payload, gpg_post) = split_gpg_and_payload(dsc_file)
+        if gpg_pre == [] and gpg_post == []:
+            dak_utils.fubar("signature required for %s but not present" 
+                % dsc_filename)
+        if signing_rules == "must be signed and valid":
+            if (gpg_pre[0] != "-----BEGIN PGP SIGNED MESSAGE-----" or
+                gpg_post[0] != "-----BEGIN PGP SIGNATURE-----"):
+                dak_utils.fubar("signature for %s invalid %r %r" % (dsc_filename, gpg_pre, gpg_post))
+
+    dsc_files = dict((entry['name'], entry) for entry in dsc['files'])
     check_dsc(dsc, current_sources, current_binaries)
 
     # Add the .dsc itself to dsc_files so it's listed in the Files: field
@@ -400,7 +479,7 @@
 
     changes = generate_changes(
         dsc, dsc_files, suite, changelog, urgency, closes, lp_closes,
-        section, priority, description, have_orig_tar_gz, requested_by,
+        section, priority, description, files_from_librarian, requested_by,
         origin)
 
     # XXX cprov 2007-07-03: Soyuz wants an unsigned changes
@@ -565,86 +644,21 @@
     if not Sources.has_key(pkg):
         dak_utils.fubar("%s doesn't exist in the Sources file." % (pkg))
 
-    have_orig_tar_gz = False
-
-    # Fetch the source
-    files = Sources[pkg]["files"]
-    for filename in files:
-        # First see if we can find the source in the librarian
-        archive_ids = [a.id for a in Options.todistro.all_distro_archives]
-        query = """
-        SELECT DISTINCT ON (LibraryFileContent.sha1, LibraryFileContent.filesize)
-            LibraryFileAlias.id
-        FROM SourcePackageFilePublishing, LibraryFileAlias, LibraryFileContent
-        WHERE
-          LibraryFileAlias.id = SourcePackageFilePublishing.libraryfilealias AND
-          LibraryFileContent.id = LibraryFileAlias.content AND
-          SourcePackageFilePublishing.libraryfilealiasfilename = %s AND
-          SourcePackageFilePublishing.archive IN %s
-        """ % sqlvalues(filename, archive_ids)
-        cur = cursor()
-        cur.execute(query)
-        results = cur.fetchall()
-        if results:
-            if not filename.endswith("orig.tar.gz"):
-                dak_utils.fubar(
-                    "%s (from %s) is in the DB but isn't an orig.tar.gz.  "
-                    "(Probably published in an older release)"
-                    % (filename, pkg))
-            if len(results) > 1:
-                dak_utils.fubar(
-                    "%s (from %s) returns multiple IDs (%s) for "
-                    "orig.tar.gz.  Help?" % (filename, pkg, results))
-            have_orig_tar_gz = filename
-            print ("  - <%s: already in distro - downloading from librarian>"
-                   % (filename))
-            output_file = open(filename, 'w')
-            librarian_input = Library.getFileByAlias(results[0][0])
-            output_file.write(librarian_input.read())
-            output_file.close()
-            continue
-
-        # Download the file
-        download_f = (
-            "%s%s" % (origin["url"], files[filename]["remote filename"]))
-        if not os.path.exists(filename):
-            print "  - <%s: downloading from %s>" % (filename, origin["url"])
-            sys.stdout.flush()
-            urllib.urlretrieve(download_f, filename)
-        else:
-            print "  - <%s: cached>" % (filename)
-
-        # Check md5sum and size match Source
-        actual_md5sum = md5sum_file(filename)
-        expected_md5sum = files[filename]["md5sum"]
-        if actual_md5sum != expected_md5sum:
-            dak_utils.fubar(
-                "%s: md5sum check failed (%s [actual] vs. %s [expected])."
-                % (filename, actual_md5sum, expected_md5sum))
-        actual_size = os.stat(filename)[stat.ST_SIZE]
-        expected_size = int(files[filename]["size"])
-        if actual_size != expected_size:
-            dak_utils.fubar(
-                "%s: size mismatch (%s [actual] vs. %s [expected])."
-                % (filename, actual_size, expected_size))
-
-        # Remember the name of the .dsc file
-        if filename.endswith(".dsc"):
-            dsc_filename = os.path.abspath(filename)
-
-    if origin["dsc"] == "must be signed and valid":
-        signing_rules = 1
-    elif origin["dsc"] == "must be signed":
-        signing_rules = 0
-    else:
-        signing_rules = -1
-
-    import_dsc(dsc_filename, suite, previous_version, signing_rules,
-               have_orig_tar_gz, requested_by, origin, current_sources,
-               current_binaries)
-
-    if have_orig_tar_gz:
-        os.unlink(have_orig_tar_gz)
+    syncsource = SyncSource(Sources[pkg]["files"], origin, Log,
+        urllib.urlretrieve, Options.todistro)
+    try:
+        files_from_librarian = syncsource.fetchLibrarianFiles()
+        dsc_filename = syncsource.fetchSyncFiles()
+        syncsource.checkDownloadedFiles()
+    except SyncSourceError, e:
+        dak_utils.fubar("Fetching files failed: %s" % (str(e),))
+
+    if dsc_filename is None:
+        dak_utils.fubar("No dsc filename in %r" % Sources[pkg]["files"].keys())
+
+    import_dsc(os.path.abspath(dsc_filename), suite, previous_version,
+               origin["dsc"], files_from_librarian, requested_by, origin,
+               current_sources, current_binaries)
 
 
 def do_diff(Sources, Suite, origin, arguments, current_binaries):