Merge ~cjwatson/launchpad:diskpool-add-source-version into launchpad:master

[Colin Watson]

Tell DiskPool about source version when adding/removing files

Some repository formats, such as Python indexes, organize files by version as well as by name. To accommodate this, tell the disk pool about the source package version as well as the source package name when adding or removing files. The Artifactory pool implementation also needs to store this in a property so that it can construct an entry for the file when updating its properties.

[Jürgen Gmach (jugmac00)]

review: Approve

diff --git a/lib/lp/archivepublisher/artifactory.py b/lib/lp/archivepublisher/artifactory.py
index 2873855..53b43a6 100644
--- a/lib/lp/archivepublisher/artifactory.py
+++ b/lib/lp/archivepublisher/artifactory.py
@@ -44,9 +44,12 @@ from lp.soyuz.interfaces.publishing import (
 class ArtifactoryPoolEntry:
 
     def __init__(self, archive: IArchive, rootpath: ArtifactoryPath,
-                 source: str, filename: str, logger: logging.Logger) -> None:
+                 source_name: str, source_version: str, filename: str,
+                 logger: logging.Logger) -> None:
+        self.archive = archive
         self.rootpath = rootpath
-        self.source = source
+        self.source_name = source_name
+        self.source_version = source_version
         self.filename = filename
         self.logger = logger
 
@@ -60,7 +63,7 @@ class ArtifactoryPoolEntry:
         # the pool structure, and doing so would introduce significant
         # complications in terms of having to keep track of components just
         # in order to update an artifact's properties.
-        return self.rootpath / poolify(self.source) / self.filename
+        return self.rootpath / poolify(self.source_name) / self.filename
 
     def makeReleaseID(self, pub_file: IPackageReleaseFile) -> str:
         """
@@ -129,7 +132,8 @@ class ArtifactoryPoolEntry:
         """
         properties = {}
         properties["launchpad.release-id"] = [release_id]
-        properties["launchpad.source-name"] = [self.source]
+        properties["launchpad.source-name"] = [self.source_name]
+        properties["launchpad.source-version"] = [self.source_version]
         if publications:
             archives = {publication.archive for publication in publications}
             if len(archives) > 1:
@@ -259,12 +263,14 @@ class ArtifactoryPool:
             session.auth = XJFrogArtApiAuth(write_creds.split(":", 1)[1])
         return session
 
-    def _getEntry(self, sourcename, file) -> ArtifactoryPoolEntry:
+    def _getEntry(self, source_name: str, source_version: str,
+                  file: str) -> ArtifactoryPoolEntry:
         """See `DiskPool._getEntry`."""
         return ArtifactoryPoolEntry(
-            self.archive, self.rootpath, sourcename, file, self.logger)
+            self.archive, self.rootpath, source_name, source_version, file,
+            self.logger)
 
