bug 1416891 - support LOCALIZED_GENERATED_FILES in the recursive make backend. r=nalexander draft
authorTed Mielczarek <ted@mielczarek.org>
Thu, 16 Nov 2017 17:07:23 -0500
changeset 705775 3febcff2afe9821bcdbc4becc47ae22b7940b44e
parent 705774 7d6cf3bf6f49c4a9d650d1f0f041634f82be027f
child 705776 86b7f45f92e2ca615eb5fba2ad9985e6d3ce429b
push id91576
push userbmo:ted@mielczarek.org
push dateThu, 30 Nov 2017 16:56:58 +0000
reviewersnalexander
bugs1416891
milestone59.0a1
bug 1416891 - support LOCALIZED_GENERATED_FILES in the recursive make backend. r=nalexander This change makes the recursive make backend emit slightly different rules when handling LOCALIZED_GENERATED_FILES vs. GENERATED_FILES. First, localized file generation is always done in the libs tier. Second, inputs are allowed to be locale-dependent, which is determined by the path starting with `en-US/`. These inputs will be run through MERGE_FILE to determine the actual file path to pass to the script. Third, the file_generate action now accepts a `--locale` option, and it gets passed the value of `$(AB_CD)` when generating localized files. If this option is passed it is also passed as a keyword argument `locale` to the generation function. Fourth, the make rules for localized files include an additional dependency on FORCE when running l10n repacks, so that the targets will always be rebuilt in that situation. MozReview-Commit-ID: BfgR8MxxJXZ
python/mozbuild/mozbuild/action/file_generate.py
python/mozbuild/mozbuild/backend/recursivemake.py
python/mozbuild/mozbuild/test/backend/data/localized-generated-files/en-US/localized-input
python/mozbuild/mozbuild/test/backend/data/localized-generated-files/foo-data
python/mozbuild/mozbuild/test/backend/data/localized-generated-files/generate-foo.py
python/mozbuild/mozbuild/test/backend/data/localized-generated-files/moz.build
python/mozbuild/mozbuild/test/backend/data/localized-generated-files/non-localized-input
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
--- a/python/mozbuild/mozbuild/action/file_generate.py
+++ b/python/mozbuild/mozbuild/action/file_generate.py
@@ -18,30 +18,35 @@ from mozbuild.pythonutil import iter_mod
 from mozbuild.makeutil import Makefile
 from mozbuild.util import FileAvoidWrite
 import buildconfig
 
 
 def main(argv):
     parser = argparse.ArgumentParser('Generate a file from a Python script',
                                      add_help=False)
+    parser.add_argument('--locale', metavar='locale', type=str,
+                        help='The locale in use.')
     parser.add_argument('python_script', metavar='python-script', type=str,
                         help='The Python script to run')
     parser.add_argument('method_name', metavar='method-name', type=str,
                         help='The method of the script to invoke')
     parser.add_argument('output_file', metavar='output-file', type=str,
                         help='The file to generate')
     parser.add_argument('dep_file', metavar='dep-file', type=str,
                         help='File to write any additional make dependencies to')
     parser.add_argument('additional_arguments', metavar='arg',
                         nargs=argparse.REMAINDER,
                         help="Additional arguments to the script's main() method")
 
     args = parser.parse_args(argv)
 
+    kwargs = {}
+    if args.locale:
+        kwargs['locale'] = args.locale
     script = args.python_script
     # Permit the script to import modules from the same directory in which it
     # resides.  The justification for doing this is that if we were invoking
     # the script as:
     #
     #    python script arg1...
     #
     # then importing modules from the script's directory would come for free.
@@ -55,17 +60,17 @@ def main(argv):
     if not hasattr(module, method):
         print('Error: script "{0}" is missing a {1} method'.format(script, method),
               file=sys.stderr)
         return 1
 
     ret = 1
     try:
         with FileAvoidWrite(args.output_file) as output:
-            ret = module.__dict__[method](output, *args.additional_arguments)
+            ret = module.__dict__[method](output, *args.additional_arguments, **kwargs)
             # The following values indicate a statement of success:
             #  - a set() (see below)
             #  - 0
             #  - False
             #  - None
             #
             # Everything else is an error (so scripts can conveniently |return
             # 1| or similar). If a set is returned, the elements of the set
--- a/python/mozbuild/mozbuild/backend/recursivemake.py
+++ b/python/mozbuild/mozbuild/backend/recursivemake.py
@@ -524,42 +524,70 @@ class RecursiveMakeBackend(CommonBackend
                 '.c',
                 '.cpp',
                 '.h',
                 '.inc',
                 '.py',
                 '.rs',
                 'new', # 'new' is an output from make-stl-wrappers.py
             )
