diff --git a/fluent/migrate/context.py b/fluent/migrate/context.py index 7cb0e023..2e74abbd 100644 --- a/fluent/migrate/context.py +++ b/fluent/migrate/context.py @@ -23,7 +23,8 @@ def getParser(path): from .cldr import get_plural_categories from .transforms import Source from .merge import merge_resource -from .errors import NotSupportedError, UnreadableReferenceError +from .errors import ( + EmptyLocalizationError, NotSupportedError, UnreadableReferenceError) class MergeContext(object): @@ -114,6 +115,47 @@ def read_legacy_resource(self, path): # Transform the parsed result which is an iterator into a dict. return {entity.key: entity.val for entity in parser} + def read_reference_ftl(self, path): + """Read and parse a reference FTL file. + + A missing resource file is a fatal error and will raise an + UnreadableReferenceError. + """ + fullpath = os.path.join(self.reference_dir, path) + try: + return self.read_ftl_resource(fullpath) + except IOError as err: + error_message = 'Missing reference file: {}'.format(fullpath) + logging.getLogger('migrate').error(error_message) + raise UnreadableReferenceError(error_message) + except UnicodeDecodeError as err: + error_message = 'Error reading file {}: {}'.format(fullpath, err) + logging.getLogger('migrate').error(error_message) + raise UnreadableReferenceError(error_message) + + def read_localization_ftl(self, path): + """Read and parse an existing localization FTL file. + + Create a new FTL.Resource if the file doesn't exist or can't be + decoded. + """ + fullpath = os.path.join(self.localization_dir, path) + try: + return self.read_ftl_resource(fullpath) + except IOError: + logger = logging.getLogger('migrate') + logger.info( + 'Localization file {} does not exist and ' + 'it will be created'.format(path)) + return FTL.Resource() + except UnicodeDecodeError: + logger = logging.getLogger('migrate') + logger.warn( + 'Localization file {} has broken encoding. ' + 'It will be re-created and some translations ' + 'may be lost'.format(path)) + return FTL.Resource() + def maybe_add_localization(self, path): """Add a localization resource to migrate translations from. @@ -159,21 +201,8 @@ def get_sources(acc, cur): acc.add((cur.path, cur.key)) return acc - refpath = os.path.join(self.reference_dir, reference) - try: - ast = self.read_ftl_resource(refpath) - except IOError as err: - error_message = 'Missing reference file: {}'.format(refpath) - logging.getLogger('migrate').error(error_message) - raise UnreadableReferenceError(error_message) - except UnicodeDecodeError as err: - error_message = 'Error reading file {}: {}'.format(refpath, err) - logging.getLogger('migrate').error(error_message) - raise UnreadableReferenceError(error_message) - else: - # The reference file will be used by the merge function as - # a template for serializing the merge results. - self.reference_resources[target] = ast + reference_ast = self.read_reference_ftl(reference) + self.reference_resources[target] = reference_ast for node in transforms: # Scan `node` for `Source` nodes and collect the information they @@ -182,29 +211,33 @@ def get_sources(acc, cur): # Set these sources as dependencies for the current transform. self.dependencies[(target, node.id.name)] = dependencies - # Read all legacy translation files defined in Source transforms. + # Keep track of localization resource paths which were defined as + # sources in the transforms. + expected_paths = set() + + # Read all legacy translation files defined in Source transforms. This + # may fail but a single missing legacy resource doesn't mean that the + # migration can't succeed. + for dependencies in self.dependencies.values(): for path in set(path for path, _ in dependencies): + expected_paths.add(path) self.maybe_add_localization(path) + # However, if all legacy resources are missing, bail out early. There + # are no translations to migrate. We'd also get errors in hg annotate. + if len(expected_paths) > 0 and len(self.localization_resources) == 0: + error_message = 'No localization files were found' + logging.getLogger('migrate').error(error_message) + raise EmptyLocalizationError(error_message) + + # Add the current transforms to any other transforms added earlier for + # this path. path_transforms = self.transforms.setdefault(target, []) path_transforms += transforms if target not in self.localization_resources: - fullpath = os.path.join(self.localization_dir, target) - try: - ast = self.read_ftl_resource(fullpath) - except IOError: - logger = logging.getLogger('migrate') - logger.info( - 'Localization file {} does not exist and ' - 'it will be created'.format(target)) - except UnicodeDecodeError: - logger = logging.getLogger('migrate') - logger.warn( - 'Localization file {} will be re-created and some ' - 'translations might be lost'.format(target)) - else: - self.localization_resources[target] = ast + target_ast = self.read_localization_ftl(target) + self.localization_resources[target] = target_ast def get_source(self, path, key): """Get an entity value from a localized legacy source. @@ -259,7 +292,7 @@ def merge_changeset(self, changeset=None): } for path, reference in self.reference_resources.iteritems(): - current = self.localization_resources.get(path, FTL.Resource()) + current = self.localization_resources[path] transforms = self.transforms.get(path, []) def in_changeset(ident): diff --git a/fluent/migrate/errors.py b/fluent/migrate/errors.py index e52984f0..466ecf70 100644 --- a/fluent/migrate/errors.py +++ b/fluent/migrate/errors.py @@ -2,6 +2,10 @@ class MigrationError(ValueError): pass +class EmptyLocalizationError(MigrationError): + pass + + class NotSupportedError(MigrationError): pass diff --git a/tests/migrate/test_context.py b/tests/migrate/test_context.py index d47ad604..81d55af1 100644 --- a/tests/migrate/test_context.py +++ b/tests/migrate/test_context.py @@ -12,7 +12,8 @@ import fluent.syntax.ast as FTL -from fluent.migrate.errors import NotSupportedError, UnreadableReferenceError +from fluent.migrate.errors import ( + EmptyLocalizationError, NotSupportedError, UnreadableReferenceError) from fluent.migrate.util import ftl, ftl_resource_to_json, to_json from fluent.migrate.context import MergeContext from fluent.migrate.transforms import COPY @@ -250,7 +251,7 @@ def test_missing_reference_file(self): @unittest.skipUnless(compare_locales, 'compare-locales requried') -class TestIncompleteLocalization(unittest.TestCase): +class TestEmptyLocalization(unittest.TestCase): def setUp(self): # Silence all logging. logging.disable(logging.CRITICAL) @@ -261,34 +262,58 @@ def setUp(self): localization_dir=here('fixtures/pl') ) - self.ctx.add_transforms('toolbar.ftl', 'toolbar.ftl', [ - FTL.Message( - id=FTL.Identifier('urlbar-textbox'), - attributes=[ - FTL.Attribute( - id=FTL.Identifier('placeholder'), - value=COPY( - 'browser.dtd', - 'urlbar.placeholder2' - ) - ), - FTL.Attribute( - id=FTL.Identifier('accesskey'), - value=COPY( - 'browser.dtd', - 'urlbar.accesskey' - ) - ), - ] - ), - ]) + def tearDown(self): + # Resume logging. + logging.disable(logging.NOTSET) + + def test_all_localization_missing(self): + pattern = ('No localization files were found') + with self.assertRaisesRegexp(EmptyLocalizationError, pattern): + self.ctx.add_transforms('existing.ftl', 'existing.ftl', [ + FTL.Message( + id=FTL.Identifier('foo'), + value=COPY( + 'missing.dtd', + 'foo' + ) + ), + ]) + + +@unittest.skipUnless(compare_locales, 'compare-locales requried') +class TestIncompleteLocalization(unittest.TestCase): + def setUp(self): + # Silence all logging. + logging.disable(logging.CRITICAL) + + self.ctx = MergeContext( + lang='pl', + reference_dir=here('fixtures/en-US'), + localization_dir=here('fixtures/pl') + ) def tearDown(self): # Resume logging. logging.disable(logging.NOTSET) def test_missing_localization_file(self): - self.maxDiff = None + self.ctx.add_transforms('existing.ftl', 'existing.ftl', [ + FTL.Message( + id=FTL.Identifier('foo'), + value=COPY( + 'existing.dtd', + 'foo' + ) + ), + FTL.Message( + id=FTL.Identifier('bar'), + value=COPY( + 'missing.dtd', + 'bar' + ) + ), + ]) + self.assertDictEqual( to_json(self.ctx.merge_changeset()), {} diff --git a/tools/migrate/blame.py b/tools/migrate/blame.py index 4fb4e905..0afa4fbf 100644 --- a/tools/migrate/blame.py +++ b/tools/migrate/blame.py @@ -15,8 +15,8 @@ def __init__(self, client): def attribution(self, file_paths): args = cmdbuilder( - b('annotate'), template='json', date=True, user=True, - cwd=self.client.root(), file=map(b, file_paths)) + b('annotate'), *map(b, file_paths), template='json', + date=True, user=True, cwd=self.client.root()) blame_json = ''.join(self.client.rawcommand(args)) file_blames = json.loads(blame_json) diff --git a/tools/migrate/migrate-l10n.py b/tools/migrate/migrate-l10n.py index ce5835c5..7f1c9ced 100755 --- a/tools/migrate/migrate-l10n.py +++ b/tools/migrate/migrate-l10n.py @@ -23,7 +23,8 @@ def main(lang, reference_dir, localization_dir, migrations, dry_run): for migration in migrations: - print('Running migration {}'.format(migration.__name__)) + print('Running migration {} for {}'.format( + migration.__name__, lang)) # For each migration create a new context. ctx = MergeContext(lang, reference_dir, localization_dir) @@ -31,8 +32,10 @@ def main(lang, reference_dir, localization_dir, migrations, dry_run): try: # Add the migration spec. migration.migrate(ctx) - except MigrationError as err: - sys.exit(err.message) + except MigrationError: + print(' Skipping migration {} for {}'.format( + migration.__name__, lang)) + continue # Keep track of how many changesets we're committing. index = 0