launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging ~wgrant/git-build-recipe:nest-part into git-build-recipe:master with ~wgrant/git-build-recipe:bug-1542673 as a prerequisite.

Commit message:
Implement nest-part (LP: #1537579).

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/git-build-recipe/+git/git-build-recipe/+merge/286479

A simpler, faster https://code.launchpad.net/~cjwatson/git-build-recipe/+git/git-build-recipe/+merge/283756.

Using an actual git subtree doesn't buy us much, since we can't easily support merges into nested parts. It would possibly vaguely work in bzr due to file IDs, but no such luck here. read-tree of the tip rev is much faster than rewriting the whole branch, so let's do that.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~wgrant/git-build-recipe:nest-part into git-build-recipe:master.

diff --git a/debian/changelog b/debian/changelog
index 0c8e1bd..8a94f44 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+git-build-recipe (0.3) UNRELEASED; urgency=medium
+
+  * Implement nest-part (LP: #1537579).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Mon, 25 Jan 2016 02:51:00 +0000
+
 git-build-recipe (0.2.1) trusty; urgency=medium
 
   * Configure "lp:" as shorthand for "https://git.launchpad.net/";.
diff --git a/gitbuildrecipe/recipe.py b/gitbuildrecipe/recipe.py
index f1ea7d0..703c2d1 100644
--- a/gitbuildrecipe/recipe.py
+++ b/gitbuildrecipe/recipe.py
@@ -368,11 +368,9 @@ def nest_part_branch(child_branch, target_path, subpath, target_subdir=None):
     child_branch.path = target_path
     fetch_branches(child_branch)
     child_branch.resolve_commit()
-    subtree_commit = child_branch.git_output(
-        "subtree", "split", "-P", subpath, child_branch.commit)
-    child_branch.path = os.path.join(target_path, target_subdir)
-    git_clone(".", child_branch.path, child_branch.remote_name)
-    child_branch.git_call("checkout", subtree_commit)
+    child_branch.git_call(
+        "read-tree", "--prefix", target_subdir, "-u",
+        child_branch.commit + ":" + subpath)
 
 
 def _build_inner_tree(base_branch, target_path):
diff --git a/gitbuildrecipe/tests/test_recipe.py b/gitbuildrecipe/tests/test_recipe.py
index 298eea7..c3c8491 100644
--- a/gitbuildrecipe/tests/test_recipe.py
+++ b/gitbuildrecipe/tests/test_recipe.py
@@ -14,10 +14,10 @@
 
 import os
 import shutil
-import unittest
 
 from testtools.matchers import (
     FileContains,
+    Not,
     PathExists,
     )
 
@@ -287,7 +287,6 @@ class RecipeParserTests(GitTestCase):
         self.assertEqual(None, location)
         self.check_recipe_branch(child_branch, "bar", "http://bar.org";)
 
-    @unittest.skip("nest_part broken")
     def test_builds_recipe_with_nest_part(self):
         base_branch = self.get_recipe(self.basic_header_and_branch
                 + "nest-part bar http://bar.org some/path")
@@ -300,7 +299,6 @@ class RecipeParserTests(GitTestCase):
         self.assertEqual("some/path", instruction.subpath)
         self.assertEqual(None, instruction.target_subdir)
 
-    @unittest.skip("nest_part broken")
     def test_builds_recipe_with_nest_part_subdir(self):
         base_branch = self.get_recipe(self.basic_header_and_branch
                 + "nest-part bar http://bar.org some/path target-subdir")
@@ -313,7 +311,6 @@ class RecipeParserTests(GitTestCase):
         self.assertEqual("some/path", instruction.subpath)
         self.assertEqual("target-subdir", instruction.target_subdir)
 
-    @unittest.skip("nest_part broken")
     def test_builds_recipe_with_nest_part_subdir_and_revspec(self):
         base_branch = self.get_recipe(self.basic_header_and_branch
                 + "nest-part bar http://bar.org some/path target-subdir 1234")
@@ -631,7 +628,6 @@ class BuildTreeTests(GitTestCase):
         self.assertEqual(source1_commit, base_branch.commit)
         self.assertEqual(source2_commit, merged_branch.commit)
 
-    @unittest.skip("nest_part broken")
     def test_build_tree_implicit_dir(self):
         # Branches nested into non-existent directories trigger creation of
         # those directories.
@@ -647,11 +643,11 @@ class BuildTreeTests(GitTestCase):
         base_branch = BaseRecipeBranch("source1", "1", 0.4)
         merged_branch_2 = RecipeBranch("merged", "source2")
         # Merge source2 into path implicit/b
-        base_branch.nest_part_branch(merged_branch_2, ".",
+        base_branch.nest_part_branch(merged_branch_2, "/",
             target_subdir="implicit/b")
         # Merge source3 into path implicit/moreimplicit/c
-        merged_branch_3 = RecipeBranch("merged", "source3")
-        base_branch.nest_part_branch(merged_branch_3, ".",
+        merged_branch_3 = RecipeBranch("merged2", "source3")
+        base_branch.nest_part_branch(merged_branch_3, "/",
             target_subdir="moreimplicit/another/c")
         build_tree(base_branch, "target")
         self.assertThat("target/implicit/b/file", FileContains("new file"))
@@ -659,7 +655,6 @@ class BuildTreeTests(GitTestCase):
             "target/moreimplicit/another/c/yetanotherfile",
             FileContains("rugby"))
 
-    @unittest.skip("nest_part broken")
     def test_build_tree_nest_part(self):
         """A recipe can specify a merge of just part of an unrelated tree."""
         source1 = self.make_source_branch("source1")
@@ -667,7 +662,9 @@ class BuildTreeTests(GitTestCase):
         source1_commit = source1.last_revision()
         # Add 'b' to source2.
         source2.build_tree_contents([
-            ("b", "new file"), ("not-b", "other file")])
+            ("b/",), ("b/a", "new file"),
+            ("not-b/",), ("not-b/a", "other file"),
+            ])
         source2.add(["b", "not-b"])
         source2_commit = source2.commit("two")
         base_branch = BaseRecipeBranch("source1", "1", 0.2)
@@ -675,15 +672,13 @@ class BuildTreeTests(GitTestCase):
         # Merge just 'b' from source2; 'a' is untouched.
         base_branch.nest_part_branch(merged_branch, "b")
         build_tree(base_branch, "target")
-        file_id = source1.path2id("a")
         self.assertThat(
-            "target/a", FileContains(source1.get_file_text(file_id)))
-        self.assertThat("target/b", FileContains("new file"))
-        self.assertNotInWorkingTree("not-b", "target")
+            "target/a", FileContains(source1._git_output("show", "HEAD:a")))
+        self.assertThat("target/b/a", FileContains("new file"))
+        self.assertThat("target/not-b", Not(PathExists()))
         self.assertEqual(source1_commit, base_branch.commit)
         self.assertEqual(source2_commit, merged_branch.commit)
 
