bug 1409276 - emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend. r?build draft
authorTed Mielczarek <ted@mielczarek.org>
Wed, 11 Jul 2018 15:52:26 -0400
changeset 822697 f999890c76992de75f521140cb3a47e6e0c513f6
parent 821479 143984185dcece46031c970179ddea4837a6c01d
child 822698 c12de60121d7b5a37019ba0fbb29ab9040c6ee62
push id117452
push userbmo:ted@mielczarek.org
push dateWed, 25 Jul 2018 19:39:28 +0000
reviewersbuild
bugs1409276
milestone63.0a1
bug 1409276 - emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend. r?build Currently the recursive make backend skips emitting COMPUTED_CFLAGS etc for Rust-only directories as an optimization. However, we need to pass CFLAGS down to Rust build scripts that compile C/C++ code, so having the COMPUTED_ variables will make it easier to pass the right set of flags. Additionally, ensure that Rust programs wind up in linkables/host_linkables so that they get computed flags emitted. MozReview-Commit-ID: Ld41080dOZN
build/templates.mozbuild
python/mozbuild/mozbuild/frontend/emitter.py
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/test/frontend/test_emitter.py
--- a/build/templates.mozbuild
+++ b/build/templates.mozbuild
@@ -48,16 +48,19 @@ def CppUnitTests(names, ext='.cpp'):
     Binary()
 
 
 @template
 def Library(name):
     '''Template for libraries.'''
     LIBRARY_NAME = name
 
+@template
+def AllowCompilerWarnings():
+    COMPILE_FLAGS['WARNINGS_AS_ERRORS'] = []
 
 @template
 def RustLibrary(name, features=None, target_dir=None):
     '''Template for Rust libraries.'''
     Library(name)
 
     IS_RUST_LIBRARY = True
 
@@ -133,20 +136,16 @@ def HostRustLibrary(name, features=None)
 def DisableStlWrapping():
     COMPILE_FLAGS['STL'] = []
 
 @template
 def NoVisibilityFlags():
     COMPILE_FLAGS['VISIBILITY'] = []
 
 @template
-def AllowCompilerWarnings():
-    COMPILE_FLAGS['WARNINGS_AS_ERRORS'] = []
-
-@template
 def ForceInclude(*headers):
     """Force includes a set of header files in C++ compilations"""
     if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'):
         include_flag = '-FI'
     else:
         include_flag = '-include'
     for header in headers:
         CXXFLAGS += [include_flag, header]
--- a/python/mozbuild/mozbuild/frontend/emitter.py
+++ b/python/mozbuild/mozbuild/frontend/emitter.py
@@ -616,16 +616,17 @@ class TreeMetadataEmitter(LoggingMixin):
                 for program in programs:
                     if program not in defined_binaries:
                         raise SandboxValidationError(
                             'Cannot find Cargo.toml definition for %s' % program,
                             context)
 
                     check_unique_binary(program, kind)
                     self._binaries[program] = cls(context, program, cargo_file)
