0

Use the chromium::import! macro to explicitly depend on 1p crates

This makes dependencies explicit in code so that:
- Crate names do not need to be unique across the entire Chromium
  codebase. Instead, the whole GN path contributes to the name.
- Users of crates can specify them by their GN paths instead of by
  some other globally unique naming scheme.
- Tooling can find unused dependencies in GN files and remove them.

This is built on the same idea as what google3 is planning internally
with a google3::import! macro that uses bazel paths. Googlers may see
also http://go/rust-import for internal docs on the same idea.

At the moment the macro is only used for 1p code, since 3p code needs
to have unique names within the cargo ecosystem and 3p crates need to
depend on each other by those names.

All 1p crates (except 2 below) are given unique mangled names that
include their full GN path, then the `import!` macro is able to find
and import them without ambiguity, even if their local GN target
names collide. For instance //a:foo and //b:foo can both be imported
by the `import!` macro and given different names in the importing
module.

There are 2 crates which are auto-imported and do not need to use the
macro (and thus have a globally unique name):
- chromium: this is imported into all 1p crates and provides the
  import! macro.
- rust_gtest_interop: this is imported into all test() targets and
  provides the gtest macro.

Care is taken to remove the crate name mangling from the generated
file names. This is good for file name length restrictions. But it's
also important for rules that refer to other rules by their target.
The unit tests linking requires adding /WHOLEARCHIVE to the command
line with the path of the rlib, and if the name is mangled then it
gets that wrong. As such, we move output_name renaming up to the
cargo_crate rule, and let other rust targets generate an output file
based on just the target name (which is normal and like c++). This
was not possible before since rlibs were all placed in
root_output_dir, but that is no longer the case, they are now placed
in the target_output_dir, ensuring uniqueness without mangling the
file name.

Change-Id: Ie2fc4ec00c9d7b4adca5c5d5efa70bb20e4ba63e
Cq-Include-Trybots: luci.chromium.try:android-rust-arm32-rel,android-rust-arm64-dbg,android-rust-arm64-rel,linux-rust-x64-dbg,linux-rust-x64-rel,mac-rust-x64-dbg,win-rust-x64-dbg,win-rust-x64-rel
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5117879
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Auto-Submit: danakj <danakj@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1238291}
This commit is contained in:
danakj
2023-12-15 22:31:22 +00:00
committed by Chromium LUCI CQ
parent aa3af61f50
commit 62e4aaedfe
39 changed files with 201 additions and 63 deletions
BUILD.gn
build/rust
cargo_crate.gnirust_executable.gnirust_static_library.gnirust_target.gni
tests
bindgen_test
test_aliased_deps
test_rust_exe
test_rust_metadata
test_rust_multiple_dep_versions_exe
test_rust_static_library_non_standard_arrangement
test_rust_unittests
mojo/public/rust
testing/rust_gtest_interop

