Bug 1398897 - Move os includes to computed flags. draft
authorChris Manchester <cmanchester@mozilla.com>
Wed, 20 Sep 2017 12:43:24 -0700
changeset 667814 2ea4731ab41ac6d7f9651390e089546e7709cb73
parent 667813 c72c5447ccfd92eaf0c90d8551cd93978d051705
child 732517 82d2d8cb718f8fb03f66da1e8f4ec7bd95aa9532
push id80853
push userbmo:cmanchester@mozilla.com
push dateWed, 20 Sep 2017 19:43:53 +0000
bugs1398897
milestone57.0a1
Bug 1398897 - Move os includes to computed flags. MozReview-Commit-ID: Ef1wu5fQo7M
config/config.mk
python/mozbuild/mozbuild/compilation/database.py
python/mozbuild/mozbuild/frontend/context.py
python/mozbuild/mozbuild/frontend/emitter.py
python/mozbuild/mozbuild/frontend/gyp_reader.py
python/mozbuild/mozbuild/test/frontend/test_emitter.py
--- a/config/config.mk
+++ b/config/config.mk
@@ -248,28 +248,16 @@ CCC = $(CXX)
 
 INCLUDES = \
   -I$(srcdir) \
   -I$(CURDIR) \
   $(LOCAL_INCLUDES) \
   -I$(ABS_DIST)/include \
   $(NULL)
 
-ifndef IS_GYP_DIR
-# NSPR_CFLAGS and NSS_CFLAGS must appear ahead of the other flags to avoid Linux
-# builds wrongly picking up system NSPR/NSS header files.
-OS_INCLUDES := \
-  $(NSPR_CFLAGS) $(NSS_CFLAGS) \
-  $(MOZ_JPEG_CFLAGS) \
-  $(MOZ_PNG_CFLAGS) \
-  $(MOZ_ZLIB_CFLAGS) \
-  $(MOZ_PIXMAN_CFLAGS) \
-  $(NULL)
-endif
-
 include $(MOZILLA_DIR)/config/static-checking-config.mk
 
 CFLAGS		= $(OS_CPPFLAGS) $(OS_CFLAGS)
 CXXFLAGS	= $(OS_CPPFLAGS) $(OS_CXXFLAGS)
 LDFLAGS		= $(OS_LDFLAGS) $(MOZBUILD_LDFLAGS) $(MOZ_FIX_LINK_PATHS)
 
 ifdef MOZ_OPTIMIZE
 ifeq (1,$(MOZ_OPTIMIZE))
@@ -319,18 +307,18 @@ endif # CLANG_CL
 
 # Use warnings-as-errors if ALLOW_COMPILER_WARNINGS is not set to 1 (which
 # includes the case where it's undefined).
 ifneq (1,$(ALLOW_COMPILER_WARNINGS))
 CXXFLAGS += $(WARNINGS_AS_ERRORS)
 CFLAGS   += $(WARNINGS_AS_ERRORS)
 endif # ALLOW_COMPILER_WARNINGS
 
-COMPILE_CFLAGS	= $(COMPUTED_CFLAGS) $(OS_INCLUDES) $(DSO_CFLAGS) $(DSO_PIC_CFLAGS) $(RTL_FLAGS) $(OS_COMPILE_CFLAGS) $(_DEPEND_CFLAGS) $(CFLAGS) $(MOZBUILD_CFLAGS) $(MK_COMPILE_DEFINES)
-COMPILE_CXXFLAGS = $(COMPUTED_CXXFLAGS) $(OS_INCLUDES) $(DSO_CFLAGS) $(DSO_PIC_CFLAGS) $(RTL_FLAGS) $(OS_COMPILE_CXXFLAGS) $(_DEPEND_CFLAGS) $(CXXFLAGS) $(MOZBUILD_CXXFLAGS) $(MK_COMPILE_DEFINES)
+COMPILE_CFLAGS	= $(COMPUTED_CFLAGS) $(DSO_CFLAGS) $(DSO_PIC_CFLAGS) $(RTL_FLAGS) $(OS_COMPILE_CFLAGS) $(_DEPEND_CFLAGS) $(CFLAGS) $(MOZBUILD_CFLAGS) $(MK_COMPILE_DEFINES)
+COMPILE_CXXFLAGS = $(COMPUTED_CXXFLAGS) $(DSO_CFLAGS) $(DSO_PIC_CFLAGS) $(RTL_FLAGS) $(OS_COMPILE_CXXFLAGS) $(_DEPEND_CFLAGS) $(CXXFLAGS) $(MOZBUILD_CXXFLAGS) $(MK_COMPILE_DEFINES)
 COMPILE_CMFLAGS = $(OS_COMPILE_CMFLAGS) $(MOZBUILD_CMFLAGS)
 COMPILE_CMMFLAGS = $(OS_COMPILE_CMMFLAGS) $(MOZBUILD_CMMFLAGS)
 ASFLAGS += $(MOZBUILD_ASFLAGS)
 
 ifndef CROSS_COMPILE
 HOST_CFLAGS += $(RTL_FLAGS)
 endif
 
