Bugfix: avoid panic on invalid json output from libtest
#108659 introduces a custom test display implementation. It does so by using libtest to output json. The stdout is read and parsed; The code trims the line read and checks whether it starts with a `{` and ends with a `}`. If so, it concludes that it must be a json encoded `Message`. Unfortunately, this does not work in all cases:
- This assumes that tests running with `--nocapture` will never start and end lines with `{` and `}` characters
- Output is generated by issuing multiple `write_message` [statements](https://github.com/rust-lang/rust/blob/master/library/test/src/formatters/json.rs#L33-L60). Where only the last one issues a `\n`. This likely results in a race condition as we see multiple json outputs on the same line when running tests for the `x86_64-fortanix-unknown-sgx` target:
```
10:21:04 [0m[0m[1m[32m Running[0m tests/run-time-detect.rs (build/x86_64-unknown-linux-gnu/stage1-std/x86_64-fortanix-unknown-sgx/release/deps/run_time_detect-8c66026bd4b1871a)
10:21:04
10:21:04 running 1 tests
10:21:04 test x86_all ... ok
10:21:04 [0m[0m[1m[32m Running[0m tests/thread.rs (build/x86_64-unknown-linux-gnu/stage1-std/x86_64-fortanix-unknown-sgx/release/deps/thread-ed5456a7d80a6193)
10:21:04 thread 'main' panicked at 'failed to parse libtest json output; error: trailing characters at line 1 column 135, line: "{ \"type\": \"suite\", \"event\": \"ok\", \"passed\": 1, \"failed\": 0, \"ignored\": 0, \"measured\": 0, \"filtered_out\": 0, \"exec_time\": 0.000725911 }{ \"type\": \"suite\", \"event\": \"started\", \"test_count\": 1 }\n"', render_tests.rs:108:25
```
This PR implements a partial fix by being much more conservative of what it asserts is a valid json encoded `Message`. This prevents panics, but still does not resolve the race condition. A discussion is needed where this race condition comes from exactly and how it best can be avoided.
cc: `@jethrogb,` `@pietroalbini`
Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF`
This option was added to LLVM in https://reviews.llvm.org/D121750?id=416339. It makes `llvm_unreachable` in builds without assertions compile to an `LLVM_BUILTIN_TRAP` instead of `LLVM_BUILTIN_UNREACHABLE` (which causes undefined behavior and is equivalent to `std::hint::unreachable_unchecked`).
Having compiler bugs triggering undefined behavior generally seems undesirable and inconsistent with Rust's goals. There is a check in `src/tools/tidy/src/style.rs` to reject code using `llvm_unreachable`. But it is used a lot within LLVM itself.
For instance, this changes a failure I get compiling `libcore` for m68k from a `SIGSEGV` to `SIGILL`, which seems better though it still doesn't provide a useful message without switching to an LLVM build with asserts.
It may be best not to do this if it noticeably degrades compiler performance, but worthwhile if it doesn't do so in any significant way. I haven't looked into what benchmarks there are for Rustc. That should be considered before merging.
refactor `fn bootstrap::builder::Builder::compiler_for` logic
- check compiler stage before forcing for stage2.
- check if download_rustc is not set before forcing for stage1.
resolves#109286
This option was added to LLVM in
https://reviews.llvm.org/D121750?id=416339. It makes `llvm_unreachable`
in builds without assertions compile to an `LLVM_BUILTIN_TRAP` instead
of `LLVM_BUILTIN_UNREACHABLE` (which causes undefined behavior and is
equivalent to `std::hint::unreachable_unchecked`).
Having compiler bugs triggering undefined behavior generally seems
undesirable and inconsistent with Rust's goals. There is a check in
`src/tools/tidy/src/style.rs` to reject code using `llvm_unreachable`.
But it is used a lot within LLVM itself.
For instance, this changes a failure I get compiling `libcore` for m68k
from a `SIGSEGV` to `SIGILL`, which seems better though it still doesn't
provide a useful message without switching to an LLVM build with asserts.
It may be best not to do this if it noticeably degrades compiler
performance, but worthwhile if it doesn't do so in any significant way. I
haven't looked into what benchmarks there are for Rustc. That should be
considered before merging.
Include executed tests in the build metrics (and use a custom test display impl)
The main goal of this PR is to include all tests executed in CI inside the build metrics JSON files. I need this for Ferrocene, and `@Mark-Simulacrum` expressed desire to have this as well to ensure all tests are executed at least once somewhere in CI.
Unfortunately implementing this required rewriting inside of bootstrap all of the code to render the test output to console. libtest supports outputting JSON instead of raw text, which we can indeed use to populate the build metrics. Doing that suppresses the console output though, and compared to rustc and Cargo the console output is not included as a JSON field.
Because of that, this PR had to reimplement both the "pretty" format (one test per line, with `rust.verbose-tests = true`), and the "terse" format (the wall of dots, with `rust.verbose-tests = false`). The current implementation should have the exact same output as libtest, except for the benchmark output. libtest's benchmark output is broken in the "terse" format, so since that's our default I slightly improved how it's rendered.
Also, to bring parity with libtest I had to introduce support for coloring output from bootstrap, using the same dependencies `annotate-snippets` uses. It's now possible to use `builder.color_for_stdout(Color::Red, "text")` and `builder.color_for_stderr(Color::Green, "text")` across all of bootstrap, automatically respecting the `--color` flag and whether the stream is a terminal or not.
I recommend reviewing the PR commit-by-commit.
r? `@Mark-Simulacrum`
Add `dist.compression-profile` option to control compression speed
PR #108534 reduced the size of compressed archives, but (as expected) it also resulted in way longer compression times and memory usage during compression.
It's desirable to keep status quo (smaller archives but more CI usage), but it should also be configurable so that downstream users don't have to waste that much time on CI. As a data point, this resulted in doubling the time of Ferrocene's dist jobs, and required us to increase the RAM allocation for one of such jobs.
This PR adds a new `config.toml` setting, `dist.compression-profile`. The values can be:
* `fast`: equivalent to the gzip and xz preset of "1"
* `balanced`: equivalent to the gzip and xz preset of "6" (the CLI defaults as far as I'm aware)
* `best`: equivalent to the gzip present of "9", and our custom xz profile
The default has also been moved back to `balanced`, to try and avoid the compression time regression for downstream users. I don't feel too strongly on the default, and I'm open to changing it.
Also, for the `best` profile the XZ settings do not match the "9" preset used by the CLI, and it might be confusing. Should we create a `custom-rustc-ci`/`ultra` profile for that?
r? ``@Mark-Simulacrum``
Distribute libntdll.a with windows-gnu toolchains
This allows the OS loader to load essential functions (e.g. read/write file) at load time instead of lazily doing so at runtime.
r? libs
migrate compiler, bootstrap and compiletest to windows-rs
This PR migrates the compiler, bootstrap, and compiletest to use [windows-rs](https://github.com/microsoft/windows-rs) instead of winapi-rs. windows-rs is the bindings crate provided by Microsoft, and is actively maintained compared to winapi-rs. Not all ecosystem crates have migrated over yet, so there will be a period of time where both crates are used.
windows-rs also provides some nice ergonomics over winapi-rs to convert return values to `Result`s (which found a case where we forgot to check the return value of `CreateFileW`).
Add tests for configure.py
I highly recommend reviewing this with whitespace disabled.
Notably, verifying that we generate valid toml relies on python 3.11 so
we can use `tomllib`, so this also switches`x86_64-gnu-llvm-14` (one of the PR builders) to use 3.11.
While fixing that, I noticed that we stopped testing python2.7 support on PR CI in https://github.com/rust-lang/rust/pull/106085. `@fee1-dead` `@pietroalbini` please be more careful in the future, there is no CI for CI itself that verifies we are testing everything we should be.
- Separate out functions so that each unit test doesn't create a file on disk
- Add a few unit tests
Set `CMAKE_SYSTEM_NAME` for Linux targets
When bootstrap compiles native dependencies like LLVM, it should set `CMAKE_SYSTEM_NAME` for the target system; otherwise cmake may not identify that it is cross-compiling.
In particular, when building a Linux rustc on a macOS host, cmake was including `-isysroot /path/to/macOS.sdk` options that caused things to break. By setting `CMAKE_SYSTEM_NAME=Linux` when building for Linux targets, the macOS SDK is no longer passed as sysroot to the compiler.
r? bootstrap
Upgrade to LLVM 16
This updates Rust to LLVM 16. It also updates our host compiler for dist-x86_64-linux to LLVM 16. The reason for that is that Bolt from LLVM 15 is not capable of compiling LLVM 16 (https://github.com/llvm/llvm-project/issues/61114).
LLVM 16.0.0 has been [released](https://discourse.llvm.org/t/llvm-16-0-0-release/69326) on March 18, while Rust 1.70 will become stable on June 1.
Tested images: `dist-x86_64-linux`, `dist-riscv64-linux` (alt), `dist-x86_64-illumos`, `dist-various-1`, `dist-various-2`, `dist-powerpc-linux`, `wasm32`, `armhf-gnu`
Tested images until the usual IPv6 failures: `test-various`
- check compiler stage before forcing for stage2.
- check if download_rustc is not set before forcing for stage1.
Signed-off-by: ozkanonur <work@onurozkan.dev>
- Separate out functions so that each unit test doesn't create a file on disk
- Add a few unit tests
Notably, verifying that we generate valid toml relies on python 3.11 so
we can use `tomllib`.
Prevent stable `libtest` from supporting `-Zunstable-options`
Took a while for me to get around to this but seems trivial (unless I'm missing some reason this will break all our tests). Fixes#75526
Basically `libtest` already tries to handle this in 501ad021b9/library/test/src/cli.rs (L310-L318)
But that env var was not passed. I'm guessing at one point [this code](501ad021b9/src/bootstrap/compile.rs (L842-L844)) (or a common ancestor) was used to compile the standard library/libtest, but that is no longer the case (or perhaps it never worked, I don't have time to go digging).
I don't love that this is a "allow unstable by default" situation, as it means things like [`rustc-build-sysroot`](https://github.com/RalfJung/rustc-build-sysroot) could accidentally get unstable (CC ````@RalfJung)```` even if this is fixed here, but it's consistent with what happens in `rustc_feature`, so... yeah.
This is user-facing after all, even if it's hard to imagine the outcome of that conversation being "lets continue allowing use of `-Zunstable-features` from stable rust" (especially since a `RUSTC_BOOTSTRAP=1`-shaped loophole remains)... I think it probably should get a vibe check in the t-libs meeting (and plausibly a relnote along the lines of "hey `cargo test -- -Zunstable-options --some --unstable --stuff=here` used to work on stable, that's been fixed, sorry").
I'll nominate it for that after CI comes up green (I've done a smoke check but don't know what (if anything) will need `bootstrap` to enable `RUSTC_BOOTSTRAP=1` when running tests)
r? ````@jyn514````
When bootstrap compiles native dependencies like LLVM, it should set
CMAKE_SYSTEM_NAME for the target system; otherwise cmake may not
identify that it is cross-compiling.
In particular, when building a Linux rustc on a macOS host, cmake was
including `-isysroot /path/to/macOS.sdk` options that caused things to
break. By setting `CMAKE_SYSTEM_NAME=Linux` when building for Linux
targets, the macOS SDK is no longer passed as sysroot to the compiler.
Rename `config.toml.example` to `config.example.toml`
This had bothered me for a while as it leads to bad (missing) syntax highlighting in most editors I've used, and `@jyn514` suggested I just make the change and that the compatibility concerns I had don't really matter.
I suspect it will be a contentious one, so will not be offended if the outcome of this is to close the PR.
bootstrap: document tidy
Enable documentation of tidy, as suggested in #106803. Jyn mentioned they should probably be added to `doc.rust-lang.org`, how should that be done?
Set `LIBC_CHECK_CFG=1` when building Rust code in bootstrap
Downstream forks of the Rust compiler might want to use a custom `libc` to add support for targets that are not yet available upstream. Adding a patch to replace `libc` with a custom one would cause compilation errors though, because Cargo would interpret the custom `libc` as part of the workspace, and apply the check-cfg lints on it.
Since https://github.com/rust-lang/libc/pull/3037, the `libc` build script emits check-cfg flags only when the `LIBC_CHECK_CFG` environment variable is set, so this PR allows the use of custom `libc`s.
Import rust-installer & adjust compression settings
This brings in rust-lang/rust-installer#123, which enables a larger compression window (8 -> 64MB) amongst other changes to the xz compression settings. The net effect should be smaller compressed tarballs which will decrease bandwidth usage for
static.rust-lang.org, download times, and decompression time.
This comes at the cost of higher baseline requirements for running rustup to use these files, which we believe should be largely acceptable (running rustc is likely to use at least this much memory) but if we get specific reports we may explore options to decrease impact (e.g., using the gzip tarballs automatically in rustup).
To simplify iteration on compression settings this also imports the rust-lang/rust-installer submodule, it is now hosted fully inside rust-lang/rust. Once we land this I'll file a followup to add a note to that repo and we can subsequently archive it.
--
CI times for dist-x86_64-linux builds:
* threads=6, master - 2h 50m
* threads=1, new - 3h 40m
* threads=6, new - 2h 50m
