Bug 1231764 - part 6 - build rust code via cargo; r=chmanchester

This patch is really two separate changes.

The first change is that rust crates are large, standalone entities that
may contain multitudes of source files.  It therefore doesn't make sense
to keep them in SOURCES, as we have been doing.  Moving to use cargo
will require a higher-level approach, which suggests that we need a
different, higher-level representation for Rust sources in the build
system.

The representation here is to have the build system refer to things
defined in Cargo.toml files as the entities dealt with in the build
system, and let Cargo deal with the details of actually building things.
This approach means that adding a new crate to an existing library just
requires editing Rust and Cargo.toml files, rather than dealing with
moz.build, which seems more natural to Rust programmers.  By having the
source files for libraries (and binaries in subsequent iterations of
this support) checked in to the tree, we can also take advantage of
Cargo.lock files.

The second is that we switch the core build system over to building via
cargo, rather than invoking rustc directly.

We also clean up a number of leftover things from the Old Way of doing
things.  A number of tests are added to confirm that we'll only permit
crates to be built that have dependencies in-tree.

build/moz.configure/rust.configure

@@ -29,6 +29,20 @@ def rustc_info(rustc):
             commit=info.get('commit-hash', 'unknown'),
         )
 
+@depends_if(cargo)
+@checking('cargo support for --frozen')
+@imports('subprocess')
+def cargo_supports_frozen(cargo):
+    try:
+        lines = subprocess.check_output(
+            [cargo, 'help', 'build']
+        ).splitlines()
+        return any('    --frozen' in l for l in lines)
+    except subprocess.CalledProcessError as e:
+        die('Failed to call cargo: %s', e.message)
+
+set_config('MOZ_CARGO_SUPPORTS_FROZEN', cargo_supports_frozen)
+
 @depends('--enable-rust', rustc, rustc_info)
 @imports(_from='textwrap', _import='dedent')
 def rust_compiler(value, rustc, rustc_info):
@@ -105,10 +119,7 @@ def rust_target(rust_compiler, rustc, target, cross_compiling):
         }.get((target.cpu, os_or_kernel), None)
 
         if rustc_target is None:
-            if cross_compiling:
-                die("Don't know how to translate {} for rustc".format(target.alias))
-            # Fall back to implicit (native) target when not cross-compiling
-            return None
+            die("Don't know how to translate {} for rustc".format(target.alias))
 
         # Check to see whether our rustc has a reasonably functional stdlib
         # for our chosen target.
@@ -141,7 +152,7 @@ def rust_target(rust_compiler, rustc, target, cross_compiling):
             os.remove(in_path)
             os.remove(out_path)
         # This target is usable.
-        return target_arg
+        return rustc_target
 
 set_config('RUST_TARGET', rust_target)
 

build/templates.mozbuild

@@ -63,6 +63,14 @@ def Library(name):
     LIBRARY_NAME = name
 
 
+@template
+def RustLibrary(name):
+    '''Template for Rust libraries.'''
+    Library(name)
+
+    IS_RUST_LIBRARY = True
+
+
 @template
 def SharedLibrary(name):
     '''Template for shared libraries.'''

config/rules.mk

@@ -240,10 +240,9 @@ CMOBJS = $(notdir $(CMSRCS:.m=.$(OBJ_SUFFIX)))
 CMMOBJS = $(notdir $(CMMSRCS:.mm=.$(OBJ_SUFFIX)))
 # ASFILES can have different extensions (.s, .asm)
 ASOBJS = $(notdir $(addsuffix .$(OBJ_SUFFIX),$(basename $(ASFILES))))
-RSOBJS = $(addprefix lib,$(notdir $(RSSRCS:.rs=.rlib)))
 RS_STATICLIB_CRATE_OBJ = $(addprefix lib,$(notdir $(RS_STATICLIB_CRATE_SRC:.rs=.$(LIB_SUFFIX))))
 ifndef OBJS
-_OBJS = $(COBJS) $(SOBJS) $(CPPOBJS) $(CMOBJS) $(CMMOBJS) $(ASOBJS) $(RSOBJS)
+_OBJS = $(COBJS) $(SOBJS) $(CPPOBJS) $(CMOBJS) $(CMMOBJS) $(ASOBJS)
 OBJS = $(strip $(_OBJS))
 endif
 