--- a/python/mozbuild/mozbuild/compilation/database.py
+++ b/python/mozbuild/mozbuild/compilation/database.py
@@ -41,17 +41,16 @@ class CompileDBBackend(CommonBackend):
         self._db = OrderedDict()
 
         # The cache for per-directory flags
         self._flags = {}
 
         self._envs = {}
         self._local_flags = defaultdict(dict)
         self._per_source_flags = defaultdict(list)
-        self._gyp_dirs = set()
 
     def consume_object(self, obj):
         # Those are difficult directories, that will be handled later.
         if obj.relativedir in (
                 'build/unix/elfhack',
                 'build/unix/elfhack/inject',
                 'build/clang-plugin',
                 'build/clang-plugin/tests'):
@@ -71,18 +70,16 @@ class CompileDBBackend(CommonBackend):
 
         elif isinstance(obj, (Sources, GeneratedSources)):
             # For other sources, include each source file.
             for f in obj.files:
                 self._build_db_line(obj.objdir, obj.relativedir, obj.config, f,
                                     obj.canonical_suffix)
 
         elif isinstance(obj, VariablePassthru):
-            if obj.variables.get('IS_GYP_DIR'):
-                self._gyp_dirs.add(obj.objdir)
             for var in ('MOZBUILD_CFLAGS', 'MOZBUILD_CXXFLAGS',
                         'MOZBUILD_CMFLAGS', 'MOZBUILD_CMMFLAGS',
                         'RTL_FLAGS'):
                 if var in obj.variables:
                     self._local_flags[obj.objdir][var] = obj.variables[var]
             if (obj.variables.get('ALLOW_COMPILER_WARNINGS') and
                     'WARNINGS_AS_ERRORS' in self._local_flags[obj.objdir]):
                 del self._local_flags[obj.objdir]['WARNINGS_AS_ERRORS']
@@ -103,31 +100,17 @@ class CompileDBBackend(CommonBackend):
 
         for (directory, filename, unified), cmd in self._db.iteritems():
             env = self._envs[directory]
             cmd = list(cmd)
             if unified is None:
                 cmd.append(filename)
             else:
                 cmd.append(unified)
-            os_includes = []
-            if directory not in self._gyp_dirs:
-                for var in (
-                    'NSPR_CFLAGS',
-                    'NSS_CFLAGS',
-                    'MOZ_JPEG_CFLAGS',
-                    'MOZ_PNG_CFLAGS',
-                    'MOZ_ZLIB_CFLAGS',
-                    'MOZ_PIXMAN_CFLAGS',
-                ):
-                    f = env.substs.get(var)
-                    if f:
-                        os_includes.extend(f)
             variables = {
-                'OS_INCLUDES': os_includes,
                 'DIST': mozpath.join(env.topobjdir, 'dist'),
                 'DEPTH': env.topobjdir,
                 'MOZILLA_DIR': env.topsrcdir,
                 'topsrcdir': env.topsrcdir,
                 'topobjdir': env.topobjdir,
             }
             variables.update(self._local_flags[directory])
             c = []
@@ -208,19 +191,16 @@ class CompileDBBackend(CommonBackend):
             if not value:
                 return
             if isinstance(value, types.StringTypes):
                 value = value.split()
             db.extend(value)
 
         db.append('$(COMPUTED_%s)' % self.CFLAGS[canonical_suffix])
 
-        db.extend((
-            '$(OS_INCLUDES)',
-        ))
         append_var('DSO_CFLAGS')
         append_var('DSO_PIC_CFLAGS')
         if canonical_suffix in ('.c', '.cpp'):
             db.append('$(RTL_FLAGS)')
         append_var('OS_COMPILE_%s' % self.CFLAGS[canonical_suffix])
         append_var('OS_CPPFLAGS')
         append_var('OS_%s' % self.CFLAGS[canonical_suffix])
         append_var('MOZ_DEBUG_FLAGS')
--- a/python/mozbuild/mozbuild/frontend/context.py
+++ b/python/mozbuild/mozbuild/frontend/context.py
@@ -307,16 +307,20 @@ class CompileFlags(ContextDerivedValue, 
              ('CXXFLAGS', 'CFLAGS')),
             ('DEFINES', None, ('CXXFLAGS', 'CFLAGS')),
             ('LIBRARY_DEFINES', None, ('CXXFLAGS', 'CFLAGS')),
             ('BASE_INCLUDES', ['-I%s' % main_src_dir, '-I%s' % context.objdir],
              ('CXXFLAGS', 'CFLAGS')),
             ('LOCAL_INCLUDES', None, ('CXXFLAGS', 'CFLAGS')),
             ('EXTRA_INCLUDES', ['-I%s/dist/include' % context.config.topobjdir],
              ('CXXFLAGS', 'CFLAGS')),
