-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update RLS to set __CARGO_TEST_ROOT when running its tests. #56177
Conversation
|
This is an internal environment variable used by cargo and not intended for other users, is there another way to fix this or perhaps something that can be officially supported by cargo? The double underscore was supposed to indicate that... |
@alexcrichton I was told to use that variable in #53586 (comment) (and that PR already landed). EDIT: I don't see how to do it short of a policy change in Cargo, to make "current package believes it's in a workspace when it's not" not an error (unless I misunderstood and it's just a warning?) |
@Mark-Simulacrum how come you suggested that? That should (like some others related to it) be an internal Cargo environment variable that isn't intended for use elsewhere. |
I did realize it was intended for Cargo-internal use but rustbuild is somewhat closely tied in anyway; I agree it's not too ideal of a state though. I mostly intended it as "this is one way" vs. "this is the right way" but perhaps I was misunderstood / that intent wasn't realized in "need to implement a more public option" |
I'm experimenting with |
@alexcrichton Given #56194 (comment), what options do we have? |
@Mark-Simulacrum oh that's what the double underscores were supposed to imply 😢 There just wasn't a way to fix this when rustbuild was originally written, hence the usage of AFAIK Cargo isn't actually that tied to rustbuild in the sense that rustbuild is only relying on public APIs of Cargo and other official ways to use it. There's been some bugs here and there over time but that's just because rustbuild exercises so much functionality. @eddyb I don't know of solutions off the top of my head, which is why I never tried to move |
Given that #56194 worked, including unbreaking RLS' tests, I'll close this and revert rust-lang/rls#1138. |
That is, update RLS to include rust-lang/rls#1138.
This fixes fallout from #53586.
r? @alexcrichton cc @Mark-Simulacrum @nrc @pietroalbini