Merge lp:~james-w/bzr-builder/fix-text-of-nest-parts into lp:bzr-builder

Proposed by James Westby
Status: Merged
Merged at revision: 107
Proposed branch: lp:~james-w/bzr-builder/fix-text-of-nest-parts
Merge into: lp:bzr-builder
Diff against target: 145 lines (+57/-25)
2 files modified
recipe.py (+45/-25)
tests/test_recipe.py (+12/-0)
To merge this branch: bzr merge lp:~james-w/bzr-builder/fix-text-of-nest-parts
Reviewer Review Type Date Requested Status
Paul Hummer Approve
bzr-builder developers Pending
Review via email: mp+37080@code.launchpad.net

This proposal supersedes a proposal from 2010-09-29.

Description of the change

Hi,

An oversight when we added nest-part meant that it wouldn't
produce the right recipe test/manifest for recipes using
that instruction.

As well as fixing the bug I changed the logic around so that
you would get an error rather than the wrong text, and it
will be more obvious that the change is needed if we add
more instructions.

Thanks,

James

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

This time without pushing to trunk.

Thanks,

James

Revision history for this message
Paul Hummer (rockstar) :
review: Approve
Revision history for this message
Andrew Bennetts (spiv) wrote :

James Westby wrote:
> James Westby has proposed merging lp:~james-w/bzr-builder/fix-text-of-nest-parts into lp:bzr-builder.
>
> Requested reviews:
> bzr-builder developers (bzr-builder-devs)
>
>
> Hi,
>
> An oversight when we added nest-part meant that it wouldn't
> produce the right recipe test/manifest for recipes using
> that instruction.

Oh, good catch. Thanks!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'recipe.py'
2--- recipe.py 2010-09-15 16:38:26 +0000
3+++ recipe.py 2010-09-29 22:51:45 +0000
4@@ -452,6 +452,8 @@
5 branch should be merged instead of nested.
6 """
7
8+ can_have_children = False
9+
10 def __init__(self, recipe_branch, nest_path=None):
11 self.recipe_branch = recipe_branch
12 self.nest_path = nest_path
13@@ -462,6 +464,15 @@
14 def as_tuple(self):
15 return (self.recipe_branch, self.nest_path)
16
17+ def _get_revid_part(self):
18+ if self.recipe_branch.revid is not None:
19+ revid_part = " revid:%s" % self.recipe_branch.revid
20+ elif self.recipe_branch.revspec is not None:
21+ revid_part = " %s" % self.recipe_branch.revspec
22+ else:
23+ revid_part = ""
24+ return revid_part
25+
26
27 class CommandInstruction(ChildBranch):
28
29@@ -474,6 +485,9 @@
30 if proc.returncode != 0:
31 raise CommandFailedError(self.nest_path)
32
33+ def as_text(self):
34+ return "%s %s" % (RUN_INSTRUCTION, self.nest_path)
35+
36
37 class MergeInstruction(ChildBranch):
38
39@@ -486,6 +500,12 @@
40 merge_branch(self.recipe_branch, tree_to, br_to,
41 possible_transports=possible_transports)
42
43+ def as_text(self):
44+ revid_part = self._get_revid_part()
45+ return "%s %s %s%s" % (
46+ MERGE_INSTRUCTION, self.recipe_branch.name,
47+ self.recipe_branch.url, revid_part)
48+
49
50 class NestPartInstruction(ChildBranch):
51
52@@ -498,14 +518,33 @@
53 nest_part_branch(self.recipe_branch, tree_to, br_to, self.subpath,
54 self.target_subdir)
55
56+ def as_text(self):
57+ revid_part = self._get_revid_part()
58+ if self.target_subdir is not None:
59+ target_revid_part = " %s%s" % (
60+ self.target_subdir, revid_part)
61+ else:
62+ target_revid_part = revid_part
63+ return "%s %s %s %s%s" % (
64+ NEST_PART_INSTRUCTION, self.recipe_branch.name,
65+ self.recipe_branch.url, self.subpath, target_revid_part)
66+
67
68 class NestInstruction(ChildBranch):
69
70+ can_have_children = True
71+
72 def apply(self, target_path, tree_to, br_to, possible_transports=None):
73 _build_inner_tree(self.recipe_branch,
74 target_path=os.path.join(target_path, self.nest_path),
75 possible_transports=possible_transports)
76
77+ def as_text(self):
78+ revid_part = self._get_revid_part()
79+ return "%s %s %s %s%s" % (
80+ NEST_INSTRUCTION, self.recipe_branch.name,
81+ self.recipe_branch.url, self.nest_path, revid_part)
82+
83
84 class RecipeBranch(object):
85 """A nested structure that represents a Recipe.
86@@ -680,31 +719,12 @@
87 def _add_child_branches_to_manifest(self, child_branches, indent_level):
88 manifest = ""
89 for instruction in child_branches:
90- child_branch = instruction.recipe_branch
91- nest_location = instruction.nest_path
92- if child_branch is None:
93- manifest += "%s%s %s\n" % (" " * indent_level, RUN_INSTRUCTION,
94- nest_location)
95- else:
96- if child_branch.revid is not None:
97- revid_part = " revid:%s" % child_branch.revid
98- elif child_branch.revspec is not None:
99- revid_part = " %s" % child_branch.revspec
100- else:
101- revid_part = ""
102- if nest_location is not None:
103- manifest += "%s%s %s %s %s%s\n" % \
104- (" " * indent_level, NEST_INSTRUCTION,
105- child_branch.name,
106- child_branch.url, nest_location,
107- revid_part)
108- manifest += self._add_child_branches_to_manifest(
109- child_branch.child_branches, indent_level+1)
110- else:
111- manifest += "%s%s %s %s%s\n" % \
112- (" " * indent_level, MERGE_INSTRUCTION,
113- child_branch.name,
114- child_branch.url, revid_part)
115+ manifest += "%s%s\n" % (
116+ " " * indent_level, instruction.as_text())
117+ if instruction.can_have_children:
118+ manifest += self._add_child_branches_to_manifest(
119+ instruction.recipe_branch.child_branches,
120+ indent_level+1)
121 return manifest
122
123
124
125=== modified file 'tests/test_recipe.py'
126--- tests/test_recipe.py 2010-09-29 21:54:48 +0000
127+++ tests/test_recipe.py 2010-09-29 22:51:45 +0000
128@@ -1075,6 +1075,18 @@
129 base_branch.nest_branch(".", nested_branch1)
130 self.assertRaises(RecipeParseError, str, base_branch)
131
132+ def test_with_nest_part(self):
133+ base_branch = BaseRecipeBranch("base_url", "1", 0.1)
134+ base_branch.revid = "base_revid"
135+ nested_branch1 = RecipeBranch("nested1", "nested1_url",
136+ revspec="tag:foo")
137+ base_branch.nest_part_branch(nested_branch1, "foo", "bar")
138+ manifest = base_branch.get_recipe_text()
139+ self.assertEqual("# bzr-builder format 0.1 deb-version 1\n"
140+ "base_url revid:base_revid\n"
141+ "nest-part nested1 nested1_url foo bar tag:foo\n",
142+ manifest)
143+
144
145 class RecipeBranchTests(TestCaseInTempDir):
146

Subscribers

People subscribed via source and target branches