Merge lp:~jelmer/bzr-builder/lp-urls into lp:bzr-builder

[Jelmer Vernooij]

Support building directly from a Launchpad recipe URL.

This means that things like:

bzr dailydeb https://code.launchpad.net/~bzr/+recipe/bzr-stats-daily

now do the right thing

[James Westby (james-w)]

78 + if safe:
79 + permitted_instructions = SAFE_INSTRUCTIONS
80 + else:
81 + permitted_instructions = None

Would you move that above the opening of the file, so that it is outside
of the try block, so that block is clearly focused on the code that
might reasonably fail?

Thanks for the cool feature.

James

review: Approve

[Jelmer Vernooij (jelmer)]

On Wed, 2011-05-18 at 18:14 +0000, James Westby wrote:
> Review: Approve
> 78 + if safe:
> 79 + permitted_instructions = SAFE_INSTRUCTIONS
> 80 + else:
> 81 + permitted_instructions = None
>
> Would you move that above the opening of the file, so that it is outside
> of the try block, so that block is clearly focused on the code that
> might reasonably fail?
Done.

> Thanks for the cool feature.
Thanks!

Cheers,

Jelmer

=== modified file 'cmds.py'
--- cmds.py	2011-05-18 16:12:22 +0000
+++ cmds.py	2011-05-18 17:14:21 +0000
@@ -15,6 +15,7 @@
 
 """Subcommands provided by bzr-builder."""
 
+from StringIO import StringIO
 import datetime
 from email import utils
 import errno
@@ -35,8 +36,10 @@
 
 from bzrlib import (
         errors,
+        lazy_regex,
         trace,
-        transport,
+        transport as _mod_transport,
+        urlutils,
         )
 from bzrlib.commands import Command
 from bzrlib.option import Option
@@ -59,54 +62,53 @@
     pass
 
 
-def write_manifest_to_path(path, base_branch):
+def write_manifest_to_transport(location, base_branch):
     """Write a manifest to disk.
 
-    :param path: Path to write to
+    :param location: Location to write to
     :param base_branch: Recipe base branch
     """
-    parent_dir = os.path.dirname(path)
-    if parent_dir != '' and not os.path.exists(parent_dir):
-        os.makedirs(parent_dir)
-    manifest_f = open(path, 'wb')
-    try:
-        manifest_f.write(str(base_branch))
-    finally:
-        manifest_f.close()
-
-
-def get_branch_from_recipe_file(recipe_file, safe=False):
+    child_transport = _mod_transport.get_transport(location)
+    base_transport = child_transport.clone('..')
+    base_transport.create_prefix()
+    basename = base_transport.relpath(child_transport.base)
+    base_transport.put_bytes(basename, str(base_branch))
+
+
+def get_branch_from_recipe_location(recipe_location, safe=False,
+        possible_transports=None):
     """Return the base branch for the specified recipe.
 
-    :param recipe_file: The URL of the recipe file to retrieve.
+    :param recipe_location: The URL of the recipe file to retrieve.
     :param safe: if True, reject recipes that would cause arbitrary code
         execution.
     """
-    recipe_transport = transport.get_transport(os.path.dirname(recipe_file))
     try:
-        recipe_contents = recipe_transport.get_bytes(
-                os.path.basename(recipe_file))
+        (basename, f) = get_recipe_from_location(recipe_location, possible_transports)
     except errors.NoSuchFile:
         raise errors.BzrCommandError("Specified recipe does not exist: "
-                "%s" % recipe_file)
-    if safe:
-        permitted_instructions = SAFE_INSTRUCTIONS
-    else:
-        permitted_instructions = None
-    parser = RecipeParser(recipe_contents, filename=recipe_file)
+                "%s" % recipe_location)
+    try:
+        if safe:
+            permitted_instructions = SAFE_INSTRUCTIONS
+        else:
+            permitted_instructions = None
+        parser = RecipeParser(f, filename=recipe_location)
+    finally:
+        f.close()
     return parser.parse(permitted_instructions=permitted_instructions)
 
 
-def get_old_recipe(if_changed_from):
-    old_manifest_transport = transport.get_transport(os.path.dirname(
-                if_changed_from))
+def get_old_recipe(if_changed_from, possible_transports=None):
     try:
-        old_manifest_contents = old_manifest_transport.get_bytes(
-                os.path.basename(if_changed_from))
+        (basename, f) = get_recipe_from_location(if_changed_from, possible_transports)
     except errors.NoSuchFile:
         return None
-    old_recipe = RecipeParser(old_manifest_contents,
-            filename=if_changed_from).parse()
+    try:
+        old_recipe = RecipeParser(f,
+                filename=if_changed_from).parse()
+    finally:
+        f.close()
     return old_recipe
 
 
