Support all combination of static/shared libraries with Rust [chromium/src : main]

28 views
Skip to first unread message

danakj (Gerrit)

unread,
Mar 1, 2022, 12:20:14 PM3/1/22
to rust...@chromium.org, Chromium LUCI CQ, Adrian Taylor, chromium...@chromium.org, Łukasz Anforowicz, Matthew Riley

Attention is currently required from: Adrian Taylor.

View Change

    To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
    Gerrit-Change-Number: 3496008
    Gerrit-PatchSet: 14
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Adrian Taylor <adet...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-CC: Łukasz Anforowicz <luk...@chromium.org>
    Gerrit-Attention: Adrian Taylor <adet...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 17:19:59 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Adrian Taylor (Gerrit)

    unread,
    Mar 1, 2022, 12:35:47 PM3/1/22
    to danakj, rust...@chromium.org, Chromium LUCI CQ, chromium...@chromium.org, Łukasz Anforowicz, Matthew Riley

    Attention is currently required from: danakj.

    Patch set 14:Code-Review +1

    View Change

    5 comments:

    • Commit Message:

    • Patchset:

    • File build/rust/mixed_target.gni:

    • 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.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
    Gerrit-Change-Number: 3496008
    Gerrit-PatchSet: 14
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Adrian Taylor <adet...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-CC: Łukasz Anforowicz <luk...@chromium.org>
    Gerrit-Attention: danakj <dan...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 17:35:36 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    danakj (Gerrit)

    unread,
    Mar 1, 2022, 12:37:37 PM3/1/22
    to rust...@chromium.org

    Attention is currently required from: danakj.

    danakj uploaded patch set #15 to this change.

    View 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.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
    Gerrit-Change-Number: 3496008
    Gerrit-PatchSet: 15
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Adrian Taylor <adet...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-CC: Łukasz Anforowicz <luk...@chromium.org>
    Gerrit-Attention: danakj <dan...@chromium.org>
    Gerrit-MessageType: newpatchset

    danakj (Gerrit)

    unread,
    Mar 1, 2022, 12:39:45 PM3/1/22
    to rust...@chromium.org, Adrian Taylor, Chromium LUCI CQ, chromium...@chromium.org, Łukasz Anforowicz, Matthew Riley

    Patch set 15:Commit-Queue +1

    View Change

    4 comments:

    • Commit Message:

    • File build/rust/mixed_target.gni:

      • Done

    • File build/rust/rust_shared_library.gni:

      • 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.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
    Gerrit-Change-Number: 3496008
    Gerrit-PatchSet: 15
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Adrian Taylor <adet...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-CC: Łukasz Anforowicz <luk...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 17:39:34 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Adrian Taylor <adet...@chromium.org>
    Gerrit-MessageType: comment

    danakj (Gerrit)

    unread,
    Mar 1, 2022, 12:41:31 PM3/1/22
    to rust...@chromium.org, Adrian Taylor, Chromium LUCI CQ, chromium...@chromium.org, Łukasz Anforowicz, Matthew Riley

    Patch set 16:Commit-Queue +2

    View Change

      To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
      Gerrit-Change-Number: 3496008
      Gerrit-PatchSet: 16
      Gerrit-Owner: danakj <dan...@chromium.org>
      Gerrit-Reviewer: Adrian Taylor <adet...@chromium.org>
      Gerrit-Reviewer: danakj <dan...@chromium.org>
      Gerrit-CC: Matthew Riley <mat...@chromium.org>
      Gerrit-CC: Łukasz Anforowicz <luk...@chromium.org>
      Gerrit-Comment-Date: Tue, 01 Mar 2022 17:41:21 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      Gerrit-MessageType: comment

      Chromium LUCI CQ (Gerrit)

      unread,
      Mar 1, 2022, 1:32:41 PM3/1/22
      to danakj, rust...@chromium.org, Adrian Taylor, chromium...@chromium.org, Łukasz Anforowicz, Matthew Riley

      Chromium LUCI CQ submitted this change.

      View 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"])
      }
      }
      ```

      Approvals: Adrian Taylor: Looks good to me danakj: Commit
      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(-)


      To view, visit change 3496008. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I113a56b1a36cdec98d6dd9bddaa49cf9e5cde12f
      Gerrit-Change-Number: 3496008
      Gerrit-PatchSet: 17
      Gerrit-Owner: danakj <dan...@chromium.org>
      Gerrit-Reviewer: Adrian Taylor <adet...@chromium.org>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: danakj <dan...@chromium.org>
      Gerrit-CC: Matthew Riley <mat...@chromium.org>
      Gerrit-CC: Łukasz Anforowicz <luk...@chromium.org>
      Gerrit-MessageType: merged
      Reply all
      Reply to author
      Forward
      0 new messages