`rust_doc_test` targets are currently broken on windows

[UebelAndre]

A change to the Bazel CI windows runners broke the rust_doc_test targets currently defined in the repo. This issue tracks fixing these.

Related to:

• Unexpected failures on windows runners continuous-integration#1201

[UebelAndre]

I've opened #888 to disable the tests while a solution is found. This way other PRs are not blocked.

[UebelAndre]

Currently RBE also does not work with rust_doc_test. I suspect that a solution to making RBE work would also fix the issues we're seeing with windows.

[sayrer]

Yes, see also: #845

[sayrer]

I suspect the problem is here:

rules_rust/rust/private/rustdoc.bzl

Line 101 in dd1c73e

# nb. rustdoc can't do anything with native link flags; we must omit them.

but I haven't had time to investigate further.

Perhaps we could set a CC env variable in the ctx.actions.run call further down.

[UebelAndre]

This rule is unlike any rule in that it is compiling rust within a test. To try and make it clearer:

build	execution
generate bash/batch script	run script -> build+run doc tests

I suspect the issue is that the test environment is not the same as the sandboxed action environment that rustc normally runs in (or even rustdoc for the rust_doc rule).

I think the fix is either compile the tests in an action (but not run them) using the --no-run feature so that the test execution is just running the test binary. I'd expect this to shape out like:

build	execution
rustdoc --no-run --persist-doctests ${CRATE}	run ${CRATE}.doc_test

Unfortunately, this is a nightly feature so not something that can be done now. The alternative is to figure out how to create a process-wrapper for the test that ensures the environment is sufficient for building and running the tests. This is probably the better path forward since it'd be backwards compatible (not needing the nightly flag) but I don't know how to do that...

[sayrer]

Given #845, I suspect we could do

ctx.actions.run(
  ...
  env = {
    "CC": "...something from the toolchain..."
  }
  ...
)

It looks like we're hitting the tool resolution described in rustc (or similar logic in the cc crate). This is roughly what happens for build.rs files:

rules_rust/cargo/cargo_build_script_runner/bin.rs

Line 92 in 79177f1

if let Some(cc_path) = env::var_os("CC") {

[UebelAndre]

I spent last weekend trying a few things but didn't come up with anything that worked 😞 I'm happy to collaborate on more ideas though!

I did try setting CC but ran into more issues on #882

Feel free to take a look and I'm happy to try and answer any questions. But feeling kinda stumped on this issue... 😞

edit: Note that that draft started simple and just exploded as I tried more things 😄

[sayrer]

I see. Looking at #882, I don't think find_sysroot is doing the right thing. I think sysroot should be a directory containing the typical Unix root paths. Something like this: https://github.com/grafica/build/tree/main/sysroots/debian_sid_amd64-sysroot

I don't have more time to look for a few days, but happy to work on this.

[UebelAndre]

I see. Looking at #882, I don't think find_sysroot is doing the right thing. I think sysroot should be a directory containing the typical Unix root paths. Something like this: https://github.com/grafica/build/tree/main/sysroots/debian_sid_amd64-sysroot

sysroot in that case is a Rust sysroot which is expected to be passed to rustdoc --sysroot. There's another section which sets the SYSROOT environment variable from the cc_toolchain.

I don't have more time to look for a few days, but happy to work on this.

I'd absolutely love the help 🤩 feeling stuck on this one...

[UebelAndre]

As mentioned earlier, I think fixing the sandboxing issues would likely solve for the issue here (which would close #804).