Skip to content

Commit 175f358

Browse files
authored
Merge pull request #39 from stasm/1435183-file
Bug 1435183 - Fix how files are passed to hg annotate
2 parents d308f1f + 5613345 commit 175f358

File tree

5 files changed

+127
-62
lines changed

5 files changed

+127
-62
lines changed

fluent/migrate/context.py

Lines changed: 66 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ def getParser(path):
2323
from .cldr import get_plural_categories
2424
from .transforms import Source
2525
from .merge import merge_resource
26-
from .errors import NotSupportedError, UnreadableReferenceError
26+
from .errors import (
27+
EmptyLocalizationError, NotSupportedError, UnreadableReferenceError)
2728

2829

2930
class MergeContext(object):
@@ -114,6 +115,47 @@ def read_legacy_resource(self, path):
114115
# Transform the parsed result which is an iterator into a dict.
115116
return {entity.key: entity.val for entity in parser}
116117

118+
def read_reference_ftl(self, path):
119+
"""Read and parse a reference FTL file.
120+
121+
A missing resource file is a fatal error and will raise an
122+
UnreadableReferenceError.
123+
"""
124+
fullpath = os.path.join(self.reference_dir, path)
125+
try:
126+
return self.read_ftl_resource(fullpath)
127+
except IOError as err:
128+
error_message = 'Missing reference file: {}'.format(fullpath)
129+
logging.getLogger('migrate').error(error_message)
130+
raise UnreadableReferenceError(error_message)
131+
except UnicodeDecodeError as err:
132+
error_message = 'Error reading file {}: {}'.format(fullpath, err)
133+
logging.getLogger('migrate').error(error_message)
134+
raise UnreadableReferenceError(error_message)
135+
136+
def read_localization_ftl(self, path):
137+
"""Read and parse an existing localization FTL file.
138+
139+
Create a new FTL.Resource if the file doesn't exist or can't be
140+
decoded.
141+
"""
142+
fullpath = os.path.join(self.localization_dir, path)
143+
try:
144+
return self.read_ftl_resource(fullpath)
145+
except IOError:
146+
logger = logging.getLogger('migrate')
147+
logger.info(
148+
'Localization file {} does not exist and '
149+
'it will be created'.format(path))
150+
return FTL.Resource()
151+
except UnicodeDecodeError:
152+
logger = logging.getLogger('migrate')
153+
logger.warn(
154+
'Localization file {} has broken encoding. '
155+
'It will be re-created and some translations '
156+
'may be lost'.format(path))
157+
return FTL.Resource()
158+
117159
def maybe_add_localization(self, path):
118160
"""Add a localization resource to migrate translations from.
119161
@@ -159,21 +201,8 @@ def get_sources(acc, cur):
159201
acc.add((cur.path, cur.key))
160202
return acc
161203

162-
refpath = os.path.join(self.reference_dir, reference)
163-
try:
164-
ast = self.read_ftl_resource(refpath)
165-
except IOError as err:
166-
error_message = 'Missing reference file: {}'.format(refpath)
167-
logging.getLogger('migrate').error(error_message)
168-
raise UnreadableReferenceError(error_message)
169-
except UnicodeDecodeError as err:
170-
error_message = 'Error reading file {}: {}'.format(refpath, err)
171-
logging.getLogger('migrate').error(error_message)
172-
raise UnreadableReferenceError(error_message)
173-
else:
174-
# The reference file will be used by the merge function as
175-
# a template for serializing the merge results.
176-
self.reference_resources[target] = ast
204+
reference_ast = self.read_reference_ftl(reference)
205+
self.reference_resources[target] = reference_ast
177206

178207
for node in transforms:
179208
# Scan `node` for `Source` nodes and collect the information they
@@ -182,29 +211,33 @@ def get_sources(acc, cur):
182211
# Set these sources as dependencies for the current transform.
183212
self.dependencies[(target, node.id.name)] = dependencies
184213

185-
# Read all legacy translation files defined in Source transforms.
214+
# Keep track of localization resource paths which were defined as
215+
# sources in the transforms.
216+
expected_paths = set()
217+
218+
# Read all legacy translation files defined in Source transforms. This
219+
# may fail but a single missing legacy resource doesn't mean that the
220+
# migration can't succeed.
221+
for dependencies in self.dependencies.values():
186222
for path in set(path for path, _ in dependencies):
223+
expected_paths.add(path)
187224
self.maybe_add_localization(path)
188225

226+
# However, if all legacy resources are missing, bail out early. There
227+
# are no translations to migrate. We'd also get errors in hg annotate.
228+
if len(expected_paths) > 0 and len(self.localization_resources) == 0:
229+
error_message = 'No localization files were found'
230+
logging.getLogger('migrate').error(error_message)
231+
raise EmptyLocalizationError(error_message)
232+
233+
# Add the current transforms to any other transforms added earlier for
234+
# this path.
189235
path_transforms = self.transforms.setdefault(target, [])
190236
path_transforms += transforms
191237

192238
if target not in self.localization_resources:
193-
fullpath = os.path.join(self.localization_dir, target)
194-
try:
195-
ast = self.read_ftl_resource(fullpath)
196-
except IOError:
197-
logger = logging.getLogger('migrate')
198-
logger.info(
199-
'Localization file {} does not exist and '
200-
'it will be created'.format(target))
201-
except UnicodeDecodeError:
202-
logger = logging.getLogger('migrate')
203-
logger.warn(
204-
'Localization file {} will be re-created and some '
205-
'translations might be lost'.format(target))
206-
else:
207-
self.localization_resources[target] = ast
239+
target_ast = self.read_localization_ftl(target)
240+
self.localization_resources[target] = target_ast
208241

209242
def get_source(self, path, key):
210243
"""Get an entity value from a localized legacy source.
@@ -259,7 +292,7 @@ def merge_changeset(self, changeset=None):
259292
}
260293

