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

./x.py test for thumb*-none-* targets fails with "cargo must succeed" #52163

Closed
sekineh opened this issue Jul 8, 2018 · 17 comments
Closed

./x.py test for thumb*-none-* targets fails with "cargo must succeed" #52163

sekineh opened this issue Jul 8, 2018 · 17 comments

Comments

@sekineh
Copy link
Contributor

sekineh commented Jul 8, 2018

Updates (2018-07-18)

  • We'll enable ./x.py test src/test/run-make for no_std targets.

Environment

  • Ubuntu 16.04 (x86_64)
  • ARM cross compiler is installed by sudo apt install gcc-arm-none-eabi libnewlib-arm-none-eabi

Problem

For cross target, we can't run the following tests:

# Both fails
./x.py test --target thumbv7m-none-eabi src/test/ui
./x.py test --target thumbv7m-none-eabi src/test/run-make

Typical error we got is cargo must succeed:

Caused by:
  process didn't exit successfully: `/home/sekineh/rustme/build/bootstrap/debug/rustc --crate-name term libterm/lib.rs --error-format json --crate-type dylib --crate-type rlib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=b839a5796ea44350 -C extra-filename=-b839a5796ea44350 --out-dir /home/sekineh/rustme/build/x86_64-unknown-linux-gnu/stage2-test/thumbv7m-none-eabi/release/deps --target thumbv7m-none-eabi -L dependency=/home/sekineh/rustme/build/x86_64-unknown-linux-gnu/stage2-test/thumbv7m-none-eabi/release/deps -L dependency=/home/sekineh/rustme/build/x86_64-unknown-linux-gnu/stage2-test/release/deps` (exit code: 101)
command did not execute successfully: "/home/sekineh/rustme/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "thumbv7m-none-eabi" "-j" "8" "--release" "--manifest-path" "/home/sekineh/rustme/src/libtest/Cargo.toml" "--message-format" "json"
expected success, got: exit code: 101
thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1091:9

The full log is posted on gist:

Notes

  • I don't believe ui tests and run-make tests require a simulator to be executed. The whole tests are run on host only (host: Linux x86_64 in this case).

Background

We need to land CI for thumb* targets before Rust 2018 is released.
The fact the test harness cannot be run is the major blocker for the CI addition.

CC @jamesmunns @nerdyvaishali @kennytm

@sekineh sekineh changed the title ./x.py test for thumb*-none-* targets fails ./x.py test for thumb*-none-* targets fails with "cargo must succeed" Jul 8, 2018
@jamesmunns
Copy link
Member

jamesmunns commented Jul 8, 2018

Here's a little more background for reference:

What we are trying to do/Context

We would like to run tests for the thumbv7m-none-eabi target from a x86_64-unknown-linux-gnu host (on CI). At the moment, we are trying to get a "base case" of getting existing compiler-only tests to run in this configuration. This presents a problem, as the thumbv7m-none-eabi target is a bare metal/no_std target, and currently the CI fails while attempting to build the libtest crate as part of the "Building stage2 std artifacts (x86_64-unknown-linux-gnu -> thumbv7m-none-eabi)" test process. The testing process fails at this section of the bootstrap process in particular.

We are also working on adding a Dockerfile to support this CI testing.

In the future, we would like to add additional tests in this style - where we would like the CI to build a binary only for our target where we can perform static analysis (e.g. check the binary output for symbols, size, etc), or dynamic analysis (running the resulting binary in QEMU).

We have tried to model our testing after the existing CI tests based on the wasm32-unknown target, which is also a no_std target, however we are unable to understand why thumbv7m-none-eabi fails and wasm32-unknown testing fails.

What we need help with

At the moment, we are not sure whether this process should be failing here. Based on wasm32-unknown tests passing the stage where our tests fail, there seems to be some existing ability to run tests solely on the host system. Is the failure we are seeing a bug (and something needs to be added/fixed in configuration or the bootstrap testing), or is this expected behavior (and larger changes may be necessary to support this configuration)?

Support for this target in dist builds was recently added, however we are also unsure how to map these changes to a test build.

@kennytm
Copy link
Member

kennytm commented Jul 10, 2018

The actual error from the gist is failure to build stage2-libtest for E0463 "can't find crate for std" 😕

Building stage2 test artifacts (x86_64-unknown-linux-gnu -> thumbv7m-none-eabi)
   Compiling term v0.0.0 (file:///home/sekineh/rustme/src/libterm)
   Compiling getopts v0.2.17
warning: dropping unsupported crate type `dylib` for target `thumbv7m-none-eabi`

error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7m-none-eabi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7m-none-eabi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `getopts`.

@sekineh
Copy link
Contributor Author

sekineh commented Jul 10, 2018

Yes, but the ‘std’ can be successfully compiled via ‘./x.py dist’ for that target.

@jamesmunns
Copy link
Member

jamesmunns commented Jul 15, 2018

I wanted to expand on my previous comment, and perhaps provide a little more context/detail. Thanks to @kennytm for taking a look already :)

