Merge lp:~sinzui/launchpad/prf-flavor-loop into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

This is my branch to fix the flavor striping and file upload looping rules in
the product release finder.

    lp:~sinzui/launchpad/prf-flavor-loop
    Diff size: 117
    Launchpad bug: https://bugs.launchpad.net/bugs/456525
                   https://bugs.launchpad.net/bugs/415595
    Test command: ./bin/test -vv -t "test_prf_finder"
    Pre-implementation: dreamcat4 and suraia, both of whom read the source
                        code with me as we diagnosed why their files were
                        missing.
    Target release: 3.1.10

= Fix the flavor striping and looping rules in the product release finder =

Bug #456525 [PRF does not strip the flavor info after the ~]
    The releases version are incorrect because the ~ that marked the start of
    the flavor information was converted to a -.

Bug #415595 [PRF should import all files]
    For example, I have sushi-1.0.1.tar.bz2 and sushi-1.0.1.tar.gz, but the
    release finder only imports the .tar.bz2. Looking at the code it seems
    that only the first matching file is added.

== Rules ==

Bug #456525 [PRF does not strip the flavor info after the ~]
    Add the '~' to the flavor_pattern.

Bug #415595 [PRF should import all files]
    hasReleaseTarball() wrongly assumes that if the release has anything like
    a code tarball, that there all the downloading is complete. So downloading
    for a release stops once the first package is downloaded which means
    release notes and installers are also being ignored.

    handleRelease() knows the filename. It should pass it to
    hasReleaseTarball() to verify if the release already has the file. Rename
    hasReleaseTarball => hasReleaseFile(self, product_name, series_name,
    release_name, file_name)

== QA ==

    * Run the PRF
    * Visit project php-fpm
    * Verify the 0.6 release exists and it has php-fpm-0.6~5.3.1.tar.gz
    * Visit /sushi.nigiri/1.0
    * Visit several of the release.
    * Verify that each has more than one milestone.

== Lint ==

Linting changed files:
  lib/lp/registry/scripts/productreleasefinder/finder.py
  lib/lp/registry/tests/test_prf_finder.py

== Test ==

    * lib/lp/registry/tests/test_prf_finder.py
      * Added a test to verify the file and an alternate file are handled
        correctly before and after one it uploaded.
      * Added a test to verify that the release.version (AKA milestone.name)
        does not contain the flavor information from the '~'.

== Implementation ==

    * lib/lp/registry/scripts/productreleasefinder/finder.py
      * Updated the flavor_pattern rules to match '~'.
      * Renamed hasReleaseTarball => hasReleaseFile and added the filename
        argument. The method now matches on file name, not file tpe so that
        a tar.gz and tar.bzr will both be uploded. This will also allow
        multiple tar.gz that contain flavor info in the name to be uploaded.

[Eleanor Berger (intellectronica)]

review: Approve (code)