+                    add_program(self._binaries[program], kind)
 
         for kind, cls in [
                 ('SIMPLE_PROGRAMS', SimpleProgram),
                 ('CPP_UNIT_TESTS', SimpleProgram),
                 ('HOST_SIMPLE_PROGRAMS', HostSimpleProgram)]:
             for program in context[kind]:
                 if program in self._binaries:
                     raise SandboxValidationError(
@@ -819,23 +820,19 @@ class TreeMetadataEmitter(LoggingMixin):
                 lib.lib_defines.update(lib_defines)
 
         # Only emit sources if we have linkables defined in the same context.
         # Note the linkables are not emitted in this function, but much later,
         # after aggregation (because of e.g. USE_LIBS processing).
         if not (linkables or host_linkables):
             return
 
-        # Avoid emitting compile flags for directories only containing rust
-        # libraries. Emitted compile flags are only relevant to C/C++ sources
-        # for the time being, however ldflags must be emitted for the benefit
-        # of cargo.
-        if not all(isinstance(l, (RustLibrary)) for l in linkables):
-            self._compile_dirs.add(context.objdir)
-        elif linkables:
+        self._compile_dirs.add(context.objdir)
+        if linkables and all(isinstance(l, (RustLibrary, RustProgram, HostRustProgram))
+                             for l in linkables):
             self._rust_compile_dirs.add(context.objdir)
 
         if host_linkables and not all(isinstance(l, HostRustLibrary) for l in host_linkables):
             self._host_compile_dirs.add(context.objdir)
             # TODO: objdirs with only host things in them shouldn't need target
             # flags, but there's at least one Makefile.in (in
             # build/unix/elfhack) that relies on the value of LDFLAGS being
             # passed to one-off rules.
@@ -1341,18 +1338,16 @@ class TreeMetadataEmitter(LoggingMixin):
 
 
         if passthru.variables:
             yield passthru
 
         if context.objdir in self._compile_dirs:
             self._compile_flags[context.objdir] = computed_flags
             yield computed_link_flags
-        elif context.objdir in self._rust_compile_dirs:
-            yield computed_link_flags
 
         if context.objdir in self._asm_compile_dirs:
             self._compile_as_flags[context.objdir] = computed_as_flags
 
         if context.objdir in self._host_compile_dirs:
             yield computed_host_flags
 
 
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -824,79 +824,89 @@ class TestRecursiveMakeBackend(BackendTe
         found = [str for str in lines if str.startswith('LOCAL_INCLUDES')]
         self.assertEqual(found, expected)
 
     def test_rust_library(self):
         """Test that a Rust library is written to backend.mk correctly."""
         env = self._consume('rust-library', RecursiveMakeBackend)
 
         backend_path = mozpath.join(env.topobjdir, 'backend.mk')
-        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
+        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]
+                 # Strip out computed flags, they're a PITA to test.
+                 if not l.startswith('COMPUTED_')]
 
         expected = [
             'RUST_LIBRARY_FILE := ./x86_64-unknown-linux-gnu/release/libtest_library.a',
             'CARGO_FILE := $(srcdir)/Cargo.toml',
             'CARGO_TARGET_DIR := %s' % env.topobjdir,
         ]
 
         self.assertEqual(lines, expected)
 
     def test_host_rust_library(self):
         """Test that a Rust library is written to backend.mk correctly."""
         env = self._consume('host-rust-library', RecursiveMakeBackend)
 
         backend_path = mozpath.join(env.topobjdir, 'backend.mk')
-        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
+        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]
+                 # Strip out computed flags, they're a PITA to test.
+                 if not l.startswith('COMPUTED_')]
 
         expected = [
             'HOST_RUST_LIBRARY_FILE := ./x86_64-unknown-linux-gnu/release/libhostrusttool.a',
             'CARGO_FILE := $(srcdir)/Cargo.toml',
             'CARGO_TARGET_DIR := %s' % env.topobjdir,
         ]
 
         self.assertEqual(lines, expected)
 
     def test_host_rust_library_with_features(self):
         """Test that a host Rust library with features is written to backend.mk correctly."""
         env = self._consume('host-rust-library-features', RecursiveMakeBackend)
 
         backend_path = mozpath.join(env.topobjdir, 'backend.mk')
-        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
+        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]
+                 # Strip out computed flags, they're a PITA to test.
+                 if not l.startswith('COMPUTED_')]
 
         expected = [
             'HOST_RUST_LIBRARY_FILE := ./x86_64-unknown-linux-gnu/release/libhostrusttool.a',
             'CARGO_FILE := $(srcdir)/Cargo.toml',
             'CARGO_TARGET_DIR := %s' % env.topobjdir,
             'HOST_RUST_LIBRARY_FEATURES := musthave cantlivewithout',
         ]
 
         self.assertEqual(lines, expected)
 
     def test_rust_library_with_features(self):
         """Test that a Rust library with features is written to backend.mk correctly."""
         env = self._consume('rust-library-features', RecursiveMakeBackend)
 
         backend_path = mozpath.join(env.topobjdir, 'backend.mk')