261294
for path, reference in self.reference_resources.iteritems():
262-
current = self.localization_resources.get(path, FTL.Resource())
295+
current = self.localization_resources[path]
263296
transforms = self.transforms.get(path, [])
264297

265298
def in_changeset(ident):

fluent/migrate/errors.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ class MigrationError(ValueError):
22
pass
33

44

5+
class EmptyLocalizationError(MigrationError):
6+
pass
7+
8+
59
class NotSupportedError(MigrationError):
610
pass
711

tests/migrate/test_context.py

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212

1313
import fluent.syntax.ast as FTL
1414

15-
from fluent.migrate.errors import NotSupportedError, UnreadableReferenceError
15+
from fluent.migrate.errors import (
16+
EmptyLocalizationError, NotSupportedError, UnreadableReferenceError)
1617
from fluent.migrate.util import ftl, ftl_resource_to_json, to_json
1718
from fluent.migrate.context import MergeContext
1819
from fluent.migrate.transforms import COPY
@@ -250,7 +251,7 @@ def test_missing_reference_file(self):
250251

251252

252253
@unittest.skipUnless(compare_locales, 'compare-locales requried')
253-
class TestIncompleteLocalization(unittest.TestCase):
254+
class TestEmptyLocalization(unittest.TestCase):
254255
def setUp(self):
255256
# Silence all logging.
256257
logging.disable(logging.CRITICAL)
@@ -261,34 +262,58 @@ def setUp(self):
261262
localization_dir=here('fixtures/pl')
262263
)
263264

264-
self.ctx.add_transforms('toolbar.ftl', 'toolbar.ftl', [
265-
FTL.Message(
266-
id=FTL.Identifier('urlbar-textbox'),
267-
attributes=[
268-
FTL.Attribute(
269-
id=FTL.Identifier('placeholder'),
270-
value=COPY(
271-
'browser.dtd',
272-
'urlbar.placeholder2'
273-
)
274-
),
275-
FTL.Attribute(
276-
id=FTL.Identifier('accesskey'),
277-
value=COPY(
278-
'browser.dtd',
279-
'urlbar.accesskey'
280-
)
281-
),
282-
]
283-
),
284-
])
265+
def tearDown(self):
266+
# Resume logging.
267+
logging.disable(logging.NOTSET)
268+
269+
def test_all_localization_missing(self):
270+
pattern = ('No localization files were found')
271+
with self.assertRaisesRegexp(EmptyLocalizationError, pattern):
272+
self.ctx.add_transforms('existing.ftl', 'existing.ftl', [
273+
FTL.Message(
274+
id=FTL.Identifier('foo'),
275+
value=COPY(
276+
'missing.dtd',
277+
'foo'
278+
)
279+
),
280+
])
281+
282+
283+
@unittest.skipUnless(compare_locales, 'compare-locales requried')
284+
class TestIncompleteLocalization(unittest.TestCase):
285+
def setUp(self):
286+
# Silence all logging.
287+
logging.disable(logging.CRITICAL)
288+
289+
self.ctx = MergeContext(
290+
lang='pl',
291+
reference_dir=here('fixtures/en-US'),
292+
localization_dir=here('fixtures/pl')
293+
)
285294

286295
def tearDown(self):
287296
# Resume logging.
288297
logging.disable(logging.NOTSET)
289298

290299
def test_missing_localization_file(self):
291-
self.maxDiff = None
300+
self.ctx.add_transforms('existing.ftl', 'existing.ftl', [
301+
FTL.Message(
302+
id=FTL.Identifier('foo'),
303+
value=COPY(
304+
'existing.dtd',
305+
'foo'
306+
)
307+
),
308+
FTL.Message(
309+
id=FTL.Identifier('bar'),
310+
value=COPY(
311+
'missing.dtd',
312+
'bar'
313+
)
314+
),
315+
])
316+
292317
self.assertDictEqual(
293318
to_json(self.ctx.merge_changeset()),
294319
{}

tools/migrate/blame.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ def __init__(self, client):
1515

1616
def attribution(self, file_paths):
1717
args = cmdbuilder(
18-
b('annotate'), template='json', date=True, user=True,
19-
cwd=self.client.root(), file=map(b, file_paths))
18+
b('annotate'), *map(b, file_paths), template='json',
19+
date=True, user=True, cwd=self.client.root())
2020
blame_json = ''.join(self.client.rawcommand(args))
2121
file_blames = json.loads(blame_json)
2222

tools/migrate/migrate-l10n.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,19 @@ def main(lang, reference_dir, localization_dir, migrations, dry_run):
2323

2424
for migration in migrations:
2525

26-
print('Running migration {}'.format(migration.__name__))
26+
print('Running migration {} for {}'.format(
27+
migration.__name__, lang))
2728

2829
# For each migration create a new context.
2930
ctx = MergeContext(lang, reference_dir, localization_dir)
3031

3132
try:
3233
# Add the migration spec.
3334
migration.migrate(ctx)
34-
except MigrationError as err:
35-
sys.exit(err.message)
35+
except MigrationError:
36+
print(' Skipping migration {} for {}'.format(
37+
migration.__name__, lang))
38+
continue
3639

3740
# Keep track of how many changesets we're committing.
3841
index = 0

0 commit comments

Comments
 (0)