bug 1361037, part 6: support multiple projects in one compare-locales run, r=flod, stas
authorAxel Hecht <axel@pike.org>
Tue, 16 May 2017 19:06:01 +0200
changeset 237 f9fb89c4d86ad2e1f6a84b24340ad1c88b74a8b1
parent 236 e2174b93de6d89a60208bfdc389d5c5b1c7eb33b
child 238 af79116532204027f849c5c673b1f0fad9e39b1c
push id48
push useraxel@mozilla.com
push dateFri, 26 May 2017 11:10:47 +0000
reviewersflod, stas
bugs1361037
bug 1361037, part 6: support multiple projects in one compare-locales run, r=flod, stas This adds a bit of horror and magic for argparse, as we're using nargs twice in one commandline, and we'll need to fixup the args from one list to the other. Turns out not too hard, and gives nice help. MozReview-Commit-ID: Enzh8Z10ra6
compare_locales/commands.py
compare_locales/compare.py
compare_locales/tests/test_merge.py
--- a/compare_locales/commands.py
+++ b/compare_locales/commands.py
@@ -1,20 +1,21 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 'Commands exposed to commandlines'
 
 import logging
 from argparse import ArgumentParser
+import os
 
 from compare_locales import version
 from compare_locales.paths import EnumerateApp
-from compare_locales.compare import compareProjects
+from compare_locales.compare import compareProjects, Observer
 
 
 class BaseCommand(object):
     """Base class for compare-locales commands.
     This handles command line parsing, and general sugar for setuptools
     entry_points.
     """
 
@@ -58,60 +59,78 @@ data in a json useful for Exhibit
     def handle_(self):
         """The instance part of the classmethod call."""
         self.parser = self.get_parser()
         args = self.parser.parse_args()
         # log as verbose or quiet as we want, warn by default
         logging.basicConfig()
         logging.getLogger().setLevel(logging.WARNING -
                                      (args.v - args.q) * 10)
-        observer = self.handle(args)
-        print observer.serialize(type=args.data).encode('utf-8', 'replace')
+        observers = self.handle(args)
+        for observer in observers:
+            print observer.serialize(type=args.data).encode('utf-8', 'replace')
 
     def handle(self, args):
         """Subclasses need to implement this method for the actual
         command handling.
         """
         raise NotImplementedError
 
 
 class CompareLocales(BaseCommand):
-    """Check the localization status of a gecko application.
-The first argument is a path to the l10n.ini file for the application,
+    """Check the localization status of gecko applications.
+The first arguments are paths to the l10n.ini files for the applications,
 followed by the base directory of the localization repositories.
 Then you pass in the list of locale codes you want to compare. If there are
 not locales given, the list of locales will be taken from the all-locales file
 of the application\'s l10n.ini."""
 
     def get_parser(self):
         parser = super(CompareLocales, self).get_parser()
