pipeline: return logs in --json mode

A pipeline run only returned logs in the `StageFailed` and
`AssemblerFailed` exceptions. Remove those and always return structured
data instead.

It only returns data for stages that actually ran (i.e., didn't come
from the cache). This is similar to the output in interactive mode.

Also change osbuildtest to be able to deal with output that is larger
than the pipe buffer by using subprocess.communicate().

osbuild/__init__.py

@@ -1,13 +1,11 @@
 
-from .pipeline import Assembler, AssemblerFailed, load, load_build, Pipeline, Stage, StageFailed
+from .pipeline import Assembler, load, load_build, Pipeline, Stage
 
 
 __all__ = [
     "Assembler",
-    "AssemblerFailed",
     "load",
     "load_build",
     "Pipeline",
     "Stage",
-    "StageFailed",
 ]

osbuild/__main__.py

@@ -38,29 +38,24 @@ def main():
         pipeline.prepend_build_env(build_pipeline, runner)
 
     try:
-        pipeline.run(args.store, interactive=not args.json, libdir=args.libdir)
+        r = pipeline.run(args.store, interactive=not args.json, libdir=args.libdir)
     except KeyboardInterrupt:
         print()
         print(f"{RESET}{BOLD}{RED}Aborted{RESET}")
         return 130
-    except (osbuild.StageFailed, osbuild.AssemblerFailed) as error:
-        print()
-        print(f"{RESET}{BOLD}{RED}{error.name} failed with code {error.returncode}{RESET}")
-        if args.json:
-            print(error.output)
-        return 1
 
     if args.json:
-        json.dump({
+        json.dump(r, sys.stdout)
-            "tree_id": pipeline.tree_id,
-            "output_id": pipeline.output_id,
-        }, sys.stdout)
         sys.stdout.write("\n")
     else:
-        print("tree id:", pipeline.tree_id)
+        if r["success"]:
-        print("output id:", pipeline.output_id)
+            print("tree id:", pipeline.tree_id)
+            print("output id:", pipeline.output_id)
+        else:
+            print()
+            print(f"{RESET}{BOLD}{RED}Failed{RESET}")
 
-    return 0
+    return 0 if r["success"] else 1
 
 
 if __name__ == "__main__":

osbuild/pipeline.py

@@ -17,22 +17,6 @@ RESET = "\033[0m"
 BOLD = "\033[1m"
 
 
-class StageFailed(Exception):
-    def __init__(self, name, returncode, output):
-        super(StageFailed, self).__init__()
-        self.name = name
-        self.returncode = returncode
-        self.output = output
-
-
-class AssemblerFailed(Exception):
-    def __init__(self, name, returncode, output):
-        super(AssemblerFailed, self).__init__()
-        self.name = name
-        self.returncode = returncode
-        self.output = output
-
-
 def print_header(title, options):
     print()
     print(f"{RESET}{BOLD}{title}{RESET} " + json.dumps(options or {}, indent=2))
@@ -62,7 +46,7 @@ class Stage:
             description["options"] = self.options
         return description
 
-    def run(self, tree, runner, build_tree, interactive=False, check=True, libdir=None):
+    def run(self, tree, runner, build_tree, interactive=False, libdir=None):
         with buildroot.BuildRoot(build_tree, runner, libdir=libdir) as build_root:
             if interactive:
                 print_header(f"{self.name}: {self.id}", self.options)
@@ -79,8 +63,13 @@ class Stage:
                     binds=[f"{tree}:/run/osbuild/tree"],
                     stdin=subprocess.DEVNULL,
                 )
-                if check and r.returncode != 0:
+                return {
-                    raise StageFailed(self.name, r.returncode, api.output)
+                    "name": self.name,
+                    "id": self.id,
+                    "options": self.options,
+                    "success": r.returncode == 0,
+                    "output": api.output
+                }
 
             return r.returncode == 0
 
@@ -108,7 +97,7 @@ class Assembler:
             description["options"] = self.options
         return description
 
-    def run(self, tree, runner, build_tree, output_dir=None, interactive=False, check=True, libdir=None):
+    def run(self, tree, runner, build_tree, output_dir=None, interactive=False, libdir=None):
         with buildroot.BuildRoot(build_tree, runner, libdir=libdir) as build_root:
             if interactive:
                 print_header(f"Assembler {self.name}: {self.id}", self.options)
