-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
migrate thumb-none-qemu
to rmake
#128639
migrate thumb-none-qemu
to rmake
#128639
Conversation
This PR modifies cc @jieyouxu |
//! $ ./x.py clean | ||
//! $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi tests/run-make | ||
//! | ||
//! For supported targets, see `example/.cargo/config` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that file notes that some targets are not quite accurate because qemu is too old. Maybe that has changed? we can try that separately though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I have no clue what the up-to-date cargo config in this case is for thumb targets and I don't have the environment to set it, I'm fine with keeping the config as-is for now. If anyone knowledgeable about thumb can update that config, I would be happy to accept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now updated the thumbv6m-none-eabi
target to use the -cpu cortex-m0
. We'll see if that causes issues, it shouldn't if the qemu version is remotely recent. Otherwise I think that config file looks fine.
Ah hah, I see this is going to require some archaeology 😄 I'll take a look at this tmrw |
yeah I've also added this to the agenda of the embedded-wg rust-embedded/wg#783, they might have thoughts on how big a deal this actually is. |
I did some of the detective work: that file is 5 years old, so it is quite plausible that more things now work. With my qemu,
anyway, we can update that separately from switching to rmake, and given that it's apparently not caused issues over the last 5 years it's probably not a high priority. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks very reasonable. I'll run a couple of try jobs to check after not using the extra directory indirection.
@@ -1,20 +0,0 @@ | |||
#!/bin/sh | |||
set -exuo pipefail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-remark:
set -exuo pipefail
-e: immediately exit on non-zero
-x: all exec cmd printed to terminal
-u: strict no-undef variables modulo `$*` and `$@`
-o pipefail: prevent errors in pipelines from being masked
//! $ ./x.py clean | ||
//! $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi tests/run-make | ||
//! | ||
//! For supported targets, see `example/.cargo/config` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I have no clue what the up-to-date cargo config in this case is for thumb targets and I don't have the environment to set it, I'm fine with keeping the config as-is for now. If anyone knowledgeable about thumb can update that config, I would be happy to accept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark: I wonder if this needs LD_LIBRARY_PATH
set.
@bors try (just to see) |
@rustbot author |
…<try> migrate `thumb-none-qemu` to rmake tracking issue: rust-lang#121876 I think this one is actually simpler than rust-lang#128636, we invoke `cargo run` with the right target and see if the expected result appears. r? `@jieyouxu` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various
@rustbot blocked (pending T-compiler discussion) |
☀️ Try build successful - checks-actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rustbot ready
//! $ ./x.py clean | ||
//! $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi tests/run-make | ||
//! | ||
//! For supported targets, see `example/.cargo/config` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now updated the thumbv6m-none-eabi
target to use the -cpu cortex-m0
. We'll see if that causes issues, it shouldn't if the qemu version is remotely recent. Otherwise I think that config file looks fine.
Thanks! |
…jieyouxu migrate `thumb-none-qemu` to rmake tracking issue: rust-lang#121876 I think this one is actually simpler than rust-lang#128636, we invoke `cargo run` with the right target and see if the expected result appears. r? `@jieyouxu` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various
💔 Test failed - checks-actions |
looks like bors still has some issues?
|
A different issue lol: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/CI.20failures.20from.20networking |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (2048386): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 0.1%, secondary 0.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 760.03s -> 761.822s (0.24%) |
tracking issue: #121876
I think this one is actually simpler than #128636, we invoke
cargo run
with the right target and see if the expected result appears.r? @jieyouxu
try-job: armhf-gnu
try-job: dist-various-1
try-job: test-various