test: make osbuild.compile method take output_dir as a parameter

Previously, the osbuild executor had its internal temporary directory that
served as the output directory. However, this approach gives no power to
the caller to control the lifetime of the produced artifacts. When more
images are built using one executor, the results will accumulate in one
place possibly leading to exhaustion of disk space.

This commit removes the executor's internal output directory. The output
directory can now be passed to osbuild.compile, so the caller can control
its lifetime. If no directory is passed in, the compile method will use
its own temporary directory - this is useful in cases when the caller
doesn't care about the built artifacts or the manifest doesn't have any
outputs.

test/run/test_assemblers.py

@@ -38,10 +38,10 @@ class TestAssemblers(test.TestBase):
         treeid = osb.treeid_from_manifest(data)
         assert treeid
 
-        osb.compile(data, checkpoints=[treeid])
+        with tempfile.TemporaryDirectory(dir="/var/tmp") as output_dir:
-        with osb.map_object(treeid) as tree, \
+            osb.compile(data, output_dir=output_dir, checkpoints=[treeid])
-             osb.map_output(output_path) as output:
+            with osb.map_object(treeid) as tree:
-            yield tree, output
+                yield tree, os.path.join(output_dir, output_path)
 
     def assertImageFile(self, filename, fmt, expected_size=None):
         info = json.loads(subprocess.check_output(["qemu-img", "info", "--output", "json", filename]))
@@ -107,18 +107,20 @@ class TestAssemblers(test.TestBase):
                 manifest = json.load(f)
 
             data = json.dumps(manifest)
-            osb.compile(data)
+            with tempfile.TemporaryDirectory(dir="/var/tmp") as output_dir:
-            with osb.map_output("compose.json") as filename:
+                osb.compile(data, output_dir=output_dir)
-                with open(filename) as f:
+                compose_file = os.path.join(output_dir, "compose.json")
-                    compose = json.load(f)
+                repo = os.path.join(output_dir, "repo")
-            commit_id = compose["ostree-commit"]
+
-            ref = compose["ref"]
+                with open(compose_file) as f:
-            rpmostree_inputhash = compose["rpm-ostree-inputhash"]
+                    compose = json.load(f)
-            assert commit_id
+                commit_id = compose["ostree-commit"]
-            assert ref
+                ref = compose["ref"]
-            assert rpmostree_inputhash
+                rpmostree_inputhash = compose["rpm-ostree-inputhash"]
+                assert commit_id
+                assert ref
+                assert rpmostree_inputhash
 
-            with osb.map_output("repo") as repo:
                 md = subprocess.check_output(
                     [
                         "ostree",
@@ -127,7 +129,7 @@ class TestAssemblers(test.TestBase):
                         "--print-metadata-key=rpmostree.inputhash",
                         commit_id
                     ], encoding="utf-8").strip()
-            self.assertEqual(md, f"'{rpmostree_inputhash}'")
+                self.assertEqual(md, f"'{rpmostree_inputhash}'")
 
     @unittest.skipUnless(test.TestBase.have_tree_diff(), "tree-diff missing")
     def test_qemu(self):

test/run/test_boot.py

@@ -24,11 +24,11 @@ class TestBoot(test.TestBase):
                                 "manifests/fedora-boot.json")
 
         with self.osbuild as osb:
-            osb.compile_file(manifest)
+            with tempfile.TemporaryDirectory(dir="/var/tmp") as temp_dir:
-            with osb.map_output("fedora-boot.qcow2") as qcow2, \
+                osb.compile_file(manifest, output_dir=temp_dir)
-                 tempfile.TemporaryDirectory() as d:
+                qcow2 = os.path.join(temp_dir, "fedora-boot.qcow2")
+                output_file = os.path.join(temp_dir, "output")
 
-                output_file = os.path.join(d, "output")
                 subprocess.run(["qemu-system-x86_64",
                                 "-snapshot",
                                 "-m", "1024",

test/test.py

@@ -221,7 +221,6 @@ class OSBuild(contextlib.AbstractContextManager):
 
     _exitstack = None
     _cachedir = None
-    _outputdir = None
 
     def __init__(self, unit_test, cache_from=None):
         self._unittest = unit_test
@@ -243,10 +242,6 @@ class OSBuild(contextlib.AbstractContextManager):
                                 self._cachedir],
                                check=True)
 
-            # Create a temporary output-directors for assembled artifacts.
-            output = tempfile.TemporaryDirectory(dir="/var/tmp")
-            self._outputdir = self._exitstack.enter_context(output)
-
             # Keep our ExitStack for `__exit__()`.
             self._exitstack = self._exitstack.pop_all()
 
@@ -257,7 +252,6 @@ class OSBuild(contextlib.AbstractContextManager):
         with self._exitstack:
             pass
 
