Merge ~cjwatson/launchpad-buildd:ttb-git into launchpad-buildd:master

[Colin Watson]

Convert translation templates builds to the VCS mixin

This adds git support. There'll be a fair bit of work needed on the Launchpad side before this is useful, but we might as well get started here.

This is essentially the same as https://code.launchpad.net/~cjwatson/launchpad-buildd/ttb-git/+merge/341389, converted to git and rebased on master.

[Ioana Lasc (ilasc)]

review: Approve

diff --git a/debian/changelog b/debian/changelog
index b6abb76..867b35e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -19,6 +19,8 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
   * Only include mock in tests_require for Python 2.
   * Fix snap proxy testing for Python 3.
   * Rename [slave] configuration section to [builder].
+  * Convert translation templates builds to the VCS mixin, thereby adding
+    git support.
 
  -- Colin Watson <[e-mail]>  Tue, 28 Apr 2020 10:19:27 +0100
 
diff --git a/lpbuildd/target/generate_translation_templates.py b/lpbuildd/target/generate_translation_templates.py
index 3e44af7..1e708ba 100644
--- a/lpbuildd/target/generate_translation_templates.py
+++ b/lpbuildd/target/generate_translation_templates.py
@@ -10,6 +10,7 @@ import os.path
 
 from lpbuildd.pottery import intltool
 from lpbuildd.target.operation import Operation
+from lpbuildd.target.vcs import VCSOperationMixin
 
 
 logger = logging.getLogger(__name__)
@@ -19,7 +20,7 @@ RETCODE_FAILURE_INSTALL = 200
 RETCODE_FAILURE_BUILD = 201
 
 
-class GenerateTranslationTemplates(Operation):
+class GenerateTranslationTemplates(VCSOperationMixin, Operation):
     """Script to generate translation templates from a branch."""
 
     description = "Generate templates for a branch."
@@ -28,42 +29,24 @@ class GenerateTranslationTemplates(Operation):
     def add_arguments(cls, parser):
         super(GenerateTranslationTemplates, cls).add_arguments(parser)
         parser.add_argument(
-            "branch_spec", help=(
-                "A branch URL or the path of a local branch.  URLs are "
-                "recognised by the occurrence of ':'.  In the case of a URL, "
-                "this will make up a path for the branch and check out the "
-                "branch to there."))
-        parser.add_argument(
             "result_name",
-            help="The name of the result tarball.  Should end in '.tar.gz'.")
+            help="the name of the result tarball; should end in '.tar.gz'")
 
     def __init__(self, args, parser):
         super(GenerateTranslationTemplates, self).__init__(args, parser)
         self.work_dir = os.environ["HOME"]
+        self.branch_dir = os.path.join(self.work_dir, "source-tree")
+
+    def install(self):
+        logger.info("Installing dependencies...")
+        deps = ["intltool"]
+        deps.extend(self.vcs_deps)
+        self.backend.run(["apt-get", "-y", "install"] + deps)
 
-    def _getBranch(self):
-        """Set `self.branch_dir`, and check out branch if needed."""
-        if ':' in self.args.branch_spec:
-            # This is a branch URL.  Check out the branch.
-            self.branch_dir = os.path.join(self.work_dir, 'source-tree')
-            logger.info("Getting remote branch %s..." % self.args.branch_spec)
-            self._checkout(self.args.branch_spec)
-        else:
-            # This is a local filesystem path.  Use the branch in-place.
-            logger.info("Using local branch %s..." % self.args.branch_spec)
-            self.branch_dir = self.args.branch_spec
-
-    def _checkout(self, branch_url):
-        """Check out a source branch to generate from.
-
-        The branch is checked out to the location specified by
-        `self.branch_dir`.
-        """
-        logger.info(
-            "Exporting branch %s to %s..." % (branch_url, self.branch_dir))
-        self.backend.run(
-            ["bzr", "export", "-q", "-d", branch_url, self.branch_dir])
-        logger.info("Exporting branch done.")
+    def fetch(self, quiet=False):
+        logger.info("Fetching %s...", self.vcs_description)
+        self.vcs_fetch(
+            os.path.basename(self.branch_dir), cwd=self.work_dir, quiet=quiet)
 
     def _makeTarball(self, files):
         """Put the given files into a tarball in the working directory."""
@@ -78,21 +61,23 @@ class GenerateTranslationTemplates(Operation):
         self.backend.run(cmd)
         logger.info("Tarball generated.")
 