@@ -395,6 +397,53 @@
             "install the devscripts package.")
 
 
+launchpad_recipe_re = lazy_regex.lazy_compile(
+    r'^https://code.launchpad.net/~(.*)/\+recipe/(.*)$')
+
+
+def get_recipe_from_launchpad(username, recipe_name, location):
+    """Load a recipe from Launchpad.
+
+    :param username: The launchpad user name
+    :param recipe_name: Recipe name
+    :param location: Original location (used for error reporting)
+    :return: Text of the recipe
+    """
+    from launchpadlib.launchpad import Launchpad
+    lp = Launchpad.login_with("bzr-builder", "production")
+    try:
+        person = lp.people[username]
+    except KeyError:
+        raise errors.NoSuchFile(location,
+            "No such Launchpad user %s" % username)
+    recipe = person.getRecipe(name=recipe_name)
+    if recipe is None:
+        raise errors.NoSuchFile(location,
+            "Launchpad user %s has no recipe %s" % (
+            username, recipe_name))
+    return recipe.recipe_text
+
+
+def get_recipe_from_location(location, possible_transports=None):
+    """Open a recipe as a file-like object from a URL.
+
+    :param location: The recipe location
+    :param possible_transports: Possible transports to use
+    :return: Tuple with basename and file-like object
+    """
+    m = launchpad_recipe_re.match(location)
+    if m:
+        (username, recipe_name) = m.groups()
+        text = get_recipe_from_launchpad(username, recipe_name,
+            location)
+        return (recipe_name, StringIO(text))
+    child_transport = _mod_transport.get_transport(location,
+        possible_transports=possible_transports)
+    recipe_transport = child_transport.clone('..')
+    basename = recipe_transport.relpath(child_transport.base)
+    return basename, recipe_transport.get(basename)
+
+
 class cmd_build(Command):
     """Build a tree based on a 'recipe'.
 
@@ -402,7 +451,7 @@
 
     See "bzr help builder" for more information on what a recipe is.
     """
-    takes_args = ["recipe_file", "working_directory"]
+    takes_args = ["recipe_location", "working_directory"]
     takes_options = [
             Option('manifest', type=str, argname="path",
                    help="Path to write the manifest to."),
@@ -411,24 +460,25 @@
                         "to that specified in the specified manifest."),
                     ]
 
-    def _get_prepared_branch_from_recipe(self, recipe_file,
-            if_changed_from=None, safe=False):
+    def _get_prepared_branch_from_recipe(self, recipe_location,
+            if_changed_from=None, safe=False, possible_transports=None):
         """Common code to prepare a branch and do substitutions.
 
-        :param recipe_file: a path to a recipe file to work from.
-        :param if_changed_from: an optional path to a manifest to
+        :param recipe_location: a path to a recipe file to work from.
+        :param if_changed_from: an optional location of a manifest to
             compare the recipe against.
         :param safe: if True, reject recipes that would cause arbitrary code
             execution.
         :return: A tuple with (retcode, base_branch). If retcode is None
             then the command execution should continue.
         """
-        base_branch = get_branch_from_recipe_file(recipe_file, safe=safe)
+        base_branch = get_branch_from_recipe_location(recipe_location, safe=safe,
+            possible_transports=possible_transports)
         time = datetime.datetime.utcnow()
         base_branch.substitute_time(time)
         old_recipe = None
         if if_changed_from is not None:
-            old_recipe = get_old_recipe(if_changed_from)
+            old_recipe = get_old_recipe(if_changed_from, possible_transports)
         # Save the unsubstituted version for dailydeb.
         self._template_version = base_branch.deb_version
         changed = resolve_revisions(base_branch, if_changed_from=old_recipe)
@@ -437,16 +487,17 @@
             return 0, base_branch
         return None, base_branch
 
-    def run(self, recipe_file, working_directory, manifest=None,
+    def run(self, recipe_location, working_directory, manifest=None,
             if_changed_from=None):
-        result, base_branch = self._get_prepared_branch_from_recipe(recipe_file,
-            if_changed_from=if_changed_from)
+        possible_transports = []
+        result, base_branch = self._get_prepared_branch_from_recipe(recipe_location,
+            if_changed_from=if_changed_from, possible_transports=possible_transports)
         if result is not None:
             return result
         manifest_path = manifest or os.path.join(working_directory,
                         "bzr-builder.manifest")
         build_tree(base_branch, working_directory)
-        write_manifest_to_path(manifest_path, base_branch)
+        write_manifest_to_transport(manifest_path, base_branch)
 
 
 def debian_source_package_name(control_path):
@@ -497,9 +548,9 @@
                        " arbitrary code execution."),
             ]
 
