Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process_wrapper removes args expectedly #2447

Closed
daivinhtran opened this issue Jan 25, 2024 · 3 comments
Closed

process_wrapper removes args expectedly #2447

daivinhtran opened this issue Jan 25, 2024 · 3 comments

Comments

@daivinhtran
Copy link
Contributor

daivinhtran commented Jan 25, 2024

This issue is related to a bug reported to by a few users (#2418, #2277 (comment) #2277 (comment)).

From these workarounds #2418 (comment) and #2418 (comment) (thanks @illicitonion ), it seems that clippy-driver doesn't allow setting SYSROOT and --sysroot at a same time. That's not true because if it is, CI builds with clippy_aspect should have failed since the --sysroot flag was added by default in #2277.

We can confirm this is not true by running SYSROOT clippy-driver --sysroot="". clippy-driver's implementation even checks that whether--sysroot is set or not before adding --sysroot=SYSROOT to rustc.

Reproduce steps:

  1. Clone https://github.com/dmeister/rules_rules_clippy_repo. Switch rules_rust in WORKSPACE to source from local.
  2. Apply changes https://github.com/bazelbuild/rules_rust/pull/2446/files to local rules_rust repo
  3. bazel build //hello_world --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect

The build is expected to fail at the clippy action and the last log from process_wrapper contains

Options { child_arguments: ["@bazel-out/k8-fastbuild/bin/hello_world/hello_world.clippy.ok-0.params.expanded", "-Lnative=/tmp/bazel-working-directory/_main/bazel-out/k8-fastbuild/bin/external/crate_index__ring-0.16.20/ring_build_script.out_dir", "-Lnative=/tmp/bazel-working-directory/_main/bazel-out/k8-fastbuild/bin/external/crate_index__ring-0.17.7/ring_build_script.out_dir"], child_environment: {"SYSROOT": "bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain"}, touch_file: Some("bazel-out/k8-fastbuild/bin/hello_world/hello_world.clippy.ok"), copy_output: None, stdout_file: None, stderr_file: None, output_file: None, rustc_quit_on_rmeta: false, rustc_output_format: None }

Surprisingly, all the args upto --sysroot are gone. I'd expect they're at least included in @bazel-out/k8-fastbuild/bin/hello_world/hello_world.clippy.ok-0.params.expanded but the file doesn't exist. Shouldn't any file be declared by Bazel instead? (cc: @UebelAndre)

👀 BUT how is "missing --sysroot" even related to this problem of "--sysroot being twice" because clippy-driver can just use SYSROOT instead? I added a log in https://github.com/rust-lang/rust-clippy/blob/900a5aa036c6070fdec8acd6d871a4bfc89b2fb4/src/driver.rs#L264 to see what exactly gets passed to rustc and got

["bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain/bin/clippy-driver", "@bazel-out/k8-fastbuild/bin/hello_world/hello_world.clippy.ok-0.params.expanded", "-Lnative=/tmp/bazel-working-directory/_main/bazel-out/k8-fastbuild/bin/external/crate_index__ring-0.16.20/ring_build_script.out_dir", "-Lnative=/tmp/bazel-working-directory/_main/bazel-out/k8-fastbuild/bin/external/crate_index__ring-0.17.7/ring_build_script.out_dir", "--sysroot", "bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain"]

Still, there's only one --sysroot which is set by SYSROOT env var. I bet the second --sysroot comes from .expanded file then. We just don't see it in the bazel output dir.

If we remove either of these attributes, the args are then processed correctly. So somehow, these attributes affect how process_wrapper parses the args.

@daivinhtran
Copy link
Contributor Author

daivinhtran commented Jan 25, 2024

I got a little further. The root cause is somewhere in these lines

let mut child_args = match flags
.parse(env::args().collect())
.map_err(OptionError::FlagError)?
{
ParseOutcome::Help(help) => {
eprintln!("{help}");
exit(0);
}
ParseOutcome::Parsed(p) => p,
};

rust_binary(
    name = "hello_world",
    srcs = glob(["src/**/*.rs"]),
    aliases = aliases(),
    edition = "2021",
    proc_macro_deps = all_crate_deps(
        proc_macro = True,
    ),
)

results to child_args being

["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", "--error-format=human", "--codegen=metadata=-2539833622", "--codegen=extra-filename=-2539833622", "--out-dir=bazel-out/k8-fastbuild/bin/hello_world", "--codegen=opt-level=0", "--codegen=debuginfo=0", "--remap-path-prefix=${pwd}=", "--emit=dep-info,metadata", "--color=always", "--target=x86_64-unknown-linux-gnu", "-L", "bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain/lib/rustlib/x86_64-unknown-linux-gnu/lib", "--edition=2021", "--extern=async_trait=bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/crate_index__async-trait-0.1.77/libasync_trait-2222682450.so", "-Ldependency=bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/crate_index__async-trait-0.1.77", "--sysroot=bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain", "-Dwarnings"]

while

rust_binary(
    name = "hello_world",
    srcs = glob(["src/**/*.rs"]),
    aliases = aliases(),
    edition = "2021",
    proc_macro_deps = all_crate_deps(
        proc_macro = True,
    ),
    deps = all_crate_deps(
        normal = True,
    ),
)

leads to

["bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain/bin/clippy-driver", "@bazel-out/k8-fastbuild/bin/hello_world/hello_world.clippy.ok-0.params"]
where .params has all the args.

@daivinhtran
Copy link
Contributor Author

The fix to this issue should be done upstream in rust-lang/rust-clippy#12203.

@daivinhtran
Copy link
Contributor Author

Closing this issue. This bug will be fixed when future rust release includues rust-lang/rust-clippy#12203.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant