trailofbits
Dylint relies on rustup
Doing cargo dylint new lint-name, trying to implement a new lint and running cargo build produces enormous amount of errors inside of clippy.
Hi, @GoldsteinE. I'm sorry this isn't working for you. I want to help you get this resolved.
Can I confirm, you are running these commands?
cargo dylint new lint-name
cd lint-name
cargo build
From within the lint-name directory, could you please share the output of the following commands?
uname -a
cargo dylint --version
cat Cargo.toml
cat rust-toolchain
Yes, I’m running these commands.
$ uname -a
Linux think 5.19.5 #1-NixOS SMP PREEMPT_DYNAMIC Mon Aug 29 09:18:05 UTC 2022 x86_64 GNU/Linux
$ cargo dylint --version
cargo-dylint 2.0.12
$ cat Cargo.toml
[package]
name = "lint_name"
version = "0.1.0"
authors = ["authors go here"]
description = "description goes here"
edition = "2021"
publish = false
[lib]
crate-type = ["cdylib"]
[dependencies]
clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "2b2190cb5667cdd276a24ef8b9f3692209c54a89" }
dylint_linting = "2.0.12"
if_chain = "1.0.2"
[dev-dependencies]
dylint_testing = "2.0.12"
[package.metadata.rust-analyzer]
rustc_private = true
rust-toolchain is irrelevant, since I’m not using rustup, so here is my rustc --version instead:
rustc 1.65.0-nightly (b44197abb 2022-09-05)
rust-toolchain is irrelevant, since I’m not using rustup, so here is my rustc --version instead:
I suspect that's the source of the problem. Each clippy_utils revision is pinned to a specific rustc version. For example, revision 2b2190cb5667cdd276a24ef8b9f3692209c54a89 is pinned to nightly-2022-08-11.
Could you try cargo +nightly-2022-08-11 build?
Is using rustup not an option for you?
Using rustup is not an option for me, but I can try specific nightly version.
In general I’d prefer for dylint to determine and use installed nightly, not the other way around. Is there a mapping of some kind? Maybe I could contribute a patch for such an option.
In general I’d prefer for dylint to determine and use installed nightly, not the other way around.
The current behavior is by design. The problem is that the compiler APIs can change. So just like a clippy_utils revision is pinned to a specific compiler revision, so too is a lint. The rustc version that a lint is pinned to is essentially recorded in its rustup-toolchain file.
In other words, if Dylint used the installed nightly, the following could happen. A lint builds fine. Then the developer updates their toolchain. Then the lint ceases to build. The current setup is meant to avoid this possibility.
In other words, if Dylint used the installed nightly, the following could happen. A lint builds fine. Then the developer updates their toolchain. Then the lint ceases to build. The current setup is meant to avoid this possibility.
I should say, one can do this---there's nothing in Dylint that prevents using the installed toolchain. But doing so could be painful, for the reason given.
I changed the toolchain to one in rust-toolchain, exported RUSTUP_TOOLCHAIN (which is required by dylint’s linker for some reason) and it worked. I’ll try to send some patches to make this more friendly to non-rustup workflow.
UI tests seem to be completely unusable without rustup, even with rustup shim answering to rustup which rustc. I’ll try fix it, so non-rustup workflows are possible.
I’ll try to send some patches to make this more friendly to non-rustup workflow.
I just want to ensure that the current rustup-based workflow doesn't break.
If you would like to propose ideas here before implementing, you would be welcome to.
I need to get a look at the source code first, but generally I think that when rustup is not found, dylint should fall back to more general solutions (like which rustc instead of rustup which rustc).
- Could we please change the title of this issue to something like "Dylnt relies on
rustup"? - In the past, I have considered using
rustupas a library, instead of using the command line tool. The reason then was to make it easier to test with a specific version ofrustup, as opposed to whatever the user had installed. Would a solution like that work for you, or are you opposed to allrustup-related tooling, generally?
Using a built in rustup would certainly be better. It'd still install a second copy of Rust, even if system Rust already has the right version.
@GoldsteinE I've attached a future enhancement label to this issue and I want to explain why.
I was seriously considering linking in rustup to avoid relying on it being available on the command line. But there is a tradeoff: linking in rustup would benefit users that do not have rustup installed, but it would also increase Dylint's overall number of dependencies.
Experience has taught me that having a large number of dependencies can be bad. Until version 3.0.0, Dylint linked in cargo, which has a large number of dependencies. On more than one occasion, a problem with one of cargo's dependencies broke cargo-dylint installation. For this reason, Dylint no longer links in cargo by default.
The number of dependencies that rustup relies on is not as large as cargo, but I think the lesson still applies.
So, again, thank you very much for raising the issue. But, for now, I think the best course of action is to rely on rustup being available on the command line.
I think this is the same story:
There is one case where I find relying on rustup excessive:
cargo-dylint --help fails with exit code 1 if there is no RUST_TOOLCHAIN env variable. However I run --help only to ensure in CI that the cargo-dylint library is properly compiled.
cargo-dylint --helpfails with exit code 1 if there is noRUST_TOOLCHAINenv variable
I can't seem to reproduce this. I agree that would be excessive, though.
Are you using a recent version of cargo-dylint? Is there a publicly available GitHub log showing the failure?
My apologies, I made a mistake - it's not a cargo-dylint but dylint-link that exits with code 1:
Ah. I agree that should not happen. Thanks for reporting. I'll get it fixed.