fix(reporters): restore old behavior for Gerrit reporting

This is an attempt to restore the old formatting, e.g. with failed
checks and a link to the URI.

At the same time, this attempts to fix the eager +1 Verified tag which
is sent when nix-eval is started (?) and not done or when the evaluation
is done instead of the whole nix-eval job seen as completed.

Signed-off-by: Raito Bezarius <raito@lix.systems>

buildbot_nix/__init__.py

@@ -19,7 +19,7 @@ from buildbot.www.auth import AuthBase
 from buildbot.www.oauth2 import OAuth2Auth
 from buildbot.changes.gerritchangesource import GerritChangeSource
 from buildbot.reporters.utils import getURLForBuildrequest
-from buildbot.reporters.generators.build import BuildStatusGenerator
+from buildbot.reporters.generators.build import BuildEndStatusGenerator
 from buildbot.reporters.message import MessageFormatterFunction
 from buildbot.process.buildstep import EXCEPTION
 from buildbot.process.buildstep import SUCCESS
@@ -33,6 +33,7 @@ from twisted.internet import defer
 from twisted.logger import Logger
 
 from .binary_cache import S3BinaryCacheConfig
+from .message_formatter import ReasonableMessageFormatter, CallbackPayloadBuild, CallbackPayloadBuildSet, CallbackReturn
 
 log = Logger()
 
@@ -838,11 +839,10 @@ class PeriodicWithStartup(schedulers.Periodic):
             yield self.setState("last_build", None)
         yield super().activate()
 
-def gerritReviewFmt(url, data):
-    if 'build' not in data:
-        raise ValueError('`build` is supposed to be present to format a build')
+def gerritReviewFmt(url: str, payload: CallbackPayloadBuild | CallbackPayloadBuildSet) -> CallbackReturn:
+    assert isinstance(payload, CallbackPayloadBuild), "BuildSet are not handled yet!"
 
-    build = data['build']
+    build = payload.build
     if 'builder' not in build and 'name' not in build['builder']:
         raise ValueError('either `builder` or `builder.name` is not present in the build dictionary, unexpected format request')
 
@@ -851,12 +851,13 @@ def gerritReviewFmt(url, data):
     if len(build['results']) != 1:
         raise ValueError('this review request contains more than one build results, unexpected format request')
 
+    log.info("Formatting a message for Gerrit, build info: '%s'", json.dumps(build))
     result = build['results'][0]
     if result == util.RETRY:
-        return dict()
+        return CallbackReturn()
 
     if builderName != f'{build["properties"].get("event.project")}/nix-eval':
-        return dict()
+        return CallbackReturn()
 
     failed = build['properties'].get('failed_builds', [[]])[0]
 
@@ -880,7 +881,7 @@ def gerritReviewFmt(url, data):
         message += "\nFor more details visit:\n"
         message += build['url'] + "\n"
 
-    return dict(message=message, labels=labels)
+    return CallbackReturn(body=message, extra_info={'labels': labels})
 
 class GerritNixConfigurator(ConfiguratorBase):
     """Janitor is a configurator which create a Janitor Builder with all needed Janitor steps"""
@@ -977,8 +978,8 @@ class GerritNixConfigurator(ConfiguratorBase):
                                        identity_file=self.gerrit_sshkey_path,
                                        generators=[
                                            # gerritReviewCB / self.url
-                                           BuildStatusGenerator(
-                                               message_formatter=MessageFormatterFunction(
+                                           BuildEndStatusGenerator(
+                                               message_formatter=ReasonableMessageFormatter(
                                                    lambda data: gerritReviewFmt(self.url, data),
                                                    "plain",
                                                    want_properties=True,

buildbot_nix/message_formatter.py

@@ -0,0 +1,45 @@
+from typing import Any, Callable, Literal
+from buildbot.reporters.message import MessageFormatterBase
+import dataclasses
+
+@dataclasses.dataclass
+class CallbackPayloadBuild:
+    # buddy i have no idea what the fuck is in this
+    build: dict[str, Any]
+
+@dataclasses.dataclass
+class CallbackPayloadBuildSet:
+    buildset: dict[str, Any]
+    # i have no idea what the fuck is in this honestly
+    builds: Any
+
+@dataclasses.dataclass
+class CallbackReturn:
+    body: str | None = None
+    subject: str | None = None
+    extra_info: dict[str, Any] | None = None
+    # FIXME: support other template types, if they actually become necessary
+    template_type: Literal['plain'] = 'plain'
+
+class ReasonableMessageFormatter(MessageFormatterBase):
+    """
+    Message formatter which uses strongly typed data classes to reduce suffering slightly.
+    """
+    CallbackFunc = Callable[[CallbackPayloadBuild | CallbackPayloadBuildSet], CallbackReturn]
+    def __init__(self, function: CallbackFunc, template_type: str, **kwargs):
+        super().__init__(**kwargs)
+        self.template_type = template_type
+        self._function = function
+
+    def format_message_for_build(self, master, build, **kwargs):
+        return dataclasses.asdict(self._function(CallbackPayloadBuild(build=build)))
+
+    def format_message_for_buildset(self, master, buildset, builds, **kwargs):
+        return dataclasses.asdict(self._function(CallbackPayloadBuildSet(buildset=buildset, builds=builds)))
+
+    # These only exist as callbacks, the only one actually used is render_message_dict
+    def render_message_body(self, context):
+        return None
+
+    def render_message_subject(self, context):
+        return None