bzr-builder

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

  1. fix-text-of-nest-parts
  2. Merge into trunk
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
=== modified file 'recipe.py'
--- recipe.py 2010-09-15 16:38:26 +0000
+++ recipe.py 2010-09-29 22:51:45 +0000
@@ -452,6 +452,8 @@
452 branch should be merged instead of nested.452 branch should be merged instead of nested.
453 """453 """
454454
455 can_have_children = False
456
455 def __init__(self, recipe_branch, nest_path=None):457 def __init__(self, recipe_branch, nest_path=None):
456 self.recipe_branch = recipe_branch458 self.recipe_branch = recipe_branch
457 self.nest_path = nest_path459 self.nest_path = nest_path
@@ -462,6 +464,15 @@
462 def as_tuple(self):464 def as_tuple(self):
463 return (self.recipe_branch, self.nest_path)465 return (self.recipe_branch, self.nest_path)
464466
467 def _get_revid_part(self):
468 if self.recipe_branch.revid is not None:
469 revid_part = " revid:%s" % self.recipe_branch.revid
470 elif self.recipe_branch.revspec is not None:
471 revid_part = " %s" % self.recipe_branch.revspec
472 else:
473 revid_part = ""
474 return revid_part
475
465476
466class CommandInstruction(ChildBranch):477class CommandInstruction(ChildBranch):
467478
@@ -474,6 +485,9 @@
474 if proc.returncode != 0:485 if proc.returncode != 0:
475 raise CommandFailedError(self.nest_path)486 raise CommandFailedError(self.nest_path)
476487
488 def as_text(self):
489 return "%s %s" % (RUN_INSTRUCTION, self.nest_path)
490
477491
478class MergeInstruction(ChildBranch):492class MergeInstruction(ChildBranch):
479493
@@ -486,6 +500,12 @@
486 merge_branch(self.recipe_branch, tree_to, br_to,500 merge_branch(self.recipe_branch, tree_to, br_to,
487 possible_transports=possible_transports)501 possible_transports=possible_transports)
488502
503 def as_text(self):
504 revid_part = self._get_revid_part()
505 return "%s %s %s%s" % (
506 MERGE_INSTRUCTION, self.recipe_branch.name,
507 self.recipe_branch.url, revid_part)
508
489509
490class NestPartInstruction(ChildBranch):510class NestPartInstruction(ChildBranch):
491511
@@ -498,14 +518,33 @@
498 nest_part_branch(self.recipe_branch, tree_to, br_to, self.subpath,518 nest_part_branch(self.recipe_branch, tree_to, br_to, self.subpath,
499 self.target_subdir)519 self.target_subdir)
500520
521 def as_text(self):
522 revid_part = self._get_revid_part()
523 if self.target_subdir is not None:
524 target_revid_part = " %s%s" % (
525 self.target_subdir, revid_part)
526 else:
527 target_revid_part = revid_part
528 return "%s %s %s %s%s" % (
529 NEST_PART_INSTRUCTION, self.recipe_branch.name,
530 self.recipe_branch.url, self.subpath, target_revid_part)
531
501532
502class NestInstruction(ChildBranch):533class NestInstruction(ChildBranch):
503534
535 can_have_children = True
536
504 def apply(self, target_path, tree_to, br_to, possible_transports=None):537 def apply(self, target_path, tree_to, br_to, possible_transports=None):
505 _build_inner_tree(self.recipe_branch,538 _build_inner_tree(self.recipe_branch,
506 target_path=os.path.join(target_path, self.nest_path),539 target_path=os.path.join(target_path, self.nest_path),
507 possible_transports=possible_transports)540 possible_transports=possible_transports)
508541
542 def as_text(self):
543 revid_part = self._get_revid_part()
544 return "%s %s %s %s%s" % (
545 NEST_INSTRUCTION, self.recipe_branch.name,
546 self.recipe_branch.url, self.nest_path, revid_part)
547
509548
510class RecipeBranch(object):549class RecipeBranch(object):
511 """A nested structure that represents a Recipe.550 """A nested structure that represents a Recipe.
@@ -680,31 +719,12 @@
680 def _add_child_branches_to_manifest(self, child_branches, indent_level):719 def _add_child_branches_to_manifest(self, child_branches, indent_level):
681 manifest = ""720 manifest = ""
682 for instruction in child_branches:721 for instruction in child_branches:
683 child_branch = instruction.recipe_branch722 manifest += "%s%s\n" % (
684 nest_location = instruction.nest_path723 " " * indent_level, instruction.as_text())
685 if child_branch is None:724 if instruction.can_have_children:
686 manifest += "%s%s %s\n" % (" " * indent_level, RUN_INSTRUCTION,725 manifest += self._add_child_branches_to_manifest(
687 nest_location)726 instruction.recipe_branch.child_branches,
688 else:727 indent_level+1)
689 if child_branch.revid is not None:
690 revid_part = " revid:%s" % child_branch.revid
691 elif child_branch.revspec is not None:
692 revid_part = " %s" % child_branch.revspec
693 else:
694 revid_part = ""
695 if nest_location is not None:
696 manifest += "%s%s %s %s %s%s\n" % \
697 (" " * indent_level, NEST_INSTRUCTION,
698 child_branch.name,
699 child_branch.url, nest_location,
700 revid_part)
701 manifest += self._add_child_branches_to_manifest(
702 child_branch.child_branches, indent_level+1)
703 else:
704 manifest += "%s%s %s %s%s\n" % \
705 (" " * indent_level, MERGE_INSTRUCTION,
706 child_branch.name,
707 child_branch.url, revid_part)
708 return manifest728 return manifest
709729
710730
711731
=== modified file 'tests/test_recipe.py'
--- tests/test_recipe.py 2010-09-29 21:54:48 +0000
+++ tests/test_recipe.py 2010-09-29 22:51:45 +0000
@@ -1075,6 +1075,18 @@
1075 base_branch.nest_branch(".", nested_branch1)1075 base_branch.nest_branch(".", nested_branch1)
1076 self.assertRaises(RecipeParseError, str, base_branch)1076 self.assertRaises(RecipeParseError, str, base_branch)
10771077
1078 def test_with_nest_part(self):
1079 base_branch = BaseRecipeBranch("base_url", "1", 0.1)
1080 base_branch.revid = "base_revid"
1081 nested_branch1 = RecipeBranch("nested1", "nested1_url",
1082 revspec="tag:foo")
1083 base_branch.nest_part_branch(nested_branch1, "foo", "bar")
1084 manifest = base_branch.get_recipe_text()
1085 self.assertEqual("# bzr-builder format 0.1 deb-version 1\n"
1086 "base_url revid:base_revid\n"
1087 "nest-part nested1 nested1_url foo bar tag:foo\n",
1088 manifest)
1089
10781090
1079class RecipeBranchTests(TestCaseInTempDir):1091class RecipeBranchTests(TestCaseInTempDir):
10801092

Subscribers

People subscribed via source and target branches