-    def pathFor(self, comp: str, source: str,
+    def pathFor(self, comp: str, source_name: str, source_version: str,
                 file: Optional[str] = None) -> Path:
         """Return the path for the given pool folder or file.
 
@@ -279,16 +285,17 @@ class ArtifactoryPool:
         # the pool structure, and doing so would introduce significant
         # complications in terms of having to keep track of components just
         # in order to update an artifact's properties.
-        path = self.rootpath / poolify(source)
+        path = self.rootpath / poolify(source_name)
         if file:
             path = path / file
         return path
 
-    def addFile(self, component: str, sourcename: str, filename: str,
-                pub_file: IPackageReleaseFile):
+    def addFile(self, component: str, source_name: str, source_version: str,
+                filename: str, pub_file: IPackageReleaseFile):
         """Add a file with the given contents to the pool.
 
-        `sourcename` and `filename` are used to calculate the location.
+        `source_name`, `source_version`, and `filename` are used to
+        calculate the location.
 
         pub_file is an `IPackageReleaseFile` providing the file's contents
         and SHA-1 hash.  The SHA-1 hash is used to compare the given file
@@ -308,10 +315,10 @@ class ArtifactoryPool:
         This is similar to `DiskPool.addFile`, except that there is no
         symlink handling and the component is ignored.
         """
-        entry = self._getEntry(sourcename, filename)
+        entry = self._getEntry(source_name, source_version, filename)
         return entry.addFile(pub_file)
 
-    def removeFile(self, component: str, sourcename: str,
+    def removeFile(self, component: str, source_name: str, source_version: str,
                    filename: str) -> int:
         """Remove the specified file from the pool.
 
@@ -324,13 +331,13 @@ class ArtifactoryPool:
         This is similar to `DiskPool.removeFile`, except that there is no
         symlink handling and the component is ignored.
         """
-        entry = self._getEntry(sourcename, filename)
+        entry = self._getEntry(source_name, source_version, filename)
         return entry.removeFile()
 
-    def updateProperties(self, sourcename, filename, publications,
-                         old_properties=None):
+    def updateProperties(self, source_name: str, source_version: str,
+                         filename: str, publications, old_properties=None):
         """Update a file's properties in Artifactory."""
-        entry = self._getEntry(sourcename, filename)
+        entry = self._getEntry(source_name, source_version, filename)
         entry.updateProperties(publications, old_properties=old_properties)
 
     def getArtifactPatterns(self, repository_format):
diff --git a/lib/lp/archivepublisher/deathrow.py b/lib/lp/archivepublisher/deathrow.py
index 06e430a..9071a53 100644
--- a/lib/lp/archivepublisher/deathrow.py
+++ b/lib/lp/archivepublisher/deathrow.py
@@ -87,10 +87,10 @@ class DeathRow:
         removed."""
         if dry_run:
             # Don't actually remove the files if we are dry running
-            def _mockRemoveFile(cn, sn, fn):
-                self.logger.debug("(Not really!) removing %s %s/%s" %
-                                  (cn, sn, fn))
-                fullpath = self.diskpool.pathFor(cn, sn, fn)
+            def _mockRemoveFile(cn, sn, sv, fn):
+                self.logger.debug("(Not really!) removing %s %s/%s/%s" %
+                                  (cn, sn, sv, fn))
+                fullpath = self.diskpool.pathFor(cn, sn, sv, fn)
                 if not fullpath.exists():
                     raise NotInPool
                 return fullpath.lstat().st_size
@@ -227,9 +227,10 @@ class DeathRow:
 
                 # Calculating the file path in pool.
                 pub_file_details = (
-                    pub_file.libraryfile.filename,
-                    pub_record.source_package_name,
                     pub_record.component_name,
+                    pub_record.source_package_name,
+                    pub_record.source_package_version,
+                    pub_file.libraryfile.filename,
                     )
                 file_path = str(self.diskpool.pathFor(*pub_file_details))
 
@@ -264,10 +265,11 @@ class DeathRow:
             "Removing %s files marked for reaping" % len(condemned_files))
 
         for condemned_file in sorted(condemned_files, reverse=True):
-            file_name, source_name, component_name = details[condemned_file]
+            component_name, source_name, source_version, file_name = (
+                details[condemned_file])
             try:
                 bytes += self._removeFile(
-                    component_name, source_name, file_name)
+                    component_name, source_name, source_version, file_name)
             except NotInPool as info:
                 # It's safe for us to let this slide because it means that
                 # the file is already gone.
diff --git a/lib/lp/archivepublisher/diskpool.py b/lib/lp/archivepublisher/diskpool.py
index 620c728..01ce763 100644
--- a/lib/lp/archivepublisher/diskpool.py
+++ b/lib/lp/archivepublisher/diskpool.py
@@ -139,11 +139,13 @@ class DiskPoolEntry:
     require manual removal after further investigation.
     """
     def __init__(self, archive: IArchive, rootpath: Path, temppath: Path,
-                 source: str, filename: str, logger: logging.Logger) -> None:
+                 source_name: str, source_version: str, filename: str,
+                 logger: logging.Logger) -> None:
         self.archive = archive
         self.rootpath = rootpath
         self.temppath = temppath
-        self.source = source
+        self.source_name = source_name
+        self.source_version = source_version
         self.filename = filename
         self.logger = logger
 
@@ -165,7 +167,9 @@ class DiskPoolEntry:
 
     def pathFor(self, component: str) -> Path:
         """Return the path for this file in the given component."""
-        return self.rootpath / poolify(self.source, component) / self.filename
+        return (
+            self.rootpath / poolify(self.source_name, component) /
+            self.filename)
 
     def preferredComponent(self, add: Optional[str] = None,
                            remove: Optional[str] = None) -> Optional[str]:
@@ -230,7 +234,7 @@ class DiskPoolEntry:
         assert not targetpath.exists()
 
         self.debug("Making new file in %s for %s/%s" %
-                   (component, self.source, self.filename))
+                   (component, self.source_name, self.filename))
 
         file_to_write = _diskpool_atomicfile(
             targetpath, "wb", rootpath=self.temppath)
@@ -253,13 +257,13 @@ class DiskPoolEntry:
         if not self.file_component:
             raise NotInPool(
                 "File for removing %s %s/%s is not in pool, skipping." %
-                (component, self.source, self.filename))
+                (component, self.source_name, self.filename))
 
         # Okay, it's there, if it's a symlink then we need to remove
         # it simply.
         if component in self.symlink_components:
             self.debug("Removing %s %s/%s as it is a symlink"
-                       % (component, self.source, self.filename))
+                       % (component, self.source_name, self.filename))
             # ensure we are removing a symbolic link and
             # it is published in one or more components
             link_path = self.pathFor(component)