-    @unittest.skip("nest_part broken")
     def test_build_tree_nest_part_explicit_target(self):
         """A recipe can specify a merge of just part of an unrelated tree into
         a specific subdirectory of the target tree.
@@ -696,7 +691,8 @@ class BuildTreeTests(GitTestCase):
         source1_commit = source1.last_revision()
         # Add 'b' to source2.
         source2.build_tree_contents([
-            ("b", "new file"), ("not-b", "other file")])
+            ("b/",), ("b/a", "new file"),
+            ("not-b/",), ("not-b/a", "other file")])
         source2.add(["b", "not-b"])
         source2_commit = source2.commit("two")
         base_branch = BaseRecipeBranch("source1", "1", 0.2)
@@ -704,8 +700,8 @@ class BuildTreeTests(GitTestCase):
         # Merge just 'b' from source2; 'a' is untouched.
         base_branch.nest_part_branch(merged_branch, "b", "dir/b")
         build_tree(base_branch, "target")
-        self.assertThat("target/dir/b", FileContains("new file"))
-        self.assertNotInWorkingTree("dir/not-b", "target")
+        self.assertThat("target/dir/b/a", FileContains("new file"))
+        self.assertThat("target/dir/not-b", Not(PathExists()))
         self.assertEqual(source1_commit, base_branch.commit)
         self.assertEqual(source2_commit, merged_branch.commit)
 
@@ -1006,7 +1002,6 @@ class StringifyTests(GitTestCase):
         base_branch.nest_branch(".", nested_branch1)
         self.assertRaises(InstructionParseError, str, base_branch)
 
-    @unittest.skip("nest_part broken")
     def test_with_nest_part(self):
         base_branch = BaseRecipeBranch("base_url", "1", 0.1)
         base_branch.commit = "base_revid"
@@ -1019,7 +1014,6 @@ class StringifyTests(GitTestCase):
                 "nest-part nested1 nested1_url foo bar tag:foo\n",
                 manifest)
 
-    @unittest.skip("nest_part broken")
     def test_with_nest_part_with_no_target_dir(self):
         base_branch = BaseRecipeBranch("base_url", "1", 0.1)
         base_branch.commit = "base_revid"
@@ -1032,7 +1026,6 @@ class StringifyTests(GitTestCase):
                 "nest-part nested1 nested1_url foo foo tag:foo\n",
                 manifest)
 
-    @unittest.skip("nest_part broken")
     def test_with_nest_part_with_no_target_dir_no_revspec(self):
         base_branch = BaseRecipeBranch("base_url", "1", 0.1)
         base_branch.commit = "base_revid"