-        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
+        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]
+                 # Strip out computed flags, they're a PITA to test.
+                 if not l.startswith('COMPUTED_')]
 
         expected = [
             'RUST_LIBRARY_FILE := ./x86_64-unknown-linux-gnu/release/libfeature_library.a',
             'CARGO_FILE := $(srcdir)/Cargo.toml',
             'CARGO_TARGET_DIR := %s' % env.topobjdir,
             'RUST_LIBRARY_FEATURES := musthave cantlivewithout',
         ]
 
         self.assertEqual(lines, expected)
 
     def test_rust_programs(self):
         """Test that {HOST_,}RUST_PROGRAMS are written to backend.mk correctly."""
         env = self._consume('rust-programs', RecursiveMakeBackend)
 
         backend_path = mozpath.join(env.topobjdir, 'code/backend.mk')
-        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
+        lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]
+                 # Strip out computed flags, they're a PITA to test.
+                 if not l.startswith('COMPUTED_')]
 
         expected = [
             'CARGO_FILE := %s/code/Cargo.toml' % env.topsrcdir,
             'CARGO_TARGET_DIR := .',
             'RUST_PROGRAMS += i686-pc-windows-msvc/release/target.exe',
             'RUST_CARGO_PROGRAMS += target',
             'HOST_RUST_PROGRAMS += i686-pc-windows-msvc/release/host.exe',
             'HOST_RUST_CARGO_PROGRAMS += host',
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -1478,18 +1478,20 @@ class TestEmitterBasic(unittest.TestCase
             self.read_topsrcdir(reader)
 
     def test_rust_library_dash_folding(self):
         '''Test that on-disk names of RustLibrary objects convert dashes to underscores.'''
         reader = self.reader('rust-library-dash-folding',
                              extra_substs=dict(RUST_TARGET='i686-pc-windows-msvc'))
         objs = self.read_topsrcdir(reader)
 
-        ldflags, lib = objs
+        self.assertEqual(len(objs), 3)
+        ldflags, lib, cflags = objs
         self.assertIsInstance(ldflags, ComputedFlags)
+        self.assertIsInstance(cflags, ComputedFlags)
         self.assertIsInstance(lib, RustLibrary)
         self.assertRegexpMatches(lib.lib_name, "random_crate")
         self.assertRegexpMatches(lib.import_name, "random_crate")
         self.assertRegexpMatches(lib.basename, "random-crate")
 
     def test_multiple_rust_libraries(self):
         '''Test that linking multiple Rust libraries throws an error'''
         reader = self.reader('multiple-rust-libraries',
@@ -1498,18 +1500,21 @@ class TestEmitterBasic(unittest.TestCase
              'Cannot link multiple Rust libraries'):
             self.read_topsrcdir(reader)
 
     def test_rust_library_features(self):
         '''Test that RustLibrary features are correctly emitted.'''
         reader = self.reader('rust-library-features',
                              extra_substs=dict(RUST_TARGET='i686-pc-windows-msvc'))
         objs = self.read_topsrcdir(reader)
-        ldflags, lib = objs
+
+        self.assertEqual(len(objs), 3)
+        ldflags, lib, cflags = objs
         self.assertIsInstance(ldflags, ComputedFlags)
+        self.assertIsInstance(cflags, ComputedFlags)
         self.assertIsInstance(lib, RustLibrary)
         self.assertEqual(lib.features, ['musthave', 'cantlivewithout'])
 
     def test_rust_library_duplicate_features(self):
         '''Test that duplicate RustLibrary features are rejected.'''
         reader = self.reader('rust-library-duplicate-features')
         with self.assertRaisesRegexp(SandboxValidationError,
              'features for .* should not contain duplicates'):
@@ -1547,51 +1552,64 @@ class TestEmitterBasic(unittest.TestCase
 
     def test_rust_programs(self):
         '''Test RUST_PROGRAMS emission.'''
         reader = self.reader('rust-programs',
                              extra_substs=dict(RUST_TARGET='i686-pc-windows-msvc',
                                                BIN_SUFFIX='.exe'))
         objs = self.read_topsrcdir(reader)
 
-        ldflags, prog = objs
+        self.assertEqual(len(objs), 3)
+        ldflags, cflags, prog = objs
         self.assertIsInstance(ldflags, ComputedFlags)
+        self.assertIsInstance(cflags, ComputedFlags)
         self.assertIsInstance(prog, RustProgram)
         self.assertEqual(prog.name, 'some')
 
     def test_host_rust_programs(self):
         '''Test HOST_RUST_PROGRAMS emission.'''
         reader = self.reader('host-rust-programs',
                              extra_substs=dict(RUST_HOST_TARGET='i686-pc-windows-msvc',
                                                HOST_BIN_SUFFIX='.exe'))
         objs = self.read_topsrcdir(reader)
 
-        self.assertEqual(len(objs), 1)
-        self.assertIsInstance(objs[0], HostRustProgram)
-        self.assertEqual(objs[0].name, 'some')
+        self.assertEqual(len(objs), 4)
+        print(objs)
+        ldflags, cflags, hostflags, prog = objs
+        self.assertIsInstance(ldflags, ComputedFlags)
+        self.assertIsInstance(cflags, ComputedFlags)
+        self.assertIsInstance(hostflags, ComputedFlags)
+        self.assertIsInstance(prog, HostRustProgram)
+        self.assertEqual(prog.name, 'some')
 
     def test_host_rust_libraries(self):
         '''Test HOST_RUST_LIBRARIES emission.'''
         reader = self.reader('host-rust-libraries',
                              extra_substs=dict(RUST_HOST_TARGET='i686-pc-windows-msvc',
                                                HOST_BIN_SUFFIX='.exe'))
         objs = self.read_topsrcdir(reader)
-        self.assertEqual(len(objs), 1)
-        self.assertIsInstance(objs[0], HostRustLibrary)
-        self.assertRegexpMatches(objs[0].lib_name, 'host_lib')
-        self.assertRegexpMatches(objs[0].import_name, 'host_lib')
+
+        self.assertEqual(len(objs), 3)
+        ldflags, lib, cflags = objs
+        self.assertIsInstance(ldflags, ComputedFlags)
+        self.assertIsInstance(cflags, ComputedFlags)
+        self.assertIsInstance(lib, HostRustLibrary)
+        self.assertRegexpMatches(lib.lib_name, 'host_lib')
+        self.assertRegexpMatches(lib.import_name, 'host_lib')
 
     def test_crate_dependency_path_resolution(self):
         '''Test recursive dependencies resolve with the correct paths.'''
         reader = self.reader('crate-dependency-path-resolution',
                              extra_substs=dict(RUST_TARGET='i686-pc-windows-msvc'))
         objs = self.read_topsrcdir(reader)
 
-        ldflags, lib = objs
+        self.assertEqual(len(objs), 3)
+        ldflags, lib, cflags = objs
         self.assertIsInstance(ldflags, ComputedFlags)
+        self.assertIsInstance(cflags, ComputedFlags)
         self.assertIsInstance(lib, RustLibrary)
 
     def test_install_shared_lib(self):
         """Test that we can install a shared library with TEST_HARNESS_FILES"""
         reader = self.reader('test-install-shared-lib')
         objs = self.read_topsrcdir(reader)
         self.assertIsInstance(objs[0], TestHarnessFiles)
         self.assertIsInstance(objs[1], VariablePassthru)