From 39213b7f44a8ac7e8a29b0cd4df10f20a67e89ea Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Tue, 18 Feb 2020 09:50:55 +0100
Subject: [PATCH] objectstore: copy on write semantics for Object

Since Object knows its base now, the initialization of the tree
with the content of its base can be delayed until the moment
someone wants to actually modify the tree, thus implementing
copy on write semantics. For this a new `write` method is added
that will initialize the base and return the writable tree. It
should be used instead of `path` whenever the a client wants to
write to the tree of the Object.
Adapt the pipeline and the tests to use the new `write` method
in all the appropriate places.
NB: since the intention can not be inferred when using `path`
directly, the Object is still being initialized there.
---
 osbuild/objectstore.py   | 14 ++++++++++++--
 osbuild/pipeline.py      |  4 ++--
 test/test_objectstore.py | 20 +++++++++++++-------
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/osbuild/objectstore.py b/osbuild/objectstore.py
index 34897531..d760ce00 100644
--- a/osbuild/objectstore.py
+++ b/osbuild/objectstore.py
@@ -73,12 +73,22 @@ class Object:
     @contextlib.contextmanager
     def open(self):
         """Open the directory and return the file descriptor"""
+        if self._base_path and not self._init:
+            path = self._base_path
+        else:
+            path = self._tree
+
         try:
-            fd = os.open(self.path, os.O_DIRECTORY)
+            fd = os.open(path, os.O_DIRECTORY)
             yield fd
         finally:
             os.close(fd)
 
+    def write(self) -> str:
+        """Return a path that can be written to"""
+        self.init()
+        return self._tree
+
     def store_tree(self, destination: str):
         """Store the tree at destination and reset itself
 
@@ -167,8 +177,8 @@ class ObjectStore:
             if base_id:
                 # if we were given a base id, resolve its path and set it
                 # as the base_path of the object
+                # NB: `obj` does not get initialized explicitly here
                 obj.base_path = self.resolve_ref(base_id)
-                obj.init()
 
             yield obj
 
diff --git a/osbuild/pipeline.py b/osbuild/pipeline.py
index bb7211da..8071fac6 100644
--- a/osbuild/pipeline.py
+++ b/osbuild/pipeline.py
@@ -272,7 +272,7 @@ class Pipeline:
                     try:
                         with object_store.new(self.tree_id, base_id=base) as tree:
                             for stage in self.stages[base_idx + 1:]:
-                                r = stage.run(tree.path,
+                                r = stage.run(tree.write(),
                                               self.runner,
                                               build_tree,
                                               store,
@@ -298,7 +298,7 @@ class Pipeline:
                             r = self.assembler.run(tree,
                                                    self.runner,
                                                    build_tree,
-                                                   output_dir=output_dir.path,
+                                                   output_dir=output_dir.write(),
                                                    interactive=interactive,
                                                    libdir=libdir,
                                                    var=store)
diff --git a/test/test_objectstore.py b/test/test_objectstore.py
index 1a3da965..c33e4fbc 100644
--- a/test/test_objectstore.py
+++ b/test/test_objectstore.py
@@ -25,7 +25,8 @@ class TestObjectStore(unittest.TestCase):
         with tempfile.TemporaryDirectory(dir="/var/tmp") as tmp:
             object_store = objectstore.ObjectStore(tmp)
             with object_store.new("a") as tree:
-                p = Path(f"{tree.path}/A")
+                path = tree.write()
+                p = Path(f"{path}/A")
                 p.touch()
 
             assert os.path.exists(f"{object_store.refs}/a")
@@ -35,9 +36,10 @@ class TestObjectStore(unittest.TestCase):
             assert len(os.listdir(f"{object_store.refs}/a/")) == 1
 
             with object_store.new("b") as tree:
-                p = Path(f"{tree.path}/A")
+                path = tree.write()
+                p = Path(f"{path}/A")
                 p.touch()
-                p = Path(f"{tree.path}/B")
+                p = Path(f"{path}/B")
                 p.touch()
 
             assert os.path.exists(f"{object_store.refs}/b")
@@ -51,12 +53,14 @@ class TestObjectStore(unittest.TestCase):
         with tempfile.TemporaryDirectory(dir="/var/tmp") as tmp:
             object_store = objectstore.ObjectStore(tmp)
             with object_store.new("a") as tree:
-                p = Path(f"{tree.path}/A")
+                path = tree.write()
+                p = Path(f"{path}/A")
                 p.touch()
 
             with object_store.new("b") as tree:
+                path = tree.write()
                 shutil.copy2(f"{object_store.refs}/a/A",
-                             f"{tree.path}/A")
+                             f"{path}/A")
 
             assert os.path.exists(f"{object_store.refs}/a")
             assert os.path.exists(f"{object_store.refs}/a/A")
@@ -70,10 +74,12 @@ class TestObjectStore(unittest.TestCase):
     def test_snapshot(self):
         object_store = objectstore.ObjectStore(self.store)
         with object_store.new("b") as tree:
-            p = Path(f"{tree.path}/A")
+            path = tree.write()
+            p = Path(f"{path}/A")
             p.touch()
             object_store.snapshot(tree, "a")
-            p = Path(f"{tree.path}/B")
+            path = tree.write()
+            p = Path(f"{path}/B")
             p.touch()
 
         # check the references exist