The issue we are seeing is that the test system x.py test ... is attempting to build the libtest crate for the target system, because that is the usual behavior of running x.py test (you want to run unit tests for/on your target).

There are two problems with that:

  1. That's not what we want. For our (initial) testing, we aren't running unit tests, only looking at the binary compiled for the target. Because of this, libtest is not needed at all. This is important because...
  2. It is not possible to build libtest for a thumb target. libtest has a dependency on std, and there is no std crate that can be downloaded or built. In general, it doesn't make sense to build unit tests for a microcontroller (or at least, it doesn't make sense for what we are trying to do at the moment) - see the bottom of this comment for more detail on std for thumb* targets.

Issue 2 describes the symptom we are seeing, where the build process is failing because there is no std crate for the thumb target. By trying to run x.py test for a thumb target, the build system assumes we want to run unit tests ON that target, which requires libtest is built for the target, which is not possible at the moment. However, that is not what we want.

So, in my opinion, there are two possible ways forward:

  1. We could teach the build system that not all cases of x.py test means "build unit tests for the target, and run them natively on that target (or in a QEMU instance)". This would mean introducing some kind of configurable option that we could disable for thumb tests. wasm targets may already have this implemented, but I am not sure.
  2. We could introduce a new verb, something like x.py host-test ..., which does what we would like it to do (build a compiler for the host, and inspect binaries built for the target, do not expect to run the produced binary).

I still don't have a good understanding of why this is an issue for us (e.g. thumb* targets, and not wasm targets, but I haven't had time to dig deeper into what is going on yet.

Another note about @sekineh's comment that dist works for this target: This is a bit of a misnomer. The std dist package that is produced by x.py for thumb* targets doesn't include std, but is instead only a packaged, distributable version of core and some other miscellaneous "under the hood" things. The nomenclature was used to fit within the current build and distribution procedure when @japaric introduced pre-built core for embedded targets (see here).

@jamesmunns
Copy link
Member

@alexcrichton - From looking at the git blame, it looks like you implemented most of the wasm tests in Rust's CI. If you have a minute, would you mind reading through this thread and chiming in? If it wasn't you who did it, would you mind pointing us in the correct direction on who we can talk to about the wasm CI tests? It would be greatly appreciated :)

@japaric
Copy link
Member

japaric commented Jul 16, 2018

wasm32 is not a no_std target; it supports std -- I don't know about the test crate. (I believe some parts of std just call unimplemented! on wasm so std supports wasm in the "at least compiles" sense).

@jamesmunns
Copy link
Member

Thanks @japaric, I'm not sure where I got that it was a no_std target. Did it used to be, and I'm just out of the loop?

I'll update my other comments at some point today to take out the details about wasm, though I think the rest of the comments should still be fine.

Perhaps it would just make sense for us to call a shell script that does something like this instead of trying to use x.py test:

# Build a compiler for us to use, just for the host
x.py build

git clone <some embedded example> && cd $_
<path to built cargo> build --target thumbv7m-none-eabi ...

# some procedure to validate binary produced
verify_bin <path to binary>

exit $?

@sekineh
Copy link
Contributor Author

sekineh commented Jul 16, 2018

I'm not sure where I got that it was a no_std target

I'm fairly confident that that misconception came from my edit of ci-plan.md. The target wasm is not strictly a no_std, just no_std'ish in some sense.
https://github.com/jamesmunns/irr-embedded-2018/blame/master/ci-plan.md

So the conclusion is that x.py test does not support no_std targets currently and it won't support them in near future?

@kennytm
Copy link
Member

kennytm commented Jul 16, 2018

Would it work if we remove this line:

builder.ensure(compile::Test { compiler, target });

? Or change to target: compiler.host?

@sekineh
Copy link
Contributor Author

sekineh commented Jul 16, 2018

The short answer is 'Yes'.

If I commented out the line 969, run-make runs and succeeds, and ui fails. I think the behavior is understandable.
If I change the line to like below, both tests are at least run. (results are: run-make: success, ui: failure)

        // builder.ensure(compile::Test { compiler, target });
        builder.ensure(compile::Test { compiler, target: compiler.host });

Huge hint. A possible hack is to add if to that line, ...... for now.

@alexcrichton
Copy link
Member

Ah yes, as @japaric mentioned the wasm target does include both libtest and libstd, so it's not quite the same. In general I don't think there's any way to get ./x.py test meaningfully working for no_std targets, 99% of our test suite assumes it links to std and "does something" (either run a binary or use items defined in std).

I'd recommend that if we wanted to add CI for this target that we'd write one-off tests in src/test/run-make. That'll allow you to define a fully custom script which only works for that one particular target. We may need to tweak rustbuild to not compile libtest for some targets (or otherwise make libtest a noop for the target), but that'll allow flavorful tests as well.

