launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21829
[Merge] lp:~cjwatson/launchpad-buildd/translation-refactor-chdir into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/translation-refactor-chdir into lp:launchpad-buildd.
Commit message:
Refactor lpbuildd.pottery.intltool to avoid calling chdir.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/translation-refactor-chdir/+merge/330415
This will make it easier to convert to the Backend infrastructure, where the caller's current directory is meaningless.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/translation-refactor-chdir into lp:launchpad-buildd.
=== modified file 'debian/changelog'
--- debian/changelog 2017-09-08 00:42:04 +0000
+++ debian/changelog 2017-09-08 12:45:03 +0000
@@ -1,3 +1,9 @@
+launchpad-buildd (152) UNRELEASED; urgency=medium
+
+ * Refactor lpbuildd.pottery.intltool to avoid calling chdir.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx> Fri, 08 Sep 2017 13:42:17 +0100
+
launchpad-buildd (151) xenial; urgency=medium
* Run snapd with SNAPPY_STORE_NO_CDN=1, since the buildd network isn't
=== modified file 'lpbuildd/pottery/intltool.py'
--- lpbuildd/pottery/intltool.py 2017-05-10 11:26:30 +0000
+++ lpbuildd/pottery/intltool.py 2017-09-08 12:45:03 +0000
@@ -13,22 +13,23 @@
'find_potfiles_in',
]
-from contextlib import contextmanager
import errno
import os.path
import re
-from subprocess import call
-
-
-def find_potfiles_in():
+import subprocess
+
+
+def find_potfiles_in(package_dir):
"""Search the current directory and its subdirectories for POTFILES.in.
- :returns: A list of names of directories that contain a file POTFILES.in.
+ :param package_dir: The directory to search.
+ :returns: A list of names of directories that contain a file
+ POTFILES.in, relative to `package_dir`.
"""
result_dirs = []
- for dirpath, dirnames, dirfiles in os.walk("."):
+ for dirpath, dirnames, dirfiles in os.walk(package_dir):
if "POTFILES.in" in dirfiles:
- result_dirs.append(dirpath)
+ result_dirs.append(os.path.relpath(dirpath, package_dir))
return result_dirs
@@ -51,51 +52,43 @@
POTFILES.in. True if all went fine and all files in POTFILES.in
actually exist.
"""
- current_path = os.getcwd()
-
- try:
- os.chdir(path)
- except OSError as e:
- # Abort nicely if the directory does not exist.
- if e.errno == errno.ENOENT:
- return False
- raise
- try:
- # Remove stale files from a previous run of intltool-update -m.
- for unlink_name in ['missing', 'notexist']:
- try:
- os.unlink(unlink_name)
- except OSError as e:
- # It's ok if the files are missing.
- if e.errno != errno.ENOENT:
- raise
- devnull = open("/dev/null", "w")
- returncode = call(
- ["/usr/bin/intltool-update", "-m"],
- stdout=devnull, stderr=devnull)
- devnull.close()
- finally:
- os.chdir(current_path)
-
- if returncode != 0:
- # An error occurred when executing intltool-update.
+ # Abort nicely if the directory does not exist.
+ if not os.path.isdir(path):
return False
+ # Remove stale files from a previous run of intltool-update -m.
+ for unlink_name in ['missing', 'notexist']:
+ try:
+ os.unlink(os.path.join(path, unlink_name))
+ except OSError as e:
+ # It's ok if the files are missing.
+ if e.errno != errno.ENOENT:
+ raise
+ with open("/dev/null", "w") as devnull:
+ try:
+ subprocess.check_call(
+ ["/usr/bin/intltool-update", "-m"],
+ stdout=devnull, stderr=devnull, cwd=path)
+ except subprocess.CalledProcessError:
+ return False
notexist = os.path.join(path, "notexist")
return not os.access(notexist, os.R_OK)
-def find_intltool_dirs():
+def find_intltool_dirs(package_dir):
"""Search for directories with intltool structure.
- The current directory and its subdiretories are searched. An 'intltool
+ `package_dir` and its subdirectories are searched. An 'intltool
structure' is a directory that contains a POFILES.in file and where all
files listed in that POTFILES.in do actually exist. The latter
condition makes sure that the file is not stale.
- :returns: A list of directory names.
+ :param package_dir: The directory to search.
+ :returns: A list of directory names, relative to `package_dir`.
"""
- return sorted(filter(check_potfiles_in, find_potfiles_in()))
+ return sorted(
+ podir for podir in find_potfiles_in(package_dir)
+ if check_potfiles_in(os.path.join(package_dir, podir)))
def _get_AC_PACKAGE_NAME(config_file):
@@ -188,14 +181,6 @@
return value
-@contextmanager
-def chdir(directory):
- cwd = os.getcwd()
- os.chdir(directory)
- yield
- os.chdir(cwd)
-
-
def generate_pot(podir, domain):
"""Generate one PO template using intltool.
@@ -212,26 +197,28 @@
"""
if domain is None or domain.strip() == "":
domain = "messages"
- with chdir(podir):
- with open("/dev/null", "w") as devnull:
- returncode = call(
+ with open("/dev/null", "w") as devnull:
+ try:
+ subprocess.check_call(
["/usr/bin/intltool-update", "-p", "-g", domain],
- stdout=devnull, stderr=devnull)
- return returncode == 0
-
-
-def generate_pots(package_dir='.'):
+ stdout=devnull, stderr=devnull, cwd=podir)
+ return True
+ except subprocess.CalledProcessError:
+ return False
+
+
+def generate_pots(package_dir):
"""Top-level function to generate all PO templates in a package."""
potpaths = []
- with chdir(package_dir):
- for podir in find_intltool_dirs():
- domain = get_translation_domain(podir)
- if generate_pot(podir, domain):
- potpaths.append(os.path.join(podir, domain + ".pot"))
+ for podir in find_intltool_dirs(package_dir):
+ full_podir = os.path.join(package_dir, podir)
+ domain = get_translation_domain(full_podir)
+ if generate_pot(full_podir, domain):
+ potpaths.append(os.path.join(podir, domain + ".pot"))
return potpaths
-class ConfigFile(object):
+class ConfigFile:
"""Represent a config file and return variables defined in it."""
def __init__(self, file_or_name):
@@ -294,7 +281,7 @@
style) or preceded by a $ sign with optional () (make style).
This class identifies a single such substitution in a variable text and
- extract the name of the variable who's value is to be inserted. It also
+ extracts the name of the variable whose value is to be inserted. It also
facilitates the actual replacement so that caller does not have to worry
about the substitution style that is being used.
"""
=== modified file 'lpbuildd/pottery/tests/test_intltool.py'
--- lpbuildd/pottery/tests/test_intltool.py 2017-05-10 11:26:30 +0000
+++ lpbuildd/pottery/tests/test_intltool.py 2017-09-08 12:45:03 +0000
@@ -35,111 +35,112 @@
def prepare_package(self, packagename, buildfiles=None):
"""Unpack the specified package in a temporary directory.
- Change into the package's directory.
+ Return the package's directory.
:param packagename: The name of the package to prepare.
:param buildfiles: A dictionary of path:content describing files to
add to the package.
"""
# First build the path for the package.
- cwd = os.getcwd()
- packagepath = os.path.join(
- cwd, os.path.dirname(__file__),
- self.test_data_dir, packagename + ".tar.bz2")
+ tarpath = os.path.join(
+ os.path.dirname(__file__), self.test_data_dir,
+ packagename + ".tar.bz2")
# Then change into the temporary directory and unpack it.
- os.chdir(self.useFixture(TempDir()).path)
- self.addCleanup(os.chdir, cwd)
- with tarfile.open(packagepath, "r|bz2") as tar:
- tar.extractall()
- os.chdir(packagename)
+ parent = self.useFixture(TempDir()).path
+ with tarfile.open(tarpath, "r|bz2") as tar:
+ tar.extractall(parent)
+ package_dir = os.path.join(parent, packagename)
if buildfiles is None:
- return
+ return package_dir
# Add files as requested.
for path, content in buildfiles.items():
directory = os.path.dirname(path)
if directory != '':
try:
- os.makedirs(directory)
+ os.makedirs(os.path.join(package_dir, directory))
except OSError as e:
# Doesn't matter if it already exists.
if e.errno != errno.EEXIST:
raise
- with open(path, 'w') as the_file:
+ with open(os.path.join(package_dir, path), 'w') as the_file:
the_file.write(content)
+ return package_dir
+
class TestDetectIntltool(TestCase, SetupTestPackageMixin):
def test_detect_potfiles_in(self):
# Find POTFILES.in in a package with multiple dirs when only one has
# POTFILES.in.
- self.prepare_package("intltool_POTFILES_in_1")
- dirs = find_potfiles_in()
- self.assertThat(dirs, MatchesSetwise(Equals("./po-intltool")))
+ package_dir = self.prepare_package("intltool_POTFILES_in_1")
+ dirs = find_potfiles_in(package_dir)
+ self.assertThat(dirs, MatchesSetwise(Equals("po-intltool")))
def test_detect_potfiles_in_module(self):
# Find POTFILES.in in a package with POTFILES.in at different levels.
- self.prepare_package("intltool_POTFILES_in_2")
- dirs = find_potfiles_in()
+ package_dir = self.prepare_package("intltool_POTFILES_in_2")
+ dirs = find_potfiles_in(package_dir)
self.assertThat(
- dirs, MatchesSetwise(Equals("./po"), Equals("./module1/po")))
+ dirs, MatchesSetwise(Equals("po"), Equals("module1/po")))
def test_check_potfiles_in_content_ok(self):
# Ideally all files listed in POTFILES.in exist in the source package.
- self.prepare_package("intltool_single_ok")
- self.assertTrue(check_potfiles_in("./po"))
+ package_dir = self.prepare_package("intltool_single_ok")
+ self.assertTrue(check_potfiles_in(os.path.join(package_dir, "po")))
def test_check_potfiles_in_content_ok_file_added(self):
# If a file is not listed in POTFILES.in, the file is still good for
# our purposes.
- self.prepare_package("intltool_single_ok")
- with open("./src/sourcefile_new.c", "w") as added_file:
+ package_dir = self.prepare_package("intltool_single_ok")
+ added_path = os.path.join(package_dir, "src/sourcefile_new.c")
+ with open(added_path, "w") as added_file:
added_file.write("/* Test file. */")
- self.assertTrue(check_potfiles_in("./po"))
+ self.assertTrue(check_potfiles_in(os.path.join(package_dir, "po")))
def test_check_potfiles_in_content_not_ok_file_removed(self):
# If a file is missing that is listed in POTFILES.in, the file
# intltool structure is probably broken and cannot be used for
# our purposes.
- self.prepare_package("intltool_single_ok")
- os.remove("./src/sourcefile1.c")
- self.assertFalse(check_potfiles_in("./po"))
+ package_dir = self.prepare_package("intltool_single_ok")
+ os.remove(os.path.join(package_dir, "src/sourcefile1.c"))
+ self.assertFalse(check_potfiles_in(os.path.join(package_dir, "po")))
def test_check_potfiles_in_wrong_directory(self):
# Passing in the wrong directory will cause the check to fail
# gracefully and return False.
- self.prepare_package("intltool_single_ok")
- self.assertFalse(check_potfiles_in("./foo"))
+ package_dir = self.prepare_package("intltool_single_ok")
+ self.assertFalse(check_potfiles_in(os.path.join(package_dir, "foo")))
def test_find_intltool_dirs(self):
# Complete run: find all directories with intltool structure.
- self.prepare_package("intltool_full_ok")
+ package_dir = self.prepare_package("intltool_full_ok")
self.assertEqual(
- ["./po-module1", "./po-module2"], find_intltool_dirs())
+ ["po-module1", "po-module2"], find_intltool_dirs(package_dir))
def test_find_intltool_dirs_broken(self):
# Complete run: part of the intltool structure is broken.
- self.prepare_package("intltool_full_ok")
- os.remove("./src/module1/sourcefile1.c")
+ package_dir = self.prepare_package("intltool_full_ok")
+ os.remove(os.path.join(package_dir, "src/module1/sourcefile1.c"))
self.assertEqual(
- ["./po-module2"], find_intltool_dirs())
+ ["po-module2"], find_intltool_dirs(package_dir))
class TestIntltoolDomain(TestCase, SetupTestPackageMixin):
def test_get_translation_domain_makevars(self):
# Find a translation domain in Makevars.
- self.prepare_package("intltool_domain_makevars")
+ package_dir = self.prepare_package("intltool_domain_makevars")
self.assertEqual(
"translationdomain",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makevars_subst_1(self):
# Find a translation domain in Makevars, substituted from
# Makefile.in.in.
- self.prepare_package(
+ package_dir = self.prepare_package(
"intltool_domain_base",
{
"po/Makefile.in.in": "PACKAGE=packagename-in-in\n",
@@ -147,12 +148,12 @@
})
self.assertEqual(
"packagename-in-in",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makevars_subst_2(self):
# Find a translation domain in Makevars, substituted from
# configure.ac.
- self.prepare_package(
+ package_dir = self.prepare_package(
"intltool_domain_base",
{
"configure.ac": "PACKAGE=packagename-ac\n",
@@ -161,21 +162,21 @@
})
self.assertEqual(
"packagename-ac",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makefile_in_in(self):
# Find a translation domain in Makefile.in.in.
- self.prepare_package("intltool_domain_makefile_in_in")
+ package_dir = self.prepare_package("intltool_domain_makefile_in_in")
self.assertEqual(
"packagename-in-in",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_ac(self):
# Find a translation domain in configure.ac.
- self.prepare_package("intltool_domain_configure_ac")
+ package_dir = self.prepare_package("intltool_domain_configure_ac")
self.assertEqual(
"packagename-ac",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def prepare_ac_init(self, parameters):
# Prepare test for various permutations of AC_INIT parameters
@@ -183,7 +184,7 @@
AC_INIT(%s)
GETTEXT_PACKAGE=AC_PACKAGE_NAME
""") % parameters
- self.prepare_package(
+ return self.prepare_package(
"intltool_domain_base",
{
"configure.ac": configure_ac_content,
@@ -191,161 +192,166 @@
def test_get_translation_domain_configure_ac_init(self):
# Find a translation domain in configure.ac in AC_INIT.
- self.prepare_ac_init("packagename-ac-init, 1.0, http://bug.org";)
+ package_dir = self.prepare_ac_init(
+ "packagename-ac-init, 1.0, http://bug.org";)
self.assertEqual(
"packagename-ac-init",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_ac_init_single_param(self):
# Find a translation domain in configure.ac in AC_INIT.
- self.prepare_ac_init("[Just 1 param]")
- self.assertIsNone(get_translation_domain("po"))
+ package_dir = self.prepare_ac_init("[Just 1 param]")
+ self.assertIsNone(
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_ac_init_brackets(self):
# Find a translation domain in configure.ac in AC_INIT with brackets.
- self.prepare_ac_init("[packagename-ac-init], 1.0, http://bug.org";)
+ package_dir = self.prepare_ac_init(
+ "[packagename-ac-init], 1.0, http://bug.org";)
self.assertEqual(
"packagename-ac-init",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_ac_init_tarname(self):
# Find a translation domain in configure.ac in AC_INIT tar name
# parameter.
- self.prepare_ac_init(
+ package_dir = self.prepare_ac_init(
"[Package name], 1.0, http://bug.org, [package-tarname]")
self.assertEqual(
"package-tarname",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_ac_init_multiline(self):
# Find a translation domain in configure.ac in AC_INIT when it
# spans multiple lines.
- self.prepare_ac_init(
+ package_dir = self.prepare_ac_init(
"[packagename-ac-init],\n 1.0,\n http://bug.org";)
self.assertEqual(
"packagename-ac-init",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_ac_init_multiline_tarname(self):
# Find a translation domain in configure.ac in AC_INIT tar name
# parameter that is on a different line.
- self.prepare_ac_init(
+ package_dir = self.prepare_ac_init(
"[Package name], 1.0,\n http://bug.org, [package-tarname]")
self.assertEqual(
"package-tarname",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_in(self):
# Find a translation domain in configure.in.
- self.prepare_package("intltool_domain_configure_in")
+ package_dir = self.prepare_package("intltool_domain_configure_in")
self.assertEqual(
"packagename-in",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makefile_in_in_substitute(self):
# Find a translation domain in Makefile.in.in with substitution from
# configure.ac.
- self.prepare_package("intltool_domain_makefile_in_in_substitute")
+ package_dir = self.prepare_package(
+ "intltool_domain_makefile_in_in_substitute")
self.assertEqual(
"domainname-ac-in-in",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makefile_in_in_substitute_same_name(self):
# Find a translation domain in Makefile.in.in with substitution from
# configure.ac from a variable with the same name as in
# Makefile.in.in.
- self.prepare_package(
+ package_dir = self.prepare_package(
"intltool_domain_makefile_in_in_substitute_same_name")
self.assertEqual(
"packagename-ac-in-in",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makefile_in_in_substitute_same_file(self):
# Find a translation domain in Makefile.in.in with substitution from
# the same file.
- self.prepare_package(
+ package_dir = self.prepare_package(
"intltool_domain_makefile_in_in_substitute_same_file")
self.assertEqual(
"domain-in-in-in-in",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_makefile_in_in_substitute_broken(self):
# Find no translation domain in Makefile.in.in when the substitution
# cannot be fulfilled.
- self.prepare_package(
+ package_dir = self.prepare_package(
"intltool_domain_makefile_in_in_substitute_broken")
- self.assertIsNone(get_translation_domain("po"))
+ self.assertIsNone(
+ get_translation_domain(os.path.join(package_dir, "po")))
def test_get_translation_domain_configure_in_substitute_version(self):
# Find a translation domain in configure.in with Makefile-style
# substitution from the same file.
- self.prepare_package(
+ package_dir = self.prepare_package(
"intltool_domain_configure_in_substitute_version")
self.assertEqual(
"domainname-in42",
- get_translation_domain("po"))
+ get_translation_domain(os.path.join(package_dir, "po")))
class TestGenerateTemplates(TestCase, SetupTestPackageMixin):
def test_generate_pot(self):
# Generate a given PO template.
- self.prepare_package("intltool_full_ok")
+ package_dir = self.prepare_package("intltool_full_ok")
self.assertTrue(
- generate_pot("./po-module1", "module1"),
+ generate_pot(os.path.join(package_dir, "po-module1"), "module1"),
"PO template generation failed.")
- expected_path = "./po-module1/module1.pot"
+ expected_path = "po-module1/module1.pot"
self.assertTrue(
- os.access(expected_path, os.F_OK),
+ os.access(os.path.join(package_dir, expected_path), os.F_OK),
"Generated PO template '%s' not found." % expected_path)
def test_generate_pot_no_domain(self):
# Generate a generic PO template.
- self.prepare_package("intltool_full_ok")
+ package_dir = self.prepare_package("intltool_full_ok")
self.assertTrue(
- generate_pot("./po-module1", None),
+ generate_pot(os.path.join(package_dir, "po-module1"), None),
"PO template generation failed.")
- expected_path = "./po-module1/messages.pot"
+ expected_path = "po-module1/messages.pot"
self.assertTrue(
- os.access(expected_path, os.F_OK),
+ os.access(os.path.join(package_dir, expected_path), os.F_OK),
"Generated PO template '%s' not found." % expected_path)
def test_generate_pot_empty_domain(self):
# Generate a generic PO template.
- self.prepare_package("intltool_full_ok")
+ package_dir = self.prepare_package("intltool_full_ok")
self.assertTrue(
- generate_pot("./po-module1", ""),
+ generate_pot(os.path.join(package_dir, "po-module1"), ""),
"PO template generation failed.")
- expected_path = "./po-module1/messages.pot"
+ expected_path = "po-module1/messages.pot"
self.assertTrue(
- os.access(expected_path, os.F_OK),
+ os.access(os.path.join(package_dir, expected_path), os.F_OK),
"Generated PO template '%s' not found." % expected_path)
def test_generate_pot_not_intltool(self):
# Fail when not an intltool setup.
- self.prepare_package("intltool_full_ok")
+ package_dir = self.prepare_package("intltool_full_ok")
# Cripple the setup.
- os.remove("./po-module1/POTFILES.in")
+ os.remove(os.path.join(package_dir, "po-module1/POTFILES.in"))
self.assertFalse(
- generate_pot("./po-module1", "nothing"),
+ generate_pot(os.path.join(package_dir, "po-module1"), "nothing"),
"PO template generation should have failed.")
- not_expected_path = "./po-module1/nothing.pot"
+ not_expected_path = "po-module1/nothing.pot"
self.assertFalse(
- os.access(not_expected_path, os.F_OK),
+ os.access(os.path.join(package_dir, not_expected_path), os.F_OK),
"Not expected PO template '%s' generated." % not_expected_path)
def test_generate_pots(self):
# Generate all PO templates in the package.
- self.prepare_package("intltool_full_ok")
+ package_dir = self.prepare_package("intltool_full_ok")
expected_paths = [
- './po-module1/packagename-module1.pot',
- './po-module2/packagename-module2.pot',
+ 'po-module1/packagename-module1.pot',
+ 'po-module2/packagename-module2.pot',
]
- pots_list = generate_pots()
+ pots_list = generate_pots(package_dir)
self.assertEqual(expected_paths, pots_list)
for expected_path in expected_paths:
self.assertTrue(
- os.access(expected_path, os.F_OK),
+ os.access(os.path.join(package_dir, expected_path), os.F_OK),
"Generated PO template '%s' not found." % expected_path)