@@ -566,7 +565,7 @@ compile:: host target
 
 host:: $(HOST_LIBRARY) $(HOST_PROGRAM) $(HOST_SIMPLE_PROGRAMS)
 
-target:: $(LIBRARY) $(SHARED_LIBRARY) $(PROGRAM) $(SIMPLE_PROGRAMS)
+target:: $(LIBRARY) $(SHARED_LIBRARY) $(PROGRAM) $(SIMPLE_PROGRAMS) $(RUST_LIBRARY_FILE)
 
 include $(MOZILLA_DIR)/config/makefiles/target_binaries.mk
 endif
@@ -915,17 +914,33 @@ $(ASOBJS):
 endif
 
 ifdef MOZ_RUST
-# Assume any system libraries rustc links against are already
-# in the target's LIBS.
-$(RSOBJS):
-	$(REPORT_BUILD)
-	$(RUSTC) $(RUST_TARGET) $(RUSTFLAGS) --crate-type rlib --emit dep-info=$(MDDEPDIR)/$(call mk_libname,$<).pp,link=$(call mk_libname,$<) $(_VPATH_SRCS)
+ifdef CARGO_FILE
 
-$(RS_STATICLIB_CRATE_OBJ):
-	$(REPORT_BUILD)
-	$(RUSTC) $(RUST_TARGET) $(RUSTFLAGS) --crate-type staticlib $(RLIB_EXTERN_CRATE_OPTIONS) --emit dep-info=$(MDDEPDIR)/$(call mk_global_crate_libname,$(RS_STATICLIB_CRATE_SRC)).pp,link=$@ $(RS_STATICLIB_CRATE_SRC)
+ifdef MOZ_DEBUG
+cargo_build_flags =
+else
+cargo_build_flags = --release
+endif
+ifdef MOZ_CARGO_SUPPORTS_FROZEN
+cargo_build_flags += --frozen
 endif
 
+cargo_build_flags += --manifest-path $(CARGO_FILE)
+cargo_build_flags += --target=$(RUST_TARGET)
+
+# Assume any system libraries rustc links against are already in the target's LIBS.
+#
+# We need to run cargo unconditionally, because cargo is the only thing that
+# has full visibility into how changes in Rust sources might affect the final
+# build.
+force-cargo-build:
+	$(REPORT_BUILD)
+	env CARGO_TARGET_DIR=. RUSTC=$(RUSTC) $(CARGO) build $(cargo_build_flags) --
+
+$(RUST_LIBRARY_FILE): force-cargo-build
+endif # CARGO_FILE
+endif # MOZ_RUST
+
 $(SOBJS):
 	$(REPORT_BUILD)
 	$(AS) -o $@ $(ASFLAGS) $($(notdir $<)_FLAGS) $(LOCAL_INCLUDES) -c $<

dom/media/gtest/moz.build

@@ -37,7 +37,6 @@ if CONFIG['MOZ_WEBM_ENCODER']:
     ]
 
 if CONFIG['MOZ_RUST']:
-    SOURCES += ['hello.rs',]
     UNIFIED_SOURCES += ['TestRust.cpp',]
 
 

media/libstagefright/binding/mp4parse-cargo.patch

@@ -0,0 +1,26 @@
+--- a/media/libstagefright/binding/mp4parse/Cargo.toml
++++ b/media/libstagefright/binding/mp4parse/Cargo.toml
+@@ -17,23 +17,6 @@ exclude = [
+   "*.mp4",
+ ]
+
+-build = "build.rs"
+-
+-[dependencies]
+-byteorder = "0.5.0"
+-afl = { version = "0.1.1", optional = true }
+-afl-plugin = { version = "0.1.1", optional = true }
+-abort_on_panic = { version = "1.0.0", optional = true }
+-
+-[dev-dependencies]
+-test-assembler = "0.1.2"
+-
+-[build-dependencies]
+-rusty-cheddar = "0.3.2"
+-
+-[features]
+-fuzz = ["afl", "afl-plugin", "abort_on_panic"]
+-
+ # Somewhat heavy-handed, but we want at least -Z force-overflow-checks=on.
+ [profile.release]
+ debug-assertions = true

media/libstagefright/binding/mp4parse/Cargo.toml

@@ -17,23 +17,6 @@ exclude = [
   "*.mp4",
 ]
 
