launchpad-reviewers team mailing list archive

[Colin Watson <mp+424872@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:soss-metadata into launchpad:master with ~cjwatson/launchpad:conda-archive-layout as a prerequisite.

Commit message:
Add commit information when publishing CI builds to Artifactory

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/424872

We publish this via the agreed `soss.source_url` and `soss.commit_id` properties.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:soss-metadata into launchpad:master.

diff --git a/lib/lp/archivepublisher/artifactory.py b/lib/lp/archivepublisher/artifactory.py
index 740519b..8031ef3 100644
--- a/lib/lp/archivepublisher/artifactory.py
+++ b/lib/lp/archivepublisher/artifactory.py
@@ -185,6 +185,17 @@ class ArtifactoryPoolEntry:
                     "%s:%s" % (
                         pub.distroseries.getSuite(pub.pocket), pub.channel)
                     for pub in publications})
+        # Additional metadata per
+        # https://docs.google.com/spreadsheets/d/15Xkdi-CRu2NiQfLoclP5PKW63Zw6syiuao8VJG7zxvw
+        # (private).
+        if IBinaryPackageFile.providedBy(self.pub_file):
+            ci_build = self.pub_file.binarypackagerelease.ci_build
+            if ci_build is not None:
+                properties.update({
+                    "soss.source_url": (
+                        ci_build.git_repository.getCodebrowseUrl()),
+                    "soss.commit_id": ci_build.commit_sha1,
+                    })
         return properties
 
     def addFile(self):
diff --git a/lib/lp/archivepublisher/tests/artifactory_fixture.py b/lib/lp/archivepublisher/tests/artifactory_fixture.py
index ba3099f..599836b 100644
--- a/lib/lp/archivepublisher/tests/artifactory_fixture.py
+++ b/lib/lp/archivepublisher/tests/artifactory_fixture.py
@@ -117,12 +117,20 @@ class FakeArtifactoryFixture(Fixture):
 
     def _handle_upload(self, request):
         """Handle a request to upload a directory or file."""
-        parsed_url = urlparse(request.url[len(self.repo_url):])
+        # Artifactory and urlparse seem to disagree about how to parse URLs
+        # where parameters contain the "/" character, so split the
+        # parameters from the rest of the URL by hand.
+        url = request.url[len(self.repo_url):]
+        if ";" in url:
+            url, params = url.split(";", 1)
+        else:
+            params = ""
+        parsed_url = urlparse(url)
         path = parsed_url.path
         if path.endswith("/"):
             self.add_dir(path.rstrip("/"))
         elif path.rsplit("/", 1)[0] in self._fs:
-            properties = self._decode_properties(parsed_url.params)
+            properties = self._decode_properties(params)
             self.add_file(
                 path, request.body,
                 int(request.headers["Content-Length"]), properties)
diff --git a/lib/lp/archivepublisher/tests/test_artifactory.py b/lib/lp/archivepublisher/tests/test_artifactory.py
index 16c2fe6..fea99fb 100644
--- a/lib/lp/archivepublisher/tests/test_artifactory.py
+++ b/lib/lp/archivepublisher/tests/test_artifactory.py
@@ -569,6 +569,9 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
                 "launchpad.source-version": ["1.0"],
+                "soss.source_url": [
+                    ci_build.git_repository.getCodebrowseUrl()],
+                "soss.commit_id": [ci_build.commit_sha1],
                 },
             path.properties)
         pool.updateProperties(bpph.pool_name, bpph.pool_version, bpf, bpphs)
@@ -579,6 +582,9 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
                 "launchpad.source-version": ["1.0"],
                 "launchpad.channel": list(
                     sorted("%s:edge" % ds.name for ds in dses)),
+                "soss.source_url": [
+                    ci_build.git_repository.getCodebrowseUrl()],
+                "soss.commit_id": [ci_build.commit_sha1],
                 },
             path.properties)
 
@@ -623,6 +629,9 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
                 "launchpad.release-id": ["binary:%d" % bpr.id],
                 "launchpad.source-name": ["foo"],
                 "launchpad.source-version": ["1.0"],
+                "soss.source_url": [
+                    ci_build.git_repository.getCodebrowseUrl()],
+                "soss.commit_id": [ci_build.commit_sha1],
                 },
             path.properties)
         pool.updateProperties(bpph.pool_name, bpph.pool_version, bpf, bpphs)
@@ -633,6 +642,9 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
                 "launchpad.source-version": ["1.0"],
                 "launchpad.channel": list(
                     sorted("%s:edge" % ds.name for ds in dses)),
+                "soss.source_url": [
+                    ci_build.git_repository.getCodebrowseUrl()],
+                "soss.commit_id": [ci_build.commit_sha1],
                 },
             path.properties)
 
diff --git a/lib/lp/archivepublisher/tests/test_pool.py b/lib/lp/archivepublisher/tests/test_pool.py
index 7ede0f4..5626b43 100644
--- a/lib/lp/archivepublisher/tests/test_pool.py
+++ b/lib/lp/archivepublisher/tests/test_pool.py
@@ -68,9 +68,10 @@ class FakeLibraryFileAlias:
 
 class FakePackageRelease:
 
-    def __init__(self, release_id, user_defined_fields=None):
+    def __init__(self, release_id, user_defined_fields=None, ci_build=None):
         self.id = release_id
         self.user_defined_fields = user_defined_fields
+        self.ci_build = ci_build
 
 
 class FakeReleaseType(EnumeratedType):
@@ -83,12 +84,13 @@ class FakeReleaseType(EnumeratedType):
 class FakePackageReleaseFile:
 
     def __init__(self, contents, filename, release_type=FakeReleaseType.BINARY,
-                 release_id=1, user_defined_fields=None):
+                 release_id=1, user_defined_fields=None, ci_build=None):
         self.libraryfile = FakeLibraryFileAlias(contents, filename)
         if release_type == FakeReleaseType.SOURCE:
             self.sourcepackagereleaseID = release_id
             self.sourcepackagerelease = FakePackageRelease(
-                release_id, user_defined_fields=user_defined_fields)
+                release_id, user_defined_fields=user_defined_fields,
+                ci_build=ci_build)
             alsoProvides(self, ISourcePackageReleaseFile)
         elif release_type == FakeReleaseType.BINARY:
             self.binarypackagereleaseID = release_id