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

Add CompileMode to Executor callbacks #5830

Merged
merged 1 commit into from
Jul 29, 2018

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented Jul 29, 2018

This came up when trying to fix rust-lang/rls#876.

So currently in the RLS we recreate our own dep graph, where we store units with a key (PackageId, TargetKind). This turned out to be not enough since
a) we can have multiple bin target kinds with different names (unrelated to this PR)
b) same package target kind (bin, lib) can be compiled regularly or including the test harness.
With this, we can distinguish these cases and properly rerun both regular compilation check and the one including unit tests.

Without this information we'd need to fall back on guessing whether the rustc invocation has --test but having this information makes it accurate and seems useful enough to add it to the callback arguments.

r? @alexcrichton or @matklad

This helps distinguish whether a given rustc invocation corresponds to a
regular compilation or is it compiled as a test harness. The difference is
important for tools like RLS, since checking test code is different than
checking what is it regularly compiled as.
@matklad
Copy link
Member

matklad commented Jul 29, 2018

@bors r+

BTW, we have a --build-plan now, might be a good idea to check if that can be useful: https://github.com/rust-lang/cargo/pulls?q=is%3Apr+build+plan+is%3Aclosed

@bors
Copy link
Collaborator

bors commented Jul 29, 2018

📌 Commit 90a5ba3 has been approved by matklad

@bors
Copy link
Collaborator

bors commented Jul 29, 2018

⌛ Testing commit 90a5ba3 with merge 1ec8c70...

bors added a commit that referenced this pull request Jul 29, 2018
Add CompileMode to Executor callbacks

This came up when trying to fix rust-lang/rls#876.

So currently in the RLS we recreate our own dep graph, where we store units with a key `(PackageId, TargetKind)`. This turned out to be not enough since
a) we can have multiple bin target kinds with different names (unrelated to this PR)
b) same package target kind (bin, lib) can be compiled regularly or including the test harness.
With this, we can distinguish these cases and properly rerun both regular compilation check and the one including unit tests.

Without this information we'd need to fall back on guessing whether the rustc invocation has `--test` but having this information makes it accurate and seems useful enough to add it to the callback arguments.

r? @alexcrichton or @matklad
@matklad
Copy link
Member

matklad commented Jul 29, 2018

Correct link: #5301

@Xanewok
Copy link
Member Author

Xanewok commented Jul 29, 2018

@matklad yep, saw that one, thanks!

In this particular case I'd prefer to use the Cargo API directly, since we're doing it and running it in-process already, however it'd definitely be good to improve the build plan further for the upcoming build system interop work!

@bors
Copy link
Collaborator

bors commented Jul 29, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 1ec8c70 to master...

@bors bors merged commit 90a5ba3 into rust-lang:master Jul 29, 2018
@Xanewok Xanewok deleted the executor-compile-mode branch July 29, 2018 21:47
@ehuss ehuss added this to the 1.29.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RLS only compiles the first member of a Cargo workspace
5 participants