+    def generate(self):
+        logger.info("Generating templates...")
+        pots = intltool.generate_pots(self.backend, self.branch_dir)
+        logger.info("Generated %d templates." % len(pots))
+        if len(pots) > 0:
+            self._makeTarball(pots)
+
     def run(self):
         """Do It.  Generate templates."""
         try:
-            self.backend.run(["apt-get", "-y", "install", "bzr", "intltool"])
+            self.install()
         except Exception:
             logger.exception("Install failed")
             return RETCODE_FAILURE_INSTALL
         try:
-            logger.info(
-                "Generating templates for %s." % self.args.branch_spec)
-            self._getBranch()
-            pots = intltool.generate_pots(self.backend, self.branch_dir)
-            logger.info("Generated %d templates." % len(pots))
-            if len(pots) > 0:
-                self._makeTarball(pots)
+            self.fetch()
+            self.generate()
         except Exception:
             logger.exception("Build failed")
             return RETCODE_FAILURE_BUILD
diff --git a/lpbuildd/target/tests/test_generate_translation_templates.py b/lpbuildd/target/tests/test_generate_translation_templates.py
index 16f6848..864e595 100644
--- a/lpbuildd/target/tests/test_generate_translation_templates.py
+++ b/lpbuildd/target/tests/test_generate_translation_templates.py
@@ -15,23 +15,37 @@ from fixtures import (
 from testtools import TestCase
 from testtools.matchers import (
     ContainsDict,
-    EndsWith,
     Equals,
+    Is,
+    MatchesDict,
     MatchesListwise,
     MatchesSetwise,
     )
 
 from lpbuildd.target.cli import parse_args
-from lpbuildd.tests.fakebuilder import FakeMethod
 
 
-class MatchesCall(MatchesListwise):
+class RanCommand(MatchesListwise):
 
-    def __init__(self, *args, **kwargs):
-        super(MatchesCall, self).__init__([
-            Equals(args),
-            ContainsDict(
-                {name: Equals(value) for name, value in kwargs.items()})])
+    def __init__(self, args, get_output=None, echo=None, cwd=None, **env):
+        kwargs_matcher = {}
+        if get_output is not None:
+            kwargs_matcher["get_output"] = Is(get_output)
+        if echo is not None:
+            kwargs_matcher["echo"] = Is(echo)
+        if cwd:
+            kwargs_matcher["cwd"] = Equals(cwd)
+        if env:
+            kwargs_matcher["env"] = MatchesDict(
+                {key: Equals(value) for key, value in env.items()})
+        super(RanCommand, self).__init__(
+            [Equals((args,)), ContainsDict(kwargs_matcher)])
+
+
+class RanAptGet(RanCommand):
+
+    def __init__(self, *args):
+        super(RanAptGet, self).__init__(["apt-get", "-y"] + list(args))
 
 
 class TestGenerateTranslationTemplates(TestCase):
@@ -45,82 +59,125 @@ class TestGenerateTranslationTemplates(TestCase):
         self.useFixture(EnvironmentVariable("HOME", self.home_dir))
         self.logger = self.useFixture(FakeLogger())
 
-    def test_getBranch_url(self):
-        # If passed a branch URL, the template generation script will
-        # check out that branch into a directory called "source-tree."
+    def make_branch_contents(self, content_map):
+        """Create a directory with the contents of a working branch.
+
+        :param content_map: A dict mapping file names to file contents.
+            Each of these files with their contents will be written to the
+            branch.  Currently only supports writing files at the root
+            directory of the branch.
+        """
+        branch_path = self.useFixture(TempDir()).path
+        for name, contents in content_map.items():
+            with open(os.path.join(branch_path, name), 'wb') as f:
+                f.write(contents)
+        return branch_path
+
+    def make_bzr_branch(self, branch_path):
+        """Make a bzr branch from an existing directory."""
+        bzr_home = self.useFixture(TempDir()).path
+        self.useFixture(EnvironmentVariable("BZR_HOME", bzr_home))
+        self.useFixture(EnvironmentVariable("BZR_EMAIL"))
+        self.useFixture(EnvironmentVariable("EMAIL"))
+
+        subprocess.check_call(["bzr", "init", "-q"], cwd=branch_path)
+        subprocess.check_call(["bzr", "add", "-q"], cwd=branch_path)
+        committer_id = "Committer <[e-mail]>"
+        with EnvironmentVariable("BZR_EMAIL", committer_id):
+            subprocess.check_call(
+                ["bzr", "commit", "-q", "-m", "Populating branch."],
+                cwd=branch_path)
+
+    def make_git_branch(self, branch_path):
+        subprocess.check_call(["git", "init", "-q"], cwd=branch_path)
+        subprocess.check_call(
+            ["git", "config", "user.name", "Committer"], cwd=branch_path)
+        subprocess.check_call(
+            ["git", "config", "user.email", "[e-mail]"],
+            cwd=branch_path)
+        subprocess.check_call(["git", "add", "."], cwd=branch_path)
+        subprocess.check_call(
+            ["git", "commit", "-q", "--allow-empty",
+             "-m", "Populating branch"],
+            cwd=branch_path)
+
+    def test_install_bzr(self):
         args = [
             "generate-translation-templates",
             "--backend=fake", "--series=xenial", "--arch=amd64", "1",
-            "lp://~my/translation/branch", self.result_name,
+            "--branch", "lp:foo", self.result_name,
             ]
         generator = parse_args(args=args).operation
-        generator._checkout = FakeMethod()
-        generator._getBranch()
-
-        self.assertEqual(1, generator._checkout.call_count)
-        self.assertThat(generator.branch_dir, EndsWith("source-tree"))
+        generator.install()
+        self.assertThat(generator.backend.run.calls, MatchesListwise([
+            RanAptGet("install", "intltool", "bzr"),
+            ]))
 
-    def test_getBranch_dir(self):
-        # If passed a branch directory, the template generation script
-        # works directly in that directory.
-        branch_dir = "/home/me/branch"
+    def test_install_git(self):
         args = [
             "generate-translation-templates",
             "--backend=fake", "--series=xenial", "--arch=amd64", "1",
-            branch_dir, self.result_name,
+            "--git-repository", "lp:foo", self.result_name,
             ]
         generator = parse_args(args=args).operation
-        generator._checkout = FakeMethod()
-        generator._getBranch()
-
-        self.assertEqual(0, generator._checkout.call_count)
-        self.assertEqual(branch_dir, generator.branch_dir)
+        generator.install()
+        self.assertThat(generator.backend.run.calls, MatchesListwise([
+            RanAptGet("install", "intltool", "git"),
+            ]))
 
-    def _createBranch(self, content_map=None):
-        """Create a working branch.
+    def test_fetch_bzr(self):
+        # fetch can retrieve branch contents from a Bazaar branch.
+        marker_text = "Ceci n'est pas cet branch."
+        branch_path = self.make_branch_contents({'marker.txt': marker_text})
+        self.make_bzr_branch(branch_path)
 
-        :param content_map: optional dict mapping file names to file
-            contents.  Each of these files with their contents will be
-            written to the branch.  Currently only supports writing files at
-            the root directory of the branch.
+        args = [
+            "generate-translation-templates",
+            "--backend=uncontained", "--series=xenial", "--arch=amd64", "1",
+            "--branch", branch_path, self.result_name,
+            ]
+        generator = parse_args(args=args).operation
+        generator.fetch(quiet=True)
 
-        :return: the URL of a fresh bzr branch.
-        """
-        branch_path = self.useFixture(TempDir()).path
-        branch_url = 'file://' + branch_path
-        subprocess.check_call(['bzr', 'init', '-q'], cwd=branch_path)
+        marker_path = os.path.join(generator.branch_dir, 'marker.txt')
+        with open(marker_path) as marker_file:
+            self.assertEqual(marker_text, marker_file.read())
 
-        if content_map is not None:
-            for name, contents in content_map.items():
-                with open(os.path.join(branch_path, name), 'wb') as f:
-                    f.write(contents)
-            subprocess.check_call(
-                ['bzr', 'add', '-q'] + list(content_map), cwd=branch_path)
-            committer_id = 'Committer <[e-mail]>'
-            with EnvironmentVariable('BZR_EMAIL', committer_id):
-                subprocess.check_call(
-                    ['bzr', 'commit', '-q', '-m', 'Populating branch.'],
-                    cwd=branch_path)
+    def test_fetch_git(self):
+        # fetch can retrieve branch contents from a Git repository.
+        marker_text = b"Ceci n'est pas cet branch."
+        branch_path = self.make_branch_contents({'marker.txt': marker_text})
+        self.make_git_branch(branch_path)
 
-        return branch_url
+        args = [
+            "generate-translation-templates",
+            "--backend=uncontained", "--series=xenial", "--arch=amd64", "1",
+            "--git-repository", branch_path, self.result_name,
+            ]
+        generator = parse_args(args=args).operation
+        generator.fetch(quiet=True)
 
-    def test_getBranch_bzr(self):
-        # _getBranch can retrieve branch contents from a branch URL.
-        bzr_home = self.useFixture(TempDir()).path
-        self.useFixture(EnvironmentVariable('BZR_HOME', bzr_home))
-        self.useFixture(EnvironmentVariable('BZR_EMAIL'))
-        self.useFixture(EnvironmentVariable('EMAIL'))
+        marker_path = os.path.join(generator.branch_dir, 'marker.txt')
+        with open(marker_path, "rb") as marker_file:
+            self.assertEqual(marker_text, marker_file.read())
 
+    def test_fetch_git_with_path(self):
+        # fetch can retrieve branch contents from a Git repository and
+        # branch name.
         marker_text = b"Ceci n'est pas cet branch."
-        branch_url = self._createBranch({'marker.txt': marker_text})
+        branch_path = self.make_branch_contents({'marker.txt': marker_text})
+        self.make_git_branch(branch_path)
+        subprocess.call(
+            ["git", "branch", "-m", "master", "next"], cwd=branch_path)
 
         args = [
             "generate-translation-templates",
             "--backend=uncontained", "--series=xenial", "--arch=amd64", "1",
-            branch_url, self.result_name,
+            "--git-repository", branch_path, "--git-path", "next",
+            self.result_name,
             ]
         generator = parse_args(args=args).operation
-        generator._getBranch()
+        generator.fetch(quiet=True)
 
         marker_path = os.path.join(generator.branch_dir, 'marker.txt')
         with open(marker_path, "rb") as marker_file:
@@ -136,22 +193,23 @@ class TestGenerateTranslationTemplates(TestCase):
             potnames = [
                 member.name
                 for member in tar.getmembers() if not member.isdir()]
+        self.make_bzr_branch(branchdir)
 
         args = [
             "generate-translation-templates",
             "--backend=uncontained", "--series=xenial", "--arch=amd64", "1",
-            branchdir, self.result_name,
+            "--branch", branchdir, self.result_name,
             ]
         generator = parse_args(args=args).operation
-        generator._getBranch()
+        generator.fetch(quiet=True)
         generator._makeTarball(potnames)
         result_path = os.path.join(self.home_dir, self.result_name)
         with tarfile.open(result_path, 'r|*') as tar:
             tarnames = tar.getnames()
         self.assertThat(tarnames, MatchesSetwise(*(map(Equals, potnames))))
 
-    def test_run(self):
-        # Install dependencies and generate a templates tarball.
+    def test_run_bzr(self):
+        # Install dependencies and generate a templates tarball from Bazaar.
         branch_url = "lp:~my/branch"
         branch_dir = os.path.join(self.home_dir, "source-tree")
         po_dir = os.path.join(branch_dir, "po")
@@ -160,7 +218,40 @@ class TestGenerateTranslationTemplates(TestCase):
         args = [
             "generate-translation-templates",
             "--backend=fake", "--series=xenial", "--arch=amd64", "1",
-            branch_url, self.result_name,
+            "--branch", branch_url, self.result_name,
+            ]
+        generator = parse_args(args=args).operation
+        generator.backend.add_file(os.path.join(po_dir, "POTFILES.in"), "")
+        generator.backend.add_file(
+            os.path.join(po_dir, "Makevars"), "DOMAIN = test\n")
+        generator.run()
+        self.assertThat(generator.backend.run.calls, MatchesListwise([
+            RanAptGet("install", "intltool", "bzr"),
+            RanCommand(
+                ["bzr", "branch", "lp:~my/branch", "source-tree"],
+                cwd=self.home_dir, LANG="C.UTF-8", SHELL="/bin/sh"),
+            RanCommand(
+                ["rm", "-f",
+                 os.path.join(po_dir, "missing"),
+                 os.path.join(po_dir, "notexist")]),
+            RanCommand(["/usr/bin/intltool-update", "-m"], cwd=po_dir),
+            RanCommand(
+                ["/usr/bin/intltool-update", "-p", "-g", "test"], cwd=po_dir),
+            RanCommand(
+                ["tar", "-C", branch_dir, "-czf", result_path, "po/test.pot"]),
+            ]))
+
+    def test_run_git(self):
+        # Install dependencies and generate a templates tarball from Git.
+        repository_url = "lp:~my/repository"
+        branch_dir = os.path.join(self.home_dir, "source-tree")
+        po_dir = os.path.join(branch_dir, "po")
+        result_path = os.path.join(self.home_dir, self.result_name)
+
+        args = [
+            "generate-translation-templates",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--git-repository", repository_url, self.result_name,
             ]
         generator = parse_args(args=args).operation
         generator.backend.add_file(os.path.join(po_dir, "POTFILES.in"), b"")
@@ -168,16 +259,20 @@ class TestGenerateTranslationTemplates(TestCase):
             os.path.join(po_dir, "Makevars"), b"DOMAIN = test\n")
         generator.run()
         self.assertThat(generator.backend.run.calls, MatchesListwise([
-            MatchesCall(["apt-get", "-y", "install", "bzr", "intltool"]),
-            MatchesCall(
-                ["bzr", "export", "-q", "-d", "lp:~my/branch", branch_dir]),
-            MatchesCall(
+            RanAptGet("install", "intltool", "git"),
+            RanCommand(
+                ["git", "clone", "lp:~my/repository", "source-tree"],
+                cwd=self.home_dir, LANG="C.UTF-8", SHELL="/bin/sh"),
+            RanCommand(
+                ["git", "submodule", "update", "--init", "--recursive"],
+                cwd=branch_dir, LANG="C.UTF-8", SHELL="/bin/sh"),
+            RanCommand(
                 ["rm", "-f",
                  os.path.join(po_dir, "missing"),
                  os.path.join(po_dir, "notexist")]),
-            MatchesCall(["/usr/bin/intltool-update", "-m"], cwd=po_dir),
-            MatchesCall(
+            RanCommand(["/usr/bin/intltool-update", "-m"], cwd=po_dir),
+            RanCommand(
                 ["/usr/bin/intltool-update", "-p", "-g", "test"], cwd=po_dir),
-            MatchesCall(
+            RanCommand(
                 ["tar", "-C", branch_dir, "-czf", result_path, "po/test.pot"]),
             ]))
diff --git a/lpbuildd/tests/test_translationtemplatesbuildmanager.py b/lpbuildd/tests/test_translationtemplatesbuildmanager.py
index a47e5f8..196f4d8 100644
--- a/lpbuildd/tests/test_translationtemplatesbuildmanager.py
+++ b/lpbuildd/tests/test_translationtemplatesbuildmanager.py
@@ -88,7 +88,7 @@ class TestTranslationTemplatesBuildManagerIteration(TestCase):
             'generate-translation-templates',
             '--backend=chroot', '--series=xenial', '--arch=i386',
             self.buildid,
-            url, 'resultarchive',
+            '--branch', url, 'resultarchive',
             ]
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(
diff --git a/lpbuildd/translationtemplates.py b/lpbuildd/translationtemplates.py
index 2e778c5..36f4335 100644
--- a/lpbuildd/translationtemplates.py
+++ b/lpbuildd/translationtemplates.py
@@ -37,16 +37,28 @@ class TranslationTemplatesBuildManager(DebianBuildManager):
 
     def initiate(self, files, chroot, extra_args):
         """See `BuildManager`."""
-        self._branch_url = extra_args['branch_url']
+        self.branch = extra_args.get('branch')
+        # XXX cjwatson 2017-11-10: Backward-compatibility; remove once the
+        # manager passes branch instead.
+        if self.branch is None:
+            self.branch = extra_args['branch_url']
+        self.git_repository = extra_args.get("git_repository")
+        self.git_path = extra_args.get("git_path")
 
         super(TranslationTemplatesBuildManager, self).initiate(
             files, chroot, extra_args)
 
     def doGenerate(self):
         """Generate templates."""
-        self.runTargetSubProcess(
-            "generate-translation-templates",
-            self._branch_url, self._resultname)
+        args = []
+        if self.branch is not None:
+            args.extend(["--branch", self.branch])
+        if self.git_repository is not None:
+            args.extend(["--git-repository", self.git_repository])
+        if self.git_path is not None:
+            args.extend(["--git-path", self.git_path])
+        args.append(self._resultname)
+        self.runTargetSubProcess("generate-translation-templates", *args)
 
     # Satisfy DebianPackageManager's needs without having a misleading
     # method name here.