=== modified file 'lib/lp/registry/scripts/productreleasefinder/finder.py'
--- lib/lp/registry/scripts/productreleasefinder/finder.py	2009-07-17 00:26:05 +0000
+++ lib/lp/registry/scripts/productreleasefinder/finder.py	2009-10-25 17:05:27 +0000
@@ -47,7 +47,8 @@
     'sparc',
     ])
 flavor_pattern = re.compile(r"""
-    (_[a-z][a-z]_[A-Z][A-Z]       # Language version
+    (~                            # Packaging target
+     |_[a-z][a-z]_[A-Z][A-Z]      # or language version
      |_(%s)                       # or processor version
      |[\.-](win32|OSX)            # or OS version
      |\.(deb|noarch|rpm|dmg|exe)  # or packaging version
@@ -139,9 +140,10 @@
                 self.log.debug("File in %s found that matched no glob: %s",
                                product_name, url)
 
-    def hasReleaseTarball(self, product_name, series_name, release_name):
+    def hasReleaseFile(self, product_name, series_name,
+                          release_name, filename):
         """Return True if we have a tarball for the given product release."""
-        has_tarball = False
+        has_file = False
         self.ztm.begin()
         try:
             product = getUtility(IProductSet).getByName(product_name)
@@ -151,13 +153,12 @@
                     release = series.getRelease(release_name)
                     if release is not None:
                         for fileinfo in release.files:
-                            if (fileinfo.filetype
-                                == UpstreamFileType.CODETARBALL):
-                                has_tarball = True
+                            if filename == fileinfo.libraryfile.filename:
+                                has_file = True
                                 break
         finally:
             self.ztm.abort()
-        return has_tarball
+        return has_file
 
     def addReleaseTarball(self, product_name, series_name, release_name,
                           filename, size, file, content_type):
@@ -221,7 +222,7 @@
                            version, url)
             return
 
-        if self.hasReleaseTarball(product_name, series_name, version):
+        if self.hasReleaseFile(product_name, series_name, version, filename):
             self.log.debug("Already have a tarball for release %s", version)
             return
 

=== modified file 'lib/lp/registry/tests/test_prf_finder.py'
--- lib/lp/registry/tests/test_prf_finder.py	2009-07-17 00:26:05 +0000
+++ lib/lp/registry/tests/test_prf_finder.py	2009-10-25 17:05:27 +0000
@@ -168,21 +168,31 @@
         ztm = self.layer.txn
         logging.basicConfig(level=logging.CRITICAL)
         prf = ProductReleaseFinder(ztm, logging.getLogger())
+        file_name = 'evolution-42.0.orig.tar.gz'
+        alt_file_name = 'evolution-42.0.orig.tar.bz2'
 
         # create a release tarball
         fp = open(os.path.join(
-            self.release_root, 'evolution-42.0.orig.tar.gz'), 'w')
+            self.release_root, file_name), 'w')
         fp.write('foo')
         fp.close()
 
-        self.assertEqual(prf.hasReleaseTarball('evolution', 'trunk', '42.0'),
-                         False)
-
-        prf.handleRelease('evolution', 'trunk',
-                          self.release_url + '/evolution-42.0.orig.tar.gz')
-
-        self.assertEqual(prf.hasReleaseTarball('evolution', 'trunk', '42.0'),
-                         True)
+        self.assertEqual(
+            prf.hasReleaseFile('evolution', 'trunk', '42.0', file_name),
+            False)
+        self.assertEqual(
+            prf.hasReleaseFile('evolution', 'trunk', '42.0', alt_file_name),
+            False)
+
+        prf.handleRelease(
+            'evolution', 'trunk', self.release_url + '/' + file_name)
+
+        self.assertEqual(
+            prf.hasReleaseFile('evolution', 'trunk', '42.0', file_name),
+            True)
+        self.assertEqual(
+            prf.hasReleaseFile('evolution', 'trunk', '42.0', alt_file_name),
+            False)
 
         # check to see that the release has been created
         evo = getUtility(IProductSet).getByName('evolution')
@@ -192,8 +202,7 @@
         self.assertEqual(release.files.count(), 1)
         fileinfo = release.files[0]
         self.assertEqual(fileinfo.filetype, UpstreamFileType.CODETARBALL)
-        self.assertEqual(fileinfo.libraryfile.filename,
-                         'evolution-42.0.orig.tar.gz')
+        self.assertEqual(fileinfo.libraryfile.filename, file_name)
 
         # verify that the fileinfo object is sane
         self.failUnless(verifyObject(IProductReleaseFile, fileinfo))
@@ -345,6 +354,8 @@
         self.assertEqual(version, '1.16.3')
         version = extract_version('partitionmanager-21-2.noarch.rpm')
         self.assertEqual(version, '21-2')
+        version = extract_version('php-fpm-0.6~5.3.1.tar.gz')
+        self.assertEqual(version, '0.6')
 
     def test_extract_version_name_with_uppercase(self):
         """Verify that the file's version is lowercases."""