@sekineh
Copy link
Contributor Author

sekineh commented Jul 16, 2018

Thanks, @japaric , @kennytm and @alexcrichton for the help!
@jamesmunns , our initial task is basically OK if run-make runs. Do you agree with me?

For the record, I ended up with this snippet:

        if !target.contains("-none-") {
            builder.ensure(compile::Test { compiler, target });
        }

And it runs run-make tests.

running 3 tests
iii
test result: ok. 0 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out

	finished in 0.052
Build completed successfully in 0:00:03

I could add this line into our (embedded CI) pull request.

@kennytm
Copy link
Member

kennytm commented Jul 16, 2018

If we only run run-make tests, directly if builder.no_std(target) that line out seems better than changing it to target: compiler.host.

(cc @Mark-Simulacrum)

@sekineh
Copy link
Contributor Author

sekineh commented Jul 16, 2018

@kennytm my current version does not use target: compiler.host and follow the just-comment(if, actually)-out strategy. Thanks for the clarification!

snippet v2:

        if builder.no_std(target) == Some(false) {
            builder.ensure(compile::Test { compiler, target });
        }

sekineh added a commit to sekineh/rust that referenced this issue Jul 17, 2018
@sekineh
Copy link
Contributor Author

sekineh commented Jul 17, 2018

I included the snippet v2 into my folk of rust.

Later, I'll open the pull request including

@sekineh
Copy link
Contributor Author

sekineh commented Jul 21, 2018

snippet v2 changed the behavior in wasm32 target and failed the CI
#52465 (comment)

printf-debug suggested builder.no_std(target) is None on wasm32 target.
I don't know if it is intended.... (the third state of the boolean)

I'd fallback to the snippet v1 which is supposed to be more conservative change.
Alternatively, snippet v3 would work.

snippet v1:

        if !target.contains("-none-") { // Skip only if it's a "-none-" target
            builder.ensure(compile::Test { compiler, target });
        }

snippet v3:

        if builder.no_std(target) != Some(true) { // Skip only if it's a genuine no_std target
            builder.ensure(compile::Test { compiler, target });
        }

@sekineh
Copy link
Contributor Author

sekineh commented Jul 21, 2018

I pushed the snippet v3 to the PR because I think it's more reviewer friendly.
Now running docker wasm32-unknown locally to confirm.

kennytm added a commit to kennytm/rust that referenced this issue Jul 21, 2018
Add CI test harness for `thumb*` targets. [IRR-2018-embedded]

This pull request will do the following (rather trivial) changes:
- Fix rust-lang#52163. In other words, we enabled `./x.py test src/test/run-make` for `no_std` targets.
- Modify `dist-various-1` Dockerfile.
  - CI now performs `run-make` test run on the targets below:
    - `thumbv6m-none-eabi`
    - `thumbv7m-none-eabi`
    - `thumbv7em-none-eabi`
    - `thumbv7em-none-eabihf`.
- ~~Add `thumb-none` Dockerfile.~~
  - ~~Initially, `thumbv7m-none-eabi`, `thumbv7em-none-eabi` and `thumbv7em-none-eabihf` are included as the tested target. `thumbv6m-none-eabi` is disabled for now because LLVM support is not certain.~~
- ~~Add `thumb-none` to .travis.yml~~

Note:
- `run-make` tests are not implemented yet. This PR is test harness only.

The amount of change is very small, but I'd like to open the pull request while the change is trivial.
Because I'm not very used to pull request process, I want to make a small progress first.  This PR will be a foundation for later additions.
kennytm added a commit to kennytm/rust that referenced this issue Jul 22, 2018
Add CI test harness for `thumb*` targets. [IRR-2018-embedded]

This pull request will do the following (rather trivial) changes:
- Fix rust-lang#52163. In other words, we enabled `./x.py test src/test/run-make` for `no_std` targets.
- Modify `dist-various-1` Dockerfile.
  - CI now performs `run-make` test run on the targets below:
    - `thumbv6m-none-eabi`
    - `thumbv7m-none-eabi`
    - `thumbv7em-none-eabi`
    - `thumbv7em-none-eabihf`.
- ~~Add `thumb-none` Dockerfile.~~
  - ~~Initially, `thumbv7m-none-eabi`, `thumbv7em-none-eabi` and `thumbv7em-none-eabihf` are included as the tested target. `thumbv6m-none-eabi` is disabled for now because LLVM support is not certain.~~
- ~~Add `thumb-none` to .travis.yml~~

Note:
- `run-make` tests are not implemented yet. This PR is test harness only.

The amount of change is very small, but I'd like to open the pull request while the change is trivial.
Because I'm not very used to pull request process, I want to make a small progress first.  This PR will be a foundation for later additions.

CC @kennytm @jamesmunns @nerdyvaishali
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

No branches or pull requests

5 participants