-        parser.add_argument('ini_file', metavar='l10n.ini',
+        parser.add_argument('config', metavar='l10n.ini', nargs='+',
                             help='INI file for the project')
         parser.add_argument('l10n_base_dir', metavar='l10n-base-dir',
                             help='Parent directory of localizations')
         parser.add_argument('locales', nargs='*', metavar='locale-code',
                             help='Locale code and top-level directory of '
                                  'each localization')
+        parser.add_argument('--unified', action="store_true",
+                            help="Show output for all projects unified")
         parser.add_argument('--clobber-merge', action="store_true",
                             default=False, dest='clobber',
                             help="""WARNING: DATALOSS.
 Use this option with care. If specified, the merge directory will
 be clobbered for each module. That means, the subdirectory will
 be completely removed, any files that were there are lost.
 Be careful to specify the right merge directory when using this option.""")
-        parser.add_argument('-r', '--reference', default='en-US',
-                            dest='reference',
-                            help='Explicitly set the reference '
-                            'localization. [default: en-US]')
         self.add_data_argument(parser)
         return parser
 
     def handle(self, args):
-        app = EnumerateApp(args.ini_file, args.l10n_base_dir, args.locales)
-        project_config = app.asConfig()
+        # using nargs multiple times in argparser totally screws things
+        # up, repair that.
+        # First files are configs, then the base dir, everything else is
+        # locales
+        all_args = args.config + [args.l10n_base_dir] + args.locales
+        del args.config[:]
+        del args.locales[:]
+        while all_args and os.path.isfile(all_args[0]):
+            args.config.append(all_args.pop(0))
+        args.l10n_base_dir = all_args.pop(0)
+        args.locales.extend(all_args)
+        configs = []
+        for config_path in args.config:
+            app = EnumerateApp(config_path, args.l10n_base_dir, args.locales)
+            configs.append(app.asConfig())
         try:
-            observer = compareProjects(
-                project_config,
+            unified_observer = None
+            if args.unified:
+                unified_observer = Observer()
+            observers = compareProjects(
+                configs,
+                other_observer=unified_observer,
                 merge_stage=args.merge, clobber_merge=args.clobber)
         except (OSError, IOError), exc:
             print "FAIL: " + str(exc)
             self.parser.exit(2)
-        return observer
+        if args.unified:
+            return [unified_observer]
+        return observers
--- a/compare_locales/compare.py
+++ b/compare_locales/compare.py
@@ -186,16 +186,21 @@ class Observer(object):
     def toJSON(self):
         return dict(summary=self.getSummary(), details=self.details.toJSON())
 
     def notify(self, category, file, data):
         rv = "error"
         if category in self.stat_cats:
             # these get called post reporting just for stats
             # return "error" to forward them to other other_observers
+            # in multi-project scenarios, this file might not be ours,
+            # check that.
+            if (self.filter is not None and
+                    self.filter(file) in (None, 'ignore')):
+                return 'ignore'
             self.summary[file.locale][category] += data
             # keep track of how many strings are in a missing file
             # we got the {'missingFile': 'error'} from the first pass
             if category == 'missingInFiles':
                 self.details[file]['strings'] = data
             return "error"
         if category in ['missingFile', 'obsoleteFile']:
             if self.filter is not None:
@@ -332,17 +337,17 @@ class ContentComparer:
     nl = re.compile('\n', re.M)
 
     def __init__(self):
         '''Create a ContentComparer.
         observer is usually a instance of Observer. The return values
         of the notify method are used to control the handling of missing
         entities.
         '''
-        self.observer = Observer()
+        self.observers = []
         self.other_observers = []
         self.merge_stage = None
 
     def add_observer(self, obs):
         '''Add a non-filtering observer.
         Results from the notify calls are ignored.
         '''
         self.other_observers.append(obs)
@@ -389,23 +394,30 @@ class ContentComparer:
 
         f.write(''.join(map(ensureNewline, trailing)))
         f.close()
 
     def notify(self, category, file, data):
         """Check observer for the found data, and if it's
         not to ignore, notify other_observers.
         """
-        rv = self.observer.notify(category, file, data)
-        if rv == 'ignore':
-            return rv
+        rvs = set(
+            observer.notify(category, file, data)
+            for observer in self.observers
+            )
+        if all(rv == 'ignore' for rv in rvs):
+            return 'ignore'
+        rvs.discard('ignore')
         for obs in self.other_observers:
             # non-filtering other_observers, ignore results
             obs.notify(category, file, data)
-        return rv
+        if 'error' in rvs:
+            return 'error'
+        assert len(rvs) == 1
+        return rvs.pop()
 
     def remove(self, obsolete):
         self.notify('obsoleteFile', obsolete, None)
         pass
 
     def compare(self, ref_file, l10n, extra_tests=None):
         try:
             p = parser.getParser(ref_file.file)
@@ -543,24 +555,27 @@ class ContentComparer:
 
     def doChanged(self, file, ref_entity, l10n_entity):
         # overload this if needed
         pass
 
 
 def compareProjects(project_configs, other_observer=None,
                     merge_stage=None, clobber_merge=False):
-    assert len(project_configs) == 1  # we're not there yet for multiple
     comparer = ContentComparer()
     if other_observer is not None:
         comparer.add_observer(other_observer)
-    project = project_configs[0]
-    comparer.observer.filter = project.filter
-    for locale in project.locales:
-        files = paths.ProjectFiles(locale, project)
+    locales = set()
+    for project in project_configs:
+        observer = Observer()
+        observer.filter = project.filter
+        comparer.observers.append(observer)
+        locales.update(project.locales)
+    for locale in sorted(locales):
+        files = paths.ProjectFiles(locale, *project_configs)
         if merge_stage is not None:
             mergedir = merge_stage.format(ab_CD=locale)
             comparer.set_merge_stage(mergedir)
             if clobber_merge:
                 modules = set(_m.get('module') for _m in files.matchers)
                 modules.discard(None)
                 for module in modules:
                     clobberdir = mozpath.join(mergedir, module)
@@ -580,9 +595,9 @@ def compareProjects(project_configs, oth
                               module=module, locale=locale)
             if not os.path.exists(l10npath):
                 comparer.add(reffile, l10n)
                 continue
             if not os.path.exists(refpath):
                 comparer.remove(l10n)
                 continue
             comparer.compare(reffile, l10n, extra_tests)
-    return comparer.observer
+    return comparer.observers
--- a/compare_locales/tests/test_merge.py
+++ b/compare_locales/tests/test_merge.py
@@ -4,17 +4,17 @@
 
 import unittest
 import os
 from tempfile import mkdtemp
 import shutil
 
 from compare_locales.parser import getParser
 from compare_locales.paths import File
-from compare_locales.compare import ContentComparer
+from compare_locales.compare import ContentComparer, Observer
 from compare_locales import mozpath
 
 
 class ContentMixin(object):
     extension = None  # OVERLOAD
 
     def reference(self, content):
         self.ref = mozpath.join(self.tmp, "en-reference" + self.extension)
@@ -42,21 +42,22 @@ class TestProperties(unittest.TestCase, 
         self.reference("""foo = fooVal
 bar = barVal
 eff = effVal""")
         self.localized("""foo = lFoo
 bar = lBar
 eff = lEff
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.properties", ""),
                    File(self.l10n, "l10n.properties", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'changed': 3
                 }},
              'details': {}
              }
         )
         self.assert_(not os.path.exists(mozpath.join(cc.merge_stage,
@@ -65,21 +66,22 @@ eff = lEff
     def testMissing(self):
         self.assertTrue(os.path.isdir(self.tmp))
         self.reference("""foo = fooVal
 bar = barVal
 eff = effVal""")
         self.localized("""bar = lBar
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.properties", ""),
                    File(self.l10n, "l10n.properties", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'changed': 1, 'missing': 2
                 }},
              'details': {
                  'children': [
                      ('l10n.properties',
                          {'value': {'missingEntity': [u'eff', u'foo']}}
@@ -99,21 +101,22 @@ eff = effVal""")
         self.reference("""foo = fooVal
 bar = %d barVal
 eff = effVal""")
         self.localized("""\
 bar = %S lBar
 eff = leffVal
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.properties", ""),
                    File(self.l10n, "l10n.properties", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'changed': 2, 'errors': 1, 'missing': 1
                 }},
              'details': {
                  'children': [
                      ('l10n.properties',
                          {'value': {
@@ -136,21 +139,22 @@ eff = leffVal
         self.assertTrue(os.path.isdir(self.tmp))
         self.reference("""foo = fooVal
 eff = effVal""")
         self.localized("""foo = fooVal
 other = obsolete
 eff = leffVal
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.properties", ""),
                    File(self.l10n, "l10n.properties", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'changed': 1, 'obsolete': 1, 'unchanged': 1
                 }},
              'details': {
                  'children': [
                      ('l10n.properties',
                          {'value': {'obsoleteEntity': [u'other']}})]},
@@ -175,21 +179,22 @@ class TestDTD(unittest.TestCase, Content
         self.reference("""<!ENTITY foo 'fooVal'>
 <!ENTITY bar 'barVal'>
 <!ENTITY eff 'effVal'>""")
         self.localized("""<!ENTITY foo 'lFoo'>
 <!ENTITY bar 'lBar'>
 <!ENTITY eff 'lEff'>
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.dtd", ""),
                    File(self.l10n, "l10n.dtd", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'changed': 3
                 }},
              'details': {}
              }
         )
         self.assert_(
@@ -198,21 +203,22 @@ class TestDTD(unittest.TestCase, Content
     def testMissing(self):
         self.assertTrue(os.path.isdir(self.tmp))
         self.reference("""<!ENTITY foo 'fooVal'>
 <!ENTITY bar 'barVal'>
 <!ENTITY eff 'effVal'>""")
         self.localized("""<!ENTITY bar 'lBar'>
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.dtd", ""),
                    File(self.l10n, "l10n.dtd", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'changed': 1, 'missing': 2
                 }},
              'details': {
                  'children': [
                      ('l10n.dtd',
                          {'value': {'missingEntity': [u'eff', u'foo']}}
@@ -232,21 +238,22 @@ class TestDTD(unittest.TestCase, Content
         self.reference("""<!ENTITY foo 'fooVal'>
 <!ENTITY bar 'barVal'>
 <!ENTITY eff 'effVal'>""")
         self.localized("""<!ENTITY foo 'fooVal'>
 <!ENTY bar 'gimmick'>
 <!ENTITY eff 'effVal'>
 """)
         cc = ContentComparer()
+        cc.observers.append(Observer())
         cc.set_merge_stage(mozpath.join(self.tmp, "merge"))
         cc.compare(File(self.ref, "en-reference.dtd", ""),
                    File(self.l10n, "l10n.dtd", ""))
         self.assertDictEqual(
-            cc.observer.toJSON(),
+            cc.observers[0].toJSON(),
             {'summary':
                 {None: {
                     'errors': 1, 'missing': 1, 'unchanged': 2
                 }},
              'details': {
                  'children': [
                      ('l10n.dtd',
                          {'value': {