Bug 1254682 - Improve checking of FinalTargetFiles; r?gps draft
authorMike Shal <mshal@mozilla.com>
Fri, 18 Mar 2016 15:40:35 -0400
changeset 343076 49fc202fead25c5bbd1f51734a4cd98233d86bd9
parent 342671 f14898695ee0dd14615914f3e1401f17df57fdd7
child 516687 d69aae4a53fe06527002b341b948a2b67a0817f8
push id13524
push userbmo:mshal@mozilla.com
push dateMon, 21 Mar 2016 22:03:01 +0000
reviewersgps
bugs1254682
milestone48.0a1
Bug 1254682 - Improve checking of FinalTargetFiles; r?gps For each moz.build file that we process, we can keep track of the generated files that it depends on, as well as those that it generates. After all moz.build files are processed, we can validate the inputs to ensure that some other moz.build file generates them. This allows us to still fail early if a moz.build file depends on a generated file that won't actually exist, but also gives us some flexibility to implement some of the edge case rules that we have in Makefiles. MozReview-Commit-ID: 4ueUCXKdTAc
python/mozbuild/mozbuild/frontend/emitter.py
python/mozbuild/mozbuild/test/frontend/test_emitter.py
--- a/python/mozbuild/mozbuild/frontend/emitter.py
+++ b/python/mozbuild/mozbuild/frontend/emitter.py
@@ -133,16 +133,19 @@ class TreeMetadataEmitter(LoggingMixin):
             paths = open(subconfigures).read().splitlines()
         self._external_paths = set(mozpath.normsep(d) for d in paths)
         # Add security/nss manually, since it doesn't have a subconfigure.
         self._external_paths.add('security/nss')
 
         self._emitter_time = 0.0
         self._object_count = 0
 
+        self._generated_inputs = OrderedDefaultDict(set)
+        self._generated_outputs = set()
+
     def summary(self):
         return ExecutionSummary(
             'Processed into {object_count:d} build config descriptors in '
             '{execution_time:.2f}s',
             execution_time=self._emitter_time,
             object_count=self._object_count)
 
     def emit(self, output):
@@ -181,16 +184,26 @@ class TreeMetadataEmitter(LoggingMixin):
         # Don't emit Linkable objects when COMPILE_ENVIRONMENT is not set
         if self.config.substs.get('COMPILE_ENVIRONMENT'):
             start = time.time()
             objs = list(self._emit_libs_derived(contexts))
             self._emitter_time += time.time() - start
 
             for o in emit_objs(objs): yield o
 
+        self._check_generated_files()
+
+    def _check_generated_files(self):
+        for mozbuild, files in self._generated_inputs.iteritems():
+            error_files = files.difference(self._generated_outputs)
+            if error_files:
+                raise Exception('Objdir file(s) listed in %s not generated by the build system: %s' % (
+                    mozbuild, error_files
+                ))
+
     def _emit_libs_derived(self, contexts):
         # First do FINAL_LIBRARY linkage.
         for lib in (l for libs in self._libs.values() for l in libs):
             if not isinstance(lib, StaticLibrary) or not lib.link_into:
                 continue
             if lib.link_into not in self._libs:
                 raise SandboxValidationError(
                     'FINAL_LIBRARY ("%s") does not match any LIBRARY_NAME'
@@ -249,16 +262,17 @@ class TreeMetadataEmitter(LoggingMixin):
                     propagate_defines(lib, defines)
 
         for lib in (l for libs in self._libs.values() for l in libs):
             if isinstance(lib, Library):
                 propagate_defines(lib, lib.lib_defines)
             yield lib
 
         for obj in self._binaries.values():
+            self._generated_outputs.add(mozpath.join(obj.objdir, obj.program))
             yield obj
 
     LIBRARY_NAME_VAR = {
         'host': 'HOST_LIBRARY_NAME',
         'target': 'LIBRARY_NAME',
     }
 
     def _link_libraries(self, context, obj, variable):
@@ -346,16 +360,18 @@ class TreeMetadataEmitter(LoggingMixin):
                     '%s/moz.build, or remove "static:".' % (variable, path,
                     name, candidates[0].relobjdir, candidates[0].relobjdir),
                     context)
 
             elif isinstance(obj, StaticLibrary) and isinstance(candidates[0],
                     SharedLibrary):
                 self._static_linking_shared.add(obj)
             obj.link_library(candidates[0])