-            tier = 'export' if any(f.endswith(export_suffixes) for f in obj.outputs) else 'misc'
+            if any(f.endswith(export_suffixes) for f in obj.outputs):
+                tier = 'export'
+            elif obj.localized:
+                tier = 'libs'
+            else:
+                tier = 'misc'
             self._no_skip[tier].add(backend_file.relobjdir)
             first_output = obj.outputs[0]
             dep_file = "%s.pp" % first_output
 
+            if obj.inputs:
+                if obj.localized:
+                    # Localized generated files can have locale-specific inputs, which are
+                    # indicated by paths starting with `en-US/`.
+                    def srcpath(p):
+                        bits = p.split('en-US/', 1)
+                        if len(bits) == 2:
+                            e, f = bits
+                            assert(not e)
+                            return '$(call MERGE_FILE,%s)' % f
+                        return self._pretty_path(p, backend_file)
+                    inputs = [srcpath(f) for f in obj.inputs]
+                else:
+                    inputs = [self._pretty_path(f, backend_file) for f in obj.inputs]
+            else:
+                inputs = []
+
             # If we're doing this during export that means we need it during
             # compile, but if we have an artifact build we don't run compile,
             # so we can skip it altogether or let the rule run as the result of
             # something depending on it.
-            if tier == 'misc' or not self.environment.is_artifact_build:
+            if tier != 'export' or not self.environment.is_artifact_build:
                 backend_file.write('%s:: %s\n' % (tier, first_output))
             for output in obj.outputs:
                 if output != first_output:
                     backend_file.write('%s: %s ;\n' % (output, first_output))
                 backend_file.write('GARBAGE += %s\n' % output)
             backend_file.write('EXTRA_MDDEPEND_FILES += %s\n' % dep_file)
             if obj.script:
-                backend_file.write("""{output}: {script}{inputs}{backend}
+                backend_file.write("""{output}: {script}{inputs}{backend}{repack_force}
 \t$(REPORT_BUILD)
-\t$(call py_action,file_generate,{script} {method} {output} $(MDDEPDIR)/{dep_file}{inputs}{flags})
+\t$(call py_action,file_generate,{locale}{script} {method} {output} $(MDDEPDIR)/{dep_file}{inputs}{flags})
 
 """.format(output=first_output,
            dep_file=dep_file,
-           inputs=' ' + ' '.join([self._pretty_path(f, backend_file) for f in obj.inputs]) if obj.inputs else '',
+           inputs=' ' + ' '.join(inputs) if inputs else '',
            flags=' ' + ' '.join(shell_quote(f) for f in obj.flags) if obj.flags else '',
            backend=' backend.mk' if obj.flags else '',
+           # Locale repacks repack multiple locales from a single configured objdir,
+           # so standard mtime dependencies won't work properly when the build is re-run
+           # with a different locale as input. IS_LANGUAGE_REPACK will reliably be set
+           # in this situation, so simply force the generation to run in that case.
+           repack_force=' $(if $(IS_LANGUAGE_REPACK),FORCE)' if obj.localized else '',
+           locale='--locale=$(AB_CD) ' if obj.localized else '',
            script=obj.script,
            method=obj.method))
 
         elif isinstance(obj, JARManifest):
             self._no_skip['libs'].add(backend_file.relobjdir)
             backend_file.write('JAR_MANIFEST := %s\n' % obj.path.full_path)
 
         elif isinstance(obj, RustProgram):
@@ -1458,27 +1486,31 @@ class RecursiveMakeBackend(CommonBackend
                     var, self._pretty_path(f, backend_file)))
             backend_file.write('%s_PATH := $(DEPTH)/%s\n'
                                % (var, mozpath.join(obj.install_target, path)))
             backend_file.write('%s_TARGET := misc\n' % var)
             backend_file.write('PP_TARGETS += %s\n' % var)
 
     def _write_localized_files_files(self, files, name, backend_file):
         for f in files:
-            # The emitter asserts that all files start with `en-US/`
-            e, f = f.split('en-US/')
-            assert(not e)
-            if '*' in f:
-                # We can't use MERGE_FILE for wildcards because it takes
-                # only the first match internally. This is only used
-                # in one place in the tree currently so we'll hardcode
-                # that specific behavior for now.
-                backend_file.write('%s += $(wildcard $(LOCALE_SRCDIR)/%s)\n' % (name, f))
+            if not isinstance(f, ObjDirPath):
+                # The emitter asserts that all srcdir files start with `en-US/`
+                e, f = f.split('en-US/')
+                assert(not e)
+                if '*' in f:
+                    # We can't use MERGE_FILE for wildcards because it takes
+                    # only the first match internally. This is only used
+                    # in one place in the tree currently so we'll hardcode
+                    # that specific behavior for now.
+                    backend_file.write('%s += $(wildcard $(LOCALE_SRCDIR)/%s)\n' % (name, f))
+                else:
+                    backend_file.write('%s += $(call MERGE_FILE,%s)\n' % (name, f))
             else:
-                backend_file.write('%s += $(call MERGE_FILE,%s)\n' % (name, f))
+                # Objdir files are allowed from LOCALIZED_GENERATED_FILES
+                backend_file.write('%s += %s\n' % (name, self._pretty_path(f, backend_file)))
 
     def _process_localized_files(self, obj, files, backend_file):
         target = obj.install_target
         path = mozpath.basedir(target, ('dist/bin', ))
         if not path:
             raise Exception('Cannot install localized files to ' + target)
         for i, (path, files) in enumerate(files.walk()):
             name = 'LOCALIZED_FILES_%d' % i
new file mode 100644
copy from python/mozbuild/mozbuild/test/backend/data/generated-files/foo-data
copy to python/mozbuild/mozbuild/test/backend/data/localized-generated-files/foo-data
copy from python/mozbuild/mozbuild/test/backend/data/generated-files/generate-foo.py
copy to python/mozbuild/mozbuild/test/backend/data/localized-generated-files/generate-foo.py
copy from python/mozbuild/mozbuild/test/backend/data/generated-files/moz.build
copy to python/mozbuild/mozbuild/test/backend/data/localized-generated-files/moz.build
--- a/python/mozbuild/mozbuild/test/backend/data/generated-files/moz.build
+++ b/python/mozbuild/mozbuild/test/backend/data/localized-generated-files/moz.build
@@ -1,12 +1,15 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
-GENERATED_FILES += [ 'bar.c', 'foo.c', 'quux.c' ]
+LOCALIZED_GENERATED_FILES += [ 'foo.xyz' ]
 
-bar = GENERATED_FILES['bar.c']
-bar.script = 'generate-bar.py:baz'
+foo = LOCALIZED_GENERATED_FILES['foo.xyz']
+foo.script = 'generate-foo.py'
+foo.inputs = [
+    'en-US/localized-input',
+    'non-localized-input',
+]
 
-foo = GENERATED_FILES['foo.c']
-foo.script = 'generate-foo.py'
-foo.inputs = ['foo-data']
+# Also check that using it in LOCALIZED_FILES does the right thing.
+LOCALIZED_FILES += [ '!foo.xyz' ]
new file mode 100644
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -413,16 +413,40 @@ class TestRecursiveMakeBackend(BackendTe
             'export:: quux.c',
             'GARBAGE += quux.c',
             'EXTRA_MDDEPEND_FILES += quux.c.pp',
         ]
 
         self.maxDiff = None
         self.assertEqual(lines, expected)
 
+    def test_localized_generated_files(self):
+        """Ensure LOCALIZED_GENERATED_FILES is handled properly."""
+        env = self._consume('localized-generated-files', RecursiveMakeBackend)
+
+        backend_path = mozpath.join(env.topobjdir, 'backend.mk')
+        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
+
+        expected = [
+            'libs:: foo.xyz',
+            'GARBAGE += foo.xyz',
+            'EXTRA_MDDEPEND_FILES += foo.xyz.pp',
+            'foo.xyz: %s/generate-foo.py $(call MERGE_FILE,localized-input) $(srcdir)/non-localized-input $(if $(IS_LANGUAGE_REPACK),FORCE)' % env.topsrcdir,
+            '$(REPORT_BUILD)',
+            '$(call py_action,file_generate,--locale=$(AB_CD) %s/generate-foo.py main foo.xyz $(MDDEPDIR)/foo.xyz.pp $(call MERGE_FILE,localized-input) $(srcdir)/non-localized-input)' % env.topsrcdir,
+            '',
+            'LOCALIZED_FILES_0_FILES += foo.xyz',
+            'LOCALIZED_FILES_0_DEST = $(FINAL_TARGET)/',
+            'LOCALIZED_FILES_0_TARGET := libs',
+            'INSTALL_TARGETS += LOCALIZED_FILES_0',
+        ]
+
+        self.maxDiff = None
+        self.assertEqual(lines, expected)
+
     def test_exports_generated(self):
         """Ensure EXPORTS that are listed in GENERATED_FILES
         are handled properly."""
         env = self._consume('exports-generated', RecursiveMakeBackend)
 
         # EXPORTS files should appear in the dist_include install manifest.
         m = InstallManifest(path=mozpath.join(env.topobjdir,
             '_build_manifests', 'install', 'dist_include'))