Bug 1304815 - rearrange Rust crate structure for newer Rust releases;…

… r=ted.mielczarek

In our current Rust world, we have the following dependency structure:

  xul.so --------------------------+
                                   |
  xul-gtest.so -+--> xul.a --------+-> gkrust
                |
                +--> gkrust-gtest

This structure results in link errors with multiply-defined symbols
between gkrust-gtest and gkrust with newer Rust releases when linking
xul-gtest.so.  So we have to do something different.

Our new structure is:

  xul.so --------------------------+
                                   |
  xul-gtest.so -+--> xul.a --------+-> gkrust --+-> gkrust-shared
                |                               |
                +--> gkrust-gtest --------------+

and we enforce that a given shared library can only have at most one
Rust library that it depends on.  Said Rust library is assumed to
include all significant Rust dependencies of the dependent static
libraries as well.  (In the above structure, gkrust is simply a wrapper
around gkrust-shared, so gkrust-gtest doesn't have to include gkrust as
a dependency.)

config/expandlibs_gen.py

@@ -21,11 +21,6 @@ def generate(args):
             else:
                 raise Exception("File not found: %s" % arg)
         elif os.path.splitext(arg)[1] == conf.LIB_SUFFIX:
-            # We want to skip static libraries with the name foo-rs-prelink
-            # as they are individually linked for every final library, and
-            # thus should not be included in the descriptor file
-            if '-rs-prelink' in os.path.basename(arg):
-                continue
             if os.path.exists(arg) or os.path.exists(arg + conf.LIBS_DESC_SUFFIX):
                 desc['LIBS'].append(os.path.abspath(arg))
             else:

config/rules.mk

@@ -796,7 +796,7 @@ endif
 # symlinks back to the originals. The symlinks are a no-op for stabs debugging,
 # so no need to conditionalize on OS version or debugging format.
 
-$(SHARED_LIBRARY): $(OBJS) $(RESFILE) $(STATIC_LIBS_DEPS) $(EXTRA_DEPS) $(GLOBAL_DEPS)
+$(SHARED_LIBRARY): $(OBJS) $(RESFILE) $(RUST_STATIC_LIB_FOR_SHARED_LIB) $(STATIC_LIBS_DEPS) $(EXTRA_DEPS) $(GLOBAL_DEPS)
 	$(REPORT_BUILD)
 ifndef INCREMENTAL_LINKER
 	$(RM) $@
@@ -805,10 +805,10 @@ ifdef DTRACE_LIB_DEPENDENT
 ifndef XP_MACOSX
 	dtrace -x nolibs -G -C -s $(MOZILLA_DTRACE_SRC) -o  $(DTRACE_PROBE_OBJ) $(shell $(EXPAND_LIBS) $(MOZILLA_PROBE_LIBS))
 endif
-	$(EXPAND_MKSHLIB) $(SHLIB_LDSTARTFILE) $(OBJS) $(SUB_SHLOBJS) $(DTRACE_PROBE_OBJ) $(MOZILLA_PROBE_LIBS) $(RESFILE) $(LDFLAGS) $(WRAP_LDFLAGS) $(STATIC_LIBS) $(SHARED_LIBS) $(EXTRA_DSO_LDOPTS) $(MOZ_GLUE_LDFLAGS) $(EXTRA_LIBS) $(OS_LIBS) $(SHLIB_LDENDFILE)
+	$(EXPAND_MKSHLIB) $(SHLIB_LDSTARTFILE) $(OBJS) $(SUB_SHLOBJS) $(DTRACE_PROBE_OBJ) $(MOZILLA_PROBE_LIBS) $(RESFILE) $(LDFLAGS) $(WRAP_LDFLAGS) $(STATIC_LIBS) $(RUST_STATIC_LIB_FOR_SHARED_LIB) $(SHARED_LIBS) $(EXTRA_DSO_LDOPTS) $(MOZ_GLUE_LDFLAGS) $(EXTRA_LIBS) $(OS_LIBS) $(SHLIB_LDENDFILE)
 	@$(RM) $(DTRACE_PROBE_OBJ)
 else # ! DTRACE_LIB_DEPENDENT
