Skip to content
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

Make sure to also wrap the initial -vV invocation #10887

Closed
wants to merge 1 commit into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jul 21, 2022

fixes #10885

This one is fairly annoying to test, as it happens before the usual cargo logic (which is also the reason it was forgotten to get wrapped).

@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 21, 2022
@ehuss
Copy link
Contributor

ehuss commented Jul 22, 2022

For testing, I think it should be sufficient to modify the rustc_wrapper test to check the target/.rustc_info.json. Just load it in a serde_json::Value, and then iterate over the "outputs" and look for one that says WRAPPER CALLED: rustc -vV in the "stderr" value.

This change seems moderately risky to me, because existing RUSTC_WRAPPERs may not be expecting this. Can you check if tools like sccache will not be affected? Presumably you've tested this with miri?

@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2022
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 30, 2022

This change seems moderately risky to me, because existing RUSTC_WRAPPERs may not be expecting this. Can you check if tools like sccache will not be affected? Presumably you've tested this with miri?

I have not tested this change at all :/ Considering it's a risky change and no on else seems to run afoul of this, and miri doesn't actually need it, I'm going to close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RUSTC_WRAPPER is ignored for rustc -vV call
3 participants