-
Notifications
You must be signed in to change notification settings - Fork 497
Description
Howdy!
I have a cargo manifest like so:
[package]
name = "my-app"
version = "0.1.0"
authors = [<me>]
[dependencies]
libc = "0.2"
log = "0.4"
env_logger = "0.5"
bytes = "0.4"
failure = "0.1"
futures = "0.1"
tokio = "0.1"
tokio-signal = "0.2"
qp-trie = "0.7"
rmp = "0.8"
rmpv = "0.4"
[lib]
crate-type = ["cdylib", "rlib"]
In src I have lib.rs
and main.rs
. Using cargo I get 3 artifacts that are used by other parts of the existing build: libmy_app.so
, libmy_app.rlib
, and the program my-app
.
I can get a working bazel build with something like this:
package(default_visibility = ["//visibility:public"])
load("@io_bazel_rules_rust//rust:rust.bzl", "rust_binary","rust_library")
COMMON_DEPS = [
"//vendor/crates:libc",
"//vendor/crates:log",
"//vendor/crates:env_logger",
"//vendor/crates:bytes",
"//vendor/crates:failure",
"//vendor/crates:futures",
"//vendor/crates:tokio",
"//vendor/crates:tokio_signal",
"//vendor/crates:qp_trie",
"//vendor/crates:rmp",
"//vendor/crates:rmpv",
]
rust_library(
name = "my_app_client",
srcs = ["src/lib.rs"],
deps = COMMON_DEPS,
crate_type = "cdylib",
)
rust_binary(
name = "my_app",
srcs = ["src/main.rs"],
deps = COMMON_DEPS,
)
But this gives me a binary my_app
instead of my-app
which is probably something I can live with; but my shared library is just totally wrong now libmy_app_client--<HASH>.so
.
I'm generating the header for the library using cbindgen and this shared library is used in other parts of our build so the name isn't entirely a problem for that purpose, but it is also part of the package given to clients so this is ultimately not going to work.
I've tried to use -o libmy_app.so
in the rustc flags but the build fails because it can't find libmy_app_client-<HASH>.so
. :D Looks like bazel and I are at another impasse.
Am I missing an obvious solution or configuration here?
Activity
photex commentedon Aug 1, 2018
I created a repo that illustrates the problem:
https://github.com/photex/bazel_rust_tests
acmcarther commentedon Aug 1, 2018
On intention
Are your expectations that a library built using Cargo alone should generate the same artifact names as with Bazel? I don't think this is impossible, but it wouldn't be trivial, and it couldn't be guaranteed to remain consistent with Cargo. Your following questions indicate to me though that you actually don't care about the output name -- you just want to be able to link it. Bazel does this using target names and
deps
, not by directly specifying filenames (apologies if this is already clear to you).Are you trying to export a shared library for other consumers outside of Bazel? In this case, you should use a packaging rule like pkg_tar (with a dependency on the rust_library) to package the file up. Are you trying to link the shared library to the binary? If so, you should add a dependency on the shared library to your deps.
It would be possible in a genrule to rename the output dylib if that was something you were interested in for the purpose of exporting the lib out of Bazel.
Conversely, if you're just trying to call Rust from C, check out the ffi example: https://github.com/bazelbuild/rules_rust/blob/master/examples/ffi/c_calling_rust/BUILD . It doesn't use a dylib, but I don't think there is a reason why it couldn't (@mfarrugi might know something I don't).
On Filenames
rules_rust uses a different mechanism to generate the output file name than cargo does for practical reasons.
bazel
rules_rust/rust/rust.bzl
Lines 197 to 198 in 1944c8a
cargo
https://github.com/rust-lang/cargo/blob/ef719bc5e5966d99cce14a092a0525eba399ea1d/src/cargo/core/compiler/context/compilation_files.rs#L97-L100
Side note: The extra "-" in your new output file is due to bazel's hash generating both negative and positive numbers. I didn't find this to be an issue in reality, so I left it that way.
On expectations
I don't have a complete picture of what you're trying to achieve, but I want to caution you that Cargo and Bazel really aren't compatible at all. If you're trying to export a compiled library for other Rust users who use Cargo, you're going to be breaking some very new ground. I've built hacks for Bazel to get at Cargo crates (because, lets face it, the language has a minuscule stdlib and you need access to crates to be able to write anything in it), but we're at a very early stage in the cross compatibility story.
I'm excited to see this discussion regardless, and happy to help sort through it.
photex commentedon Aug 1, 2018
Sorry if I'm dropping a lot of comments and issues today, I decided to really sit down and dedicate the day to working through this stuff. Thanks for your patience and thoughtful comments!
In terms of the names of targets my only concern is with the shared object. That is both used internally (and it could be called anything for all it matters) and externally (whereby it's important that it is named libmy_app.so). The externally provided "dev package" with the client library and associated header would potentially be used by any number of build systems.
The difference in filenames for the executable is purely one of convention. Most of the executables are meant to be installed via a product deb package and managed by systemd. As long as clients can link with the client library internal tools and software built by clients should all be on the same page.
Let's ignore the rlib to simplify matters for the time being. I included that to provide a rust library for potentially external rust client software even though I honestly have no idea how those dots connect yet 😁.
The majority of our product is C++, but this facet of Rust is a keystone so that's why I'm starting with it. We currently build for aarch64 and amd64 and our existing build drops things into a particularly arranged output directory (this allows for easy on-device testing over nfs for example). I figure a genrule or some package rules are totally in-order and acceptable but since I'm just getting my feet wet here it's hard to know when that's the correct solution versus having the primary rule just create the correctly named file.
For packaging up the client library and services to clients I anticipated using the pkg_deb rule.
I admit I didn't think of using a genrule to rename the shared object... I just assumed it wouldn't be linkable with the name changed.
Part of the reason I'm stepping into these issues is because I'm trying to bootstrap a bazel build that lives beside our current cmake/cargo/bash extravaganza (soon to include docker). I'm open to any organization changes as long as the end results are largely compatible; and since it's clear that the support for a completely multi-arch build isn't quite ready yet I can't just convert everything all at once.
acmcarther commentedon Aug 1, 2018
Ok, I think I have a better picture for what you've got in mind. Lets see:
For usage internally (within Bazel as well), you can just add a dep to whatever wants the crate. If we're talking rust-depending-on-rust, that means your dependent should look like
For an internal C(++) user in bazel, you'd go with
For an
internalexternal user of either variety, you'll probably want to export the so with a name that isn't whack. You could do this with a genrule.Heres a kind of sketch. This declares a macro to wrap a genrule that copies (and implicitly renames) the file.
I strongly believe that this renamed
so
should be directly includeable as a dep in either a rust_library or a cc_library, but I'm not 100% certain. It would certainly be usable in any pkg_* rule.I don't have much experience bringing up Bazel alongside an existing build system, though I know it has been done. I suspect that a hybrid build system where cmake depends on Bazel output or vice versa might be perilous though compared to a "big bang" where you make Bazel work alongside cmake and eventually throw a lever and switch everything over to Bazel.
photex commentedon Aug 1, 2018
These examples are great! Putting them to use right now as I march along to the next target. :D
I'll quickly verify that renamed libs work as expected and report back. I have very likely just internalized some relic of autotools behavior. Although on OSX I believe I'd also have to use otool to fixup the name for dyld... but it's been a while so maybe even that knowledge is just cobwebs.
I'm actually aiming for the "big bang" with the caveat that I've chosen the unenviable task of working and supporting two builds for the time being. For Bazel, the idea is that I'm putting together all the targets for amd64 first and will work out the details of cross-compiling for aarch64 after that. Until we can cross-compile for aarch64 it wouldn't be possible to switch. I very much hope I can be of service in this effort.
acmcarther commentedon Aug 1, 2018
I have a pretty good idea of what is left as far as finishing implementing cross compilation support, but I just need to find some time to actually do it. I suspect it might take me longer to mentor you to finish up the implementation than it would take for me to just sit down and do it. Having someone who actually has a need for the feature is already pretty helpful though as it gives me someone to bounce the API+Implementation off of that might actually use it.
I can commit to having a PR up for you to examine / try by the end of Friday (UTC-8).
photex commentedon Aug 2, 2018
Exciting stuff! I verified that the renamed library works as intended. I now have the core application, client library, and two tools building and running.
I wasn't able to use the macro you specified. I just used the genrule portion of it.
If you manage to get something together by Friday then I'm totally happy to kick the tires but please don't go too far out of your way. Tomorrow I'll be attending to a family health issue, but after that I'll back and digging around.
Cheers!
acmcarther commentedon Aug 4, 2018
TL;DR: Not done yet, unfortunately
I worked on this somewhat over the last couple of days, but hit a pretty annoying roadblock that I'd like to put down in text somewhere so that I have something to come back to after I sleep on it. Here seems good.
Currently rules_rust piggybacks off of the C++ toolchain for its linker (and
ar
, but this doesn't matter that much):rules_rust/rust/toolchain.bzl
Lines 50 to 53 in 1944c8a
This means that we sort-of implicitly start to get mired in CROSSTOOL chaos. I spent some time trying to cut that corner by adding an optional linker_bin param to the rust_toolchain decl, but this caused so many unrelated headaches around paths in the Bazel sandbox that I think just using CROSSTOOL properly is advisable.
The consequence of this is that I need to learn how to write a proper CROSSTOOL so that I can put together a usage example. All told this will require only minor rules_rust changes to the tune of https://github.com/bazelbuild/rules_rust/compare/acm-cross-compilation plus making
rust_toolchain.attr._crosstool
public via therust_repository_set
attrs, but the actual usage and CROSSTOOL shenanigans will be non-trivial for users.For a sense of what happens without the above fixes:
As a side note, we wouldn't have to deal with this at all and could drop our CROSSTOOL dependency if LLD was production ready. Unfortunately the Rust integration is still nightly only (since it's a -Z flag to rustc), and has some issues that make trying to use it anyway inadvisable.
Long term LLD tracking issue:
rust-lang/rust#39915
Tertiary LLD PR:
rust-lang/rust#51936
photex commentedon Aug 6, 2018
Howdy :D
I admit I do not yet know enough of the spirit of Bazel to immediately make some suggestions on this although I really appreciate you writing it all out because it helps me understand what I'm seeing when I peek under the hood.
In the meantime I attempted to write a non-sandboxed genrule to just run cargo... which didn't work. I felt like it's a potential trade-off for the short-term and would work the same as the cmake integration:
This does in fact build everything correctly; but the genrule fails stating that it was not responsible for those files being created. 🤒
acmcarther commentedon Aug 7, 2018
Running cargo as a genrule is seriously brittle, in that Bazel has little means of:
Furthermore, Cargo itself isn't really amenable to fixing any of those problems because it plays so many roles in the build process (dep resolution, dep fetching, build planning, compiler execution) and isn't modular at all.
I was very serious in #110 (comment) when I said
mfarrugi commentedon Aug 7, 2018
I feel like this a bit overstated. At the edges (ie. relatively low level libraries) this is true, but many libraries (all of those that I've touched), and so most users, only want to enumerate the crates they depend on.
@photex you would need to declare 100% of the input files, including what 'cargo' is, the Cargo.toml, and the source fields that it's allowed to use; though I would also recommend against it in general.
photex commentedon Aug 7, 2018
I'm not suggesting a permanent approach, just something to kick the tires that works at all for aarch64.
I was trying to do this without the sandbox. Things build perfectly fine, but genrule doesn't seem to be able to see or find the resulting build.
This was only attempting to have bazel run a command that would be otherwise just required for someone to run themselves.
In either case I'm not the biggest fan of the Cargo monoculture in rust-land so I'm very much in favor of finding the correct bazel methodology.
photex commentedon Aug 7, 2018
Some thoughts on specifying the linker for cross-compilation:
Currently we make use of
.cargo/config
to specify the linker we require. This is for our uses practical (if not a little bit of a pain) and we definitely need an easy way to control that. I'm attempting now to follow examples for bazel and I'm setting up an aarch64 toolchain by downloading a prebuilt linaro toolchain and sysroot; but it'll also be the case that we need to leverage a toolchain and sysroot provided with the development hardware. Ideally this would mean that the rust rules would just use whatever linker is currently selected via the active crosstool_top option; but being able to specify a linker via command line and then bundling that behind a--config=foo
option would also be totally fine.photex commentedon Aug 10, 2018
Howdy @acmcarther
I'm going through the repo and trying to understand everything.
Currently I'm trying to understand why
rules_rust/rust/toolchain.bzl
Lines 50 to 53 in 1944c8a
With my current CROSSTOOL and cc_toolchain:
I only need to use
bazel build --cpu=aarch64
to have it select my aarch64 toolchain and I'd expect then that the cpp fragment is using information from there.Obviously it's not, and it's always getting the linker/compiler from my clang6 toolchain instead. I'm trying to connect the dots there and understand why.
photex commentedon Aug 10, 2018
Ok, I fiddled around until I could cross_compile.
I don't understand how fragments and host_fragments are being selected, so I just added attrs to the rust_toolchain rule for linker_bin and ar_bin. Since I have relative paths built-in to the repo this worked, but the next snag is actually with a proc-macro crate 'failure_derive'. That's resulting in an *.so built for aarch64 instead of for the host platform.
Also, toolchain selection by cpu wasn't working at all; I had to use a platform definition instead.
photex commentedon Aug 10, 2018
Welp, procedural macros are a can of worms that are beyond the bounds of my bazel knowledge at the moment. I can always check for the type of that crate and build for the host then, but every one of it's dependencies need that treatment as well and that's where I run into trouble.
I've run out of time on this so I think I'm going to have to either hack up a genrule, abandon bazel for now, or perhaps wrap a cargo/bazel combo build with a python script. 🤷