-	$(EXPAND_MKSHLIB) $(SHLIB_LDSTARTFILE) $(OBJS) $(SUB_SHLOBJS) $(RESFILE) $(LDFLAGS) $(WRAP_LDFLAGS) $(STATIC_LIBS) $(SHARED_LIBS) $(EXTRA_DSO_LDOPTS) $(MOZ_GLUE_LDFLAGS) $(EXTRA_LIBS) $(OS_LIBS) $(SHLIB_LDENDFILE)
+	$(EXPAND_MKSHLIB) $(SHLIB_LDSTARTFILE) $(OBJS) $(SUB_SHLOBJS) $(RESFILE) $(LDFLAGS) $(WRAP_LDFLAGS) $(STATIC_LIBS) $(RUST_STATIC_LIB_FOR_SHARED_LIB) $(SHARED_LIBS) $(EXTRA_DSO_LDOPTS) $(MOZ_GLUE_LDFLAGS) $(EXTRA_LIBS) $(OS_LIBS) $(SHLIB_LDENDFILE)
 endif # DTRACE_LIB_DEPENDENT
 	$(call CHECK_BINARY,$@)
 
@@ -898,7 +898,7 @@ $(ASOBJS):
 endif
 
 ifdef MOZ_RUST
-ifdef CARGO_FILE
+ifdef RUST_LIBRARY_FILE
 
 ifdef MOZ_DEBUG
 cargo_build_flags =
@@ -927,30 +927,6 @@ force-cargo-build:
 
 $(RUST_LIBRARY_FILE): force-cargo-build
 endif # CARGO_FILE
-
-ifdef RUST_PRELINK
-# Make target for building a prelinked rust library. This merges rust .rlibs
-# together into a single .a file which is used within the FINAL_LIBRARY.
-#
-# RUST_PRELINK_FLAGS, RUST_PRELINK_SRC, and RUST_PRELINK_DEPS are set in
-# recursivemake.py, and together tell rustc how to find the libraries to link
-# together, but we compute the optimization flags below
-
-RUST_PRELINK_FLAGS += -g
-RUST_PRELINK_FLAGS += -C panic=abort
-
-ifdef MOZ_DEBUG
-RUST_PRELINK_FLAGS += -C opt-level=1
-RUST_PRELINK_FLAGS += -C debug-assertions
-else
-RUST_PRELINK_FLAGS += -C opt-level=2
-RUST_PRELINK_FLAGS += -C lto
-endif
-
-$(RUST_PRELINK): $(RUST_PRELINK_DEPS) $(RUST_PRELINK_SRC)
-	$(REPORT_BUILD)
-	$(RUSTC) -o $@ --crate-type staticlib --target $(RUST_TARGET) $(RUST_PRELINK_FLAGS) $(RUST_PRELINK_SRC)
-endif # RUST_PRELINK
 endif # MOZ_RUST
 
 $(SOBJS):

python/mozbuild/mozbuild/backend/recursivemake.py

@@ -1238,49 +1238,8 @@ def pretty_relpath(lib):
         # 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.
-
-        def find_rlibs(obj):
-            if isinstance(obj, RustLibrary):
-                yield obj
-            elif isinstance(obj, StaticLibrary) and not obj.no_expand_lib:
-                for l in obj.linked_libraries:
-                    for rlib in find_rlibs(l):
-                        yield rlib
-
-        # Check if we have any rust libraries to prelink and include in our
-        # final library. If we do, write out the RUST_PRELINK information
-        rlibs = []
-        if isinstance(obj, (SharedLibrary, StaticLibrary)):
-            for l in obj.linked_libraries:
-                rlibs += find_rlibs(l)
-        if rlibs:
-            prelink_libname = '%s/%s%s-rs-prelink%s' \
-                              % (relpath,
-                                 obj.config.lib_prefix,
-                                 obj.basename,
-                                 obj.config.lib_suffix)
-            backend_file.write('RUST_PRELINK := %s\n' % prelink_libname)
-            backend_file.write_once('STATIC_LIBS += %s\n' % prelink_libname)
-
-            extern_crate_file = mozpath.join(
-                obj.objdir, '%s-rs-prelink.rs' % obj.basename)
-            with self._write_file(extern_crate_file) as f:
-                f.write('// AUTOMATICALLY GENERATED.  DO NOT EDIT.\n\n')
-                for rlib in rlibs:
-                    f.write('extern crate %s;\n'
-                            % rlib.basename.replace('-', '_'))
-            backend_file.write('RUST_PRELINK_SRC := %s\n' % extern_crate_file)
-
-            backend_file.write('RUST_PRELINK_FLAGS :=\n')
-            backend_file.write('RUST_PRELINK_DEPS :=\n')
-            for rlib in rlibs:
-                rlib_relpath = pretty_relpath(rlib)
-                backend_file.write('RUST_PRELINK_FLAGS += --extern %s=%s/%s\n'
-                                   % (rlib.basename.replace('-', '_'), rlib_relpath, rlib.import_name))
-                backend_file.write('RUST_PRELINK_FLAGS += -L %s/%s\n'
-                                   % (rlib_relpath, rlib.deps_path))
-                backend_file.write('RUST_PRELINK_DEPS += %s/%s\n'
-                                   % (rlib_relpath, rlib.import_name))
+        if isinstance(obj, SharedLibrary):
+            self._process_rust_libraries(obj, backend_file, pretty_relpath)
 
         for lib in obj.linked_system_libs:
             if obj.KIND == 'target':
