launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/git-build-recipe:nest-part into git-build-recipe:master.

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Implement nest-part (LP: #1537579).  Not too difficult once I finally figured out the right sequence of filter-branch and subtree operations needed.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/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 1d0faa3..a961997 100644
--- a/gitbuildrecipe/recipe.py
+++ b/gitbuildrecipe/recipe.py
@@ -385,14 +385,28 @@ def nest_part_branch(child_branch, target_path, subpath, target_subdir=None):
         target_subdir = os.path.basename(subpath)
     # XXX should handle updating as well
     assert not os.path.exists(target_subdir)
+    target_subdir_parent = os.path.dirname(target_subdir)
+    if target_subdir_parent:
+        target_subdir_parent_path = os.path.join(
+            target_path, target_subdir_parent)
+        if not os.path.exists(target_subdir_parent_path):
+            os.makedirs(target_subdir_parent_path)
     child_branch.path = target_path
     ensure_remote(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.git_call("checkout", subtree_commit)
+    child_branch.git_call(
+        "branch", "-q", "-f", "tmp-gbr-nest-part", child_branch.commit)
+    if subpath != ".":
+        child_branch.git_call(
+            "filter-branch", "-f", "--original", "refs/git-build-recipe",
+            "--subdirectory-filter", subpath, "tmp-gbr-nest-part")
+    for original in child_branch.git_output(
+            "for-each-ref", "--format=%(refname)",
+            "refs/git-build-recipe").splitlines():
+        child_branch.git_call("update-ref", "-d", original)
+    child_branch.git_output(
+        "subtree", "add", "-q", "-P", target_subdir, "tmp-gbr-nest-part")
+    child_branch.git_call("branch", "-q", "-D", "tmp-gbr-nest-part")
 
 
 def _build_inner_tree(base_branch, target_path):
diff --git a/gitbuildrecipe/tests/test_recipe.py b/gitbuildrecipe/tests/test_recipe.py
index 298eea7..337b741 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.
@@ -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"