@@ -269,13 +273,13 @@ class DiskPoolEntry:
         if component != self.file_component:
             raise MissingSymlinkInPool(
                 "Symlink for %s/%s in %s is missing, skipping." %
-                (self.source, self.filename, component))
+                (self.source_name, self.filename, component))
 
         # It's not a symlink, this means we need to check whether we
         # have symlinks or not.
         if len(self.symlink_components) == 0:
             self.debug("Removing %s/%s from %s" %
-                       (self.source, self.filename, component))
+                       (self.source_name, self.filename, component))
         else:
             # The target for removal is the real file, and there are symlinks
             # pointing to it. In order to avoid breakage, we need to first
@@ -398,33 +402,34 @@ class DiskPool:
         self.entries = {}
         self.logger = logger
 
-    def _getEntry(self, sourcename: str, file: str) -> DiskPoolEntry:
-        """Return a new DiskPoolEntry for the given sourcename and file."""
+    def _getEntry(self, source_name: str, source_version: str,
+                  file: str) -> DiskPoolEntry:
+        """Return a new DiskPoolEntry for the given source and file."""
         return DiskPoolEntry(
-            self.archive, self.rootpath, self.temppath, sourcename,
-            file, self.logger)
+            self.archive, self.rootpath, self.temppath, source_name,
+            source_version, file, self.logger)
 
