Make sure to also wrap the initial `-vV` invocation #13659

RalfJung · 2024-03-27T07:00:47Z

Fixes #10885 and therefore helps unblock rust-lang/miri#3422.

This ensures that the version info actually matches the compiler that will later be doing the builds.

rustbot · 2024-03-27T07:00:53Z

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

DO NOT MERGE: update cargo for experiment This incorporates rust-lang/cargo#13659 so that we can test it.

RalfJung · 2024-03-27T08:46:20Z

All right now in rust-lang/rust#123124 we have a toolchain with this cargo. CI also still works (including Miri).

So now that's ready for experimentation with sccache. Does someone happen to have a setup ready for this? :D

RalfJung · 2024-03-27T08:46:57Z

tests/testsuite/cargo_env_config.rs

-                let status = std::process::Command::new(&args.next().unwrap())
+                let mut cmd = std::env::args().skip(1).collect::<Vec<_>>();
+                if cmd.get(1).map(|s| &**s) == Some("-vV") {
+                    // This is the version query, not the target info query, so skip this.


I guess this raises the question, should the env stuff also be set for the -vV query?

Sorry, did you mean to add an environment variable similar to RUSTC_VERSION? Is there any benefit to this?

This is about env vars configured with [env] in config.toml. Should they be set here?

We have a test that they are set for the --print ... invocation. Should they be set for the -vV invocation as well?

Yea, it probably makes sense to also set the env vars. Can you add that? It looks like apply_env_config might need to be pub(crate) or something to make that work.

Sure, done.

RalfJung · 2024-03-27T18:43:03Z

I have installed sccache (apt install sccache), set the env var (export RUSTC_WRAPPER=$(which sccache)), and then tried building a few crates with the toolchain from rust-lang/rust#123124:

rustup-toolchain-install-master -c cargo -- c656380a167bb05da121130f7364bfe5785f1c8b
cargo +c656380a167bb05da121130f7364bfe5785f1c8b build

That seems to work fine. I can see via sccache --show-stats that sccache was invoked. I can see via RUSTC_WRAPPER=/bin/false cargo +c656380a167bb05da121130f7364bfe5785f1c8b build that the -vV call is being wrapped:

error: process didn't exit successfully: `/bin/false /home/r/.rustup/toolchains/c656380a167bb05da121130f7364bfe5785f1c8b/bin/rustc -vV` (exit status: 1)

I have also confirmed that Miri still works, and that setting RUSTC to a non-existent path but using a suitable RUSTC_WRAPPER now makes the test-cargo-miri call succeed (after removing autocfg). So cargo itself seems to be clean then in terms of always applying the wrapper, at least for the code paths needed by that crate.

So... I think we're all good? @ehuss anything else you think should be checked?

(We also still need a decision about applying the env keys though, see above.)

weihanglo · 2024-03-28T23:45:16Z

r? @ehuss

since you seems to involve in the discussion already.

ehuss · 2024-04-10T16:50:41Z

Thanks! There looks like there might be a few more things to get this ready to merge:

Add an explicit test for this. There are a few ways to go about this, either what is suggested in Make sure to also wrap the initial -vV invocation #10887 (comment), or just have a separate test that has a custom wrapper that checks for the appropriate behavior, or do something similar to the rustc_info_cache test which sets CARGO_LOG=cargo::util::rustc=debug and check for the [..]running [..]my_wrapper rustc -Vv[..] or similar. (That last option might be the easiest if it works.)
Update the PR description to remove the "experimentation" description and include an explanation of why the change is being made.
Squash the commits.

RalfJung · 2024-04-12T10:03:07Z

All right, should be all done.

Based on an earlier draft by oli-obk

ehuss · 2024-04-15T17:17:07Z

Thanks!

@bors r+

bors · 2024-04-15T17:17:11Z

📌 Commit 8a7ba8f has been approved by ehuss

It is now in the queue for this repository.

bors · 2024-04-15T17:18:29Z

⌛ Testing commit 8a7ba8f with merge 624233b...

bors · 2024-04-15T17:48:29Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 624233b to master...

Update cargo 11 commits in 48eca1b164695022295ce466b64b44e4e0228b08..6f06fe908a5ee0f415c187f868ea627e82efe07d 2024-04-12 21:16:36 +0000 to 2024-04-16 18:47:44 +0000 - fix(toml): Error on `[project]` in Edition 2024 (rust-lang/cargo#13747) - feat(update): Include a Locking message (rust-lang/cargo#13759) - chore(deps): update rust crate gix to 0.62.0 [security] (rust-lang/cargo#13760) - test(schemas): Ensure tests cover the correct case (rust-lang/cargo#13761) - feat(resolve): Tell the user the style of resovle done (rust-lang/cargo#13754) - Make sure to also wrap the initial `-vV` invocation (rust-lang/cargo#13659) - docs: update `checkout` GitHub action version (rust-lang/cargo#13757) - Recategorize cargo test's `--doc` flag under "Target Selection" (rust-lang/cargo#13756) - Reword sentence describing workspace toml for clarity (rust-lang/cargo#13753) - docs(ref): Update unstable docs for msrv-policy (rust-lang/cargo#13751) - refactor(config): Consistently use kebab-case (rust-lang/cargo#13748) r? ghost

rustbot assigned weihanglo Mar 27, 2024

rustbot added A-cache-messages Area: caching of compiler messages S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2024

RalfJung mentioned this pull request Mar 27, 2024

DO NOT MERGE: update cargo for experiment rust-lang/rust#123124

Closed

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2024

Auto merge of rust-lang#123124 - RalfJung:cargo-rustc-wrapper, r=<try>

c656380

DO NOT MERGE: update cargo for experiment This incorporates rust-lang/cargo#13659 so that we can test it.

RalfJung commented Mar 27, 2024

View reviewed changes

RalfJung mentioned this pull request Mar 27, 2024

cargo-miri: ensure all rustc invocations go via our wrapper rust-lang/miri#3422

Closed

3 tasks

RalfJung marked this pull request as ready for review March 27, 2024 18:58

rustbot assigned ehuss and unassigned weihanglo Mar 28, 2024

weihanglo mentioned this pull request Mar 29, 2024

cargo rustc --version should print the version of rustc called by cargo #13611

Open

rustbot added the A-build-execution Area: anything dealing with executing the compiler label Apr 7, 2024

RalfJung force-pushed the rustc-wrapper branch from d67f485 to 45caab2 Compare April 7, 2024 08:18

RalfJung force-pushed the rustc-wrapper branch 3 times, most recently from 6535b01 to c692d8b Compare April 12, 2024 10:03

Also wrap the initial -vV invocation in the rustc_(workspace_)wrapper

8a7ba8f

Based on an earlier draft by oli-obk

RalfJung force-pushed the rustc-wrapper branch from c692d8b to 8a7ba8f Compare April 12, 2024 10:34

weihanglo mentioned this pull request Apr 12, 2024

docs(ref): Update unstable docs for msrv-policy #13751

Merged

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 Apr 15, 2024

bors merged commit 624233b into rust-lang:master Apr 15, 2024
21 of 22 checks passed

weihanglo mentioned this pull request Apr 16, 2024

Update cargo rust-lang/rust#124039

Merged

rustbot added this to the 1.79.0 milestone Apr 17, 2024

RalfJung mentioned this pull request Apr 20, 2024

clippy-driver acts as a bogus rustc wrapper rust-lang/rust-clippy#12697

Open

This was referenced Jul 13, 2024

bootstrap: fully rely on RUSTC_WRAPPER rust-lang/rust#127682

Closed

Build script does not respect RUSTC_WRAPPER rust-lang/libc#3772

Closed

RalfJung mentioned this pull request Jul 30, 2024

bootstrap should be able to fully rely on RUSTC_WRAPPER rust-lang/rust#128383

Open

RalfJung deleted the rustc-wrapper branch August 6, 2024 11:55

epage mentioned this pull request Aug 13, 2024

Cargo caches failed cargo clippy -- --bad-arg #14385

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure to also wrap the initial `-vV` invocation #13659

Make sure to also wrap the initial `-vV` invocation #13659

RalfJung commented Mar 27, 2024 •

edited

Loading

rustbot commented Mar 27, 2024

RalfJung commented Mar 27, 2024 •

edited

Loading

RalfJung Mar 27, 2024

heisen-li Apr 3, 2024

RalfJung Apr 3, 2024 •

edited

Loading

ehuss Apr 6, 2024

RalfJung Apr 7, 2024

RalfJung commented Mar 27, 2024 •

edited

Loading

weihanglo commented Mar 28, 2024

ehuss commented Apr 10, 2024

RalfJung commented Apr 12, 2024

ehuss commented Apr 15, 2024

bors commented Apr 15, 2024

bors commented Apr 15, 2024

bors commented Apr 15, 2024

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

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

Conversation

RalfJung commented Mar 27, 2024 • edited Loading

rustbot commented Mar 27, 2024

RalfJung commented Mar 27, 2024 • edited Loading

RalfJung Mar 27, 2024

Choose a reason for hiding this comment

heisen-li Apr 3, 2024

Choose a reason for hiding this comment

RalfJung Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

ehuss Apr 6, 2024

Choose a reason for hiding this comment

RalfJung Apr 7, 2024

Choose a reason for hiding this comment

RalfJung commented Mar 27, 2024 • edited Loading

weihanglo commented Mar 28, 2024

ehuss commented Apr 10, 2024

RalfJung commented Apr 12, 2024

ehuss commented Apr 15, 2024

bors commented Apr 15, 2024

bors commented Apr 15, 2024

bors commented Apr 15, 2024

Make sure to also wrap the initial `-vV` invocation #13659

Make sure to also wrap the initial `-vV` invocation #13659

RalfJung commented Mar 27, 2024 •

edited

Loading

RalfJung commented Mar 27, 2024 •

edited

Loading

RalfJung Apr 3, 2024 •

edited

Loading

RalfJung commented Mar 27, 2024 •

edited

Loading