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

Real cross-compiling tests instead of #![no_core] silliness #130375

Open
workingjubilee opened this issue Sep 14, 2024 · 14 comments
Open

Real cross-compiling tests instead of #![no_core] silliness #130375

workingjubilee opened this issue Sep 14, 2024 · 14 comments
Assignees
Labels
A-compiletest Area: The compiletest test runner A-cross Area: Cross compilation A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@workingjubilee
Copy link
Member

We have a zillion tests that use the #![no_core] hack, like this one:

A given test involves:

  • Host A
  • Target B
  • Some Rust source (compiled for Target B)
  • Test tooling (run on Host A)

Most of our test infra assumes that Host A and Target B are the same thing. This greatly simplifies a lot of assumptions. But this hack we have introduced in so many tests essentially exists so that we can be sure the test runs even when cross-compiling from Host A to Target B. It's possible there's some weird finagling of test flags that can enable this without the hack, but that would be another hack itself. Cross-compiled tests should be relatively easy, like just writing something like:

//@ cross-compile-targets: arch_1-unknown-os_3-env_5 arch_2-unknown-os_4-env_6 arch_1-unknown-os_4-env_5 arch_2-unknown-os_3-env_7
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 14, 2024
@workingjubilee workingjubilee added A-testsuite Area: The testsuite used to check the correctness of rustc A-compiletest Area: The compiletest test runner labels Sep 14, 2024
@workingjubilee workingjubilee added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 14, 2024
@workingjubilee workingjubilee added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 14, 2024
@workingjubilee
Copy link
Member Author

sorry @jieyouxu!

@jieyouxu jieyouxu added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 15, 2024
@workingjubilee workingjubilee added the A-cross Area: Cross compilation label Sep 15, 2024
@nikic
Copy link
Contributor

nikic commented Sep 16, 2024

The problem here is that without no_core, you actually have to build core/std for the target. We have some tests that test ~all targets, and having everyone build core/std for 100 targets just to run some codegen or assembly tests would be ... not great.

@workingjubilee
Copy link
Member Author

That would probably be bad, yes. Handling this correctly probably requires some coordination between bootstrap and compiletest about which stdlibs are built (or can be built, or may be desirable to build).

@workingjubilee
Copy link
Member Author

workingjubilee commented Sep 16, 2024

I don't know that we'd even need much to extract information like "Ignored 20 assembly tests with 50 revisions because you didn't build std for aarch64-apple-darwin and 69 other targets."

That would presume treating it like something between needs-llvm-component and compile-flags: --target.

It probably wouldn't replace every no_core test. But there are many cases where no_core tests aren't good enough or are too tedious to write so they simply don't get written, and then there's no way to run the test that didn't get written, but if we had at least the core built we could write the tests.

@RalfJung
Copy link
Member

We also have the problem that for most of our targets, you can't easily cross-compile the sysroot. Building a windows std on my Linux host doesn't "just work", at the very least it requires a bunch of tooling to be installed system-wide (because std has C dependencies, and because a linker is needed to build the final artifact). As long as that requirement stands, I think the no_core approach is crucial to be able to let people run and --bless those tests without having to install all sorts of stuff on their system.

We don't actually need std for these tests, though. Can we build a "core-only sysroot" for these targets when needed? That should be faster than building a full sysroot, and it should in principle be possible without any target-specific tools.

That said, looking at one of my own no_core tests (tests/ui/abi/compatibility.rs), I definitely don't want to have to build core 12 times just to be able to run that test on all targets. So I think we want to keep tests like this no_core. I don't think that's actually such a bad hack, TBH -- the only thing that bothers me about it is having to update all those lang items in a bunch of tests. That can be fixed with a minicore.rs file that we include! into these no_core tests, rather than having to copy-paste the same stuff everywhere.

@chrisnc
Copy link
Contributor

chrisnc commented Sep 16, 2024

