launchpad-reviewers team mailing list archive

[William Grant <mp+459249@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging ~wgrant/launchpad:buildinfo-exposure into launchpad:master.

Commit message:
Expose .buildinfo files in the API and web UI

We've been capturing these for BinaryPackageBuilds for years, and
storing them in the DB and librarian, but they haven't been available to
users.

LP: #1686242

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1686242 in Launchpad itself: ".changes files reference .buildinfo files that aren't exposed"
  https://bugs.launchpad.net/launchpad/+bug/1686242

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/+git/launchpad/+merge/459249
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~wgrant/launchpad:buildinfo-exposure into launchpad:master.

diff --git a/lib/lp/soyuz/doc/build-files.rst b/lib/lp/soyuz/doc/build-files.rst
index ddca43d..2ba7f52 100644
--- a/lib/lp/soyuz/doc/build-files.rst
+++ b/lib/lp/soyuz/doc/build-files.rst
@@ -28,6 +28,7 @@ following file type in its context.
  * Binary changesfile: '.changes';
  * Build logs: '.txt.gz';
  * Build upload logs: '_log.txt';
+ * .buildinfo file: '.buildinfo';
  * Built files: '*deb';
 
     >>> print(build.title)
@@ -70,6 +71,14 @@ Adding and retrieving a upload_log.
     >>> build.upload_log == build.getFileByName(upload_log_name)
     True
 
+Adding and retrieving a .buildinfo.
+
+    >>> build.addBuildInfo(
+    ...     factory.makeLibraryFileAlias(filename="foo.buildinfo")
+    ... )
+    >>> build.buildinfo == build.getFileByName("foo.buildinfo")
+    True
+
 Retrieve a built file:
 
     >>> deb == build.getFileByName("test_1.0_all.deb")
diff --git a/lib/lp/soyuz/interfaces/binarypackagebuild.py b/lib/lp/soyuz/interfaces/binarypackagebuild.py
index 18bc50f..e72b450 100644
--- a/lib/lp/soyuz/interfaces/binarypackagebuild.py
+++ b/lib/lp/soyuz/interfaces/binarypackagebuild.py
@@ -139,8 +139,9 @@ class IBinaryPackageBuildView(IPackageBuildView):
 
     changesfile_url = exported(
         TextLine(
-            title=_("Changes File URL"),
+            title=_("Changes file URL"),
             required=False,
+            readonly=True,
             description=_(
                 "The URL for the changes file for this build. "
                 "Will be None if the build was imported by Gina."
@@ -153,6 +154,15 @@ class IBinaryPackageBuildView(IPackageBuildView):
         "this build, if any."
     )
 
+    buildinfo_url = exported(
+        TextLine(
+            title=_("buildinfo file URL"),
+            required=False,
+            readonly=True,
+            description=_("The URL for the .buildinfo file for this build."),
+        )
+    )
+
     package_upload = Attribute(
         "The `PackageUpload` record corresponding to the original upload "
         "of the binaries resulted from this build. It's 'None' if it is "
diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py
index a801a6a..4ac614c 100644
--- a/lib/lp/soyuz/model/binarypackagebuild.py
+++ b/lib/lp/soyuz/model/binarypackagebuild.py
@@ -408,6 +408,14 @@ class BinaryPackageBuild(PackageBuildMixin, StormBase):
         return ProxiedLibraryFileAlias(self.upload_log, self).http_url
 
     @property
+    def buildinfo_url(self):
+        """See `IBinaryPackageBuild`."""
+        buildinfo = self.buildinfo
+        if buildinfo is None:
+            return None
+        return ProxiedLibraryFileAlias(buildinfo, self).http_url
+
+    @property
     def distributionsourcepackagerelease(self):
         """See `IBuild`."""
         from lp.soyuz.model.distributionsourcepackagerelease import (
@@ -832,6 +840,8 @@ class BinaryPackageBuild(PackageBuildMixin, StormBase):
         """See `IBuild`."""
         if filename.endswith(".changes"):
             file_object = self.upload_changesfile
+        elif filename.endswith(".buildinfo"):
+            file_object = self.buildinfo
         elif filename.endswith(".txt.gz"):
             file_object = self.log
         elif is_upload_log(filename):
diff --git a/lib/lp/soyuz/stories/soyuz/xx-build-record.rst b/lib/lp/soyuz/stories/soyuz/xx-build-record.rst
index 9da0cb4..593e1c7 100644
--- a/lib/lp/soyuz/stories/soyuz/xx-build-record.rst
+++ b/lib/lp/soyuz/stories/soyuz/xx-build-record.rst
@@ -310,6 +310,7 @@ appropriate 'Build status' section the user will see 2 new sections,
     >>> build.buildqueue_record.destroySelf()
     >>> build.updateStatus(BuildStatus.FULLYBUILT, builder=bob_builder)
     >>> build.setLog(stp.addMockFile("fake-buildlog"))
+    >>> build.addBuildInfo(stp.addMockFile("testing_1.0_all.buildinfo"))
     >>> binaries = stp.uploadBinaryForBuild(build, "testing-bin")
     >>> upload = stp.distroseries.createQueueEntry(
     ...     PackagePublishingPocket.RELEASE,
@@ -329,10 +330,14 @@ appropriate 'Build status' section the user will see 2 new sections,
     Finished on 2008-01-01 (took 5 minutes, 0.0 seconds)
     buildlog (7 bytes)
     testing_1.0_all.changes (15 bytes)
+    testing_1.0_all.buildinfo (7 bytes)
 
     >>> print(anon_browser.getLink("testing_1.0_all.changes").url)
     http://.../+build/.../+files/testing_1.0_all.changes
 
+    >>> print(anon_browser.getLink("testing_1.0_all.buildinfo").url)
+    http://.../+build/.../+files/testing_1.0_all.buildinfo
+
     >>> print(extract_text(find_tag_by_id(anon_browser.contents, "binaries")))
     Binary packages
     Binary packages awaiting approval in NEW queue:
diff --git a/lib/lp/soyuz/stories/webservice/xx-builds.rst b/lib/lp/soyuz/stories/webservice/xx-builds.rst
index 203e503..93a399a 100644
--- a/lib/lp/soyuz/stories/webservice/xx-builds.rst
+++ b/lib/lp/soyuz/stories/webservice/xx-builds.rst
@@ -53,6 +53,7 @@ of properties:
     arch_tag: 'i386'
     archive_link: 'http://.../beta/~cprov/+archive/ubuntu/ppa'
     builder_link: 'http://.../beta/builders/bob'
+    buildinfo_url: None
     can_be_cancelled: False
     can_be_rescored: False
     can_be_retried: True
@@ -89,6 +90,7 @@ Whereas the 1.0 webservice for builds maintains the old property names
     build_log_url:
     'http://.../~cprov/+archive/ubuntu/ppa/+build/26/+files/netapplet-1.0.0.tar.gz'
     builder_link: 'http://.../builders/bob'
+    buildinfo_url: None
     buildstate: 'Failed to build'
     can_be_cancelled: False
     can_be_rescored: False
@@ -122,6 +124,7 @@ devel webservice also contains build date_started and duration.
     build_log_url:
     'http://.../~cprov/+archive/ubuntu/ppa/+build/26/+files/netapplet-1.0.0.tar.gz'
     builder_link: 'http://.../builders/bob'
+    buildinfo_url: None
     buildstate: 'Failed to build'
     can_be_cancelled: False
     can_be_rescored: False
diff --git a/lib/lp/soyuz/templates/build-index.pt b/lib/lp/soyuz/templates/build-index.pt
index e00428b..9883dfc 100644
--- a/lib/lp/soyuz/templates/build-index.pt
+++ b/lib/lp/soyuz/templates/build-index.pt
@@ -197,6 +197,13 @@
            tal:content="changesfile/filename">CHANGESFILE</a>
         (<span tal:replace="changesfile/content/filesize/fmt:bytes" />)
       </li>
+      <li tal:define="buildinfo context/buildinfo;"
+          tal:condition="buildinfo">
+        <a class="sprite download"
+           tal:attributes="href context/buildinfo_url"
+           tal:content="buildinfo/filename">BUILDINFO</a>
+        (<span tal:replace="buildinfo/content/filesize/fmt:bytes" />)
+      </li>
     </ul>
   </metal:macro>
 
diff --git a/lib/lp/soyuz/tests/test_build.py b/lib/lp/soyuz/tests/test_build.py
index 8ee97c4..e14f26d 100644
--- a/lib/lp/soyuz/tests/test_build.py
+++ b/lib/lp/soyuz/tests/test_build.py
@@ -277,6 +277,35 @@ class TestBuild(TestCaseWithFactory):
         expected_url = "%s/%s" % (url_start, expected_filename)
         self.assertEqual(expected_url, build.upload_log_url)
 
+    def test_buildinfo(self):
+        # The .buildinfo file can be attached to a build
+        spph = self.publisher.getPubSource(
+            sourcename=self.factory.getUniqueString(),
+            version="%s.1" % self.factory.getUniqueInteger(),
+            distroseries=self.distroseries,
+        )
+        [build] = spph.createMissingBuilds()
+
+        self.assertIsNone(build.buildinfo)
+        self.assertIsNone(build.buildinfo_url)
+
+        buildinfo = self.factory.makeLibraryFileAlias()
+        with person_logged_in(self.admin):
+            build.addBuildInfo(buildinfo)
+        self.assertEqual(buildinfo, build.buildinfo)
+
+        url_start = (
+            "http://launchpad.test/%s/+source/%s/%s/+build/%s/+files";
+            % (
+                self.distroseries.distribution.name,
+                spph.source_package_name,
+                spph.source_package_version,
+                build.id,
+            )
+        )
+        expected_url = "%s/%s" % (url_start, buildinfo.filename)
+        self.assertEqual(expected_url, build.buildinfo_url)
+
     def test_retry_resets_state(self):
         # Retrying a build resets most of the state attributes, but does
         # not modify the first dispatch time.