launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02912
[Merge] lp:~stevenk/launchpad/derive-common-ancestor into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/derive-common-ancestor into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/derive-common-ancestor/+merge/52796
Change how base versions are calculated for DistroSeriesDifferences.
When DSDs were first implemented, they based base version calculation on the assumption that the highest common version is published in both series. Unfortunately, this assumption is false.
Based on the work from a Soyuz mini-sprint (the last one ever, in fact) it was determined that the safest and most correct way to determine the base version is parsing the changelog of the two packages. See https://dev.launchpad.net/Soyuz/BaseVersionDetermination for more information.
I have implemented a method into the object factory that creates a changelog with as many entries as required, which is then stored in the Librarian, and the LFA returned. I have extended makeSourcePackageRelease to accept a changelog argument, which is set on creation, and finally I have extended makeDistroSeriesDifference to set changelog if they are passed in.
--
https://code.launchpad.net/~stevenk/launchpad/derive-common-ancestor/+merge/52796
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/derive-common-ancestor into lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-10-29 19:28:14 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2011-03-10 03:26:14 +0000
@@ -103,11 +103,19 @@
def test_requestDiffs(self):
# The generation of package diffs can be requested via the API.
+ dervied_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.2'])
+ parent_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.3'])
+ transaction.commit() # Yay, librarian.
ds_diff = self.factory.makeDistroSeriesDifference(
source_package_name_str='foo', versions={
'derived': '1.2',
'parent': '1.3',
- 'base': '1.0',
+ 'base': '1.0'},
+ changelogs={
+ 'derived': dervied_changelog,
+ 'parent': parent_changelog,
})
ws_diff = ws_object(self.factory.makeLaunchpadService(
ds_diff.owner), ds_diff)
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-03-04 05:54:40 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-03-10 03:26:14 +0000
@@ -9,6 +9,7 @@
'DistroSeriesDifference',
]
+from debian.changelog import Changelog
from lazr.enum import DBItem
from storm.expr import Desc
from storm.locals import (
@@ -299,29 +300,22 @@
DistroSeriesDifferenceType.DIFFERENT_VERSIONS):
return False
- # Find all source package releases for the derived and parent
- # series and get the most recent common version.
- derived_sprs = self.source_pub.meta_sourcepackage.distinctreleases
- derived_versions = [
- Version(spr.version) for spr in derived_sprs]
-
- parent_sourcepkg = self.parent_source_pub.meta_sourcepackage
- parent_sprs = parent_sourcepkg.distinctreleases
- parent_versions = [
- Version(spr.version) for spr in parent_sprs]
-
- common_versions = list(
- set(derived_versions).intersection(parent_versions))
- if common_versions:
- common_versions.sort()
- self.base_version = unicode(common_versions.pop())
- return True
-
- if self.base_version is None:
- return False
- else:
- self.base_version = None
- return True
+ changelog = self.source_pub.sourcepackagerelease.changelog
+ parent_changelog = (
+ self.parent_source_pub.sourcepackagerelease.changelog)
+
+ # If both the parents and descendants changelog are available, we
+ # can parse the changelog versions and reliably work out the most
+ # recent common ancestor using set arithmetic.
+ if changelog is not None and parent_changelog is not None:
+ ancestry = set(Changelog(changelog.read()).versions)
+ parent_ancestry = set(
+ Changelog(parent_changelog.read()).versions)
+ intersection = ancestry.intersection(parent_ancestry)
+ if len(intersection) > 0:
+ self.base_version = unicode(max(intersection))
+ return True
+ return False
def addComment(self, commenter, comment):
"""See `IDistroSeriesDifference`."""
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-12-22 14:50:08 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2011-03-10 03:26:14 +0000
@@ -5,10 +5,9 @@
__metaclass__ = type
-import unittest
-
from storm.exceptions import IntegrityError
from storm.store import Store
+import transaction
from zope.component import getUtility
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
@@ -39,7 +38,7 @@
class DistroSeriesDifferenceTestCase(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def test_implements_interface(self):
# The implementation implements the interface correctly.
@@ -445,37 +444,25 @@
self.assertIs(None, ds_diff.base_version)
- def test_base_version_common(self):
- # The common base version is set when the difference is created.
- # Publish v1.0 of foo in both series.
- ds_diff = self.factory.makeDistroSeriesDifference(versions={
- 'derived': '1.2',
- 'parent': '1.3',
- 'base': '1.0',
- })
-
- self.assertEqual('1.0', ds_diff.base_version)
-
def test_base_version_multiple(self):
# The latest common base version is set as the base-version.
- # Publish v1.0 and 1.1 of foo in both series.
derived_series = self.factory.makeDistroSeries(
parent_series=self.factory.makeDistroSeries())
source_package_name = self.factory.getOrMakeSourcePackageName('foo')
- for series in [derived_series, derived_series.parent_series]:
- for version in ['1.0', '1.1']:
- self.factory.makeSourcePackagePublishingHistory(
- distroseries=series,
- version=version,
- sourcepackagename=source_package_name,
- status=PackagePublishingStatus.SUPERSEDED)
-
+ # Create changelogs for both.
+ changelog_lfa = self.factory.makeChangelog('foo', ['1.2', '1.1'])
+ parent_changelog_lfa = self.factory.makeChangelog('foo', ['1.1'])
+ transaction.commit() # Yay, librarian.
+
ds_diff = self.factory.makeDistroSeriesDifference(
derived_series=derived_series, source_package_name_str='foo',
versions={
'derived': '1.2',
'parent': '1.3',
- })
+ },
+ changelogs={
+ 'derived': changelog_lfa,
+ 'parent': parent_changelog_lfa})
self.assertEqual('1.1', ds_diff.base_version)
@@ -488,22 +475,61 @@
def test_base_source_pub(self):
# The publishing in the derived series with base_version is returned.
+ dervied_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.2'])
+ parent_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.3'])
+ transaction.commit() # Yay, librarian.
+
ds_diff = self.factory.makeDistroSeriesDifference(versions={
'derived': '1.2',
'parent': '1.3',
'base': '1.0',
+ },
+ changelogs={
+ 'derived': dervied_changelog,
+ 'parent': parent_changelog,
})
base_pub = ds_diff.base_source_pub
self.assertEqual('1.0', base_pub.source_package_version)
self.assertEqual(ds_diff.derived_series, base_pub.distroseries)
+ def test_base_source_pub_not_published(self):
+ # If the base version isn't published, the base version is
+ # calculated, but the source publication isn't set.
+ dervied_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.2'])
+ parent_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.3'])
+ transaction.commit() # Yay, librarian.
+
+ ds_diff = self.factory.makeDistroSeriesDifference(versions={
+ 'derived': '1.2',
+ 'parent': '1.3',
+ },
+ changelogs={
+ 'derived': dervied_changelog,
+ 'parent': parent_changelog,
+ })
+ self.assertEqual('1.0', ds_diff.base_version)
+ self.assertIs(None, ds_diff.base_source_pub)
+
def test_requestPackageDiffs(self):
# IPackageDiffs are created for the corresponding versions.
+ dervied_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.2'])
+ parent_changelog = self.factory.makeChangelog(
+ versions=['1.0', '1.3'])
+ transaction.commit() # Yay, librarian.
ds_diff = self.factory.makeDistroSeriesDifference(versions={
'derived': '1.2',
'parent': '1.3',
'base': '1.0',
+ },
+ changelogs={
+ 'derived': dervied_changelog,
+ 'parent': parent_changelog,
})
with person_logged_in(ds_diff.owner):
@@ -647,7 +673,3 @@
ds_diff.derived_series, 'fooname')
self.assertEqual(ds_diff, result)
-
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-03-10 00:37:49 +0000
+++ lib/lp/testing/factory.py 2011-03-10 03:26:14 +0000
@@ -2049,6 +2049,23 @@
removeSecurityProxy(code_import).review_status = review_status
return code_import
+ def makeChangelog(self, spn=None, versions=[]):
+ """Create and return a LFA of a valid Debian-style changelog."""
+ if spn is None:
+ spn = self.getUniqueString()
+ changelog = ''
+ for version in versions:
+ entry = dedent('''
+ %s (%s) unstable; urgency=low
+
+ * %s.
+
+ -- Foo Bar <foo@xxxxxxxxxxx> Tue, 01 Jan 1970 01:50:41 +0000
+
+ ''' % (spn, version, version))
+ changelog += entry
+ return self.makeLibraryFileAlias(content=changelog)
+
def makeCodeImportEvent(self):
"""Create and return a CodeImportEvent."""
code_import = self.makeCodeImport()
@@ -2248,7 +2265,8 @@
self, derived_series=None, source_package_name_str=None,
versions=None,
difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
- status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
+ status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
+ changelogs=None):
"""Create a new distro series source package difference."""
if derived_series is None:
parent_series = self.makeDistroSeries()
@@ -2263,32 +2281,38 @@
if versions is None:
versions = {}
+ if changelogs is None:
+ changelogs = {}
base_version = versions.get('base')
if base_version is not None:
for series in [derived_series, derived_series.parent_series]:
- self.makeSourcePackagePublishingHistory(
- distroseries=series,
- version=base_version,
+ spr = self.makeSourcePackageRelease(
sourcepackagename=source_package_name,
+ version=base_version)
+ self.makeSourcePackagePublishingHistory(
+ distroseries=series, sourcepackagerelease=spr,
status=PackagePublishingStatus.SUPERSEDED)
if difference_type is not (
DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES):
-
- self.makeSourcePackagePublishingHistory(
- distroseries=derived_series,
+ spr = self.makeSourcePackageRelease(
+ sourcepackagename=source_package_name,
version=versions.get('derived'),
- sourcepackagename=source_package_name,
+ changelog=changelogs.get('derived'))
+ self.makeSourcePackagePublishingHistory(
+ distroseries=derived_series, sourcepackagerelease=spr,
status = PackagePublishingStatus.PUBLISHED)
if difference_type is not (
DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES):
-
+ spr = self.makeSourcePackageRelease(
+ sourcepackagename=source_package_name,
+ version=versions.get('parent'),
+ changelog=changelogs.get('parent'))
self.makeSourcePackagePublishingHistory(
distroseries=derived_series.parent_series,
- version=versions.get('parent'),
- sourcepackagename=source_package_name,
+ sourcepackagerelease=spr,
status = PackagePublishingStatus.PUBLISHED)
diff = getUtility(IDistroSeriesDifferenceSource).new(
@@ -2592,7 +2616,7 @@
source_package_recipe_build=sprb,
sourcepackagename=sourcepackagename,
distroseries=distroseries, archive=archive)
- spph = self.makeSourcePackagePublishingHistory(
+ self.makeSourcePackagePublishingHistory(
sourcepackagerelease=spr, archive=archive,
distroseries=distroseries)
@@ -3196,7 +3220,8 @@
dscsigningkey=None,
user_defined_fields=None,
changelog_entry=None,
- homepage=None):
+ homepage=None,
+ changelog=None):
"""Make a `SourcePackageRelease`."""
if distroseries is None:
if source_package_recipe_build is not None:
@@ -3252,7 +3277,7 @@
build_conflicts=build_conflicts,
build_conflicts_indep=build_conflicts_indep,
architecturehintlist=architecturehintlist,
- changelog=None,
+ changelog=changelog,
changelog_entry=changelog_entry,
dsc=None,
copyright=self.getUniqueString(),
Follow ups