+            self._generated_outputs.add(mozpath.join(context.objdir, candidates[0].lib_name))
+            self._generated_outputs.add(mozpath.join(self.config.topobjdir, obj.install_target, candidates[0].lib_name))
 
         # Link system libraries from OS_LIBS/HOST_OS_LIBS.
         for lib in context.get(variable.replace('USE', 'OS'), []):
             obj.link_system_library(lib)
 
     @memoize
     def _get_external_library(self, dir, name, force_static):
         # Create ExternalStaticLibrary or ExternalSharedLibrary object with a
@@ -406,16 +422,17 @@ class TreeMetadataEmitter(LoggingMixin):
         if host_libname:
             if host_libname == libname:
                 raise SandboxValidationError('LIBRARY_NAME and '
                     'HOST_LIBRARY_NAME must have a different value', context)
             lib = HostLibrary(context, host_libname)
             self._libs[host_libname].append(lib)
             self._linkage.append((context, lib, 'HOST_USE_LIBS'))
             has_linkables = True
+            self._generated_outputs.add(mozpath.join(context.objdir, lib.lib_name))
 
         final_lib = context.get('FINAL_LIBRARY')
         if not libname and final_lib:
             # If no LIBRARY_NAME is given, create one.
             libname = context.relsrcdir.replace('/', '_')
 
         static_lib = context.get('FORCE_STATIC_LIB')
         shared_lib = context.get('FORCE_SHARED_LIB')
@@ -553,16 +570,17 @@ class TreeMetadataEmitter(LoggingMixin):
                         symbols_file.full_path), context)
                 shared_args['symbols_file'] = True
 
             if shared_lib:
                 lib = SharedLibrary(context, libname, **shared_args)
                 self._libs[libname].append(lib)
                 self._linkage.append((context, lib, 'USE_LIBS'))
                 has_linkables = True
+                self._generated_outputs.add(mozpath.join(context.objdir, lib.lib_name))
                 if is_component and not context['NO_COMPONENTS_MANIFEST']:
                     yield ChromeManifestEntry(context,
                         'components/components.manifest',
                         ManifestBinaryComponent('components', lib.lib_name))
                 if symbols_file:
                     script = mozpath.join(
                         mozpath.dirname(mozpath.dirname(__file__)),
                         'action', 'generate_symbols_file.py')
@@ -572,16 +590,17 @@ class TreeMetadataEmitter(LoggingMixin):
                     yield GeneratedFile(context, script,
                         'generate_symbols_file', lib.symbols_file,
                         [symbols_file], defines)
             if static_lib:
                 lib = StaticLibrary(context, libname, **static_args)
                 self._libs[libname].append(lib)
                 self._linkage.append((context, lib, 'USE_LIBS'))
                 has_linkables = True
+                self._generated_outputs.add(mozpath.join(context.objdir, lib.lib_name))
 
             if lib_defines:
                 if not libname:
                     raise SandboxValidationError('LIBRARY_DEFINES needs a '
                         'LIBRARY_NAME to take effect', context)
                 lib.lib_defines.update(lib_defines)
 
         # Only emit sources if we have linkables defined in the same context.
@@ -706,16 +725,18 @@ class TreeMetadataEmitter(LoggingMixin):
         if any(k in context for k in ('FINAL_TARGET', 'XPI_NAME', 'DIST_SUBDIR')):
             yield InstallationTarget(context)
 
         # We always emit a directory traversal descriptor. This is needed by
         # the recursive make backend.
         for o in self._emit_directory_traversal_from_context(context): yield o
 
         for obj in self._process_xpidl(context):
