Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Cargo clippy with cargo-sysroot warns on sysroot #6

Closed
XAMPPRocky opened this issue Nov 18, 2020 · 16 comments
Closed

Cargo clippy with cargo-sysroot warns on sysroot #6

XAMPPRocky opened this issue Nov 18, 2020 · 16 comments
Assignees
Labels
needs-info Needs more information

Comments

@XAMPPRocky
Copy link
Contributor

Hey while working on EmbarkStudios/rust-gpu#239 I noticed that when running cargo clippy on a project that uses cargo-sysroot in a build script will also run clippy on core, which isn't ideal since there's a lot of warnings, and there's not much I as a user can do about it. It would be nice if clippy didn't warn about sysroot.

@XAMPPRocky XAMPPRocky changed the title Cargo clippy with cargo-sysroot runs clippy on core Cargo clippy with cargo-sysroot warns on sysroot Nov 18, 2020
@DianaNites DianaNites self-assigned this Nov 18, 2020
@DianaNites
Copy link
Owner

Gonna need more information here, as i'm unable to reproduce, and have never had that happen before.

@DianaNites DianaNites added the needs-info Needs more information label Nov 18, 2020
@XAMPPRocky
Copy link
Contributor Author

Here's steps to reproduce. I'm running on macOS 11.0.1

  1. Clone & Checkout https://github.com/EmbarkStudios/rust-gpu/tree/vuild-system
  2. Run cargo clippy --manifest-path examples/runners/ash/Cargo.toml -vv

You can also see the output here, as I first noticed on CI. https://github.com/EmbarkStudios/rust-gpu/runs/1418118000?check_suite_focus=true#step:8:333

@DianaNites DianaNites added bug Something isn't working needs-info Needs more information and removed needs-info Needs more information bug Something isn't working labels Nov 18, 2020
@DianaNites
Copy link
Owner

DianaNites commented Nov 18, 2020

Does not reproduce on my machine, clippy ran successfully.

For reference I'm on Arch Linux, if that makes any difference.

@XAMPPRocky
Copy link
Contributor Author

Hmm, okay. I can reproduce it here, so I'll try to debug this later this week.

@DianaNites
Copy link
Owner

Apologies, seems I out the branch wrong. I'm getting failures, but nothing mentioning core, "Error loading target specification: Could not find specification for target "spirv-unknown-unknown"".
A bunch of warnings for other dependencies under ~/.cargo/registry, though?

@XAMPPRocky
Copy link
Contributor Author

XAMPPRocky commented Nov 19, 2020

"Error loading target specification: Could not find specification for target "spirv-unknown-unknown"".

That's actually a different bug, the rustc flags in the builder aren't working.

@DianaNites
Copy link
Owner

Yeah oops, I noticed that. Fixed in 48f811f

@DianaNites
Copy link
Owner

Now it seems to be linker errors

librustc_codegen_spirv.so: undefined symbol: _ZN8spvtools3val8Function13current_blockEv"

@XAMPPRocky
Copy link
Contributor Author

I have a fix for that locally one moment.

@XAMPPRocky
Copy link
Contributor Author

@DianaNites Okay it's working now so running cargo clippy --manifest-path examples/runners/ash/Cargo.toml -vv (I recommend trying to compile without -vv once, to remove most of the noise) in the vuild-system branch should show clippy warnings in the example-ash build script.

@DianaNites
Copy link
Owner

Clippy now successfully runs, and i'm not seeing any clippy warnings from the sysroot, just all the dependencies?

@DianaNites
Copy link
Owner

Ah build output, If I insert a panic into the build script I see non-fatal clippy warnings.

No idea why thats happening..

@XAMPPRocky
Copy link
Contributor Author

I’ll investigate more tomorrow, but I would hazard to guess it’s something with flags and or cargo rustc causing issues.

@XAMPPRocky
Copy link
Contributor Author

Potnentially related: EmbarkStudios/rust-gpu#294

@DianaNites
Copy link
Owner

Cause is probably the new RUSTC_WORKSPACE_WRAPPER, documented on nightly https://doc.rust-lang.org/nightly/cargo/reference/environment-variables.html, added specifically for clippy

Removing it from the env should probably fix it

@DianaNites
Copy link
Owner

Should be fixed now but I can't test so

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-info Needs more information
Projects
None yet
Development

No branches or pull requests

2 participants