@ -851,7 +851,7 @@ group("all_rust") {
if (enable_rust_mojo) {
deps += [
"//mojo/public/rust",
"//mojo/public/rust:mojo_rust",
"//mojo/public/rust:mojo_rust_integration_unittests",
"//mojo/public/rust:mojo_rust_unittests",
]

@ -195,6 +195,19 @@ template("cargo_crate") {
}
}
if (defined(invoker.output_name)) {
_output_name = invoker.output_name
} else if (_crate_type != "bin") {
# Note that file names of libraries must start with the crate name in
# order for the compiler to find transitive dependencies in the
# directory search paths (since they are not all explicitly specified).
#
# For bin targets, we expect the target name to be unique, and the name
# of the exe should not add magic stuff to it. And bin crates can not be
# transitive dependencies.
_output_name = "${_crate_name}_${_orig_target_name}"
}
_testonly = false
if (defined(invoker.testonly)) {
_testonly = invoker.testonly
@ -228,6 +241,11 @@ template("cargo_crate") {
# See comments above about cdylib.
crate_type = "rlib"
}
crate_name = _crate_name
if (defined(_output_name)) {
output_name = _output_name
}
# Don't import the `chromium` crate into third-party code.
no_chromium_prelude = true
@ -405,6 +423,7 @@ template("cargo_crate") {
# the rust_macro_toolchain for it to unblock building them while the
# Chromium stdlib is still being compiled.
rust_executable(_build_script_name) {
crate_name = _build_script_name
sources = invoker.build_sources
crate_root = invoker.build_root
testonly = _testonly

@ -60,6 +60,14 @@ template("rust_executable") {
executable_configs = invoker.configs
target_type = "executable"
assert(!defined(cxx_bindings))
# Executable targets should be unique names as they all get placed in the
# root output dir. We want their exe file name to be the same as the GN
# target, not a mangled name including the full GN path, and the exe file
# name comes from the crate name.
if (!defined(invoker.crate_name)) {
crate_name = target_name
}
}
}

@ -169,9 +169,13 @@ template("rust_static_library") {
# files to the linker, but Rust does not have a parallel to this. Instead,
# force the linker to always include the whole archive.
#
# The library name is hardcoded here, since `get_target_outputs()` can't
# be used with a target that doesn't exist yet!
_rlib_path = "${_output_dir}/lib${_target_name}.rlib"
# The crate name mangling is copied from the rust_target template, and
# the library name is hardcoded here since `get_target_outputs()` can't be
# used with a target that doesn't exist yet!
_dir = rebase_path(target_out_dir, root_out_dir + "/obj")
_crate_name =
string_join("_", string_split(_dir, "/")) + "_" + target_name
_rlib_path = "${_output_dir}/lib${_crate_name}.rlib"
if (current_os == "aix") {
# The AIX linker does not implement an option for this.
} else if (is_win) {

@ -33,9 +33,14 @@ if (build_with_chromium) {
template("rust_target") {
_target_name = target_name
_crate_name = target_name
if (defined(invoker.crate_name)) {
_crate_name = invoker.crate_name
} else {
# Build a unique mangled crate name from the full GN path that the
# chromium::import! macro can find.
_dir = rebase_path(target_out_dir, root_out_dir + "/obj")
_crate_name = string_join("_", string_split(_dir, "/")) + "_" + target_name
}
_generate_crate_root =
defined(invoker.generate_crate_root) && invoker.generate_crate_root
@ -44,11 +49,6 @@ template("rust_target") {
# neither.
assert(!defined(invoker.crate_root) || !_generate_crate_root)
if (defined(invoker.output_dir) && invoker.output_dir != "") {
# This is where the build target (.exe, .rlib, etc) goes.
_output_dir = invoker.output_dir
}
# This is where the OUT_DIR environment variable points to when running a
# build script and when compiling the build target, for consuming generated
# files.
@ -223,12 +223,6 @@ template("rust_target") {
_rustc_metadata = invoker.rustc_metadata
}
# Add a metadata-influenced suffix to the output name for libraries only.
_output_suffix = ""
if (invoker.target_type == "rust_library" && _rustc_metadata != "") {
_output_suffix = "-${_rustc_metadata}"
}
group(_target_name) {
testonly = _testonly
if (defined(_visibility)) {
@ -301,6 +295,7 @@ template("rust_target") {
rust_unit_test(_unit_test_target) {
testonly = true
crate_name = _unit_test_target
crate_root = _crate_root
sources = invoker.sources + [ crate_root ]
rustflags = _rustflags
@ -378,14 +373,24 @@ template("rust_target") {
sources += [ _crate_root ]
}
# The Rust tool() declarations, like C++ ones, use the output_name and
# output_dir, so that GN targets can override these if needed. Here we
# give them their default values, or allow them to be overridden.
if (defined(_output_dir)) {
output_dir = _output_dir
}
if (!defined(output_name) || output_name == "") {
output_name = "${crate_name}${_output_suffix}"
if (!defined(output_name)) {
# Note that file names of libraries must start with the crate name in
# order for the compiler to find transitive dependencies in the
# directory search paths (since they are not all explicitly specified).
#
# For bin targets, we expect the target name to be unique, and the name
# of the exe should not add magic stuff to it. And bin crates can not be
# transitive dependencies.
if (invoker.target_type == "executable") {
output_name = _target_name
} else {
# TODO(danakj): Since the crate name includes the whole path for 1p
# libraries, we could move the output_dir to `root_out_dir` here for
# them, which would make for shorter file paths. But we need to not
# do the same for 3p crates or those with a `crate_name` set
# explicitly.
output_name = _crate_name
}
}
if (!_allow_unsafe) {

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//build/rust/tests/bindgen_test:bindgen_test_lib";
}
use bindgen_test_lib::add_two_numbers_in_c;
fn main() {

@ -19,12 +19,15 @@ rust_static_library("test_aliased_deps") {
aliased_deps = {
# Unfortunately we have to know the `__rlib` suffix which is attached to the
# actual rlib in `rust_static_library()`.
other_name = ":real_name__rlib"
build_rust_tests_test_aliased_deps_other_name = ":real_name__rlib"
}
build_native_rust_unit_tests = true
}
rust_static_library("real_name") {
# Using a fixed crate_name here which does not work with `import!` because
# we're testing `aliased_deps` which also does not work with `import!`.
crate_name = "real_name"
crate_root = "real_name.rs"
sources = [ crate_root ]
}

@ -2,7 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
pub use other_name;
chromium::import! {
pub "//build/rust/tests/test_aliased_deps:other_name";
}
#[cfg(test)]
#[test]

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//build/rust/tests/test_aliased_deps";
}
fn main() {
test_aliased_deps::other_name::hello_world();
}

@ -11,7 +11,7 @@ rust_executable("test_rust_exe") {
"//build/rust/tests/test_proc_macro_crate",
"//build/rust/tests/test_rlib_crate:target1",
"//build/rust/tests/test_rust_static_library",
"//build/rust/tests/test_rust_static_library_non_standard_arrangement",
"//build/rust/tests/test_rust_static_library_non_standard_arrangement:lib",
]
build_native_rust_unit_tests = true
}

@ -2,6 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//build/rust/tests/test_rust_static_library";
"//build/rust/tests/test_rust_static_library_non_standard_arrangement:lib" as
test_rust_static_library_non_standard_arrangement;
}
// To mimic third-party, test_rlib_crate has a short crate_name which we do not
// need to import!.
use test_rlib_crate::say_hello_from_crate;
fn main() {

@ -3,14 +3,14 @@
# found in the LICENSE file.
import("//build/config/rust.gni")
import("//build/rust/cargo_crate.gni")
import("//build/rust/rust_executable.gni")
import("//build/rust/rust_static_library.gni")
import("//build/rust/rust_unit_test.gni")
# This target depends on two variants of the same crate: one directly, and one
# transitively. With correct metadata handling, this will work.
rust_static_library("test_rust_metadata_lib") {
crate_name = "lib"
rust_static_library("lib") {
crate_root = "lib.rs"
sources = [ "lib.rs" ]
deps = [
@ -36,24 +36,24 @@ if (can_build_rust_unit_tests) {
rust_unit_test("test_rust_metadata_unittests") {
crate_root = "tests.rs"
sources = [ "tests.rs" ]
deps = [ ":test_rust_metadata_lib" ]
deps = [ ":lib" ]
}
}
rust_executable("test_rust_metadata_exe") {
crate_root = "main.rs"
sources = [ "main.rs" ]
deps = [ ":test_rust_metadata_lib" ]
deps = [ ":lib" ]
}
# Check that the metadata handling works when linking into a C++ binary too.
executable("test_rust_metadata_cc_exe") {
sources = [ "main.cc" ]
deps = [ ":test_rust_metadata_lib" ]
deps = [ ":lib" ]
}
# A source file whose behavior depends on cfg options.
rust_static_library("transitive_dep_1") {
cargo_crate("transitive_dep_1") {
crate_name = "transitive_dep"
crate_root = "transitive_dep.rs"
sources = [ "transitive_dep.rs" ]
@ -63,7 +63,7 @@ rust_static_library("transitive_dep_1") {
# Build the same source again, but with a feature enabled. The metadata should
# disambiguate the symbols when linking.
rust_static_library("transitive_dep_2") {
cargo_crate("transitive_dep_2") {
crate_name = "transitive_dep"
crate_root = "transitive_dep.rs"
sources = [ "transitive_dep.rs" ]

@ -11,6 +11,10 @@
// what, rustc will see the conflict.
extern crate transitive_dep;
chromium::import! {
"//build/rust/tests/test_rust_metadata:foo_dependency";
}
pub use foo_dependency::say_foo;
pub use foo_dependency::say_foo_directly;
pub use transitive_dep::say_something;

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//build/rust/tests/test_rust_metadata:lib";
}
fn main() {
lib::print_foo_bar();
println!("{} from re-exported function", lib::say_foo_directly());

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//build/rust/tests/test_rust_metadata:lib";
}
#[test]
fn test_expected_outputs() {
assert_eq!(lib::say_foo(), "foo");

@ -5,7 +5,7 @@
import("//build/rust/rust_executable.gni")
import("//build/rust/rust_static_library.gni")
# The exe depends on lib v1.But it also transitively depends on lib v2.
# The exe depends on lib v1. But it also transitively depends on lib v2.
# The code in the exe should use v1, and the code in the transitive lib should
# use v2.
rust_executable("test_rust_multiple_dep_versions_exe") {

@ -2,6 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//build/rust/tests/test_rust_multiple_dep_versions_exe:transitive_v2";
}
// To mimic third-party, the `test_lib` crate has a short name which does not
// need to be import!ed.
fn main() {
test_lib::say_hello_from_v1();
transitive_v2::transitively_say_hello();

@ -2,6 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// To mimic third-party, the `test_lib` crate has a short name which does not
// need to be import!ed.
pub fn transitively_say_hello() {
test_lib::say_hello_from_v2();
}

@ -5,6 +5,8 @@
import("//build/rust/cargo_crate.gni")
cargo_crate("test_lib") {
crate_name = "test_lib"
# This crate has the same name as v2/test_lib, but a different epoch. The GN
# target for the unit tests should not collide.
epoch = "1"

@ -5,6 +5,8 @@
import("//build/rust/cargo_crate.gni")
cargo_crate("test_lib") {
crate_name = "test_lib"
# This crate has the same name as v1/test_lib, but a different epoch. The GN
# target for the unit tests should not collide.
epoch = "2"

@ -4,7 +4,7 @@
import("//build/rust/rust_static_library.gni")
rust_static_library("test_rust_static_library_non_standard_arrangement") {
rust_static_library("lib") {
sources = [ "foo.rs" ]
crate_root = "foo.rs"
unit_test_target = "foo_tests"

@ -5,6 +5,10 @@
#![feature(test)]
extern crate test;
chromium::import! {
"//build/rust/tests/test_rust_static_library";
}
use test::Bencher;
use test_rust_static_library::add_two_ints_via_rust;

@ -8,8 +8,8 @@ import("//build/rust/rust_static_library.gni")
import("//testing/test.gni")
# Meta target to build everything
group("rust") {
deps = [ ":mojo_rust" ]
group("mojo_rust") {
deps = [ ":mojo" ]
}
# These tests require real mojo and can't be mixed with test-only mojo. Mojo can
@ -45,9 +45,8 @@ rust_bindgen("mojo_c_system_binding") {
visibility = [ ":*" ]
}
rust_static_library("mojo_rust_system") {
rust_static_library("mojo_system") {
crate_root = "system/lib.rs"
crate_name = "mojo_system"
allow_unsafe = true
sources = [
@ -78,10 +77,9 @@ rust_static_library("mojo_rust_system") {
rebase_path(bindgen_output[0], get_path_info(crate_root, "dir")) ]
}
rust_static_library("mojo_rust_system_test_support") {
rust_static_library("mojo_system_test_support") {
testonly = true
crate_root = "system/test_support/lib.rs"
crate_name = "mojo_system_test_support"
# Calls Mojo FFI functions.
allow_unsafe = true
@ -89,14 +87,13 @@ rust_static_library("mojo_rust_system_test_support") {
sources = [ "system/test_support/lib.rs" ]
deps = [
":mojo_rust_system",
":mojo_system",
"//mojo/public/c/system",
]
}
rust_static_library("mojo_rust_bindings") {
rust_static_library("mojo_bindings") {
crate_root = "bindings/lib.rs"
crate_name = "mojo_bindings"
allow_unsafe = true
sources = [
@ -108,18 +105,17 @@ rust_static_library("mojo_rust_bindings") {
"bindings/mojom.rs",
]
deps = [ ":mojo_rust_system" ]
deps = [ ":mojo_system" ]
}
# Convenience target to link both Mojo Rust components and reexport them under a
# single `mojo` name.
rust_static_library("mojo_rust") {
rust_static_library("mojo") {
crate_root = "lib.rs"
crate_name = "mojo"
sources = [ "lib.rs" ]
deps = [
":mojo_rust_bindings",
":mojo_rust_system",
":mojo_bindings",
":mojo_system",
]
}

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//mojo/public/rust:mojo_system" as system;
}
use crate::encoding::{
Bits, Context, DataHeader, DataHeaderValue, MojomPrimitive, DATA_HEADER_SIZE,
};
@ -11,7 +15,7 @@ use std::mem;
use std::ptr;
use std::vec::Vec;
use system::{self, CastHandle, Handle, UntypedHandle};
use system::{CastHandle, Handle, UntypedHandle};
#[derive(Debug, Eq, PartialEq)]
pub enum ValidationError {

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//mojo/public/rust:mojo_system" as system;
}
use crate::mojom::MOJOM_NULL_POINTER;
use std::mem;

@ -6,8 +6,10 @@
// Require unsafe blocks for unsafe operations even in an unsafe fn.
#![deny(unsafe_op_in_unsafe_fn)]
/// `pub` since a macro refers to `$crate::system`.
pub extern crate mojo_system as system;
chromium::import! {
// `pub` since a macro refers to `$crate::system`.
pub "//mojo/public/rust:mojo_system" as system;
}
pub mod macros;

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//mojo/public/rust:mojo_system" as system;
}
use crate::decoding::{Decoder, DecodingState, ValidationError};
use crate::encoding;
use crate::encoding::{

@ -2,7 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
pub use mojo_bindings as bindings;
pub use mojo_system as system;
chromium::import! {
pub "//mojo/public/rust:mojo_bindings" as bindings;
pub "//mojo/public/rust:mojo_system" as system;
}
pub use system::MojoResult;

@ -5,7 +5,9 @@
//! Utilities to support testing Mojo clients and the Mojo system implementation
//! itself.
extern crate mojo_system as system;
chromium::import! {
"//mojo/public/rust:mojo_system" as system;
}
macro_rules! gen_panic_stub {
($name:ident $(, $arg:ident : $arg_ty:ty)*) => {

@ -18,7 +18,7 @@ rust_static_library("mojo_rust_system_tests") {
deps = [
":c_test_support",
":test_util",
"//mojo/public/rust:mojo_rust",
"//mojo/public/rust:mojo",
"//testing/rust_gtest_interop",
"//third_party/rust/lazy_static/v1:lib",
]
@ -40,8 +40,8 @@ rust_static_library("mojo_rust_encoding_tests") {
":c_test_support",
":test_util",
"//mojo/public/c/system",
"//mojo/public/rust:mojo_rust",
"//mojo/public/rust:mojo_rust_system_test_support",
"//mojo/public/rust:mojo",
"//mojo/public/rust:mojo_system_test_support",
"//testing/rust_gtest_interop",
]
@ -72,7 +72,7 @@ rust_static_library("mojo_rust_integration_tests") {
deps = [
":c_test_support",
":test_util",
"//mojo/public/rust:mojo_rust",
"//mojo/public/rust:mojo",
"//testing/rust_gtest_interop",
]
}

@ -8,6 +8,11 @@
//! and the result being caught in the test! macro. If a test function
//! returns without panicking, it is assumed to pass.
chromium::import! {
"//mojo/public/rust:mojo";
"//mojo/public/rust/tests:test_util" as util;
}
use crate::mojom_validation::*;
use mojo::bindings::encoding::Context;

@ -2,9 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
extern crate mojo_system_test_support as test_support;
extern crate rust_gtest_interop;
extern crate test_util as util;
chromium::import! {
"//mojo/public/rust:mojo_system_test_support" as test_support;
}
/// Macro to produce a test which uses the stub Mojo backend. This is used
/// instead of the macro from `test_util`, which initializes the full Mojo

@ -8,6 +8,10 @@
#![allow(unused_variables)]
#![allow(dead_code)]
chromium::import! {
"//mojo/public/rust:mojo";
}
use mojo::bindings::decoding::{self, Decoder, ValidationError};
use mojo::bindings::encoding::{
self, Context, DataHeaderValue, Encoder, EncodingState, DATA_HEADER_SIZE,

@ -8,6 +8,10 @@
//! and the result being caught in the test! macro. If a test function
//! returns without panicking, it is assumed to pass.
chromium::import! {
"//mojo/public/rust:mojo";
}
use mojo::bindings::decoding::{Decoder, ValidationError};
use mojo::bindings::encoding::{self, Context, DataHeaderValue, Encoder, EncodingState};
use mojo::bindings::impl_encodable_for_pointer;

@ -8,6 +8,11 @@
//! and the result being caught in the test! macro. If a test function
//! returns without panicking, it is assumed to pass.
chromium::import! {
"//mojo/public/rust:mojo";
"//mojo/public/rust/tests:test_util" as util;
}
use crate::mojom_validation::*;
use mojo::bindings::mojom::MojomMessageOption;

@ -4,6 +4,11 @@
//! Tests for system + encoding that use a real Mojo implementation.
chromium::import! {
"//mojo/public/rust:mojo";
"//mojo/public/rust/tests:test_util";
}
use mojo::bindings::mojom::{MojomInterface, MojomInterfaceRecv, MojomInterfaceSend};
use mojo::system::message_pipe;
use mojo::system::{Handle, HandleSignals};

@ -11,6 +11,11 @@
#![feature(assert_matches)]
#![feature(maybe_uninit_write_slice)]
chromium::import! {
"//mojo/public/rust:mojo";
"//mojo/public/rust/tests:test_util";
}
mod run_loop;
use mojo::system::shared_buffer::{self, SharedBuffer};

@ -8,6 +8,11 @@
//! and the result being caught in the test! macro. If a test function
//! returns without panicking, it is assumed to pass.
chromium::import! {
"//mojo/public/rust:mojo";
"//mojo/public/rust/tests:test_util";
}
use mojo::bindings::run_loop::{self, Handler, RunLoop, Token, WaitError};
use mojo::system::{message_pipe, HandleSignals, MOJO_INDEFINITE};
use rust_gtest_interop::prelude::*;

@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
chromium::import! {
"//testing/rust_gtest_interop:gtest_attribute";
}
use std::pin::Pin;
/// Use `prelude:::*` to get access to all macros defined in this crate.