@@ -1291,6 +1250,23 @@ def find_rlibs(obj):
         # Process library-based defines
         self._process_defines(obj.lib_defines, backend_file)
 
+    def _process_rust_libraries(self, obj, backend_file, pretty_relpath):
+        assert isinstance(obj, SharedLibrary)
+
+        # If this library does not depend on any Rust libraries, then we are done.
+        direct_linked = [l for l in obj.linked_libraries if isinstance(l, RustLibrary)]
+        if not direct_linked:
+            return
+
+        # We should have already checked this in Linkable.link_library.
+        assert len(direct_linked) == 1
+
+        # TODO: see bug 1310063 for checking dependencies are set up correctly.
+
+        direct_linked = direct_linked[0]
+        backend_file.write('RUST_STATIC_LIB_FOR_SHARED_LIB := %s/%s\n' %
+                           (pretty_relpath(direct_linked), direct_linked.import_name))
+
     def _process_final_target_files(self, obj, files, backend_file):
         target = obj.install_target
         path = mozpath.basedir(target, (

python/mozbuild/mozbuild/frontend/data.py

@@ -310,6 +310,10 @@ class LinkageWrongKindError(Exception):
     """Error thrown when trying to link objects of the wrong kind"""
 
 
+class LinkageMultipleRustLibrariesError(Exception):
+    """Error thrown when trying to link multiple Rust libraries to an object"""
+
+
 class Linkable(ContextDerived):
     """Generic context derived container object for programs and libraries"""
     __slots__ = (
@@ -333,6 +337,13 @@ def link_library(self, obj):
                 'Linkable.link_library() does not take components.')
         if obj.KIND != self.KIND:
             raise LinkageWrongKindError('%s != %s' % (obj.KIND, self.KIND))
+        # Linking multiple Rust libraries into an object would result in
+        # multiple copies of the Rust standard library, as well as linking
+        # errors from duplicate symbols.
+        if isinstance(obj, RustLibrary) and any(isinstance(l, RustLibrary)
+                                                for l in self.linked_libraries):
+            raise LinkageMultipleRustLibrariesError("Cannot link multiple Rust libraries into %s",
+                                                    self)
         self.linked_libraries.append(obj)
         if obj.cxx_link:
             self.cxx_link = True
@@ -468,19 +479,23 @@ class RustLibrary(StaticLibrary):
     __slots__ = (
         'cargo_file',
         'crate_type',
+        'dependencies',
         'deps_path',
     )
 
-    def __init__(self, context, basename, cargo_file, crate_type, **args):
+    def __init__(self, context, basename, cargo_file, crate_type, dependencies, **args):
         StaticLibrary.__init__(self, context, basename, **args)
         self.cargo_file = cargo_file
         self.crate_type = crate_type
         # We need to adjust our naming here because cargo replaces '-' in
         # package names defined in Cargo.toml with underscores in actual
         # filenames. But we need to keep the basename consistent because
         # many other things in the build system depend on that.
-        assert self.crate_type == 'rlib'
-        self.lib_name = 'lib%s.rlib' % basename.replace('-', '_')
+        assert self.crate_type == 'staticlib'
+        self.lib_name = '%s%s%s' % (context.config.lib_prefix,
+                                     basename.replace('-', '_'),
+                                     context.config.lib_suffix)
+        self.dependencies = dependencies
         # cargo creates several directories and places its build artifacts
         # in those directories.  The directory structure depends not only
         # on the target, but also what sort of build we are doing.

python/mozbuild/mozbuild/frontend/emitter.py

@@ -437,7 +437,7 @@ def _rust_library(self, context, libname, static_args):
                 context)
 
         crate_type = crate_type[0]
-        if crate_type != 'rlib':
+        if crate_type != 'staticlib':
             raise SandboxValidationError(
                 'crate-type %s is not permitted for %s' % (crate_type, libname),
                 context)
@@ -463,7 +463,10 @@ def _rust_library(self, context, libname, static_args):
                      ' in [profile.%s] section') % (libname, profile_name),
                     context)
 
