Previously we were adding these early on in the link args.
This was a bug - if some transitive dep foo has a linkopt of `-lbar`,
the `-lbar` needs to be later in the link args list than `-lfoo`.
Rather than arbitrarily adding them early on and hoping things work,
this adds the args for each library just after we add a dependency on
that library.
Partially addresses https://github.com/bazelbuild/rules_rust/issues/1156
This pull-request implements an additional change to enable this
behavior which aggregates all transitive `BuildInfo.compile_data` into
`Rustc` actions. While this seems to bloat these actions with
unnecessary data, it addresses a catastrophic flaw in how Windows works
at all.
As of Bazel 7.3.1, Windows does not run any actions in a sandbox
(https://github.com/bazelbuild/bazel/discussions/18401), this means that
references to the current working directory will be consistent since
they always refer to the execroot. On top of this fact,
`cargo_build_script` will assign
[CARGO_MANIFEST_DIR](https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates)
to a [path within the runfiles directory of the build
script](d9ee6172d1/cargo/private/cargo_build_script.bzl (L218)).
The combination of these two facts leads crates like
[windows_x86_64_msvc](https://crates.io/crates/windows_x86_64_msvc),
which assign a linker path using `CARGO_MANIFEST_DIR`
([@windows_x86_64_msvc//build.rs](https://github.com/microsoft/windows-rs/blob/0.59.0/crates/targets/x86_64_msvc/build.rs#L1-L8))
to introduce un-tracked dependencies into dependent `Rustc` actions.
This then leads to build failures if the `windows_x86_64_msvc` crate is
ever a remote cache hit for the dependents as runfiles (or
`.cargo_runfiles` in the case of this PR) are not fetched and will not
exist on the host at link time.
This change addresses this issue of untracked dependencies by ensuring
the runfiles of `cargo_build_script.script` targets are aggregated into
`Rustc` actions.
This change expands the use of `cargo tree` to resolve features for
different combinations of `host -> target` platform triples where before
`cargo-bazel` was only capable of resolving host dependencies for the
current host platform and not a foreign platform.
A lengthy description of how this was done can be found at
`crate_universe/src/metadata/cargo_tree_resolver.rs -
TreeResolver::create_rustc_wrapper`.
closes https://github.com/bazelbuild/rules_rust/issues/2212
---------
Co-authored-by: Daniel Wagner-Hall <dawagner@gmail.com>
https://github.com/bazelbuild/rules_rust/pull/2826 ended up introducing
a regression for crates with build scripts that require extra data at
compile time. This change adds a default glob for all
`cargo_build_script` targets generated by `crate_universe` and also
introduces an `annotation.build_script_compile_data` attribute to add
custom targets to generated outputs.
I hit this when profiling for something else, we spent 1.1sec of 1.3sec
of `cargo-bazel generate` compiling this regular expression. It seems to
be quite bad on the regex compiler, as even compiling it once takes 8ms.
I had to add the `once_cell` crate to unwrap the Result type - the
corresponding stdlib function is still nightly-only unfortunately.
However, that crate was already in the transitive dependency tree, so
it's a net-same change.
The Cargo [Feature Resolver version
2](https://doc.rust-lang.org/cargo/reference/resolver.html#feature-resolver-version-2)
behavior is currently not supported by the `cargo metadata` sub command
(https://github.com/rust-lang/cargo/issues/9863) which `crate_universe`
uses to determine the dependencies of a target, leading to inaccuracies
when dependencies are introduced via feature resolution for a particular
configuration.
In https://github.com/bazelbuild/rules_rust/pull/1710 functionality was
added to use `cargo tree` to perform feature resolution for each
supported platform. This change expands on this trick to collect
dependency information at the same time and use that to determine
whether or not to include optional dependencies located in standard
`cargo metadata` output in the rendered Bazel targets. Non optional or
`target.cfg` (conditional) dependencies behave as they did before this
change.
Implementation details:
- `FeatureGenerator` was replaced by `TreeResolver`
- Optional dependencies are now rendered as selects on explicit
platforms. This will expand the size of `cargo-bazel-lock.json` files
but is expected to be more correct.
This also plumbs `target_compatible_with` and `exec_compatible_with`
into the `rust_bindgen` target from `rust_bindgen_library` to account
for cases where the `"manual"` tag is not sufficient in disabling the
target (e.g. `cquery`).
On windows (and some other platforms), the file extension of cargo,
rustc, etc have an extension. The module extension for loading crates
did not take this into account, causing it to error on windows.
Additionally, when using bzlmod to build vendored crates, the runfiles
tree would exceed the 260 char windows path limit. To mitigate this, I
have shortened the internal_deps module extension to just `i` and
changed the build script suffix to `_bs` from `_build_script`. This
makes the path names below the 260 char limit.
This makes the bzlmod CI run on windows, to avoid regressing this.
Currently gen_rust_project does not run on windows for other reasons,
although we do build this.
### Problem
As of now (before this PR), we seem to mix `link_flags` file into using
for two purposes.
For
```
cc_library(
name = "simple",
srcs = ["simple.cc"],
hdrs = ["simple.h"],
linkopts = ["-framework"],
)
rust_bindgen_library(
name = "simple_bindgen",
cc_lib = ":simple",
header = "simple.h",
)
rust_binary(
name = "simple_example",
srcs = ["main.rs"],
deps = [":simple_bindgen"],
)
```
`rust_bindgen_library` produces a `link_flags` file with
```
-lstatic=simple
-C
link-args=-framework
```
`-lstatic=simple` is consumed by `rustc` whereas `-framework` is
expected to be consumed by an actual linker (either invoked by `rustc`
or `cc_common.link`)
The flags are then passed to `rustc` command to compile
`libsimple_bindgen.rlib`. It does not yield any error because
`-lstatic=simple` is correctly used whereas `-framework` is no-op (there
is no linker being invoked for producing `rlib`). However, the rustc
***doesn't*** pass `-framework` to the linker that link the
`rust_binary` (which is where the cc linkopts matters).
Another problem is that this approach is not compatible with
`experimental_use_cc_common_link` option which let `cc_common.link`
instead of `rustc` invoke the linker. See #2413
### Solution
We're referring "link" as at least two things (which I think what causes
problem here):
1.
https://doc.rust-lang.org/rustc/command-line-arguments.html#-l-link-the-generated-crate-to-a-native-library
2. https://doc.rust-lang.org/rustc/codegen-options/index.html#linkerhttps://doc.rust-lang.org/rustc/codegen-options/index.html#link-args
As proposed in
https://github.com/bazelbuild/rules_rust/pull/2415#issuecomment-1890109092,
this PR splits `<rust_library_bindgen>__bindgen.link_flags` produced by
`rust_bindgen` rule into two files:
1. `rustc_flags`
```
-lstatic=simple
```
2. `linker_flags`
```
-framework
```
We "sort-of" (but not perfectly) had it correct before
https://github.com/bazelbuild/rules_rust/pull/2361 when we link the
binary directly with the cc_library (aka both kinds of flags).
But since we want to support the Cargo style of linking
```
cc_lib -> bindgen -> lib_a -> bin
```
instead of
```
cc_lib -> bindgen -> lib_a -> bin
\___________________________^
```
we can pass `-lstatic=simple` to the `rustc` command that builds
`simple_bindgen` (rust_library) and propagate `linkopts` to
`simple_example` (rust_binary) so that the linker can use it.
```
cc_library -> rust_bindgen -> rust_library -> rust_binary
-lstatic=simple
-Clink-args="-framework"
```
This is long and sounds like a proposal. I'm open for suggestion
@UebelAndre @illicitonion @krasimirgg
Fixes #2413
This commit fixes #2402 by using [`get_lib_name_default`] to pass
libraries names to `rustc` through `-lstatic` instead of relying on the
owner's name of the `File` object.
[`get_lib_name_default`]:
a1e9f9600c/rust/private/utils.bzl (L101)
Co-authored-by: UebelAndre <github@uebelandre.com>
Most uses of [bindgen](https://github.com/rust-lang/rust-bindgen) are
via [build
scripts](https://doc.rust-lang.org/cargo/reference/build-scripts.html)
which typically use either `-Lnative=` to specify linker paths for the
current target being built. The way the rules are currently defined, has
the bindgen library linked directly into the final library which leads
to some annoying ordering errors compared to the same project building
with Cargo. This PR aims to support the Cargo style of linking via a
`leak_symbols = False` flag, thus changing the dependency graph when
linking from:
```
cc_lib -> bindgen -> lib_a -> lib_b -> bin
\__________________________________^
```
to:
```
cc_lib -> bindgen -> lib_a -> lib_b -> bin
```
Note that `leak_symbols` is intended to be used as an escape hatch to
restore the original behavior and will be deleted in the future.
This change parses `cc_toolchain` flags for `c++-compile` actions for
use in bindgen. This change aims to allow the action to gain toolchain
specific include paths as well as a more accurate `--sysroot`.
See #2246 for how we declare dependencies automatically for third party
crates. I'm using the same mechanism for all other repositories.
Note that the MODULE.bazel changes in use_repo are autogenerated, and if
you change the third party crates you depend on, you should get an error
message saying something like "run this command to fix it" on your
bzlmod presubmit environment.
Currently, when you run `bazel build --enable_bzlmod
//crate_universe:bin`, you get the error `undefined repo @cui`.
When you add a dependency on CUI, you get errors like `undefined repo
@cui__<crate>-<version>` (the repo for the third party crate).
This change will allow us to automatically declare dependencies on our
third party crates from bzlmod (see
https://docs.google.com/document/d/1dj8SN5L6nwhNOufNqjBhYkk5f-BJI_FPYWKxlB3GAmA/edit#heading=h.5mcn15i0e1ch).
---------
Co-authored-by: UebelAndre <github@uebelandre.com>
While running from CARGO_MANIFEST_DIR is the simplest thing for
compatibility, there are cases where a cargo build script may be easier
to run from the exec root as most bazel actions do (e.g. where a C++
toolchain specifies in-repo include paths).
This mirrors the `rundir` attribute of `go_test`, which has similar
concerns:
https://github.com/bazelbuild/rules_go/blob/master/docs/go/core/rules.md#go_test-rundir
Some minor optimizations to analysis time memory usage (avoid creating
some list temporaries, some extra strings, etc.)
Some of the usage might be simpler as `args.add("foo", foo)` instead of
`args.add(foo, format="foo=%s")` but I opted to keep the usage the same
as before for safety.
* Exclude .tmp_git_root from globs
This directory contains the original git repo when the crate is from git. Including this directory currently makes these rules re-run whenever the repository rule re-runs, although this is fixed by https://github.com/bazelbuild/bazel/pull/18271. Even after this fix, excluding this directory avoids depending on unnecessary files.
Fixes #1927.
* Regenerate vendored crates
Some toolchains, notably https://github.com/bazelembedded/rules_cc_toolchain,
use different and incompatbile link arguments for executables and
shared libraries.
Co-authored-by: Daniel Wagner-Hall <dwagnerhall@apple.com>
At first I was confused why some of the generated files were coming out with loads formatted like this:
4ef3d4aaa2/crate_universe/3rdparty/crates/BUILD.anyhow-1.0.68.bazel (L9-L13)
Surely this should be as follows? The wrapped line is even shorter than the unwrapped line.
```bzl
load("@rules_rust//cargo:defs.bzl", "cargo_build_script")
load("@rules_rust//rust:defs.bzl", "rust_library")
```
It turned out to be because crate_universe unconditionally generates _all_ the loads that might be needed:
```bzl
load("@rules_rust//cargo:defs.bzl", "cargo_build_script")
load(
"@rules_rust//rust:defs.bzl",
"rust_library",
"rust_binary",
"rust_proc_macro",
)
```
and then Buildifier deletes any that are unused, but without rewrapping any lines:
```bzl
load("@rules_rust//cargo:defs.bzl", "cargo_build_script")
load(
"@rules_rust//rust:defs.bzl",
"rust_library",
)
```
This PR tweaks crate_universe to generate only loads that are used, so that we retain control over how they are wrapped. After this PR, single loads get put on one line while multiple loads get put on multiple lines.
e987f2041d/crate_universe/3rdparty/crates/BUILD.anyhow-1.0.68.bazel (L9-L10)e987f2041d/bindgen/3rdparty/crates/BUILD.bindgen-0.60.1.bazel (L9-L14)
Daniel Wagner-Hall