launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22245
[Merge] lp:~cjwatson/launchpad-buildd/ttb-git into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/ttb-git into lp:launchpad-buildd with lp:~cjwatson/launchpad-buildd/refactor-vcs as a prerequisite.
Commit message:
Convert translation templates builds to the new VCS mixin, thereby adding git support.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/ttb-git/+merge/341389
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/ttb-git into lp:launchpad-buildd.
=== modified file 'debian/changelog'
--- debian/changelog 2018-03-14 14:15:15 +0000
+++ debian/changelog 2018-03-14 14:15:15 +0000
@@ -7,6 +7,8 @@
* Allow optionally installing snapcraft as a snap (LP: #1737994).
* Refactor VCS operations from lpbuildd.target.build_snap out to a module
that can be used by other targets.
+ * Convert translation templates builds to the new VCS mixin, thereby
+ adding git support.
-- Colin Watson <cjwatson@xxxxxxxxxx> Fri, 02 Feb 2018 16:14:46 +0000
=== modified file 'lpbuildd/target/generate_translation_templates.py'
--- lpbuildd/target/generate_translation_templates.py 2017-11-10 22:13:03 +0000
+++ lpbuildd/target/generate_translation_templates.py 2018-03-14 14:15:15 +0000
@@ -10,6 +10,7 @@
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_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 @@
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"]
-
- 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.")
+ 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 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 @@
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
=== modified file 'lpbuildd/target/tests/test_generate_translation_templates.py'
--- lpbuildd/target/tests/test_generate_translation_templates.py 2017-10-27 08:08:49 +0000
+++ lpbuildd/target/tests/test_generate_translation_templates.py 2018-03-14 14:15:15 +0000
@@ -15,23 +15,37 @@
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.fakeslave import FakeMethod
-
-
-class MatchesCall(MatchesListwise):
-
- def __init__(self, *args, **kwargs):
- super(MatchesCall, self).__init__([
- Equals(args),
- ContainsDict(
- {name: Equals(value) for name, value in kwargs.items()})])
+
+
+class RanCommand(MatchesListwise):
+
+ 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 @@
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."
- args = [
- "generate-translation-templates",
- "--backend=fake", "--series=xenial", "--arch=amd64", "1",
- "lp://~my/translation/branch", 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"))
-
- def test_getBranch_dir(self):
- # If passed a branch directory, the template generation script
- # works directly in that directory.
- branch_dir = "/home/me/branch"
- args = [
- "generate-translation-templates",
- "--backend=fake", "--series=xenial", "--arch=amd64", "1",
- branch_dir, 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)
-
- def _createBranch(self, content_map=None):
- """Create a working branch.
-
- :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.
-
- :return: the URL of a fresh bzr branch.
+ 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
- branch_url = 'file://' + branch_path
- subprocess.check_call(['bzr', 'init', '-q'], cwd=branch_path)
-
- 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)
+ 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 <committer@xxxxxxxxxxx>"
+ with EnvironmentVariable("BZR_EMAIL", committer_id):
subprocess.check_call(
- ['bzr', 'add', '-q'] + list(content_map), cwd=branch_path)
- committer_id = 'Committer <committer@xxxxxxxxxxx>'
- with EnvironmentVariable('BZR_EMAIL', committer_id):
- subprocess.check_call(
- ['bzr', 'commit', '-q', '-m', 'Populating branch.'],
- cwd=branch_path)
-
- return branch_url
-
- 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_text = "Ceci n'est pas cet branch."
- branch_url = self._createBranch({'marker.txt': marker_text})
-
- args = [
- "generate-translation-templates",
- "--backend=uncontained", "--series=xenial", "--arch=amd64", "1",
- branch_url, self.result_name,
- ]
- generator = parse_args(args=args).operation
- generator._getBranch()
+ ["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", "committer@xxxxxxxxxxx"],
+ 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",
+ "--branch", "lp:foo", self.result_name,
+ ]
+ generator = parse_args(args=args).operation
+ generator.install()
+ self.assertThat(generator.backend.run.calls, MatchesListwise([
+ RanAptGet("install", "intltool", "bzr"),
+ ]))
+
+ def test_install_git(self):
+ args = [
+ "generate-translation-templates",
+ "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+ "--git-repository", "lp:foo", self.result_name,
+ ]
+ generator = parse_args(args=args).operation
+ generator.install()
+ self.assertThat(generator.backend.run.calls, MatchesListwise([
+ RanAptGet("install", "intltool", "git"),
+ ]))
+
+ 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)
+
+ 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)
+
+ marker_path = os.path.join(generator.branch_dir, 'marker.txt')
+ with open(marker_path) as marker_file:
+ self.assertEqual(marker_text, marker_file.read())
+
+ def test_fetch_git(self):
+ # fetch can retrieve branch contents from a Git repository.
+ marker_text = "Ceci n'est pas cet branch."
+ branch_path = self.make_branch_contents({'marker.txt': marker_text})
+ self.make_git_branch(branch_path)
+
+ 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)
+
+ marker_path = os.path.join(generator.branch_dir, 'marker.txt')
+ with open(marker_path) 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 = "Ceci n'est pas cet branch."
+ 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",
+ "--git-repository", branch_path, "--git-path", "next",
+ self.result_name,
+ ]
+ generator = parse_args(args=args).operation
+ generator.fetch(quiet=True)
marker_path = os.path.join(generator.branch_dir, 'marker.txt')
with open(marker_path) as marker_file:
@@ -136,22 +193,23 @@
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,24 +218,61 @@
args = [
"generate-translation-templates",
"--backend=fake", "--series=xenial", "--arch=amd64", "1",
- 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([
- MatchesCall(["apt-get", "-y", "install", "bzr", "intltool"]),
- MatchesCall(
- ["bzr", "export", "-q", "-d", "lp:~my/branch", branch_dir]),
- MatchesCall(
- ["rm", "-f",
- os.path.join(po_dir, "missing"),
- os.path.join(po_dir, "notexist")]),
- MatchesCall(["/usr/bin/intltool-update", "-m"], cwd=po_dir),
- MatchesCall(
- ["/usr/bin/intltool-update", "-p", "-g", "test"], cwd=po_dir),
- MatchesCall(
+ "--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"), "")
+ 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", "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")]),
+ 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"]),
]))
=== modified file 'lpbuildd/tests/test_translationtemplatesbuildmanager.py'
--- lpbuildd/tests/test_translationtemplatesbuildmanager.py 2017-09-08 15:57:18 +0000
+++ lpbuildd/tests/test_translationtemplatesbuildmanager.py 2018-03-14 14:15:15 +0000
@@ -82,7 +82,7 @@
'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(
=== modified file 'lpbuildd/translationtemplates.py'
--- lpbuildd/translationtemplates.py 2017-09-08 15:57:18 +0000
+++ lpbuildd/translationtemplates.py 2018-03-14 14:15:15 +0000
@@ -36,16 +36,28 @@
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.
Follow ups