-        return RustLibrary(context, libname, cargo_file, crate_type, **static_args)
+        dependencies = set(config.get('dependencies', {}).iterkeys())
+
+        return RustLibrary(context, libname, cargo_file, crate_type,
+                           dependencies, **static_args)
 
     def _handle_linkables(self, context, passthru, generated_files):
         linkables = []

python/mozbuild/mozbuild/test/frontend/data/crate-dependency-path-resolution/Cargo.toml

@@ -6,7 +6,7 @@ authors = [
 ]
 
 [lib]
-crate-type = ["rlib"]
+crate-type = ["staticlib"]
 
 [dependencies]
 deep-crate = { version = "0.1.0", path = "the/depths" }

python/mozbuild/mozbuild/test/frontend/data/multiple-rust-libraries/moz.build

@@ -0,0 +1,27 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+@template
+def Library(name):
+    '''Template for libraries.'''
+    LIBRARY_NAME = name
+
+
+@template
+def RustLibrary(name):
+    '''Template for Rust libraries.'''
+    Library(name)
+
+    IS_RUST_LIBRARY = True
+
+Library('test')
+
+DIRS += [
+    'rust1',
+    'rust2',
+]
+
+USE_LIBS += [
+    'rust1',
+    'rust2',
+]

python/mozbuild/mozbuild/test/frontend/data/multiple-rust-libraries/rust1/Cargo.toml

@@ -0,0 +1,15 @@
+[package]
+name = "rust1"
+version = "0.1.0"
+authors = [
+  "Nobody <nobody@mozilla.org>",
+]
+
+[lib]
+crate-type = ["staticlib"]
+
+[profile.dev]
+panic = "abort"
+
+[profile.release]
+panic = "abort"

python/mozbuild/mozbuild/test/frontend/data/multiple-rust-libraries/rust1/moz.build

@@ -0,0 +1,4 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+RustLibrary('rust1')

python/mozbuild/mozbuild/test/frontend/data/multiple-rust-libraries/rust2/Cargo.toml

@@ -0,0 +1,15 @@
+[package]
+name = "rust2"
+version = "0.1.0"
+authors = [
+  "Nobody <nobody@mozilla.org>",
+]
+
+[lib]
+crate-type = ["staticlib"]
+
+[profile.dev]
+panic = "abort"
+
+[profile.release]
+panic = "abort"

python/mozbuild/mozbuild/test/frontend/data/multiple-rust-libraries/rust2/moz.build

@@ -0,0 +1,4 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+RustLibrary('rust2')

python/mozbuild/mozbuild/test/frontend/data/rust-library-dash-folding/Cargo.toml

@@ -6,7 +6,7 @@ authors = [
 ]
 
 [lib]
-crate-type = ["rlib"]
+crate-type = ["staticlib"]
 
 [profile.dev]
 panic = "abort"

python/mozbuild/mozbuild/test/frontend/data/rust-library-no-profile-section/Cargo.toml

@@ -6,7 +6,7 @@ authors = [
 ]
 
 [lib]
-crate-type = ["rlib"]
+crate-type = ["staticlib"]
 
 [profile.release]
 panic = "abort"

python/mozbuild/mozbuild/test/frontend/data/rust-library-non-abort-panic/Cargo.toml

@@ -6,7 +6,7 @@ authors = [
 ]
 
 [lib]
-crate-type = ["rlib"]
+crate-type = ["staticlib"]
 
 [profile.dev]
 panic = "unwind"

python/mozbuild/mozbuild/test/frontend/test_emitter.py

@@ -28,6 +28,7 @@
     HostSources,
     IPDLFile,
     JARManifest,
+    LinkageMultipleRustLibrariesError,
     LocalInclude,
     Program,
     RustLibrary,
@@ -1075,6 +1076,14 @@ def test_rust_library_dash_folding(self):
         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',
+                             extra_substs=dict(RUST_TARGET='i686-pc-windows-msvc'))
+        with self.assertRaisesRegexp(LinkageMultipleRustLibrariesError,
+             'Cannot link multiple Rust libraries'):
+            self.read_topsrcdir(reader)
+
     def test_crate_dependency_path_resolution(self):
         '''Test recursive dependencies resolve with the correct paths.'''
         reader = self.reader('crate-dependency-path-resolution',