Repository navigation
Effects of RUSTUP_WINDOWS_PATH_ADD_BIN change #3825
Description
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
Activity
sunshowers commentedon May 14, 2024
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 commentedon May 14, 2024
@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 commentedon May 14, 2024
I'm not sure. Is
RUSTUP_WINDOWS_PATH_ADD_BINgoing away?djc commentedon May 14, 2024
There are no concrete plans in that direction, but it'd be good to avoid depending on workarounds for deprecated features.
sunshowers commentedon May 14, 2024
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 commentedon May 14, 2024
OK. Dylint's CI still uses this environment variable to run one obscure doc test, but Dylint itself no longer uses it.
sagudev commentedon May 16, 2024
In servo we also hit this with our custom linter.
Auto merge of #12812 - Alexendoo:driver-integration, r=flip1995
Darksonn commentedon May 17, 2024
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 commentedon May 17, 2024
@ehuss do you think Rustup is responsible for this breakage?
... unfortunately that's probably because few have actually tried 😅
rbtcollins commentedon May 17, 2024
This is in a super frustrating part of unix/windows difference; I'm sad we've created some pain - but the good news seems to be it is fairly limited - 3 projects out of the entire ecosystem is not terrible ;).
I have no strongly held view on what the right answer here is, but see #3036 (comment) - and there is a lot of previous discussion and work such as rust-lang/cargo#11917 and #3178
I do think its important that rustup is consistent on all platforms except where we really cannot (and the edge case of linked toolchains missing binaries is one such place). And we are now consistent again, so with the limited breakage, I'm inclined to suggest we shouldn't panic but should take some time to look at that the impact is on these downstream projects...
we could solve this by documenting what they need to do, or possibly by some sort of config they could opt-into ... but if we offer that, we should offer it for all platforms IMO.
sunshowers commentedon May 17, 2024
(Note that this introduces an inconsistency -- on Unix platforms, with rustup-managed cargo, you still don't need to do anything special with proc macro tests -- but on Windows you do.)
Wanted to reiterate that any projects that are broken by this are also probably broken by scenarios where rustup is not in use (e.g. distro-packaged cargo).
rbtcollins commentedon May 18, 2024
@sunshowers I think that visible inconsistency is properly at a higher layer though: it is a symptom of dlopen semantics on the underlying platforms. Because PATH affects both command line lookup and dlopen, we can't have detach them,
ChrisDenton commentedon May 18, 2024
Is there a minimal example of the proc-macro test issue? This could perhaps be fixed on the rustc/cargo side of things by being more explicit about where to load a specific DLL from.
Maybe a possible workaround for rustup is to have a third mode that adds the bin path to the end of
PATH?2 remaining items
ferdnyc commentedon Sep 15, 2024
That's true, but there's an escape hatch on Windows because an executable will ALWAYS look in its own directory for DLLs, first and always. Regardless of
%PATH%, regardless of things likeC:\Windows\System32\, etc...Just arranging to have an executable's DLL dependencies end up in the same directory as that executable is almost always a surer way of ensuring that the application can be run, vs. any sort of path shenanigans.
Do that, and while it might be helpful to have
C:\path\to\my\binary\on the path, so that you can runC:\path\to\my\binary\program.exeas justprogram.exe, it's no longer required to be on the path because you can always just runC:\path\to\my\binary\program.exeby its full path, and it will pick up any DLL inC:\path\to\my\binary\even if that directory isn't on the path.(TBH, what people need to get over, when building applications for Windows, is this idea that there should be a "libdir" at all. There should not; the bindir IS the libdir. They are one and the same. Import libraries can go in the "libdir", if you're installing libs to be used by other projects. But DLLs belong in the same output directory as the executables.)
ChrisDenton commentedon Mar 11, 2025
The trouble is that rustc DLLs are shared by other applications (e.g. proc macros or rustc_driver apps). It would be necessary for tools (such as Cargo) to copy all the DLLs from rustc's directory to the new application's directory. Well it could be more selective but that's a harder problem.
Using a common libdir is more appropriate for this case and more performant. An alternative is using SxS to install the DLLs but that's possibly more involved than rust would like.
zjp-CN commentedon Mar 12, 2025
Can rustup create a dir only containing rust dll libs, and add it to
PATH?Coming from
rustup rundoesn't setup the dylib path on windows #4246rustup rundoesn't setup the dylib path on windows #4246 as a duplicate of this issuefix(charon CLI): set RUSTUP_WINDOWS_PATH_ADD_BIN=1 on Windows
Nadrieril commentedon Mar 12, 2025
This was found to be a workaround for #4246, so plz count https://github.com/AeneasVerif/charon as one user of this special variable until a better solution is found.
ChrisDenton commentedon Mar 12, 2025
@ehuss I'm looking back through the implementation history and as far as I can see only prepending to
PATHwas tried (in various different orders)? Wouldn't appending toPATHwork better for this? So it has the lowest possible precedence.The other thing I'm investigating is changing the layout of the rustc toolchain directory on Windows to separate bins and libs. But that's more complicated and I'm not sure if it'll be breaking for some people.
ehuss commentedon Mar 12, 2025
Hm, I can't think of a particular reason that appending shouldn't work. There are some circumstances where you can end up with multiple toolchains in PATH, but assuming the DLLs have unique filenames, it should work. It also assumes that rustup prepends
~/.cargo/bin, which it still does with your patch.This I'm less confident about. I feel like there are some situations where we are relying on the DLL lookup order to look in the exe's directory as a priority. That could have larger ramifications.
ChrisDenton commentedon Mar 12, 2025
This can be somewhat controlled using SxS Application Manifests. It would require every binary in the sysroot's bin directory to have an
assemblyIdentityand declare adependentAssembly. I would strongly prefer a simpler solution though.Diggsey commentedon Mar 12, 2025
TBH I think Cargo probably should copy (or at least link) those DLLs that are needed by a rust binary.
ChrisDenton commentedon Mar 13, 2025
Could you open a Cargo issue about that, if there isn't one already?