-    takes_args = ["recipe_file", "working_basedir?"]
+    takes_args = ["recipe_location", "working_basedir?"]
 
-    def run(self, recipe_file, working_basedir=None, manifest=None,
+    def run(self, recipe_location, working_basedir=None, manifest=None,
             if_changed_from=None, package=None, distribution=None,
             dput=None, key_id=None, no_build=None, watch_ppa=False,
             append_version=None, safe=False):
@@ -515,8 +566,10 @@
                 # Check we can calculate a PPA url.
                 target_from_dput(dput)
 
-        result, base_branch = self._get_prepared_branch_from_recipe(recipe_file,
-            if_changed_from=if_changed_from, safe=safe)
+        possible_transports = []
+        result, base_branch = self._get_prepared_branch_from_recipe(recipe_location,
+            if_changed_from=if_changed_from, safe=safe,
+            possible_transports=possible_transports)
         if result is not None:
             return result
         if working_basedir is None:
@@ -526,7 +579,7 @@
             temp_dir = None
             if not os.path.exists(working_basedir):
                 os.makedirs(working_basedir)
-        package_name = self._calculate_package_name(recipe_file, package)
+        package_name = self._calculate_package_name(recipe_location, package)
         working_directory = os.path.join(working_basedir,
             "%s-%s" % (package_name, self._template_version))
         try:
@@ -544,7 +597,7 @@
                     raise errors.BzrCommandError("Missing debian/control file to "
                         "read package name from.")
                 package = debian_source_package_name(control_path)
-            write_manifest_to_path(manifest_path, base_branch)
+            write_manifest_to_transport(manifest_path, base_branch)
             # Add changelog also substitutes {debupstream}.
             add_changelog_entry(base_branch, working_directory,
                 distribution=distribution, package=package,
@@ -556,7 +609,7 @@
             os.rename(working_directory, package_dir)
             if no_build:
                 if manifest is not None:
-                    write_manifest_to_path(manifest, base_branch)
+                    write_manifest_to_transport(manifest, base_branch)
                 return 0
             try:
                 build_source_package(package_dir)
@@ -570,7 +623,7 @@
                 os.rename(package_dir, working_directory)
             # Note that this may write a second manifest.
             if manifest is not None:
-                write_manifest_to_path(manifest, base_branch)
+                write_manifest_to_transport(manifest, base_branch)
         finally:
             if temp_dir is not None:
                 shutil.rmtree(temp_dir)
@@ -580,9 +633,9 @@
             if not watch(owner, archive, package_name, base_branch.deb_version):
                 return 2
 
-    def _calculate_package_name(self, recipe_file, package):
+    def _calculate_package_name(self, recipe_location, package):
         """Calculate the directory name that should be used while debuilding."""
-        recipe_name = os.path.basename(recipe_file)
+        recipe_name = urlutils.basename(recipe_location)
         if recipe_name.endswith(".recipe"):
             recipe_name = recipe_name[:-len(".recipe")]
         return package or recipe_name

=== modified file 'ppa.py'
--- ppa.py	2011-03-28 10:31:13 +0000
+++ ppa.py	2011-05-18 17:14:21 +0000
@@ -14,25 +14,17 @@
 # You should have received a copy of the GNU General Public License along
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
 import time
 
-
-from launchpadlib.launchpad import (
-    Launchpad,
-    EDGE_SERVICE_ROOT,
-    )
-from launchpadlib.credentials import Credentials
-
 from bzrlib import (
     errors,
     trace,
     )
+from launchpadlib.launchpad import Launchpad
 
 
 def get_lp():
-    oauth_file = os.path.expanduser('~/.cache/launchpadlib/bzr-builder')
-    return Launchpad.login_with('bzr-builder', 'production', credentials_file=oauth_file)
+    return Launchpad.login_with('bzr-builder', 'production')
 
 
 def watch(owner_name, archive_name, package_name, version):

=== modified file 'tests/test_blackbox.py'
--- tests/test_blackbox.py	2011-05-18 16:12:22 +0000
+++ tests/test_blackbox.py	2011-05-18 17:14:21 +0000
@@ -41,7 +41,7 @@
     def test_cmd_builder_requires_recipe_file_argument(self):
         err = self.run_bzr("build", retcode=3)[1]
         self.assertEqual("bzr: ERROR: command 'build' requires argument "
-                "RECIPE_FILE\n", err)
+                "RECIPE_LOCATION\n", err)
 
     def test_cmd_builder_requires_working_dir_argument(self):
         err = self.run_bzr("build recipe", retcode=3)[1]