From 3ab2ddd481a6882d73990fa58e81cbc3ca639c36 Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Mon, 13 Jun 2022 20:33:03 +0200
Subject: [PATCH] pipeline, fmt: use build result object internally

Instead of serializing the `BuildResult` to a dict in `build_stages`,
we keep the object and then only serialize it in the corresponding
formatting code. This doubles down on the separation between the
internal data structures and the external representation of them. It
was partially already done in the v2 format which hand-picked which
elements of the BuildResult it would return for each stage.
---
 osbuild/formats/v1.py | 21 ++++++++++++++++++---
 osbuild/formats/v2.py | 26 +++++++++++++-------------
 osbuild/pipeline.py   |  2 +-
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/osbuild/formats/v1.py b/osbuild/formats/v1.py
index b9b8522e..7c68948c 100644
--- a/osbuild/formats/v1.py
+++ b/osbuild/formats/v1.py
@@ -9,7 +9,7 @@ to fetch resources.
 """
 from typing import Dict
 from osbuild.meta import Index, ValidationResult
-from ..pipeline import Manifest, Pipeline, detect_host_runner
+from ..pipeline import BuildResult, Manifest, Pipeline, detect_host_runner
 
 
 VERSION = "1"
@@ -194,6 +194,16 @@ def load(description: Dict, index: Index) -> Manifest:
 def output(manifest: Manifest, res: Dict) -> Dict:
     """Convert a result into the v1 format"""
 
+    def result_for_stage(result: BuildResult):
+        return {
+            "id": result.id,
+            "type": result.name,
+            "success": result.success,
+            "error": result.error,
+            "output": result.output,
+            "metadata": result.metadata,
+        }
+
     def result_for_pipeline(pipeline):
         # The pipeline might not have been built one of its
         # dependencies, i.e. its build pipeline, failed to
@@ -211,7 +221,9 @@ def output(manifest: Manifest, res: Dict) -> Dict:
 
         stages = current.get("stages")
         if stages:
-            retval["stages"] = stages
+            retval["stages"] = [
+                result_for_stage(r) for r in stages
+            ]
         return retval
 
     result = result_for_pipeline(manifest["tree"])
@@ -226,7 +238,10 @@ def output(manifest: Manifest, res: Dict) -> Dict:
     if not current:
         return result
 
-    result["assembler"] = current["stages"][0]
+    # The assembler pipeline must have exactly one stage
+    # which is the v1 assembler
+    stage = current["stages"][0]
+    result["assembler"] = result_for_stage(stage)
     if not result["assembler"]["success"]:
         result["success"] = False
 
diff --git a/osbuild/formats/v2.py b/osbuild/formats/v2.py
index a90975e7..4a43ba81 100644
--- a/osbuild/formats/v2.py
+++ b/osbuild/formats/v2.py
@@ -395,10 +395,10 @@ def output(manifest: Manifest, res: Dict) -> Dict:
                 "type": "org.osbuild.error.stage",
                 "details": {
                     "stage": {
-                        "id": failed["id"],
-                        "type": failed["name"],
-                        "output": failed["output"],
-                        "error": failed["error"]
+                        "id": failed.id,
+                        "type": failed.name,
+                        "output": failed.output,
+                        "error": failed.error,
                     }
                 }
             }
@@ -415,10 +415,10 @@ def output(manifest: Manifest, res: Dict) -> Dict:
             data = {}
             r = res.get(p.id, {})
             for stage in r.get("stages", []):
-                md = stage.get("metadata")
+                md = stage.metadata
                 if not md:
                     continue
-                name = stage["name"]
+                name = stage.name
                 val = data.setdefault(name, {})
                 val.update(md)
 
@@ -433,14 +433,14 @@ def output(manifest: Manifest, res: Dict) -> Dict:
 
         for stage in r.get("stages", []):
             data = {
-                "id": stage["id"],
-                "type": stage["name"],
-                "output": stage["output"]
+                "id": stage.id,
+                "type": stage.name,
+                "output": stage.output,
             }
-            if not stage["success"]:
-                data["success"] = stage["success"]
-                if stage["error"]:
-                    data["error"] = stage["error"]
+            if not stage.success:
+                data["success"] = stage.success
+                if stage.error:
+                    data["error"] = stage.error
 
             log.append(data)
 
diff --git a/osbuild/pipeline.py b/osbuild/pipeline.py
index 8d8e0624..407a0816 100644
--- a/osbuild/pipeline.py
+++ b/osbuild/pipeline.py
@@ -333,7 +333,7 @@ class Pipeline:
 
                 monitor.result(r)
 
-            results["stages"].append(r.as_dict())
+            results["stages"].append(r)
             if not r.success:
                 cleanup(build_tree, tree)
                 results["success"] = False