+1 for a minicore.rs with all the lang_items defined. As I was writing out a half-dozen lang items trying to get the arm fpu feature test to work with #![no_core], this is exactly what I was wishing for, as nothing about them was specific to the test I was trying to write. I think I would have needed about a half dozen more to get the test to work but couldn't figure out how to use some of them and failed at finding examples to follow.

@workingjubilee
Copy link
Member Author

I'd be satisfied-enough with that, as long as we can cut down the silliness!

@jieyouxu

This comment has been minimized.

@jieyouxu
Copy link
Member

I can work on this, I'm just not sure what the exact design should be.

@jieyouxu
Copy link
Member

As @compiler-errors mentioned in #130466 (comment), we can probably have something like //@ aux-build: minicore.rs but a separate compiletest flag since we don't want to have a copy of minicore.rs under a auxiliary folder relative to every test that needs it.

@RalfJung
Copy link
Member

Yeah I am not sure what is the best way to do this -- something like tests/util/minicore.rs that we can include! everywhere would work, but maybe an aux-build is better?

Maybe this should be an MCP, then more t-compiler people can weigh in.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 17, 2024

Yeah, I can file a new MCP ~tomorrow so more compiler people can advice.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 17, 2024

Yeah I am not sure what is the best way to do this -- something like tests/util/minicore.rs that we can include! everywhere would work, but maybe an aux-build is better?

ui tests have the precedent of a C test_helpers.c file, so I suppose assembly and codegen tests could get their own special support file lol. Just don't want any hard-to-debug cursedness due to staging and whatever.

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2024
[PROTOTYPE] Add minicore test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests

**TODO: work in progress prototype implementation, opened early for MCP to reference**

Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375)

This PR introduces a prototype implementation of `minicore` auxiliary test helper. `minicore` contains stub definitions of std/core prelude items intended for consumption by tests that want the typical prelude items like `Copy` or `Result` in cross-compilation scenarios, but don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`).

The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`. The path to this auxiliary is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag. The `minicore` auxiliary is then built, on demand via `//@ use-minicore` compiletest directives, for each test revision for the given target (this distinction is important for when host != target in cross-compilation scenario).

### Implementation steps

- [ ] 1. File an MCP to describe `tests/auxiliary/minicore.rs`, `--minicore-path` compiletest flag, `//@ use-minicore` compiletest directive and the behavior.
- [ ] 2. And some self-tests to sanity check the behavior.
- [ ] 3. Update rustc-dev-guide to describe the new `use-minicore` directive and provide an example, noting that `use-minicore` both requires `no_std` + `no_core` and implies `-C panic=abort` for the test file.

r? `@ghost` (not yet ready for full review, still needs some self-tests and some try-jobs)

(TODO: cc interested people once this passes initial try jobs in cross-compilation scenarios)
cc `@/workingjubilee` `@/RalfJung` `@/nikic` `@/chrisnc` (if this makes sense to you in terms of functionality and UX)

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
@jieyouxu
Copy link
Member

I have a prototype PR at #130693 and opened MCP at rust-lang/compiler-team#786.

@jieyouxu jieyouxu added the E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. label Oct 17, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 31, 2024
Add `minicore` test auxiliary and support `//@ add-core-stubs` directive in ui/assembly/codegen tests

Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375)
MCP: rust-lang/compiler-team#786
Tracking issue: rust-lang#131485

This prototype PR is subject to further changes based on feedback.

### New `minicore` test auxiliary and `//@ add-core-stubs` compiletest directive

This PR introduces a prototype implementation of a `minicore` auxiliary test helper that provides `core` stubs for `#![no_core]` ui/assembly/codegen tests that need to build but not run on both the host platform and the cross-compiled target platform.

Key summary:

- `tests/auxiliary/minicore.rs` contains stub definitions of `core` items intended for consumption by `check-pass`/`build-pass` tests that want the typical prelude items like `Copy` to be stubbed out under `#![no_core]` scenarios, so that the test can be built (not run) for cross-compiled target platforms. Such tests don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`).
- `minicore` is intended for `core` items **only**, not `std`- or `alloc`-exclusive items. If stubs for `alloc` or `std` are wanted, they should be provided by an additional directive and test auxiliary, and not be conflated with `minicore` or `core` stubs. This is because a wider range of tests can benefit from `core`-only stubs.

### Implementation

- The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`.
- The path to `minicore` is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag.
- `minicore` is then built on-demand via the `//@ add-core-stubs` compiletest directive, for each test revision for the given target platform (this distinction is important for when host platform != target platform in cross-compilation scenario).
- `minicore` is then made available to the test as an [extern prelude].

[extern prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude

### Example usage

```rs
// tests/ui/abi/my-abi-test.rs

//@ check-pass
//@ add-core-stubs
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

#![feature(no_core, lang_items)]
#![no_std]
#![no_core]
#![allow(unused, internal_features)]

extern crate minicore;
use minicore::*;

#[lang = "clone"]
pub trait Clone: Sized {      // `Sized` is provided by `minicore`
    fn clone(&self) -> Self;
}
```

### Implementation steps

- [x] 1. Add an initial `minicore` test auxiliary.
- [x] 2. Build `minicore` in bootstrap.
- [x] 3. Setup a `--minicore-path` compiletest cli flag and pass `minicore` build artifact path from bootstrap to compiletest.
- [x] 4. Assert `add-core-stubs` is mutually incompatible with tests that require to be `run`, as the stubs are only good for tests that only need to be built (i.e. no `run-{pass,fail}`).
- [x] 5. Add some self-tests to sanity check the behavior.
- [x] 6. Ensure that `tests/auxiliary/minicore.rs` is input stamped, i.e. modifying `tests/auxiliary/minicore.rs` should invalidate test cache and force the test to be rerun.

### Known limitations

- The current `minicore` is very minimal, because this PR is intended to focus on supporting the test infrastructure first. Further stubs could be added in follow-up PRs and/or on a as-needed basis.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 31, 2024
Add `minicore` test auxiliary and support `//@ add-core-stubs` directive in ui/assembly/codegen tests

Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375)
MCP: rust-lang/compiler-team#786
Tracking issue: rust-lang#131485

This prototype PR is subject to further changes based on feedback.

### New `minicore` test auxiliary and `//@ add-core-stubs` compiletest directive

This PR introduces a prototype implementation of a `minicore` auxiliary test helper that provides `core` stubs for `#![no_core]` ui/assembly/codegen tests that need to build but not run on both the host platform and the cross-compiled target platform.

Key summary:

- `tests/auxiliary/minicore.rs` contains stub definitions of `core` items intended for consumption by `check-pass`/`build-pass` tests that want the typical prelude items like `Copy` to be stubbed out under `#![no_core]` scenarios, so that the test can be built (not run) for cross-compiled target platforms. Such tests don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`).
- `minicore` is intended for `core` items **only**, not `std`- or `alloc`-exclusive items. If stubs for `alloc` or `std` are wanted, they should be provided by an additional directive and test auxiliary, and not be conflated with `minicore` or `core` stubs. This is because a wider range of tests can benefit from `core`-only stubs.

### Implementation

- The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`.
- The path to `minicore` is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag.
- `minicore` is then built on-demand via the `//@ add-core-stubs` compiletest directive, for each test revision for the given target platform (this distinction is important for when host platform != target platform in cross-compilation scenario).
- `minicore` is then made available to the test as an [extern prelude].