+            ('OS_INCLUDES', list(itertools.chain(*(context.config.substs.get(v, []) for v in (
+                'NSPR_CFLAGS', 'NSS_CFLAGS', 'MOZ_JPEG_CFLAGS', 'MOZ_PNG_CFLAGS',
+                'MOZ_ZLIB_CFLAGS', 'MOZ_PIXMAN_CFLAGS')))),
+             ('CXXFLAGS', 'CFLAGS')),
         )
         self._known_keys = set(k for k, v, _ in self.flag_variables)
 
         # Providing defaults here doesn't play well with multiple templates
         # modifying COMPILE_FLAGS from the same moz.build, because the merge
         # done after the template runs can't tell which values coming from
         # a template were set and which were provided as defaults.
         template_name = getattr(context, 'template', None)
--- a/python/mozbuild/mozbuild/frontend/emitter.py
+++ b/python/mozbuild/mozbuild/frontend/emitter.py
@@ -946,19 +946,16 @@ class TreeMetadataEmitter(LoggingMixin):
                 for dll in context['DELAYLOAD_DLLS']])
             context['OS_LIBS'].append('delayimp')
 
         for v in ['CFLAGS', 'CXXFLAGS', 'CMFLAGS', 'CMMFLAGS', 'ASFLAGS',
                   'LDFLAGS', 'HOST_CFLAGS', 'HOST_CXXFLAGS']:
             if v in context and context[v]:
                 passthru.variables['MOZBUILD_' + v] = context[v]
 
-        if isinstance(context, TemplateContext) and context.template == 'Gyp':
-            passthru.variables['IS_GYP_DIR'] = True
-
         dist_install = context['DIST_INSTALL']
         if dist_install is True:
             passthru.variables['DIST_INSTALL'] = True
         elif dist_install is False:
             passthru.variables['NO_DIST_INSTALL'] = True
 
         # Ideally, this should be done in templates, but this is difficult at
         # the moment because USE_STATIC_LIBS can be set after a template
--- a/python/mozbuild/mozbuild/frontend/gyp_reader.py
+++ b/python/mozbuild/mozbuild/frontend/gyp_reader.py
@@ -322,16 +322,17 @@ def process_gyp_result(gyp_result, gyp_d
               '/ipc/chromium/src',
               '/ipc/glue',
           ]
           # These get set via VC project file settings for normal GYP builds.
           if config.substs['OS_TARGET'] == 'WINNT':
               context['DEFINES']['UNICODE'] = True
               context['DEFINES']['_UNICODE'] = True
         context['COMPILE_FLAGS']['STL'] = []
+        context['COMPILE_FLAGS']['OS_INCLUDES'] = []
 
         for key, value in gyp_dir_attrs.sandbox_vars.items():
             if context.get(key) and isinstance(context[key], list):
                 # If we have a key from sanbox_vars that's also been
                 # populated here we use the value from sandbox_vars as our
                 # basis rather than overriding outright.
                 context[key] = value + context[key]
             elif context.get(key) and isinstance(context[key], dict):
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -219,21 +219,36 @@ class TestEmitterBasic(unittest.TestCase
             self.read_topsrcdir(reader)
 
         reader = self.reader('compile-flags-type-validation')
         with self.assertRaisesRegexp(BuildReaderError,
                                      'A list of strings must be provided'):
             self.read_topsrcdir(reader)
 
     def test_compile_flags_templates(self):
-        reader = self.reader('compile-flags-templates')
+        reader = self.reader('compile-flags-templates', extra_substs={
+            'NSPR_CFLAGS': ['-I/nspr/path'],
+            'NSS_CFLAGS': ['-I/nss/path'],
+            'MOZ_JPEG_CFLAGS': ['-I/jpeg/path'],
+            'MOZ_PNG_CFLAGS': ['-I/png/path'],
+            'MOZ_ZLIB_CFLAGS': ['-I/zlib/path'],
+            'MOZ_PIXMAN_CFLAGS': ['-I/pixman/path'],
+        })
         sources, lib, flags = self.read_topsrcdir(reader)
         self.assertIsInstance(flags, ComputedFlags)
         self.assertEqual(flags.flags['STL'], [])
         self.assertEqual(flags.flags['VISIBILITY'], [])
+        self.assertEqual(flags.flags['OS_INCLUDES'], [
+            '-I/nspr/path',
+            '-I/nss/path',
+            '-I/jpeg/path',
+            '-I/png/path',
+            '-I/zlib/path',
+            '-I/pixman/path',
+        ])
 
     def test_disable_stl_wrapping(self):
         reader = self.reader('disable-stl-wrapping')
         sources, lib, flags = self.read_topsrcdir(reader)
         self.assertIsInstance(flags, ComputedFlags)
         self.assertEqual(flags.flags['STL'], [])
 
     def test_visibility_flags(self):