@@ -141,10 +130,13 @@ class Assembler:
                     readonly_binds=ro_binds,
                     stdin=subprocess.DEVNULL,
                 )
-                if check and r.returncode != 0:
+                return {
-                    raise AssemblerFailed(self.name, r.returncode, api.output)
+                    "name": self.name,
-
+                    "id": self.id,
-            return r.returncode == 0
+                    "options": self.options,
+                    "success": r.returncode == 0,
+                    "output": api.output
+                }
 
 
 class Pipeline:
@@ -206,13 +198,14 @@ class Pipeline:
                 finally:
                     subprocess.run(["umount", "--lazy", tmp], check=True)
 
-    def run(self, store, interactive=False, check=True, libdir=None):
+    def run(self, store, interactive=False, libdir=None):
         os.makedirs("/run/osbuild", exist_ok=True)
         object_store = objectstore.ObjectStore(store)
         if self.build:
-            if not self.build.run(store, interactive, check, libdir):
+            if not self.build.run(store, interactive, libdir):
                 return False
 
+        results = {}
         with self.get_buildtree(object_store) as build_tree:
             if self.stages:
                 if not object_store.contains(self.tree_id):
@@ -225,34 +218,46 @@ class Pipeline:
                             base = self.stages[i].id
                             base_idx = i
                             break
+
                     # The tree does not exist. Create it and save it to the object store. If
                     # two run() calls race each-other, two trees may be generated, and it
                     # is nondeterministic which of them will end up referenced by the tree_id
                     # in the content store. However, we guarantee that all tree_id's and all
                     # generated trees remain valid.
+                    results["stages"] = []
                     with object_store.new(self.tree_id, base_id=base) as tree:
                         for stage in self.stages[base_idx + 1:]:
-                            if not stage.run(tree,
+                            r = stage.run(tree,
-                                             self.runner,
+                                          self.runner,
-                                             build_tree,
+                                          build_tree,
-                                             interactive=interactive,
+                                          interactive=interactive,
-                                             check=check,
+                                          libdir=libdir)
-                                             libdir=libdir):
+                            results["stages"].append(r)
-                                return False
+                            if not r["success"]:
+                                results["success"] = False
+                                return results
 
-            if self.assembler and not object_store.contains(self.output_id):
+                results["tree_id"] = self.tree_id
-                with object_store.get(self.tree_id) as tree, \
-                    object_store.new(self.output_id) as output_dir:
-                    if not self.assembler.run(tree,
-                                              self.runner,
-                                              build_tree,
-                                              output_dir=output_dir,
-                                              interactive=interactive,
-                                              check=check,
-                                              libdir=libdir):
-                        return False
 
-        return True
+            if self.assembler:
+                if not object_store.contains(self.output_id):
+                    with object_store.get(self.tree_id) as tree, \
+                        object_store.new(self.output_id) as output_dir:
+                        r = self.assembler.run(tree,
+                                               self.runner,
+                                               build_tree,
+                                               output_dir=output_dir,
+                                               interactive=interactive,
+                                               libdir=libdir)
+                        results["assembler"] = r
+                        if not r["success"]:
+                            results["success"] = False
+                            return results
+
+                results["output_id"] = self.output_id
+
+        results["success"] = True
+        return results
 
 
 def load_build(description):

test/osbuildtest.py

@@ -42,23 +42,19 @@ class TestCase(unittest.TestCase):
 
         stdin = subprocess.PIPE if input else None
 
-        p = subprocess.Popen(osbuild_cmd, encoding="utf-8", stdin=stdin, stdout=subprocess.PIPE)
+        p = subprocess.Popen(osbuild_cmd, encoding="utf-8", stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-        if input:
-            p.stdin.write(input)
-            p.stdin.close()
         try:
-            r = p.wait()
+            output, _ = p.communicate(input)
-            if r != 0:
+            if p.returncode != 0:
-                print(p.stdout.read())
+                print(output)
-            self.assertEqual(r, 0)
+            self.assertEqual(p.returncode, 0)
         except KeyboardInterrupt:
             # explicitly wait again to let osbuild clean up
             p.wait()
             raise
 
-        result = json.load(p.stdout)
+        result = json.loads(output)
-        p.stdout.close()
+        return result.get("tree_id"), result.get("output_id")
-        return result["tree_id"], result["output_id"]
 
     def run_tree_diff(self, tree1, tree2):
         tree_diff_cmd = ["./tree-diff", tree1, tree2]