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

expand RUSTC_WRAPPER docs #10896

Merged
merged 1 commit into from
Jul 24, 2022
Merged

expand RUSTC_WRAPPER docs #10896

merged 1 commit into from
Jul 24, 2022

Conversation

RalfJung
Copy link
Member

Fixes #10886

@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 24, 2022
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nicer to me! Thank you.

Comment on lines +27 to +28
[`build.rustc-wrapper`] to set via config. Setting this to the empty string
overwrites the config and resets cargo to not use a wrapper.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workspace wrapper can also be reset with the same trick. However, this seems to be an undocumented behaviour, attributed to these lines.

IMHO, if users start to rely on an undocumented but reasonable behaivour, we should treat it as a feature and document it well, like what you just did.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the reset is reasonable to document. Could you mention it for rustc-workspace-wrapper as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. If this is going to be documented, adding some tests to prevent regression would be better. Sorry for the reply bombing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you mention it for rustc-workspace-wrapper as well?

Done.

However I won't have the time to add tests, sorry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is what you were referring to, but I opened #10899 to update the test to make this behavior explicitly tested.

@ehuss
Copy link
Contributor

ehuss commented Jul 24, 2022

Looks good, thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 24, 2022

📌 Commit c492216 has been approved by ehuss

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2022
@bors
Copy link
Collaborator

bors commented Jul 24, 2022

⌛ Testing commit c492216 with merge 27f4f02...

@bors
Copy link
Collaborator

bors commented Jul 24, 2022

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 27f4f02 to master...

@bors bors merged commit 27f4f02 into rust-lang:master Jul 24, 2022
@RalfJung RalfJung deleted the rustc-wrapper branch July 25, 2022 11:44
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2022
Update cargo

5 commits in d8d30a75376f78bb0fabe3d28ee9d87aa8035309..85b500ccad8cd0b63995fd94a03ddd4b83f7905b
2022-07-19 13:59:17 +0000 to 2022-07-24 21:10:46 +0000
- Make the empty rustc-wrapper test more explicit. (rust-lang/cargo#10899)
- expand RUSTC_WRAPPER docs (rust-lang/cargo#10896)
- Stabilize Workspace Inheritance (rust-lang/cargo#10859)
- Fix typo in unstable docs: s/PROGJCT/PROJECT/ (rust-lang/cargo#10890)
- refactor(source): Open query API for adding more types of queries (rust-lang/cargo#10883)
@ehuss ehuss added this to the 1.64.0 milestone Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document behavior of mixing RUSTC and RUSTC_WRAPPER
5 participants