Effects of `RUSTUP_WINDOWS_PATH_ADD_BIN` change

[smoelius]

In #3703 (comment), @djc asked:

Can one of you open a new issue discussing what the use case here is and how/why we broke it?

Dylint works similar to Clippy. It sets RUSTC_WORKSPACE_WRAPPER to the path of a dylint-driver binary, and then invokes cargo check. There are several dylint-driver binaries, each associated with a nightly toolchain. Each binary is linked against the rustc_driver and std libraries of its corresponding toolchain. On Windows, this means that the directories containing those libraries must be in PATH. If PATH does not include those directories, then the call to cargo check fails because dylint-driver cannot be loaded.

Here is an example: https://github.com/trailofbits/dylint/actions/runs/8977891579/job/24657422248#step:8:2321

[sunshowers]

Thanks for filing this! This is essentially what nextest has too, specifically with test binaries built by proc-macro crates. Though this has been a long-standing issue with nextest (nextest-rs/nextest#267), because ideally nextest would work with Rust installations not managed by rustup.

We expect nextest-rs/nextest#1499 to fix this the right way, by querying rustc to obtain the target libdir and adding it manually to the dylib path variable.

[djc]

@sunshowers sounds good, thanks for chiming in!

@smoelius thanks for the detailed description. Do you think the proposed path forward for nextest would also address the dylint use cases?

[smoelius]

@smoelius thanks for the detailed description. Do you think the proposed path forward for nextest would also address the dylint use cases?

I'm not sure. Is RUSTUP_WINDOWS_PATH_ADD_BIN going away?

[djc]

@smoelius thanks for the detailed description. Do you think the proposed path forward for nextest would also address the dylint use cases?

I'm not sure. Is RUSTUP_WINDOWS_PATH_ADD_BIN going away?

There are no concrete plans in that direction, but it'd be good to avoid depending on workarounds for deprecated features.

[sunshowers]

I think a general decision to make is whether a tool needs to support Cargo installations not managed by rustup. For nextest the answer is yes, but it could be no for other tools.

[smoelius]

There are no concrete plans in that direction, but it'd be good to avoid depending on workarounds for deprecated features.

OK. Dylint's CI still uses this environment variable to run one obscure doc test, but Dylint itself no longer uses it.

[sagudev]

In servo we also hit this with our custom linter.

[Darksonn]

This affected Tokio's CI setup too, and broke the case where we run tests on windows with proc macros. Not sure why exactly it broke us, but adding the flag seems to fix it.

[rami3l]

@ehuss do you think Rustup is responsible for this breakage?

Modifying PATH was originally done for various reasons that are complicated and not really relevant anymore.
https://internals.rust-lang.org/t/help-test-windows-behavior-between-rustup-and-cargo/20237

AFAIK, there have not been any indications of anyone having problems.
#3703

... unfortunately that's probably because few have actually tried 😅