bootstrap: don't use rustflags for --rustc-args

[lqd]

r? @onur-ozkan

This is going to require a bit of context.

#47558 has added --rustc-args to ./x test to allow passing flags when building compiletest tests. It was made specifically because using RUSTFLAGS would rebuild the compiler/stdlib, which would in turn require the flag you want to build tests with to successfully bootstrap.

#113178 made the request that it also works for other tests and doctests. This is not trivial to support across the board for library/compiler unit-tests/doctests and across stages. This issue was closed in #113948 by using RUSTFLAGS, seemingly incorrectly since #123489 fixed that part to make it work.

Unfortunately #123489/#113948 have regressed the goals of --rustc-args:

• now we can't use rustc args that don't bootstrap, to run the UI tests: we can't test incomplete features. The new trait solver doesn't bootstrap, in-progress borrowck/polonius changes don't bootstrap, some other features are similarly incomplete, etc.
• using the flag now rebuilds everything from scratch: stage0 stdlib, stage1 compiler, stage1 stdlib. You don't need to re-do all this to compile UI tests, you only need the latter to run stdlib tests with a new flag, etc. This happens for contributors, but also on CI today. (Not to mention that in doing that it will rebuild things with flags that are not meant to be used, e.g. stdlib cfgs that don't exist in the compiler; or you could also imagine that this silently enables flags that were not meant to be enabled in this way).

Since then, bd71c71 has started using it to test a stdlib feature, relying on the fact that it now rebuilds everything. So #125011 also regressed CI times more than necessary because it rebuilds everything instead of just stage 1 stdlib.

It's not easy for me to know how to properly fix #113178 in bootstrap, but #113948/#123489 are not it since they regress the initial intent. I'd think bootstrap would have to know from the list of test targets that are passed that the library or compiler paths that are passed could require rebuilding these crates with different rustflags, probably also depending on stages. Therefore I would not be able to fix it, and will just try in this PR to unregress the situation to unblock the initial use-case.

It seems miri now also uses ./x miri --rustc-args in this incorrect meaning to rebuild the library paths they support to run with the new args. I've not made any bootstrap changes related to ./x miri in this PR, so --rustc-args wouldn't work there anymore. I'd assume this would need to use rustflags again but I don't know how to make that work properly in bootstrap, hence opening as draft, so you can tell me how to do that. I assume we don't want to break their use-case again now that it exists, even though there are ways to use ./x test to do exactly that.

RUSTFLAGS_NOT_BOOTSTRAP=flag ./x test library/std is a way to run unit tests with a new flag without rebuilding everything, while with #123489 there is no way anymore to run tests with a flag that doesn't bootstrap.

---

edit: after review, this PR:

• renames ./x test --rustc-args to ./x test --compiletest-rustc-args as it only applies there, and cannot use rustflags for this purpose.
• fixes the regression that using these args rebuilt everything from scratch
• speeds up some CI jobs via the above point
• removes ./x miri --rustc-args as only library tests are supported, needs to rebuild libstd, and ./x miri --compiletest-rustc-args wouldn't work since compiletests are not supported.

[onur-ozkan]

We had an issue with setting --rustc-args for any test (which is also needed). We could add another flag specifically for handling compiletest which should be much easier than modifying the current flag (as such a change could impact existing bootstrap calls).

[lqd]

Yeah I linked to that issue in the PR description. The way it was fixed broke the flag that had existed for compiletests. Maybe there could be a dedicated flag for these other tests, or they can use rustflags. We can’t do that for compiletests.

[onur-ozkan]

Maybe there could be a dedicated flag for these other tests, or they can use rustflags

We don’t have any - right, using RUSTFLAGS can achieve the same thing. I think we should rename --rustc-args to something that better reflects its purpose (like compiletest-rustc-args). The name rustc-args feels too general and doesn’t clearly cover what it does without looking at the source code.

It seems miri now also uses ./x miri --rustc-args in this incorrect meaning to rebuild the library paths they support to run with the new args. I've not made any bootstrap changes related to ./x miri in this PR, so --rustc-args wouldn't work there anymore.

Can't we replace --rustc-args with RUSTFLAGS_NOT_BOOTSTRAP?

cc @RalfJung

[lqd]

Can't we replace --rustc-args with RUSTFLAGS_NOT_BOOTSTRAP?

That's what I did in the PR to change the std size-optimized tests, but I'm not sure if it'd work exactly in the same way for miri tests, in particular I wanted to avoid possibly breaking use-cases or CI they might have relying on --rustc-args by now.

---

Maybe an alternative is making sure RUSTFLAGS_NOT_BOOTSTRAP are applied to compiletest, I don't believe it's the case today. It may unify --rustc-args to then only modify that env var in some stages. Until we have per-stage granularity on all these, it may also not work depending on the stage with which you actually want to perform the test. --rustc-args historically never wants to rebuild anything for itself, on any stage...

Maybe renaming to --compiletest-rustc-args or similar is indeed the least bad way to do it.

[RalfJung]

I don't know --rustc-args and whatever it does in ./x.py miri is likely copied from somewhere else without understanding what the flag is supposed to do. Feel free to change its behavior in any way you see fit.

[onur-ozkan]

Maybe renaming to --compiletest-rustc-args or similar is indeed the least bad way to do it.

Let's do that before landing this.

[lqd]

Ok. I'll also try to do some archeology on ./x miri's rustc-args, and remove it if it exists only from copying ./x test.

[RalfJung]

I can assure you it exists only by copying -- adding x.py miri was the first time I even learned that --rustc-flags existed.

We usually use the MIRIFLAGS env var to pass flags to Miri with cargo miri, so with x.py miri I did the same.

[lqd]

Let's do that before landing this.

Done, I've renamed the flag as you asked.

I've also removed ./x miri --rustc-args since it seems it's copy-paste rather than particularly useful, and ./x miri --compiletest-rustc-args wouldn't make sense: it currently only supports library tests.

[rustbot]

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[lqd]

Context for the last ping: cg_gcc uses ./x test --rustc-args which is being renamed in this PR. It should also speed up your tests/CI since it fixes the regression that rebuilt everything when using it.

[onur-ozkan]

@bors r+

[bors]

📌 Commit 2abfa35 has been approved by onur-ozkan

It is now in the queue for this repository.