-build = "build.rs"
-
-[dependencies]
-byteorder = "0.5.0"
-afl = { version = "0.1.1", optional = true }
-afl-plugin = { version = "0.1.1", optional = true }
-abort_on_panic = { version = "1.0.0", optional = true }
-
-[dev-dependencies]
-test-assembler = "0.1.2"
-
-[build-dependencies]
-rusty-cheddar = "0.3.2"
-
-[features]
-fuzz = ["afl", "afl-plugin", "abort_on_panic"]
-
 # Somewhat heavy-handed, but we want at least -Z force-overflow-checks=on.
 [profile.release]
 debug-assertions = true

media/libstagefright/binding/update-rust.sh

@@ -38,6 +38,7 @@ cp _upstream/byteorder/src/new.rs mp4parse/src/byteorder/new.rs
 echo "Applying patches..."
 patch -p4 < byteorder-mod.patch
 patch -p4 < mp4parse-mod.patch
+patch -p4 < mp4parse-cargo.patch
 
 echo "Cleaning up..."
 rm -rf _upstream

media/libstagefright/moz.build

@@ -80,9 +80,6 @@ SOURCES += [
 ]
 
 if CONFIG['MOZ_RUST']:
-    SOURCES += [
-        'binding/mp4parse/src/lib.rs',
-    ]
     EXPORTS += [
         'binding/include/mp4parse.h',
     ]

python/mozbuild/mozbuild/backend/common.py

@@ -30,7 +30,6 @@
     GeneratedWebIDLFile,
     PreprocessedTestWebIDLFile,
     PreprocessedWebIDLFile,
-    RustCrate,
     SharedLibrary,
     TestManifest,
     TestWebIDLFile,

python/mozbuild/mozbuild/backend/recursivemake.py

@@ -59,7 +59,7 @@
     ObjdirPreprocessedFiles,
     PerSourceFlag,
     Program,
-    RustCrate,
+    RustLibrary,
     SharedLibrary,
     SimpleProgram,
     Sources,
@@ -577,9 +577,11 @@ def consume_object(self, obj):
             else:
                 return False
 
-        elif isinstance(obj, RustCrate):
-            # Nothing to do because |Sources| has done the work for us.
-            pass
+        elif isinstance(obj, RustLibrary):
+            self.backend_input_files.add(obj.cargo_file)
+            self._process_rust_library(obj, backend_file)
+            # No need to call _process_linked_libraries, because Rust
+            # libraries are self-contained objects at this point.
 
         elif isinstance(obj, SharedLibrary):
             self._process_shared_library(obj, backend_file)
@@ -1162,17 +1164,6 @@ def _process_shared_library(self, libdef, backend_file):
         if libdef.symbols_file:
             backend_file.write('SYMBOLS_FILE := %s\n' % libdef.symbols_file)
 
-        rust_rlibs = [o for o in libdef.linked_libraries if isinstance(o, RustCrate)]
-        if rust_rlibs:
-            # write out Rust file with extern crate declarations.
-            extern_crate_file = mozpath.join(libdef.objdir, 'rul.rs')
-            with self._write_file(extern_crate_file) as f:
-                f.write('// AUTOMATICALLY GENERATED.  DO NOT EDIT.\n\n')
-                for rlib in rust_rlibs:
-                    f.write('extern crate %s;\n' % rlib.crate_name)
-
-            backend_file.write('RS_STATICLIB_CRATE_SRC := %s\n' % extern_crate_file)
-
     def _process_static_library(self, libdef, backend_file):
         backend_file.write_once('LIBRARY_NAME := %s\n' % libdef.basename)
         backend_file.write('FORCE_STATIC_LIB := 1\n')
@@ -1182,6 +1173,10 @@ def _process_static_library(self, libdef, backend_file):
         if libdef.no_expand_lib:
             backend_file.write('NO_EXPAND_LIBS := 1\n')
 
+    def _process_rust_library(self, libdef, backend_file):
+        backend_file.write_once('RUST_LIBRARY_FILE := %s\n' % libdef.import_name)
+        backend_file.write('CARGO_FILE := $(srcdir)/Cargo.toml')
+
     def _process_host_library(self, libdef, backend_file):
         backend_file.write('HOST_LIBRARY_NAME = %s\n' % libdef.basename)
 
@@ -1192,7 +1187,7 @@ def _build_target_for_obj(self, obj):
     def _process_linked_libraries(self, obj, backend_file):
         def write_shared_and_system_libs(lib):
             for l in lib.linked_libraries:
