Merge ~racb/git-ubuntu:commit-graph-builder into git-ubuntu:master

[Robie Basak]

Make Jenkins happy

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:94dbfe16b413bf16fb1fb203d7a96976c2e6259d
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/280/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/280/rebuild

review: Approve (continuous-integration)

[Nish Aravamudan (nacc)]

review: Approve

diff --git a/gitubuntu/repo_builder.py b/gitubuntu/repo_builder.py
index cb93278..04973f7 100644
--- a/gitubuntu/repo_builder.py
+++ b/gitubuntu/repo_builder.py
@@ -17,6 +17,11 @@ its constructor. This allows a build in a single data structure instead of
 having to use multiple statements to grab references to other parts of the same
 structure.
 
+Use a Commit object to represent a git commit (constructed with a Tree object
+to represent the git tree contained within it), and a CommitGraph object to
+contain and create multiple commits as needed, including ones that reference
+each other using Placeholder objects.
+
 Write any object (including any children as necessary) using the write method,
 which takes a single pygit2.Repository parameter. This returns the pygit2.Oid
 of the top level written object.
@@ -28,6 +33,7 @@ Objects must be treated as immutable. The result of mutating an object after it 
 __all__ = [
     'Blob',
     'Commit',
+    'CommitGraph',
     'ExecutableBlob',
     'Symlink',
     'Placeholder',
@@ -42,6 +48,13 @@ __all__ = [
 # hashes or something?
 
 
+DEFAULT_SIGNATURE = pygit2.Signature(
+    name='Test Builder',
+    email='[e-mail]',
+    time=0,  # don't default to current time for hash reproducibility
+)
+
+
 class NamedMixin:
     def __init__(self, name=None):
         self.name = name
@@ -122,25 +135,6 @@ class Tree(NamedMixin, WriteMixin):
         super().__init__(**kwargs)
         self.entries = entries
 
-    def __getitem__(self, k):
-        if k == self.name:
-            return self
-
-        # XXX inefficient
-        for entry in self.entries.values():
-            if isinstance(entry, Placeholder):
-                # As an exception, Placeholders don't have names so cannot be
-                # located using this method.
-                continue
-            if entry.name == k:
-                return entry
-            elif isinstance(entry, type(self)):
-                try:
-                    return entry[k]
-                except KeyError:
-                    pass
-        raise KeyError('Object named %r not found' % k)
-
     def replace(self, old, new):
         # XXX inefficient
         for name, entry in self.entries.items():
@@ -166,31 +160,80 @@ class Tree(NamedMixin, WriteMixin):
 
 
 class Commit(NamedMixin, WriteMixin):
-    def __init__(self, tree, **kwargs):
+    def __init__(self, tree, parents=None, message=None, **kwargs):
         super().__init__(**kwargs)
         self.tree = tree
+        self.parents = parents or []
+        self.message = 'Test commit' if message is None else message
+
+    def replace(self, old, new):
+        for i, parent in enumerate(self.parents):
+            if parent is old:
+                self.parents[i] = new
+                return
+        raise KeyError("Cannot find %r in parents" % old)
 
     def walk(self, parent=None):
+        for commit_parent in self.parents:
+            yield from commit_parent.walk(parent=self)
+        yield from self.tree.walk(parent=self)
         yield parent, self
 
     def _obj_to_oid(self, repo, record=None):
         return repo.create_commit(
             None,
-            pygit2.Signature(name='Test Builder', email='[e-mail]'),
-            pygit2.Signature(name='Test Builder', email='[e-mail]'),
-            'Test commit',
+            DEFAULT_SIGNATURE,
+            DEFAULT_SIGNATURE,
+            self.message,
             self.tree.write(repo),
-            [],
+            [parent.write(repo, record=record) for parent in self.parents],
         )
 
-    def __getitem__(self, k):
-        return self if self.name == k else self.tree[k]
+
+class CommitGraph:
+    """Represent a graph of commits
+
+    We keep the commits as a list, but understand that they may relate to each
+    other using parenting relationships. This is affectively a commit container
+    that exists for the convenience of writing them to a git repository at
+    once.
+    """
+    def __init__(self, commit_list):
+        """Construct a CommitGraph instance
+
+        :param list(Commit) commit_list: the commits this CommitGraph should
+            contain. These may be related to each other with parenting
+            relationships, but do not have to be.
+        """
+        self.commit_list = commit_list
+        replace_placeholders(self)
+
+    def write(self, repo, record=None):
+        record = record or dict()
+        written_commits = [
+            commit.write(repo=repo, record=record)
+            for commit
+            in self.commit_list
+        ]
+        return written_commits[0]
+
+    def walk(self):
+        for commit in self.commit_list:
+            yield from commit.walk(parent=self)
+
+
+def find_node(top, name):
+    for parent, obj in top.walk():
+        if isinstance(obj, NamedMixin):
+            if obj.name == name:
+                return obj
+    raise KeyError("Object named %r not found" % name)
 
 
 def replace_placeholders(top):
     for parent, obj in top.walk():
         if isinstance(obj, Placeholder):
-            parent.replace(obj, top[obj.target_name])
+            parent.replace(obj, find_node(top, obj.target_name))
 
 
 class TestObjectCreation(unittest.TestCase):
@@ -224,16 +267,50 @@ class TestObjectCreation(unittest.TestCase):
         commit = self.repo.get(commit_ref)
         assert commit.tree_id == tree_ref
 
+    def testCommitMessage(self):
+        ref = Commit(Tree({}), message='foo').write(self.repo)
+        commit = self.repo.get(ref)
+        assert commit.message == 'foo'
+
+    def testCommitParents(self):
+        tree = Tree({})
+        top = Commit(tree, message='top')
+        top_ref = top.write(self.repo)
+        child_a = Commit(tree, parents=[top], message='child_a')
+        child_a_ref = child_a.write(self.repo)
+        child_b = Commit(tree, parents=[top], message='child_b')
+        child_b_ref = child_b.write(self.repo)
+        merge = Commit(tree, parents=[child_a, child_b], message='merge')
+        merge_ref = merge.write(self.repo)
+
+        merge_commit = self.repo.get(merge_ref)
+        assert merge_commit.parent_ids == [child_a_ref, child_b_ref]
+
+        child_a_commit = self.repo.get(child_a_ref)
+        assert child_a_commit.parent_ids == [top_ref]
+
+        top_commit = self.repo.get(top_ref)
+        assert top_commit.parent_ids == []
+
     def testNameSearch(self):
         inner_tree = Tree({'foo': Blob(b'bar', name='blob')}, name='inner')
         outer_tree = Tree({'baz': inner_tree}, name='outer')
         commit = Commit(outer_tree, name='commit')
-        assert commit['commit'] is commit
-        assert commit['inner'] is inner_tree
-        assert commit['outer'] is outer_tree
-        assert isinstance(commit['blob'], Blob)
+        assert find_node(commit, 'commit') is commit
+        assert find_node(commit, 'inner') is inner_tree
+        assert find_node(commit, 'outer') is outer_tree
+        assert isinstance(find_node(commit, 'blob'), Blob)
         with pytest.raises(KeyError):
-            commit['absent']
+            find_node(commit, 'absent')
+
+    def testCommitGraph(self):
+        graph = CommitGraph([
+            Commit(Tree({}), parents=[Placeholder('parent')]),
+            Commit(Tree({}), name='parent'),
+        ])
+        child_ref = graph.write(self.repo)
+        child = self.repo.get(child_ref)
+        assert child.parent_ids == [graph.commit_list[1].write(self.repo)]
 
 
 @pytest.mark.parametrize('cls', [
@@ -248,3 +325,12 @@ def test_placeholder(cls):
     assert top.entries['foo'] is not top.entries['bar']
     replace_placeholders(top)
     assert top.entries['foo'] is top.entries['bar']
+
+def test_commit_graph_placeholder():
+    empty_tree = Tree({})
+    graph = CommitGraph([
+        Commit(empty_tree, message='top', name='top'),
+        Commit(empty_tree, message='child', parents=[Placeholder('top')]),
+    ])
+    replace_placeholders(graph)
+    assert graph.commit_list[1].parents[0] is graph.commit_list[0]