Merge lp:~maxb/udd/import-ordering-fix-take-2 into lp:udd

[Max Bowsher]

Jelmer, you were so very right when you suggested import ordering could be better done based on versions at the bzr sprint in London.

What neither of us realized at the time was that importing sid first in all cases only holds true as a correct method if sid contains all older releases in its ancestry .... which is true in theory, but not in Launchpad's database.

In fact, Launchpad's "sid" only goes back as far as lenny, and woody, sarge, and etch are imported by the UDD importer via package indices from archive.debian.org.

Fortunately, this miscalculation of mine only causes suboptimal history to be produced when doing fresh imports of package versions around the etch / lenny boundary, so should not have had much if any impact.

[James Westby (james-w)]

The comments are much clearer now too, thanks.

James

review: Approve

=== modified file 'icommon.py'
--- icommon.py	2011-06-02 00:32:31 +0000
+++ icommon.py	2011-06-03 07:49:30 +0000
@@ -324,60 +324,43 @@
         series or pockets are imported before imports to the likely
         destinations, such that the source imported version is available to be
         pulled."""
-        # Debian versions should be imported before Ubuntu versions, so
-        # that any Debian version an Ubuntu version may be based on is
-        # already imported when the Ubuntu version is imported.
+        # Core assumption: packages always have higher versions than their
+        # ancestors... except for backports.  Therefore, sort backports after
+        # all non-backports.
+        if a.pocket == "backports" and b.pocket != "backports":
+            return 1
+        if a.pocket != "backports" and b.pocket == "backports":
+            return -1
+        # Next sort by version.
+        if a.version < b.version:
+            return -1
+        if a.version > b.version:
+            return 1
+        # From here on, a.version == b.version
+        # Sort Debian before Ubuntu, so that synced packages are first imported
+        # into Debian.
         if a.distro != b.distro:
             if a.distro == "debian":
                 return -1
             return 1
         # From here on, a.distro == b.distro
-        if a.distro == "ubuntu":
-            # Ubuntu backports should be imported after all other Ubuntu
-            # pockets, so that the version the backport is based on is
-            # imported before the backport itself.
-            if a.pocket == "backports" and b.pocket != "backports":
-                return 1
-            if a.pocket != "backports" and b.pocket == "backports":
-                return -1
-            # Within Ubuntu, release series should be imported in ascending
-            # order, so that a package version is imported into the first
-            # series in which it is present.
-            if a.release != b.release:
-                a_release = distro_releases[a.distro].index(a.release)
-                b_release = distro_releases[b.distro].index(b.release)
-                if a_release < b_release:
-                    return -1
-                return 1
-            # Within a single Ubuntu release series, import pockets such
-            # that any pocket that can contain versions based on versions
-            # in another pocket, is imported after that pocket.
-            if a.pocket != b.pocket:
-                a_pocket = distro_pockets[a.distro].index(a.pocket)
-                b_pocket = distro_pockets[b.distro].index(b.pocket)
-                if a_pocket < b_pocket:
-                    return -1
-                return 1
-        if a.distro == "debian":
-            # Debian sid/experimental should be imported before any other
-            # codename, to get the proper ancestry.
-            if a.release in ("sid", "experimental") and b.release not in ("sid", "experimental"):
-                return -1
-            if a.release not in ("sid", "experimental") and b.release in ("sid", "experimental"):
-                return 1
-            # Other than that special case, import in ascending order.
-            if a.release != b.release:
-                a_release = distro_releases[a.distro].index(a.release)
-                b_release = distro_releases[b.distro].index(b.release)
-                if a_release < b_release:
-                    return -1
-                return 1
-        # Within a single distro, release, and pocket, import in version
-        # order.
-        if a.version < b.version:
-            return -1
-        if a.version > b.version:
-            return 1
+        # Release series should be imported in ascending order, modelling a
+        # packaging being carried forward unchanged into new series.
+        if a.release != b.release:
+            a_release = distro_releases[a.distro].index(a.release)
+            b_release = distro_releases[b.distro].index(b.release)
+            if a_release < b_release:
+                return -1
+            return 1
+        # Within a single release series, import pockets such that for all
+        # typical pocket copy workflows, the source pocket is imported first.
+        if a.pocket != b.pocket:
+            a_pocket = distro_pockets[a.distro].index(a.pocket)
+            b_pocket = distro_pockets[b.distro].index(b.pocket)
+            if a_pocket < b_pocket:
+                return -1
+            return 1
+        # Same version, distro, release and pocket
         return 0
 
 

=== modified file 'tests/test_package_to_import.py'
--- tests/test_package_to_import.py	2011-05-17 13:26:32 +0000
+++ tests/test_package_to_import.py	2011-06-03 07:49:30 +0000
@@ -68,22 +68,25 @@
         self.assertTrue(PTI.import_order_comparator(a, b) < 0)
 
     def test_comparison(self):
-        # Distro is the most-significant factor, with all of Ubuntu coming
-        # after all of Debian, irregardless of releases and versions:
-        self.assertOrdering(PTI("a", V("999999"), "debian", "sid", "release"),
-                            PTI("a", V("1"), "ubuntu", "warty", "release"))
-
-        # Within a distro, release is the next most significant:
+        # Version is the most-significant factor:
+        self.assertOrdering(PTI("a", V("1"), "ubuntu", "warty", "release"),
+                            PTI("a", V("999999"), "debian", "sid", "release"))
+
+        # For something that looks like a sync, Debian comes before Ubuntu:
+        self.assertOrdering(PTI("a", V("1"), "debian", "lenny", "release"),
+                            PTI("a", V("1"), "ubuntu", "lucid", "release"))
+
+        # With same version, same distro, release is the next most significant:
         self.assertOrdering(PTI("a", V("1"), "debian", "lenny", "release"),
                             PTI("a", V("1"), "debian", "squeeze", "release"))
 
-        # Except for Ubuntu backports, which are a special case, imported after
-        # all other Ubuntu imports in the same per-package run:
+        # Except for backports, which are a special case, imported after all
+        # other imports in the same per-package run:
         self.assertOrdering(PTI("a", V("1"), "ubuntu", "natty", "release"),
                             PTI("a", V("1~hardy1"), "ubuntu", "hardy",
                                 "backports"))
 
-        # Within Ubuntu, pockets matter:
+        # Pockets have a defined ordering:
         self.assertOrdering(PTI("a", V("1"), "ubuntu", "natty", "release"),
                             PTI("a", V("1"), "ubuntu", "natty", "security"))
         self.assertOrdering(PTI("a", V("1"), "ubuntu", "natty", "security"),