+            xpt = mozpath.join(self.config.topobjdir, obj.install_target, 'components', '%s.xpt' % obj.module)
+            self._generated_outputs.add(xpt)
             yield obj
 
         # Proxy some variables as-is until we have richer classes to represent
         # them. We should aim to keep this set small because it violates the
         # desired abstraction of the build definition away from makefiles.
         passthru = VariablePassthru(context)
         varlist = [
             'ALLOW_COMPILER_WARNINGS',
@@ -773,25 +794,24 @@ class TreeMetadataEmitter(LoggingMixin):
                               not context.config.substs.get('MOZ_ASAN'))
             rtl_flag = '-MT' if use_static_lib else '-MD'
             if (context.config.substs.get('MOZ_DEBUG') and
                     not context.config.substs.get('MOZ_NO_DEBUG_RTL')):
                 rtl_flag += 'd'
             # Use a list, like MOZBUILD_*FLAGS variables
             passthru.variables['RTL_FLAGS'] = [rtl_flag]
 
-        generated_files = set()
         for obj in self._process_generated_files(context):
-            generated_files.add(obj.output)
+            self._generated_outputs.add(mozpath.join(context.objdir, obj.output))
             yield obj
 
         for path in context['CONFIGURE_SUBST_FILES']:
             sub = self._create_substitution(ConfigFileSubstitution, context,
                 path)
-            generated_files.add(str(sub.relpath))
+            self._generated_outputs.add(mozpath.join(context.objdir, str(sub.relpath)))
             yield sub
 
         defines = context.get('DEFINES')
         if defines:
             yield Defines(context, defines)
 
         host_defines = context.get('HOST_DEFINES')
         if host_defines:
@@ -817,18 +837,16 @@ class TreeMetadataEmitter(LoggingMixin):
                 raise SandboxValidationError('Path specified in LOCAL_INCLUDES '
                     'does not exist: %s (resolved to %s)' % (local_include,
                     local_include.full_path), context)
             yield LocalInclude(context, local_include)
 
         for obj in self._handle_linkables(context, passthru):
             yield obj
 
