Comments (14)
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.
This is now out as part of cargo-nextest 0.9.72. With this, nextest no longer depends on whether it's being invoked via a rustup-wrapped cargo.
from rustup.
@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.
from rustup.
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.
from rustup.
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.
from rustup.
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.
from rustup.
@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?
from rustup.
@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?
from rustup.
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.
from rustup.
In servo we also hit this with our custom linter.
from rustup.
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.
from rustup.
@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 😅
from rustup.
(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).
from rustup.
@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,
from rustup.
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
?
from rustup.
Related Issues (20)
- rustup fails to open non-existing windows registry path HOT 3
- `rustup component (add|remove)` should not rely on hardcoded target triples HOT 9
- Decide on a better strategy for release notes/changelogs HOT 3
- Simplify download and/or TLS backends HOT 1
- Rustup fails with os error 10054 on a new Windows 11 machine HOT 13
- `sh.rustup.rs` should only resolve to ipv4 HOT 7
- rust-analyzer is not being automatically proxied after installing HOT 4
- Rust installing script incompatible with older macOS HOT 1
- rustup should use the configured profile as fallback when the key is not present in `rust-toolchain.toml` HOT 5
- CfT: Test out Rustup's `reqwest` backend with `rustls` HOT 5
- error: toolchain 'stable-x86_64-pc-windows-msvc' is not installable with rustup 1.27.0 on wine HOT 4
- Failures while downloading https://static.rust-lang.org/rustup/dist/x86_64-unknown-linux-gnu/rustup-init HOT 1
- `rustup update self` should suggest `rustup self update` HOT 8
- Windows: Explorer 'Quick Access' polluted with deleted folders inside `.rustup`
- Lack of support for p521 signatures with the `ring`-based `reqwest/rustls` backend HOT 15
- New env `RUSTUP_AUTO_SELF_UPDATE` to configure `rustup self update` HOT 12
- (🐞) rustup didn't install Visual Studio HOT 7
- Disable Rustup's self update by default if the `CI` environment variable is detected HOT 3
- #3827 has broken the ETA format when downloading/installing components HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rustup.