-                if isinstance(l, StaticLibrary):
+                if isinstance(l, (StaticLibrary, RustLibrary)):
                     write_shared_and_system_libs(l)
                 else:
                     backend_file.write_once('SHARED_LIBS += %s/%s\n'
@@ -1214,14 +1209,15 @@ def pretty_relpath(lib):
                     self._build_target_for_obj(lib))
             relpath = pretty_relpath(lib)
             if isinstance(obj, Library):
-                if isinstance(lib, StaticLibrary):
+                if isinstance(lib, RustLibrary):
+                    # We don't need to do anything here; we will handle
+                    # linkage for any RustLibrary elsewhere.
+                    continue
+                elif isinstance(lib, StaticLibrary):
                     backend_file.write_once('STATIC_LIBS += %s/%s\n'
                                         % (relpath, lib.import_name))
                     if isinstance(obj, SharedLibrary):
                         write_shared_and_system_libs(lib)
-                elif isinstance(lib, RustCrate):
-                    backend_file.write_once('RLIB_EXTERN_CRATE_OPTIONS += --extern %s=%s/%s\n'
-                                            % (lib.crate_name, relpath, lib.rlib_filename))
                 elif isinstance(obj, SharedLibrary):
                     assert lib.variant != lib.COMPONENT
                     backend_file.write_once('SHARED_LIBS += %s/%s\n'
@@ -1240,13 +1236,28 @@ def pretty_relpath(lib):
                 backend_file.write_once('HOST_LIBS += %s/%s\n'
                                    % (relpath, lib.import_name))
 
-        # We have to link the Rust super-crate after all intermediate static
-        # libraries have been listed to ensure that the Rust objects are
+        # We have to link any Rust libraries after all intermediate static
+        # libraries have been listed to ensure that the Rust libraries are
         # searched after the C/C++ objects that might reference Rust symbols.
-        # Building the Rust super-crate will take care of Rust->Rust linkage.
-        if isinstance(obj, SharedLibrary) and any(isinstance(o, RustCrate)
+        # Building Rust crates normally takes care of Rust->Rust linkage, but
+        # we have to be careful: a savvy user might have specified that there
+        # is a staticlib (that contains the Rust runtime) and several other
+        # rlibs (which are plain archive files) to be linked into a given
+        # library.  We need to ensure that the staticlib comes after the
+        # rlibs to ensure symbols are found correctly.
+        if isinstance(obj, SharedLibrary) and any(isinstance(o, RustLibrary)
                                                   for o in obj.linked_libraries):
-            backend_file.write('STATIC_LIBS += $(RS_STATICLIB_CRATE_OBJ)\n')
+            libs = [l for l in obj.linked_libraries if isinstance(l, RustLibrary)]
+            def name_cmp(l1, l2):
+                if l1.crate_type == 'staticlib':
+                    return 1
+                if l2.crate_type == 'staticlib':
+                    return -1
+                return cmp(l1.basename, l2.basename)
+            libs.sort(cmp=name_cmp)
+            for l in libs:
+                relpath = pretty_relpath(l)
+                backend_file.write('STATIC_LIBS += %s/%s\n' % (relpath, l.import_name))
 
         for lib in obj.linked_system_libs:
             if obj.KIND == 'target':
@@ -1439,7 +1450,6 @@ def _handle_linked_rust_crates(self, obj, extern_crate_file):
         backend_file = self._get_backend_file_for(obj)
 
         backend_file.write('RS_STATICLIB_CRATE_SRC := %s\n' % extern_crate_file)
-        backend_file.write('STATIC_LIBS += librul.$(LIB_SUFFIX)\n')
 
     def _handle_ipdl_sources(self, ipdl_dir, sorted_ipdl_sources,
                              unified_ipdl_cppsrcs_mapping):

python/mozbuild/mozbuild/frontend/context.py

@@ -945,6 +945,13 @@ def aggregate(files):
 
         """),
 
+    'IS_RUST_LIBRARY': (bool, bool,
+        """Whether the current library defined by this moz.build is built by Rust.
+
+        The library defined by this moz.build should have a build definition in
+        a Cargo.toml file that exists in this moz.build's directory.
+        """),
+
     'UNIFIED_SOURCES': (ContextDerivedTypedList(SourcePath, StrictOrderingOnAppendList), list,
         """Source code files that can be compiled together.