-        generated_files.update(['%s%s' % (k, self.config.substs.get('BIN_SUFFIX', '')) for k in self._binaries.keys()])
-
         components = []
         for var, cls in (
             ('BRANDING_FILES', BrandingFiles),
             ('EXPORTS', Exports),
             ('FINAL_TARGET_FILES', FinalTargetFiles),
             ('FINAL_TARGET_PP_FILES', FinalTargetPreprocessedFiles),
             ('SDK_FILES', SdkFiles),
             ('TEST_HARNESS_FILES', TestHarnessFiles),
@@ -837,49 +855,43 @@ class TreeMetadataEmitter(LoggingMixin):
             if not all_files:
                 continue
             if dist_install is False and var != 'TEST_HARNESS_FILES':
                 raise SandboxValidationError(
                     '%s cannot be used with DIST_INSTALL = False' % var,
                     context)
             has_prefs = False
             has_resources = False
+            track_generated_files = False
             for base, files in all_files.walk():
                 if var == 'TEST_HARNESS_FILES' and not base:
                     raise SandboxValidationError(
                         'Cannot install files to the root of TEST_HARNESS_FILES', context)
                 if base == 'components':
                     components.extend(files)
                 if base == 'defaults/pref':
                     has_prefs = True
                 if mozpath.split(base)[0] == 'res':
                     has_resources = True
                 for f in files:
-                    if (var == 'FINAL_TARGET_PP_FILES' and
-                        not isinstance(f, SourcePath)):
-                        raise SandboxValidationError(
+                    if var == 'FINAL_TARGET_PP_FILES':
+                        track_generated_files = True
+                        if not isinstance(f, SourcePath):
+                            raise SandboxValidationError(
                                 ('Only source directory paths allowed in ' +
-                                'FINAL_TARGET_PP_FILES: %s')
+                                 'FINAL_TARGET_PP_FILES: %s')
                                 % (f,), context)
                     if not isinstance(f, ObjDirPath):
                         path = f.full_path
                         if '*' not in path and not os.path.exists(path):
                             raise SandboxValidationError(
                                 'File listed in %s does not exist: %s'
                                 % (var, path), context)
                     else:
-                        # TODO: Bug 1254682 - The '/' check is to allow
-                        # installing files generated from other directories,
-                        # which is done occasionally for tests. However, it
-                        # means we don't fail early if the file isn't actually
-                        # created by the other moz.build file.
-                        if f.target_basename not in generated_files and '/' not in f:
-                            raise SandboxValidationError(
-                                ('Objdir file listed in %s not in ' +
-                                 'GENERATED_FILES: %s') % (var, f), context)
+                        self._generated_inputs[context.main_path].add(f.full_path)
 
             # Addons (when XPI_NAME is defined) and Applications (when
             # DIST_SUBDIR is defined) use a different preferences directory
             # (default/preferences) from the one the GRE uses (defaults/pref).
             # Hence, we move the files from the latter to the former in that
             # case.
             if has_prefs and (context.get('XPI_NAME') or
                               context.get('DIST_SUBDIR')):
@@ -887,17 +899,24 @@ class TreeMetadataEmitter(LoggingMixin):
                 del all_files.defaults._children['pref']
 
             if has_resources and (context.get('DIST_SUBDIR') or
                                   context.get('XPI_NAME')):
                 raise SandboxValidationError(
                     'RESOURCES_FILES cannot be used with DIST_SUBDIR or '
                     'XPI_NAME.', context)
 
-            yield cls(context, all_files)
+            obj = cls(context, all_files)
+            if track_generated_files:
+                for subdir, files in obj.files.walk():
+                    for f in files:
+                        basename = os.path.basename(f)
+                        output = basename[:-3] if basename.endswith('.in') else basename
+                        self._generated_outputs.add(mozpath.join(self.config.topobjdir, obj.install_target, subdir, output))
+            yield obj
 
         # Check for manifest declarations in EXTRA_{PP_,}COMPONENTS.
         if any(e.endswith('.js') for e in components) and \
                 not any(e.endswith('.manifest') for e in components) and \
                 not context.get('NO_JS_MANIFEST', False):
             raise SandboxValidationError('A .js component was specified in EXTRA_COMPONENTS '
                                          'or EXTRA_PP_COMPONENTS without a matching '
                                          '.manifest file.  See '
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -312,21 +312,21 @@ class TestEmitterBasic(unittest.TestCase
         '''
         reader = self.reader('exports-missing')
         with self.assertRaisesRegexp(SandboxValidationError,
              'File listed in EXPORTS does not exist:'):
             objs = self.read_topsrcdir(reader)
 
     def test_exports_missing_generated(self):
         '''
-        An objdir file in EXPORTS that is not in GENERATED_FILES is an error.
+        An objdir file that is not generated by moz.build is an error
         '''
         reader = self.reader('exports-missing-generated')
-        with self.assertRaisesRegexp(SandboxValidationError,
-             'Objdir file listed in EXPORTS not in GENERATED_FILES:'):
+        with self.assertRaisesRegexp(Exception,
+                                     'Objdir file\(s\) listed in .*/exports-missing-generated/moz.build not generated by the build system:.*bar.h'):
             objs = self.read_topsrcdir(reader)
 
     def test_exports_generated(self):
         reader = self.reader('exports-generated')
         objs = self.read_topsrcdir(reader)
 
         self.assertEqual(len(objs), 2)
         self.assertIsInstance(objs[0], GeneratedFile)