-        self._outputdir = None
         self._cachedir = None
         self._exitstack = None
 
@@ -275,52 +269,57 @@ class OSBuild(contextlib.AbstractContextManager):
         print(data_stderr)
         print("-- END ---------------------------------")
 
-    def compile(self, data_stdin, checkpoints=None):
+    def compile(self, data_stdin, output_dir=None, checkpoints=None):
         """Compile an Artifact
 
         This takes a manifest as `data_stdin`, executes the pipeline, and
         assembles the artifact. No intermediate steps are kept, unless you
         provide suitable checkpoints.
 
-        The produced artifact (if any) is stored in the output directory. Use
+        The produced artifact (if any) is stored in the directory passed via
-        `map_output()` to temporarily map the file and get access. Note that
+        the output_dir parameter. If it's set to None, a temporary directory
-        the output directory becomes invalid when you leave the context-manager
+        is used and thus the caller cannot access the built artifact.
-        of this class.
         """
 
-        cmd_args = []
+        if output_dir is None:
+            output_dir_context = tempfile.TemporaryDirectory(dir="/var/tmp")
+        else:
+            output_dir_context = contextlib.nullcontext(output_dir)
 
-        cmd_args += ["--json"]
+        with output_dir_context as osbuild_output_dir:
-        cmd_args += ["--libdir", "."]
+            cmd_args = []
-        cmd_args += ["--output-directory", self._outputdir]
-        cmd_args += ["--store", self._cachedir]
 
-        for c in (checkpoints or []):
+            cmd_args += ["--json"]
-            cmd_args += ["--checkpoint", c]
+            cmd_args += ["--libdir", "."]
+            cmd_args += ["--output-directory", osbuild_output_dir]
+            cmd_args += ["--store", self._cachedir]
 
-        # Spawn the `osbuild` executable, feed it the specified data on
+            for c in (checkpoints or []):
-        # `STDIN` and wait for completion. If we are interrupted, we always
+                cmd_args += ["--checkpoint", c]
-        # wait for `osbuild` to shut down, so we can clean up its file-system
+
-        # trees (they would trigger `EBUSY` if we didn't wait).
+            # Spawn the `osbuild` executable, feed it the specified data on
-        try:
+            # `STDIN` and wait for completion. If we are interrupted, we always
-            p = subprocess.Popen(
+            # wait for `osbuild` to shut down, so we can clean up its file-system
-                ["python3", "-m", "osbuild"] + cmd_args + ["-"],
+            # trees (they would trigger `EBUSY` if we didn't wait).
-                encoding="utf-8",
+            try:
-                stdin=subprocess.PIPE,
+                p = subprocess.Popen(
-                stdout=subprocess.PIPE,
+                    ["python3", "-m", "osbuild"] + cmd_args + ["-"],
-                stderr=subprocess.PIPE,
+                    encoding="utf-8",
-            )
+                    stdin=subprocess.PIPE,
-            data_stdout, data_stderr = p.communicate(data_stdin)
+                    stdout=subprocess.PIPE,
-        except KeyboardInterrupt:
+                    stderr=subprocess.PIPE,
-            p.wait()
+                )
-            raise
+                data_stdout, data_stderr = p.communicate(data_stdin)
+            except KeyboardInterrupt:
+                p.wait()
+                raise
 
         # If execution failed, print results to `STDOUT`.
         if p.returncode != 0:
             self._print_result(p.returncode, data_stdout, data_stderr)
             self._unittest.assertEqual(p.returncode, 0)
 
-    def compile_file(self, file_stdin, checkpoints=None):
+    def compile_file(self, file_stdin, output_dir=None, checkpoints=None):
         """Compile an Artifact
 
         This is similar to `compile()` but takes a file-path instead of raw
@@ -330,7 +329,7 @@ class OSBuild(contextlib.AbstractContextManager):
 
         with open(file_stdin, "r") as f:
             data_stdin = f.read()
-            return self.compile(data_stdin, checkpoints=checkpoints)
+            return self.compile(data_stdin, output_dir, checkpoints=checkpoints)
 
     @staticmethod
     def treeid_from_manifest(manifest_data):
@@ -363,19 +362,3 @@ class OSBuild(contextlib.AbstractContextManager):
         # as a context-manager so the caller does not retain the path for
         # later access.
         yield path
-
-    @contextlib.contextmanager
-    def map_output(self, filename):
-        """Temporarily Map an Output Object
-
-        This takes a filename (or relative path) and looks it up in the output
-        directory. It then provides the absolute path to that file back to the
-        caller.
-        """
-
-        path = os.path.join(self._outputdir, filename)
-        assert os.access(path, os.R_OK)
-
-        # Similar to `map_object()` we provide the path through a
-        # context-manager so the caller does not retain the path.
-        yield path