Attention is currently required from: Adrian Taylor.
To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: danakj.
Patch set 14:Code-Review +1
5 comments:
Commit Message:
Patch Set #14, Line 11: A rust crate produces the equivalent of a C++ static library: Ot is a
Typo
Patchset:
LGTM % some nits
File build/rust/mixed_target.gni:
Patch Set #14, Line 230: #not_needed(invoker, ["export_symbols"])
Commented
File build/rust/rust_shared_library.gni:
Patch Set #14, Line 20: crate_type = "cdylib"
Why is crate_type needed here? I'd hope that making a shared_library with Rust code would result in gn rules automatically figuring out crate_type = "cdylib" somewhere? If not, we probably need to look into that.
File build/rust/tests/BUILD.gn:
Patch Set #14, Line 41: "test_mixed_shared_library",
Could these not be moved down to the `if (rustc_can_link)` section lower?
To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: danakj.
danakj uploaded patch set #15 to this change.
Support all combination of static/shared libraries with Rust
1) Rename mixed_source_set/rust_source_set to *_static_library
A rust crate produces the equivalent of a C++ static library: It is a
set of object files for the sources (but not any deps) which will be
linked in the final linking step to produce a shared library or
executable. This differs from a C++ source set, which is linked
directly into some C++ static library. Rust rlibs are passed along
to the "link" or "solink" tool, as seen by the reference to {{rlibs}}
in //build/toolchain/gcc_toolchain.gni and described in
https://crbug.com/gn/276.
2) Add a test for the mixed_shared_library target type.
However the test is currently disabled, because on the bots the .so file
is missing from the isolate - we get a file not found for it.
3) Introduce a rust_shared_library type, which is the parallel to the
rust_static_library type, along with a test.
Similarly, the test is disabled for now but can be manually compiled
with the full target name.
4) Export cxx-generated C++ symbols for shared_library targets always,
even when not using component build. We ensure this happens by
having rust_cxx read invoker.export_symbols. The invoker is the only one
who knows if symbols must be exported, because it knows if the cxx
bindings are going to be linked into a shared_library or not. Previously
it was trying to let the invoker set it but was not doing so correctly.
With this, we have a way to make a component, static lib, or shared
lib that is either C++, Rust, or mixed C++-and-Rust.
R=adet...@chromium.org
Bug: 1297592
Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
Cq-Include-Trybots: luci.chromium.try:android-rust-arm-rel,linux-rust-x64-rel
---
M build/rust/cargo_crate.gni
M build/rust/mixed_component.gni
M build/rust/mixed_shared_library.gni
D build/rust/mixed_source_set.gni
A build/rust/mixed_static_library.gni
M build/rust/mixed_target.gni
M build/rust/rust_cxx.gni
A build/rust/rust_shared_library.gni
R build/rust/rust_static_library.gni
M build/rust/rust_target.gni
M build/rust/rust_unit_test.gni
M build/rust/tests/BUILD.gn
M build/rust/tests/test_cpp_including_rust/BUILD.gn
M build/rust/tests/test_cpp_including_rust/main.cc
M build/rust/tests/test_cpp_including_rust/unittests.cc
A build/rust/tests/test_mixed_shared_library/BUILD.gn
A build/rust/tests/test_mixed_shared_library/dependency_header.h
C build/rust/tests/test_mixed_shared_library/src/lib.rs
A build/rust/tests/test_mixed_shared_library/test_mixed_shared_library.cc
A build/rust/tests/test_mixed_shared_library/test_mixed_shared_library.h
D build/rust/tests/test_mixed_source_set/dependency_header.h
D build/rust/tests/test_mixed_source_set/test_mixed_source_set.h
R build/rust/tests/test_mixed_static_library/BUILD.gn
A build/rust/tests/test_mixed_static_library/dependency_header.h
R build/rust/tests/test_mixed_static_library/src/lib.rs
R build/rust/tests/test_mixed_static_library/test_mixed_static_library.cc
A build/rust/tests/test_mixed_static_library/test_mixed_static_library.h
M build/rust/tests/test_rust_exe/BUILD.gn
M build/rust/tests/test_rust_exe/main.rs
M build/rust/tests/test_rust_multiple_dep_versions_exe/BUILD.gn
M build/rust/tests/test_rust_multiple_dep_versions_exe/v1/BUILD.gn
M build/rust/tests/test_rust_multiple_dep_versions_exe/v2/BUILD.gn
A build/rust/tests/test_rust_shared_library/BUILD.gn
A build/rust/tests/test_rust_shared_library/src/lib.rs
R build/rust/tests/test_rust_static_library/BUILD.gn
R build/rust/tests/test_rust_static_library/src/lib.rs
R build/rust/tests/test_rust_static_library_non_standard_arrangement/BUILD.gn
R build/rust/tests/test_rust_static_library_non_standard_arrangement/foo.rs
M build/rust/tests/test_rust_unittests/BUILD.gn
M build/rust/tests/test_rust_unittests/main.rs
M build/rust/tests/test_serde_jsonrc/BUILD.gn
D build/rust/tests/test_variable_source_set/test_variable_source_set.h
R build/rust/tests/test_variable_static_library/BUILD.gn
R build/rust/tests/test_variable_static_library/demo.cc
R build/rust/tests/test_variable_static_library/src/lib.rs
R build/rust/tests/test_variable_static_library/test_variable_static_library.cc
A build/rust/tests/test_variable_static_library/test_variable_static_library.h
M mojo/public/rust/BUILD.gn
M testing/scripts/rust/rust_main_program_unittests.py
49 files changed, 436 insertions(+), 206 deletions(-)
To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 15:Commit-Queue +1
4 comments:
Commit Message:
Patch Set #14, Line 11: A rust crate produces the equivalent of a C++ static library: Ot is a
Typo
Done
File build/rust/mixed_target.gni:
Patch Set #14, Line 230: #not_needed(invoker, ["export_symbols"])
Commented
Done
File build/rust/rust_shared_library.gni:
Patch Set #14, Line 20: crate_type = "cdylib"
Why is crate_type needed here? I'd hope that making a shared_library with Rust code would result in […]
Without this it tries to may a dylib, and we have no tool() defined for it.
File build/rust/tests/BUILD.gn:
Patch Set #14, Line 41: "test_mixed_shared_library",
Could these not be moved down to the `if (rustc_can_link)` section lower?
They could. But those are also guided inside build_rust_unit_tests, and this is not a rust_unit_test target, it's a library.
To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 16:Commit-Queue +2
Chromium LUCI CQ submitted this change.
14 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: build/rust/mixed_target.gni
Insertions: 0, Deletions: 2.
@@ -226,7 +226,5 @@
inputs = invoker.rs_cxx_bindings
generate_source_set = false
}
- } else {
- #not_needed(invoker, ["export_symbols"])
}
}
```
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3496008
Reviewed-by: Adrian Taylor <adet...@chromium.org>
Commit-Queue: danakj <dan...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#976312}
49 files changed, 438 insertions(+), 206 deletions(-)