launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/safe-v3 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


= Summary =
Update bzr-builder dependency to 0.5 or better, use --safe
flag.  0.5 or better enables 0.3 format, and thefore the nest-part instruction.

== Proposed fix ==
Same

== Pre-implementation notes ==
None

== Implementation details ==
None

== Tests ==
Tested interactively.  The 'run' command is safely rejected.

== Demo and Q/A ==
None.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/buildd/buildrecipe
  lib/canonical/buildd/debian/control

./lib/canonical/buildd/debian/control
      11: Line exceeds 78 characters.
-- 
https://code.launchpad.net/~abentley/launchpad/safe-v3/+merge/34465
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/safe-v3 into lp:launchpad/devel.

=== modified file 'lib/canonical/buildd/buildrecipe'
--- lib/canonical/buildd/buildrecipe	2010-08-20 17:23:15 +0000
+++ lib/canonical/buildd/buildrecipe	2010-09-02 19:11:23 +0000
@@ -55,8 +55,8 @@
         """
         # XXX: AaronBentley 2010-07-07 bug=602463: pbuilder uses aptitude but
         # does not depend on it.
-        return self.chroot(['apt-get', 'install', '-y', 'pbuilder',
-                            'bzr-builder', 'sudo', 'aptitude'])
+        return self.chroot([
+            'apt-get', 'install', '-y', 'pbuilder', 'aptitude'])
 
     def buildTree(self):
         """Build the recipe into a source tree.
@@ -65,37 +65,35 @@
         :return: a retcode from `bzr dailydeb`.
         """
         assert not os.path.exists(self.tree_path)
-        recipe_path_relative = os.path.join(self.work_dir_relative, 'recipe')
-        self.tree_path_relative = os.path.join(self.work_dir_relative, 'tree')
-        manifest_path_relative = os.path.join(
-            self.tree_path_relative, 'manifest')
-        recipe_path = os.path.join(self.chroot_path, recipe_path_relative[1:])
+        recipe_path = os.path.join(self.work_dir, 'recipe')
+        manifest_path = os.path.join(self.tree_path, 'manifest')
         recipe_file = open(recipe_path, 'rb')
         try:
             recipe = recipe_file.read()
         finally:
             recipe_file.close()
-        # As of bzr 2.2, a defined identity is needed.  In this case, we're using
-        # buildd@<hostname>.
+        # As of bzr 2.2, a defined identity is needed.  In this case, we're
+        # using buildd@<hostname>.
         hostname = socket.gethostname()
         bzr_email = 'buildd@%s' % hostname
 
         print 'Building recipe:'
         print recipe
-        retcode = self.chroot([
-            'sudo', '-i', '-u', self.username, 'DEBEMAIL=%s' % self.author_email,
-            'DEBFULLNAME=%s' % self.author_name.encode('utf-8'),
-            'BZR_EMAIL=%s' % bzr_email, 'bzr',
-            'dailydeb', '--no-build', recipe_path_relative,
-            self.tree_path_relative, '--manifest', manifest_path_relative,
-            '--append-version', '~%s1' % self.distroseries_name],
-            echo=True)
+        sys.stdout.flush()
+        env = {
+            'DEBEMAIL': self.author_email,
+            'DEBFULLNAME': self.author_name.encode('utf-8'),
+            'BZR_EMAIL': bzr_email}
+        retcode = call([
+            'bzr', 'dailydeb', '--safe', '--no-build', recipe_path,
+            self.tree_path, '--manifest', manifest_path,
+            '--append-version', '~%s1' % self.distroseries_name], env=env)
         if retcode != 0:
             return retcode
         (source,) = [name for name in os.listdir(self.tree_path)
                      if name != 'manifest']
-        self.source_dir_relative = os.path.join(self.tree_path_relative,
-                                                source)
+        self.source_dir_relative = os.path.join(
+            self.work_dir_relative, 'tree', source)
         return retcode
 
     def getPackageName(self):
@@ -163,10 +161,10 @@
 
 if __name__ == '__main__':
     builder = RecipeBuilder(*sys.argv[1:])
+    if builder.buildTree() != 0:
+        sys.exit(RETCODE_FAILURE_BUILD_TREE)
     if builder.install() != 0:
         sys.exit(RETCODE_FAILURE_INSTALL)
-    if builder.buildTree() != 0:
-        sys.exit(RETCODE_FAILURE_BUILD_TREE)
     if builder.installBuildDeps() != 0:
         sys.exit(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
     if builder.buildSourcePackage() != 0:

=== modified file 'lib/canonical/buildd/debian/control'
--- lib/canonical/buildd/debian/control	2010-07-22 16:15:58 +0000
+++ lib/canonical/buildd/debian/control	2010-09-02 19:11:23 +0000
@@ -8,7 +8,7 @@
 Package: launchpad-buildd
 Section: misc
 Architecture: all
-Depends: python-twisted-core, python-twisted-web, debootstrap, dpkg-dev, linux32, file, bzip2, sudo, ntpdate, adduser, apt-transport-https, lsb-release, apache2, ${misc:Depends}
+Depends: python-twisted-core, python-twisted-web, debootstrap, dpkg-dev, linux32, file, bzip2, sudo, ntpdate, adduser, apt-transport-https, lsb-release, apache2, bzr-builder (>=0.5), ${misc:Depends}
 Description: Launchpad buildd slave
  This is the launchpad buildd slave package. It contains everything needed to
  get a launchpad buildd going apart from the database manipulation required to