-    def pathFor(self, comp: str, source: str,
+    def pathFor(self, comp: str, source_name: str, source_version: str,
                 file: Optional[str] = None) -> Path:
         """Return the path for the given pool folder or file.
 
         If file is none, the path to the folder containing all packages
-        for the given component and source package name will be returned.
+        for the given component and source package will be returned.
 
         If file is specified, the path to the specific package file will
         be returned.
         """
-        path = self.rootpath / poolify(source, comp)
+        path = self.rootpath / poolify(source_name, comp)
         if file:
             path = path / file
         return path
 
-    def addFile(self, component: str, sourcename: str, filename: str,
-                pub_file: IPackageReleaseFile):
+    def addFile(self, component: str, source_name: str, source_version: str,
+                filename: str, pub_file: IPackageReleaseFile):
         """Add a file with the given contents to the pool.
 
-        Component, sourcename and filename are used to calculate the
-        on-disk location.
+        `component`, `source_name`, `source_version`, and `filename` are
+        used to calculate the on-disk location.
 
         pub_file is an `IPackageReleaseFile` providing the file's contents
         and SHA-1 hash.  The SHA-1 hash is used to compare the given file
@@ -450,10 +455,10 @@ class DiskPool:
         either as a file or a symlink, and the hash check passes,
         results.NONE will be returned and nothing will be done.
         """
-        entry = self._getEntry(sourcename, filename)
+        entry = self._getEntry(source_name, source_version, filename)
         return entry.addFile(component, pub_file)
 
-    def removeFile(self, component: str, sourcename: str,
+    def removeFile(self, component: str, source_name: str, source_version: str,
                    filename: str) -> int:
         """Remove the specified file from the pool.
 
@@ -469,5 +474,5 @@ class DiskPool:
         will be deleted, and the file will be moved to replace it. The
         size of the deleted symlink will be returned.
         """
-        entry = self._getEntry(sourcename, filename)
+        entry = self._getEntry(source_name, source_version, filename)
         return entry.removeFile(component)
diff --git a/lib/lp/archivepublisher/model/ftparchive.py b/lib/lp/archivepublisher/model/ftparchive.py
index 469b1cb..7e94441 100644
--- a/lib/lp/archivepublisher/model/ftparchive.py
+++ b/lib/lp/archivepublisher/model/ftparchive.py
@@ -693,8 +693,15 @@ class FTPArchiveHandler:
 
         def updateFileList(sourcepackagename, filename, component,
                            architecturetag=None):
+            # DiskPool.pathFor takes a source package version parameter.  We
+            # could fetch that in getSourceFiles/getBinaryFiles and pass it
+            # down here.  However, it adds another column to a query with an
+            # already large number of rows, and it's only needed for
+            # non-Debian-format archives, which by definition aren't
+            # involved here; so we just pass None as the version.
             ondiskname = str(
-                self._diskpool.pathFor(component, sourcepackagename, filename))
+                self._diskpool.pathFor(
+                    component, sourcepackagename, None, filename))
             if architecturetag is None:
                 architecturetag = "source"
             filelist[component][architecturetag].append(ondiskname)
diff --git a/lib/lp/archivepublisher/publishing.py b/lib/lp/archivepublisher/publishing.py
index 69fb7e3..b0ddd32 100644
--- a/lib/lp/archivepublisher/publishing.py
+++ b/lib/lp/archivepublisher/publishing.py
@@ -752,12 +752,13 @@ class Publisher:
         for path, properties in sorted(artifacts.items()):
             release_id = properties.get("launchpad.release-id")
             source_name = properties.get("launchpad.source-name")
-            if not release_id or not source_name:
+            source_version = properties.get("launchpad.source-version")
+            if not release_id or not source_name or not source_version:
                 # Skip any files that Launchpad didn't put in Artifactory.
                 continue
             self._diskpool.updateProperties(
-                source_name[0], path.name, pubs_by_id.get(release_id[0]),
-                old_properties=properties)
+                source_name[0], source_version[0], path.name,
+                pubs_by_id.get(release_id[0]), old_properties=properties)
 
     def D_writeReleaseFiles(self, is_careful):
         """Write out the Release files for the provided distribution.
diff --git a/lib/lp/archivepublisher/tests/deathrow.txt b/lib/lp/archivepublisher/tests/deathrow.txt
index 92b0752..5f8ac79 100644
--- a/lib/lp/archivepublisher/tests/deathrow.txt
+++ b/lib/lp/archivepublisher/tests/deathrow.txt
@@ -208,6 +208,7 @@ Publish files on disk and build a list of all created file paths
     ...             file_path = quiet_disk_pool.pathFor(
     ...                 pub.component.name,
     ...                 pub.source_package_name,
+    ...                 pub.source_package_version,
     ...                 pub_file.libraryfile.filename
     ...              )
     ...             unique_file_paths.add(file_path)
diff --git a/lib/lp/archivepublisher/tests/test_artifactory.py b/lib/lp/archivepublisher/tests/test_artifactory.py
index 49f7ace..241f277 100644
--- a/lib/lp/archivepublisher/tests/test_artifactory.py
+++ b/lib/lp/archivepublisher/tests/test_artifactory.py
@@ -62,7 +62,8 @@ class ArtifactoryPoolTestingFile(PoolTestingFile):
         return super().checkIsFile(None)
 
     def getProperties(self):
-        path = self.pool.pathFor(None, self.sourcename, self.filename)
+        path = self.pool.pathFor(
+            None, self.source_name, self.source_version, self.filename)
         return path.properties
 
 
@@ -81,8 +82,9 @@ class TestArtifactoryPool(TestCase):
 
     def test_addFile(self):
         foo = ArtifactoryPoolTestingFile(
-            self.pool, "foo", "foo-1.0.deb",
-            release_type=FakeReleaseType.BINARY, release_id=1)
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb", release_type=FakeReleaseType.BINARY,
+            release_id=1)
         self.assertFalse(foo.checkIsFile())
         result = foo.addToPool()
         self.assertEqual(self.pool.results.FILE_ADDED, result)
@@ -91,13 +93,15 @@ class TestArtifactoryPool(TestCase):
             {
                 "launchpad.release-id": ["binary:1"],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 },
             foo.getProperties())
 
     def test_addFile_exists_identical(self):
         foo = ArtifactoryPoolTestingFile(
-            self.pool, "foo", "foo-1.0.deb",
-            release_type=FakeReleaseType.BINARY, release_id=1)
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb", release_type=FakeReleaseType.BINARY,
+            release_id=1)
         foo.addToPool()
         self.assertTrue(foo.checkIsFile())
         result = foo.addToPool()
@@ -106,15 +110,18 @@ class TestArtifactoryPool(TestCase):
 
     def test_addFile_exists_overwrite(self):
         foo = ArtifactoryPoolTestingFile(
-            self.pool, "foo", "foo-1.0.deb",
-            release_type=FakeReleaseType.BINARY, release_id=1)
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb", release_type=FakeReleaseType.BINARY,
+            release_id=1)
         foo.addToPool()
         self.assertTrue(foo.checkIsFile())
         foo.contents = b"different"
         self.assertRaises(PoolFileOverwriteError, foo.addToPool)
 
     def test_removeFile(self):
-        foo = ArtifactoryPoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = ArtifactoryPoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         foo.addToPool()
         self.assertTrue(foo.checkIsFile())
         size = foo.removeFromPool()
@@ -145,23 +152,28 @@ class TestArtifactoryPool(TestCase):
         # with it.  This test mainly ensures that we transform the response
         # correctly.
         ArtifactoryPoolTestingFile(
-            self.pool, "foo", "foo-1.0.deb",
-            release_type=FakeReleaseType.BINARY, release_id=1).addToPool()
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb", release_type=FakeReleaseType.BINARY,
+            release_id=1).addToPool()
         ArtifactoryPoolTestingFile(
-            self.pool, "foo", "foo-1.1.deb",
-            release_type=FakeReleaseType.BINARY, release_id=2).addToPool()
+            pool=self.pool, source_name="foo", source_version="1.1",
+            filename="foo-1.1.deb", release_type=FakeReleaseType.BINARY,
+            release_id=2).addToPool()
         ArtifactoryPoolTestingFile(
-            self.pool, "bar", "bar-1.0.whl",
-            release_type=FakeReleaseType.BINARY, release_id=3).addToPool()
+            pool=self.pool, source_name="bar", source_version="1.0",
+            filename="bar-1.0.whl", release_type=FakeReleaseType.BINARY,
+            release_id=3).addToPool()
         self.assertEqual(
             {
                 PurePath("pool/f/foo/foo-1.0.deb"): {
                     "launchpad.release-id": ["binary:1"],
                     "launchpad.source-name": ["foo"],
+                    "launchpad.source-version": ["1.0"],
                     },
                 PurePath("pool/f/foo/foo-1.1.deb"): {
                     "launchpad.release-id": ["binary:2"],
                     "launchpad.source-name": ["foo"],
+                    "launchpad.source-version": ["1.1"],
                     },
                 },
             self.pool.getAllArtifacts(
@@ -171,6 +183,7 @@ class TestArtifactoryPool(TestCase):
                 PurePath("pool/b/bar/bar-1.0.whl"): {
                     "launchpad.release-id": ["binary:3"],
                     "launchpad.source-name": ["bar"],
+                    "launchpad.source-version": ["1.0"],
                     },
                 },
             self.pool.getAllArtifacts(
@@ -199,7 +212,7 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
         spph = self.factory.makeSourcePackagePublishingHistory(
             archive=self.archive, distroseries=dses[0],
             pocket=PackagePublishingPocket.RELEASE, component="main",
-            sourcepackagename="foo")
+            sourcepackagename="foo", version="1.0")
         spr = spph.sourcepackagerelease
         sprf = self.factory.makeSourcePackageReleaseFile(
             sourcepackagerelease=spr,
@@ -210,7 +223,8 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
         spphs.append(spph.copyTo(
             dses[1], PackagePublishingPocket.RELEASE, self.archive))
         transaction.commit()
-        self.pool.addFile(None, spr.name, sprf.libraryfile.filename, sprf)
+        self.pool.addFile(
+            None, spr.name, spr.version, sprf.libraryfile.filename, sprf)
         path = self.pool.rootpath / "f" / "foo" / "foo_1.0.dsc"
         self.assertTrue(path.exists())
         self.assertFalse(path.is_symlink())
@@ -218,13 +232,16 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             {
                 "launchpad.release-id": ["source:%d" % spr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 },
             path.properties)
-        self.pool.updateProperties(spr.name, sprf.libraryfile.filename, spphs)
+        self.pool.updateProperties(
+            spr.name, spr.version, sprf.libraryfile.filename, spphs)
         self.assertEqual(
             {
                 "launchpad.release-id": ["source:%d" % spr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 "deb.distribution": list(sorted(ds.name for ds in dses)),
                 "deb.component": ["main"],
                 },
@@ -240,10 +257,12 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             self.factory.makeDistroArchSeries(
                 distroseries=ds, architecturetag=processor.name)
             for ds in dses]
+        spr = self.factory.makeSourcePackageRelease(
+            archive=self.archive, sourcepackagename="foo", version="1.0")
         bpph = self.factory.makeBinaryPackagePublishingHistory(
             archive=self.archive, distroarchseries=dases[0],
             pocket=PackagePublishingPocket.RELEASE, component="main",
-            sourcepackagename="foo", binarypackagename="foo",
+            source_package_release=spr, binarypackagename="foo",
             architecturespecific=True)
         bpr = bpph.binarypackagerelease
         bpf = self.factory.makeBinaryPackageFile(
@@ -256,7 +275,8 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             dses[1], PackagePublishingPocket.RELEASE, self.archive)[0])
         transaction.commit()
         self.pool.addFile(
-            None, bpr.sourcepackagename, bpf.libraryfile.filename, bpf)
+            None, bpr.sourcepackagename, bpr.sourcepackageversion,
+            bpf.libraryfile.filename, bpf)
         path = (
             self.pool.rootpath / "f" / "foo" /
             ("foo_1.0_%s.deb" % processor.name))
@@ -266,14 +286,17 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             {
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 },
             path.properties)
         self.pool.updateProperties(
-            bpr.sourcepackagename, bpf.libraryfile.filename, bpphs)
+            bpr.sourcepackagename, bpr.sourcepackageversion,
+            bpf.libraryfile.filename, bpphs)
         self.assertEqual(
             {
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 "deb.distribution": list(sorted(ds.name for ds in dses)),
                 "deb.component": ["main"],
                 "deb.architecture": [processor.name],
@@ -286,9 +309,11 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
         dases = [
             self.factory.makeDistroArchSeries(distroseries=ds)
             for _ in range(2)]
+        spr = self.factory.makeSourcePackageRelease(
+            archive=self.archive, sourcepackagename="foo", version="1.0")
         bpb = self.factory.makeBinaryPackageBuild(
-            archive=self.archive, distroarchseries=dases[0],
-            pocket=PackagePublishingPocket.RELEASE, sourcepackagename="foo")
+            archive=self.archive, source_package_release=spr,
+            distroarchseries=dases[0], pocket=PackagePublishingPocket.RELEASE)
         bpr = self.factory.makeBinaryPackageRelease(
             binarypackagename="foo", build=bpb, component="main",
             architecturespecific=False)
@@ -302,7 +327,8 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             {bpr: (bpr.component, bpr.section, bpr.priority, None)})
         transaction.commit()
         self.pool.addFile(
-            None, bpr.sourcepackagename, bpf.libraryfile.filename, bpf)
+            None, bpr.sourcepackagename, bpr.sourcepackageversion,
+            bpf.libraryfile.filename, bpf)
         path = self.pool.rootpath / "f" / "foo" / "foo_1.0_all.deb"
         self.assertTrue(path.exists())
         self.assertFalse(path.is_symlink())
@@ -310,14 +336,17 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             {
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 },
             path.properties)
         self.pool.updateProperties(
-            bpr.sourcepackagename, bpf.libraryfile.filename, bpphs)
+            bpr.sourcepackagename, bpr.sourcepackageversion,
+            bpf.libraryfile.filename, bpphs)
         self.assertEqual(
             {
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 "deb.distribution": [ds.name],
                 "deb.component": ["main"],
                 "deb.architecture": list(sorted(
@@ -331,9 +360,11 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
         ds = self.factory.makeDistroSeries(
             distribution=self.archive.distribution)
         das = self.factory.makeDistroArchSeries(distroseries=ds)
+        spr = self.factory.makeSourcePackageRelease(
+            archive=self.archive, sourcepackagename="foo", version="1.0")
         bpb = self.factory.makeBinaryPackageBuild(
-            archive=self.archive, distroarchseries=das,
-            pocket=PackagePublishingPocket.RELEASE, sourcepackagename="foo")
+            archive=self.archive, source_package_release=spr,
+            distroarchseries=das, pocket=PackagePublishingPocket.RELEASE)
         bpr = self.factory.makeBinaryPackageRelease(
             binarypackagename="foo", build=bpb, component="main",
             architecturespecific=False)
@@ -347,22 +378,26 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             {bpr: (bpr.component, bpr.section, bpr.priority, None)})
         transaction.commit()
         self.pool.addFile(
-            None, bpr.sourcepackagename, bpf.libraryfile.filename, bpf)
+            None, bpr.sourcepackagename, bpr.sourcepackageversion,
+            bpf.libraryfile.filename, bpf)
         path = self.pool.rootpath / "f" / "foo" / "foo_1.0_all.deb"
         path.set_properties({"deb.version": ["1.0"]}, recursive=False)
         self.assertEqual(
             {
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 "deb.version": ["1.0"],
                 },
             path.properties)
         self.pool.updateProperties(
-            bpr.sourcepackagename, bpf.libraryfile.filename, bpphs)
+            bpr.sourcepackagename, bpr.sourcepackageversion,
+            bpf.libraryfile.filename, bpphs)
         self.assertEqual(
             {
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
+                "launchpad.source-version": ["1.0"],
                 "deb.distribution": [ds.name],
                 "deb.component": ["main"],
                 "deb.architecture": [das.architecturetag],
diff --git a/lib/lp/archivepublisher/tests/test_deathrow.py b/lib/lp/archivepublisher/tests/test_deathrow.py
index 10efb6f..9fd62e5 100644
--- a/lib/lp/archivepublisher/tests/test_deathrow.py
+++ b/lib/lp/archivepublisher/tests/test_deathrow.py
@@ -53,6 +53,7 @@ class TestDeathRow(TestCase):
         return diskpool.pathFor(
             pub.component.name,
             pub.source_package_name,
+            pub.source_package_version,
             pub_file.libraryfile.filename)
 
     def assertIsFile(self, path: Path) -> None:
diff --git a/lib/lp/archivepublisher/tests/test_ftparchive.py b/lib/lp/archivepublisher/tests/test_ftparchive.py
index 9ea5778..9b407a7 100755
--- a/lib/lp/archivepublisher/tests/test_ftparchive.py
+++ b/lib/lp/archivepublisher/tests/test_ftparchive.py
@@ -147,10 +147,11 @@ class TestFTPArchive(TestCaseWithFactory):
         with open_func(path) as result_file:
             self.assertEqual(b"", result_file.read())
 
-    def _addRepositoryFile(self, component, sourcename, leafname,
-                           samplename=None):
+    def _addRepositoryFile(self, component, source_name, source_version,
+                           leafname, samplename=None):
         """Create a repository file."""
-        fullpath = self._dp.pathFor(component, sourcename, leafname)
+        fullpath = self._dp.pathFor(
+            component, source_name, source_version, leafname)
         fullpath.parent.mkdir(parents=True, exist_ok=True)
         if samplename is None:
             samplename = leafname
@@ -515,9 +516,9 @@ class TestFTPArchive(TestCaseWithFactory):
         self._publishDefaultFileLists(fa, 'main')
 
         # Add mentioned files in the repository pool/.
-        self._addRepositoryFile('main', 'tiny', 'tiny_0.1.dsc')
-        self._addRepositoryFile('main', 'tiny', 'tiny_0.1.tar.gz')
-        self._addRepositoryFile('main', 'tiny', 'tiny_0.1_i386.deb')
+        self._addRepositoryFile('main', 'tiny', '0.1', 'tiny_0.1.dsc')
+        self._addRepositoryFile('main', 'tiny', '0.1', 'tiny_0.1.tar.gz')
+        self._addRepositoryFile('main', 'tiny', '0.1', 'tiny_0.1_i386.deb')
 
         # When include_long_descriptions is set, apt.conf has
         # LongDescription "true" for that series.
@@ -649,9 +650,9 @@ class TestFTPArchive(TestCaseWithFactory):
         fa.createEmptyPocketRequests(fullpublish=True)
         self._publishDefaultOverrides(fa, "main")
         self._publishDefaultFileLists(fa, "main")
-        self._addRepositoryFile("main", "tiny", "tiny_0.1.dsc")
-        self._addRepositoryFile("main", "tiny", "tiny_0.1.tar.gz")
-        self._addRepositoryFile("main", "tiny", "tiny_0.1_i386.deb")
+        self._addRepositoryFile("main", "tiny", "0.1", "tiny_0.1.dsc")
+        self._addRepositoryFile("main", "tiny", "0.1", "tiny_0.1.tar.gz")
+        self._addRepositoryFile("main", "tiny", "0.1", "tiny_0.1_i386.deb")
         comp_dir = os.path.join(self._distsdir, "hoary-test", "main")
         os.makedirs(os.path.join(comp_dir, "signed"))
         with open(os.path.join(comp_dir, "signed", "stuff"), "w"):
@@ -778,10 +779,10 @@ class TestFTPArchive(TestCaseWithFactory):
         fa.publishFileLists(
             self._distribution["hoary-test"], PackagePublishingPocket.RELEASE,
             source_files, binary_files)
-        self._addRepositoryFile("main", "tiny", "tiny_0.1.dsc")
+        self._addRepositoryFile("main", "tiny", "0.1", "tiny_0.1.dsc")
         for i in range(50):
             self._addRepositoryFile(
-                "main", "bin%d" % i, "bin%d_1_i386.deb" % i,
+                "main", "bin%d" % i, "1", "bin%d_1_i386.deb" % i,
                 samplename="tiny_0.1_i386.deb")
         apt_conf = fa.generateConfig(fullpublish=True)
         fa.runApt(apt_conf)
@@ -790,7 +791,7 @@ class TestFTPArchive(TestCaseWithFactory):
         # something to do.
         for i in range(49):
             self._dp.pathFor(
-                "main", "bin%d" % i, "bin%d_1_i386.deb" % i).unlink()
+                "main", "bin%d" % i, "1", "bin%d_1_i386.deb" % i).unlink()
 
         cache_path = os.path.join(self._config.cacheroot, "packages-i386.db")
         old_cache_size = os.stat(cache_path).st_size
diff --git a/lib/lp/archivepublisher/tests/test_pool.py b/lib/lp/archivepublisher/tests/test_pool.py
index 4beeea5..97669ff 100644
--- a/lib/lp/archivepublisher/tests/test_pool.py
+++ b/lib/lp/archivepublisher/tests/test_pool.py
@@ -83,30 +83,34 @@ class FakePackageReleaseFile:
 
 class PoolTestingFile:
 
-    def __init__(self, pool, sourcename, filename,
+    def __init__(self, pool, source_name, source_version, filename,
                  release_type=FakeReleaseType.BINARY, release_id=1):
         self.pool = pool
-        self.sourcename = sourcename
+        self.source_name = source_name
+        self.source_version = source_version
         self.filename = filename
-        self.contents = sourcename.encode("UTF-8")
+        self.contents = source_name.encode("UTF-8")
         self.release_type = release_type
         self.release_id = release_id
 
     def addToPool(self, component: str):
         return self.pool.addFile(
-            component, self.sourcename, self.filename,
+            component, self.source_name, self.source_version, self.filename,
             FakePackageReleaseFile(
                 self.contents, self.release_type, self.release_id))
 
     def removeFromPool(self, component: str) -> int:
-        return self.pool.removeFile(component, self.sourcename, self.filename)
+        return self.pool.removeFile(
+            component, self.source_name, self.source_version, self.filename)
 
     def checkExists(self, component: str) -> bool:
-        path = self.pool.pathFor(component, self.sourcename, self.filename)
+        path = self.pool.pathFor(
+            component, self.source_name, self.source_version, self.filename)
         return path.exists()
 
     def checkIsLink(self, component: str) -> bool:
-        path = self.pool.pathFor(component, self.sourcename, self.filename)
+        path = self.pool.pathFor(
+            component, self.source_name, self.source_version, self.filename)
         return path.is_symlink()
 
     def checkIsFile(self, component: str) -> bool:
@@ -140,14 +144,18 @@ class TestPool(unittest.TestCase):
 
     def testSimpleAdd(self):
         """Adding a new file should work."""
-        foo = PoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = PoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         result = foo.addToPool("main")
         self.assertEqual(self.pool.results.FILE_ADDED, result)
         self.assertTrue(foo.checkIsFile("main"))
 
     def testSimpleSymlink(self):
         """Adding a file twice should result in a symlink."""
-        foo = PoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = PoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         foo.addToPool("main")
         result = foo.addToPool("universe")
         self.assertEqual(self.pool.results.SYMLINK_ADDED, result)
@@ -156,7 +164,9 @@ class TestPool(unittest.TestCase):
 
     def testSymlinkShuffleOnAdd(self):
         """If the second add is a more preferred component, links shuffle."""
-        foo = PoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = PoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         foo.addToPool("universe")
         result = foo.addToPool("main")
         self.assertEqual(self.pool.results.SYMLINK_ADDED, result)
@@ -165,7 +175,9 @@ class TestPool(unittest.TestCase):
 
     def testRemoveSymlink(self):
         """Remove file should just remove a symlink"""
-        foo = PoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = PoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         foo.addToPool("main")
         foo.addToPool("universe")
 
@@ -175,7 +187,9 @@ class TestPool(unittest.TestCase):
 
     def testRemoveLoneFile(self):
         """Removing a file with no symlinks removes it."""
-        foo = PoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = PoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         foo.addToPool("main")
 
         size = foo.removeFromPool("main")
@@ -184,7 +198,9 @@ class TestPool(unittest.TestCase):
 
     def testSymlinkShuffleOnRemove(self):
         """Removing a file with a symlink shuffles links."""
-        foo = PoolTestingFile(self.pool, "foo", "foo-1.0.deb")
+        foo = PoolTestingFile(
+            pool=self.pool, source_name="foo", source_version="1.0",
+            filename="foo-1.0.deb")
         foo.addToPool("universe")
         foo.addToPool("main")
 
diff --git a/lib/lp/archivepublisher/tests/test_publisher.py b/lib/lp/archivepublisher/tests/test_publisher.py
index f247242..a2cbb54 100644
--- a/lib/lp/archivepublisher/tests/test_publisher.py
+++ b/lib/lp/archivepublisher/tests/test_publisher.py
@@ -3607,6 +3607,7 @@ class TestArtifactoryPublishing(TestPublisherBase):
                 "launchpad.release-id":
                     ["source:%d" % source.sourcepackagereleaseID],
                 "launchpad.source-name": ["hello"],
+                "launchpad.source-version": ["1.0"],
                 },
             source_path.properties)
         binary_path = (
@@ -3620,6 +3621,7 @@ class TestArtifactoryPublishing(TestPublisherBase):
                 "launchpad.release-id":
                     ["binary:%d" % binary.binarypackagereleaseID],
                 "launchpad.source-name": ["hello"],
+                "launchpad.source-version": ["1.0"],
                 },
             binary_path.properties)
 
@@ -3669,6 +3671,7 @@ class TestArtifactoryPublishing(TestPublisherBase):
                 "launchpad.release-id":
                     ["source:%d" % source.sourcepackagereleaseID],
                 "launchpad.source-name": ["hello"],
+                "launchpad.source-version": ["1.0"],
                 },
             source_path.properties)
         self.assertEqual(
@@ -3679,6 +3682,7 @@ class TestArtifactoryPublishing(TestPublisherBase):
                 "launchpad.release-id":
                     ["binary:%d" % binary.binarypackagereleaseID],
                 "launchpad.source-name": ["hello"],
+                "launchpad.source-version": ["1.0"],
                 },
             binary_path.properties)
 
@@ -3722,6 +3726,7 @@ class TestArtifactoryPublishing(TestPublisherBase):
                 "launchpad.release-id":
                     ["source:%d" % source.sourcepackagereleaseID],
                 "launchpad.source-name": ["hello"],
+                "launchpad.source-version": ["1.0"],
                 },
             source_path.properties)
         self.assertEqual(
@@ -3729,6 +3734,7 @@ class TestArtifactoryPublishing(TestPublisherBase):
                 "launchpad.release-id":
                     ["binary:%d" % binary.binarypackagereleaseID],
                 "launchpad.source-name": ["hello"],
+                "launchpad.source-version": ["1.0"],
                 },
             binary_path.properties)
 
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index b229c47..b17194e 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -178,14 +178,16 @@ class ArchivePublisherBase:
         """See `IPublishing`"""
         try:
             for pub_file in self.files:
-                source = self.source_package_name
+                source_name = self.source_package_name
+                source_version = self.source_package_version
                 component = (
                     None if self.component is None else self.component.name)
                 filename = pub_file.libraryfile.filename
-                path = diskpool.pathFor(component, source, filename)
+                path = diskpool.pathFor(
+                    component, source_name, source_version, filename)
 
                 action = diskpool.addFile(
-                    component, source, filename, pub_file)
+                    component, source_name, source_version, filename, pub_file)
                 if action == diskpool.results.FILE_ADDED:
                     log.debug("Added %s from library" % path)
                 elif action == diskpool.results.SYMLINK_ADDED: