Clippy Builds fail on 0.36.2

[SvenKratzEasel]

Building of clippy targets fail with the following message : error: Option 'sysroot' given more than once

[UebelAndre]

Can you give an example workspace for this?

[tidefield]

Yeah I'm surprised CI isn't failing. Several folks have reported this problem before and it seems like something I should look into fixing.

@SvenKratzEasel I would appreciate if you have some steps to reproduce the bug.

[SvenKratzEasel]

Here are the rust-related items from the WORKSPACE file:

http_archive(
    name = "rules_rust",
    sha256 = "9d04e658878d23f4b00163a72da3db03ddb451273eb347df7d7c50838d698f49",
    urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.26.0/rules_rust-v0.26.0.tar.gz"],
)

load("@rules_rust//rust:repositories.bzl", "rules_rust_dependencies", "rust_register_toolchains")

rules_rust_dependencies()

rust_register_toolchains(
    edition = "2021",
)

load("@rules_rust//crate_universe:repositories.bzl", "crate_universe_dependencies")

crate_universe_dependencies(bootstrap = True)

load("@rules_rust//crate_universe:defs.bzl", "crates_repository")

crates_repository(
    name = "crate_index",
    cargo_config = "//:.cargo/config.toml",
    cargo_lockfile = "//:Cargo.lock",
    # See load satement for `cargo_bazel_bootstrap`.
    generator = "@cargo_bazel_bootstrap//:cargo-bazel",
    lockfile = "//:cargo-bazel-lock.json",
    manifests = [
       ...
    ],
)

load(
    "@crate_index//:defs.bzl",
    cargo_workspace_crate_repositories = "crate_repositories",
)

cargo_workspace_crate_repositories()

Building any rust_clippy target will produce the above error.

[illicitonion]

There's a full minimal-ish repro at https://github.com/dmeister/rules_rules_clippy_repo from https://bazelbuild.slack.com/archives/CSV56UT0F/p1704923287157729

A workaround is to set --@rules_rust//rust/settings:experimental_toolchain_generated_sysroot=False.

I suspect the fix may be just to not add the flag in clippy specifically.

[tidefield]

Thanks! I'll take a look.

[UebelAndre]

A workaround is to set --@rules_rust//rust/settings:experimental_toolchain_generated_sysroot=False.

I suspect the fix may be just to not add the flag in clippy specifically.

I think clippy should definitely be given an explicit sysroot. I struggle to see where the flag is being passed twice. I only see one instance if I print the args in the process wrapper

[illicitonion]

I think clippy is setting it based on the env var being set - we set both SYSROOT= and --sysroot, and I think clippy notices the env var and sets the flag itself as a result.

If you apply this patch (which isn't on its own a solution, but could be cleaned up into one), the error goes away:

% git diff
diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl
index 9fd9842c..636212a6 100644
--- a/rust/private/clippy.bzl
+++ b/rust/private/clippy.bzl
@@ -171,6 +171,8 @@ See https://github.com/bazelbuild/rules_rust/pull/1264#discussion_r853241339 for
     env["CLIPPY_CONF_DIR"] = "${{pwd}}/{}".format(ctx.file._config.dirname)
     compile_inputs = depset([ctx.file._config], transitive = [compile_inputs])
 
+    env.pop("SYSROOT", None)
+
     ctx.actions.run(
         executable = ctx.executable._process_wrapper,
         inputs = compile_inputs,

[UebelAndre]

Oh, I’m happy to remove the end var. I wanted that to happen when —sysroot was added. But why doesn’t this fail in CI now?

[UebelAndre]

I've opened #2428, does this help avoid the issue?

[tidefield]

An incomplete recap of this interesting problem

As of rust-lang/rust-clippy#10149, setting both --sysroot and SYSROOT will fail clippy-driver. To support setting --sysroot to clippy-driver, we have two options:

1. Removing SYSROOT from env in clippy_aspect specifically like what @illicitonion did above.
2. Explicitly skip setting --sysroot if constructor_argument is called from clippy_aspect. Skip setting --sysroot in clippy_aspect #2439
3. Change clippy-driver's implementation to prefer --sysroot if both are set. Don't pass --sysroot twice if SYSROOT is set rust-lang/rust-clippy#10149 (comment)

Now, why is CI not failing? 👀

When I build a target in rules_rust/examples locally with --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect, the action does have both SYSROOT and --sysroot set as expected so I expected it to fail.
Strangely, it also doesn't fail.

Environment: [SYSROOT=bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain]
Command Line: (exec bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/rules_rust/util/process_wrapper/process_wrapper \
    --subst \
    'pwd=${pwd}' \
    --touch-file \
    bazel-out/k8-fastbuild/bin/hello_world/hello_world.clippy.ok \
    -- \
    bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain/bin/clippy-driver \
    hello_world/src/main.rs \
    '--crate-name=hello_world' \
    '--crate-type=bin' \
   ...
    '--sysroot=bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain' \
    -Dwarnings)

This matches the action produced by https://github.com/dmeister/rules_rules_clippy_repo which also sets --sysroot and SYSROOT.

One difference between

https://github.com/dmeister/rules_rules_clippy_repo/blob/8b3e5c2fb1cded5197663ff5f1b0c0a350b89214/hello_world/BUILD#L7-L18

and

rules_rust/examples/hello_world/BUILD.bazel

Lines 5 to 9 in fe610da

	rust_binary(
	name = "hello_world",
	srcs = ["src/main.rs"],
	deps = ["@examples//hello_lib"],
	)

is the former sets proc_macro_deps attribute. If we uncomment proc_macro_deps attribute, the error disappears. proc_macro_deps seems completely unrelated to clippy which makes the issue a little mysterical. I'm looking more into this now.

[tidefield]

I suspect passing SYSROOT and --sysroot shouldn't be a problem because if it is, our CI should have failed. So I tried running clippy-driver directly

~/github/dmeister/rules_rules_clippy_repo on  main! ⌚ 11:51:48
$ SYSROOT="" clippy-driver hello_world/src/main.rs  --crate-name=hello_world --crate-type=bin --out-dir=manual_out --sysroot=""

and it passed

Apparently, setting proc_macro_deps and deps attributes with @crate_index dependencies results to a bunch of --extern and -L being set to clippy-driver and it fails. I'm still looking into this problem but I think the problem is worth investigating more holistically.