[extern prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude

### Example usage

```rs
// tests/ui/abi/my-abi-test.rs

//@ check-pass
//@ add-core-stubs
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

#![feature(no_core, lang_items)]
#![no_std]
#![no_core]
#![allow(unused, internal_features)]

extern crate minicore;
use minicore::*;

#[lang = "clone"]
pub trait Clone: Sized {      // `Sized` is provided by `minicore`
    fn clone(&self) -> Self;
}
```

### Implementation steps

- [x] 1. Add an initial `minicore` test auxiliary.
- [x] 2. Build `minicore` in bootstrap.
- [x] 3. Setup a `--minicore-path` compiletest cli flag and pass `minicore` build artifact path from bootstrap to compiletest.
- [x] 4. Assert `add-core-stubs` is mutually incompatible with tests that require to be `run`, as the stubs are only good for tests that only need to be built (i.e. no `run-{pass,fail}`).
- [x] 5. Add some self-tests to sanity check the behavior.
- [x] 6. Ensure that `tests/auxiliary/minicore.rs` is input stamped, i.e. modifying `tests/auxiliary/minicore.rs` should invalidate test cache and force the test to be rerun.

### Known limitations

- The current `minicore` is very minimal, because this PR is intended to focus on supporting the test infrastructure first. Further stubs could be added in follow-up PRs and/or on a as-needed basis.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 31, 2024
Rollup merge of rust-lang#130693 - jieyouxu:minicore, r=bjorn3

Add `minicore` test auxiliary and support `//@ add-core-stubs` directive in ui/assembly/codegen tests

Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375)
MCP: rust-lang/compiler-team#786
Tracking issue: rust-lang#131485

This prototype PR is subject to further changes based on feedback.

### New `minicore` test auxiliary and `//@ add-core-stubs` compiletest directive

This PR introduces a prototype implementation of a `minicore` auxiliary test helper that provides `core` stubs for `#![no_core]` ui/assembly/codegen tests that need to build but not run on both the host platform and the cross-compiled target platform.

Key summary:

- `tests/auxiliary/minicore.rs` contains stub definitions of `core` items intended for consumption by `check-pass`/`build-pass` tests that want the typical prelude items like `Copy` to be stubbed out under `#![no_core]` scenarios, so that the test can be built (not run) for cross-compiled target platforms. Such tests don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`).
- `minicore` is intended for `core` items **only**, not `std`- or `alloc`-exclusive items. If stubs for `alloc` or `std` are wanted, they should be provided by an additional directive and test auxiliary, and not be conflated with `minicore` or `core` stubs. This is because a wider range of tests can benefit from `core`-only stubs.

### Implementation

- The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`.
- The path to `minicore` is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag.
- `minicore` is then built on-demand via the `//@ add-core-stubs` compiletest directive, for each test revision for the given target platform (this distinction is important for when host platform != target platform in cross-compilation scenario).
- `minicore` is then made available to the test as an [extern prelude].

[extern prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude

### Example usage

```rs
// tests/ui/abi/my-abi-test.rs

//@ check-pass
//@ add-core-stubs
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

#![feature(no_core, lang_items)]
#![no_std]
#![no_core]
#![allow(unused, internal_features)]

extern crate minicore;
use minicore::*;

#[lang = "clone"]
pub trait Clone: Sized {      // `Sized` is provided by `minicore`
    fn clone(&self) -> Self;
}
```

### Implementation steps

- [x] 1. Add an initial `minicore` test auxiliary.
- [x] 2. Build `minicore` in bootstrap.
- [x] 3. Setup a `--minicore-path` compiletest cli flag and pass `minicore` build artifact path from bootstrap to compiletest.
- [x] 4. Assert `add-core-stubs` is mutually incompatible with tests that require to be `run`, as the stubs are only good for tests that only need to be built (i.e. no `run-{pass,fail}`).
- [x] 5. Add some self-tests to sanity check the behavior.
- [x] 6. Ensure that `tests/auxiliary/minicore.rs` is input stamped, i.e. modifying `tests/auxiliary/minicore.rs` should invalidate test cache and force the test to be rerun.

### Known limitations

- The current `minicore` is very minimal, because this PR is intended to focus on supporting the test infrastructure first. Further stubs could be added in follow-up PRs and/or on a as-needed basis.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-cross Area: Cross compilation A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Development

No branches or pull requests

6 participants