migrate `thumb-none-cortex-m` to rmake #128636

folkertdev · 2024-08-04T11:07:39Z

tracking issue: #121876

I'll leave some comments/questions inline

try-job: armhf-gnu
try-job: dist-various-1
try-job: test-various

rustbot · 2024-08-04T11:07:48Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

tests/run-make/thumb-none-cortex-m/rmake.rs

folkertdev · 2024-08-04T11:10:35Z

tests/run-make/thumb-none-cortex-m/rmake.rs

+    let target_dir = PathBuf::from("target");
+    let target = env_var("TARGET");
+
+    let manifest_path = PathBuf::from("Cargo.toml");
+
+    let path = env_var("PATH");
+    let rustc = env_var("RUSTC");
+    let bootstrap_cargo = env_var("BOOTSTRAP_CARGO");
+    let mut cmd = cmd(bootstrap_cargo);
+    cmd.args(&[
+        "build",
+        "--manifest-path",
+        manifest_path.to_str().unwrap(),
+        "-Zbuild-std=core",
+        "--target",
+        &target,
+    ])
+    .env("PATH", path)
+    .env("RUSTC", rustc)


should this be extracted into some cargo helper? it occurs a couple of times and is kind of inscrutable with all the env variables.

I would actually prefer if we keep the current bootstrap cargo incantations because there's another test compiler_builtins which does something similar but also subtly different. I want to investigate them together in a follow-up PR, so can you please leave me a FIXME pointing to #128734?

folkertdev · 2024-08-04T11:11:44Z

tests/run-make/thumb-none-cortex-m/rmake.rs

+    // Visual Studio 2022 requires that the LIB env var be set so it can
+    // find the Windows SDK.
+    .env("LIB", std::env::var("LIB").unwrap_or_default());


is this required for these embedded targets? I guess it was in the makefile for a reason?

We can always try to run try-jobs and see if they pass without this env var. But I don't know if the embedded targets need the LIB env var.

well the windows SDK just does not seem relevant. let's try removing it.

folkertdev · 2024-08-04T11:12:36Z

tests/run-make/thumb-none-cortex-m/Makefile

-	# HACK(eddyb) sets `RUSTC_BOOTSTRAP=1` so Cargo can accept nightly features.
-	# These come from the top-level Rust workspace, that this crate is not a
-	# member of, but Cargo tries to load the workspace `Cargo.toml` anyway.


is this comment still relevant?

setting that environment variable is not relevant on my system anyway. It's still in the code right now to be a faithful translation, but maybe this can be dropped now?

I believe RUSTC_BOOTSTRAP=1 is not needed if the crate and its transitive dependencies do not depend on nightly features. We can always try removing it and see if it fails.

jieyouxu · 2024-08-04T11:26:22Z

Oof. Thank you for taking this one on, this is one of the tests I spoke with @Oneirical about being very ~~cursed~~ interesting. Expect some delay from me, because I probably will also be asking around for help about what this is trying to test and in particular how it achieves that 😅

folkertdev · 2024-08-04T11:43:38Z

yeah I figured that this one was a bit of an outlier. I and several of my colleagues are quite familiar with this target platform, so we're happy to help out where that expertise is required.

…<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

jieyouxu · 2024-08-06T10:31:11Z

Update: let me get back to you after discussing this test (thumb-none-cortex-m) and thumb-none-qemu with the compiler people regarding needing internet connection (due to cargo external deps) in order to run the test...
@rustbot blocked (pending T-compiler discussion)

jieyouxu · 2024-08-06T12:26:06Z

I don't want to block this PR on the external dep challenges, they can be done separately (cc #128733).
@rustbot review

jieyouxu

Thank you for tackling this, I have some suggestions, but this LGTM in general.

tests/run-make/thumb-none-cortex-m/rmake.rs

jieyouxu · 2024-08-06T12:28:45Z

tests/run-make/thumb-none-cortex-m/rmake.rs

+//! How to run this
+//! $ ./x.py clean
+//! $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi tests/run-make
+//!
+//! Supported targets:
+//! - thumbv6m-none-eabi (Bare Cortex-M0, M0+, M1)
+//! - thumbv7em-none-eabi (Bare Cortex-M4, M7)
+//! - thumbv7em-none-eabihf (Bare Cortex-M4F, M7F, FPU, hardfloat)
+//! - thumbv7m-none-eabi (Bare Cortex-M3)


Question: do we know if these instructions and support targets are up-to-date?

it is correct, but today probably should also include

thumbv8m.base-none-eabi

thumbv8m.main-none-eabi

thumbv8m.main-none-eabihf

I'm not sure whether CI even runs those though?

based on https://hackmd.io/@thejpster/H1iTNJSRT the comment can be extended by

//! - thumbv8m.base-none-eabi (Bare Cortex-M23) //! - thumbv8m.main-none-eabi (Bare Cortex-M33, M55, M85) //! - thumbv8m.main-none-eabihf (Bare Cortex-M33, M55, M85, FPU, hardfloat)

this appears to work fine locally, but I think is only useful if CI actually runs these targets

it is correct, but today probably should also include

* thumbv8m.base-none-eabi * thumbv8m.main-none-eabi * thumbv8m.main-none-eabihf

I'm not sure whether CI even runs those though?

cc @rust-lang/infra dear infra people, do we know if any of our CI jobs exercise these targets?

tests/run-make/thumb-none-cortex-m/rmake.rs

jieyouxu · 2024-08-06T12:34:00Z

In the meantime, let's see what our CI try jobs say.
@bors try

…, r=<try> migrate `thumb-none-cortex-m` to rmake tracking issue: rust-lang#121876 I'll leave some comments/questions inline r? `@jieyouxu` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various

bors · 2024-08-06T12:35:13Z

⌛ Trying commit f71a627 with merge e493763...

bors · 2024-08-06T13:46:33Z

☀️ Try build successful - checks-actions
Build commit: e493763 (e49376372b0e6d6c3b34a248d3049fd33027dae9)

folkertdev

@rustbot ready

I can squash the commits if that's better, but know that github's interface does not always work well with force pushes.

we might want to try this for a windows host (due to removing the LIB variable)

folkertdev · 2024-08-06T19:52:30Z

tests/run-make/thumb-none-cortex-m/rmake.rs

+//! How to run this
+//! $ ./x.py clean
+//! $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi tests/run-make
+//!
+//! Supported targets:
+//! - thumbv6m-none-eabi (Bare Cortex-M0, M0+, M1)
+//! - thumbv7em-none-eabi (Bare Cortex-M4, M7)
+//! - thumbv7em-none-eabihf (Bare Cortex-M4F, M7F, FPU, hardfloat)
+//! - thumbv7m-none-eabi (Bare Cortex-M3)


it is correct, but today probably should also include

thumbv8m.base-none-eabi

thumbv8m.main-none-eabi

thumbv8m.main-none-eabihf

I'm not sure whether CI even runs those though?

tests/run-make/thumb-none-cortex-m/rmake.rs

folkertdev · 2024-08-06T19:55:05Z

tests/run-make/thumb-none-cortex-m/rmake.rs

+    // Visual Studio 2022 requires that the LIB env var be set so it can
+    // find the Windows SDK.
+    .env("LIB", std::env::var("LIB").unwrap_or_default());


well the windows SDK just does not seem relevant. let's try removing it.

folkertdev · 2024-08-06T20:05:31Z

tests/run-make/thumb-none-cortex-m/rmake.rs

+//! How to run this
+//! $ ./x.py clean
+//! $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi tests/run-make
+//!
+//! Supported targets:
+//! - thumbv6m-none-eabi (Bare Cortex-M0, M0+, M1)
+//! - thumbv7em-none-eabi (Bare Cortex-M4, M7)
+//! - thumbv7em-none-eabihf (Bare Cortex-M4F, M7F, FPU, hardfloat)
+//! - thumbv7m-none-eabi (Bare Cortex-M3)


based on https://hackmd.io/@thejpster/H1iTNJSRT the comment can be extended by

//! - thumbv8m.base-none-eabi (Bare Cortex-M23) //! - thumbv8m.main-none-eabi (Bare Cortex-M33, M55, M85) //! - thumbv8m.main-none-eabihf (Bare Cortex-M33, M55, M85, FPU, hardfloat)

this appears to work fine locally, but I think is only useful if CI actually runs these targets

…<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

jieyouxu

Thanks, this looks good to me. I'll run try-jobs to check.

jieyouxu · 2024-08-07T02:02:35Z

@bors try

…, r=<try> migrate `thumb-none-cortex-m` to rmake tracking issue: rust-lang#121876 I'll leave some comments/questions inline r? `@jieyouxu` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various

bors · 2024-08-07T02:03:46Z

⌛ Trying commit 201ca3f with merge 5f8274c...

bors · 2024-08-07T03:11:07Z

☀️ Try build successful - checks-actions
Build commit: 5f8274c (5f8274c1a3b38f75938f5069ff39e543b83acfe8)

jieyouxu · 2024-08-07T04:38:55Z

@bors r+ rollup=iffy (thumb tests)

bors · 2024-08-07T04:38:58Z

📌 Commit 201ca3f has been approved by jieyouxu

It is now in the queue for this repository.

…-m, r=jieyouxu migrate `thumb-none-cortex-m` to rmake tracking issue: rust-lang#121876 I'll leave some comments/questions inline r? `@jieyouxu` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various

…-m, r=jieyouxu migrate `thumb-none-cortex-m` to rmake tracking issue: rust-lang#121876 I'll leave some comments/questions inline r? ``@jieyouxu`` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#128384 (Add tests to ensure MTE tags are preserved across FFI boundaries) - rust-lang#128407 (Migrate `min-global-align` and `no-alloc-shim` `run-make` tests to rmake) - rust-lang#128584 (Add a set of tests for LLVM 19) - rust-lang#128636 (migrate `thumb-none-cortex-m` to rmake) - rust-lang#128696 (Migrate `staticlib-dylib-linkage` `run-make` test to rmake) Failed merges: - rust-lang#128639 (migrate `thumb-none-qemu` to rmake) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#128363 (Migrate `pdb-buildinfo-cl-cmd` and `pgo-indirect-call-promotion` `run-make` tests to rmake) - rust-lang#128384 (Add tests to ensure MTE tags are preserved across FFI boundaries) - rust-lang#128636 (migrate `thumb-none-cortex-m` to rmake) - rust-lang#128696 (Migrate `staticlib-dylib-linkage` `run-make` test to rmake) Failed merges: - rust-lang#128407 (Migrate `min-global-align` and `no-alloc-shim` `run-make` tests to rmake) - rust-lang#128639 (migrate `thumb-none-qemu` to rmake) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#128636 - folkertdev:rmake-thumb-none-cortex-m, r=jieyouxu migrate `thumb-none-cortex-m` to rmake tracking issue: rust-lang#121876 I'll leave some comments/questions inline r? ```@jieyouxu``` try-job: armhf-gnu try-job: dist-various-1 try-job: test-various

…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

…-file, r=jieyouxu Remove unused script from run-make tests Its last usage was removed in rust-lang#128636. Tracking issue: rust-lang#121876 r? jieyouxu

Rollup merge of rust-lang#129013 - Kobzol:remove-unused-git-clone-sha-file, r=jieyouxu Remove unused script from run-make tests Its last usage was removed in rust-lang#128636. Tracking issue: rust-lang#121876 r? jieyouxu

migrate thumb-none-cortex-m to rmake

f71a627

rustbot assigned jieyouxu Aug 4, 2024

folkertdev commented Aug 4, 2024

View reviewed changes

jieyouxu mentioned this pull request Aug 4, 2024

Tracking Issue for porting run-make tests to use Rust #121876

Open

folkertdev mentioned this pull request Aug 4, 2024

migrate thumb-none-qemu to rmake #128639

Merged

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 6, 2024

jieyouxu approved these changes Aug 6, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2024

changes after review

201ca3f

folkertdev commented Aug 6, 2024

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 6, 2024

jieyouxu approved these changes Aug 7, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2024

tgross35 mentioned this pull request Aug 7, 2024

Rollup of 3 pull requests #128774

Closed

matthiaskrgr mentioned this pull request Aug 7, 2024

Rollup of 5 pull requests #128785

Closed

matthiaskrgr mentioned this pull request Aug 7, 2024

Rollup of 4 pull requests #128793

Merged

bors merged commit 38dc3b2 into rust-lang:master Aug 8, 2024
7 checks passed

rustbot added this to the 1.82.0 milestone Aug 8, 2024

Kobzol mentioned this pull request Aug 12, 2024

Remove unused script from run-make tests #129013

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate `thumb-none-cortex-m` to rmake #128636

migrate `thumb-none-cortex-m` to rmake #128636

folkertdev commented Aug 4, 2024 •

edited by jieyouxu

Loading

rustbot commented Aug 4, 2024

folkertdev Aug 4, 2024

jieyouxu Aug 6, 2024

folkertdev Aug 4, 2024

jieyouxu Aug 6, 2024

folkertdev Aug 6, 2024

folkertdev Aug 4, 2024

folkertdev Aug 4, 2024

jieyouxu Aug 6, 2024 •

edited

Loading

jieyouxu commented Aug 4, 2024

folkertdev commented Aug 4, 2024

jieyouxu commented Aug 6, 2024 •

edited

Loading

jieyouxu commented Aug 6, 2024

jieyouxu left a comment

jieyouxu Aug 6, 2024

folkertdev Aug 6, 2024

folkertdev Aug 6, 2024 •

edited

Loading

jieyouxu Aug 7, 2024

jieyouxu commented Aug 6, 2024

bors commented Aug 6, 2024

bors commented Aug 6, 2024

folkertdev left a comment

folkertdev Aug 6, 2024

folkertdev Aug 6, 2024

folkertdev Aug 6, 2024 •

edited

Loading

jieyouxu left a comment

jieyouxu commented Aug 7, 2024

bors commented Aug 7, 2024

bors commented Aug 7, 2024

jieyouxu commented Aug 7, 2024

bors commented Aug 7, 2024

migrate thumb-none-cortex-m to rmake #128636

migrate thumb-none-cortex-m to rmake #128636

Conversation

folkertdev commented Aug 4, 2024 • edited by jieyouxu Loading

rustbot commented Aug 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

jieyouxu commented Aug 4, 2024

folkertdev commented Aug 4, 2024

jieyouxu commented Aug 6, 2024 • edited Loading

jieyouxu commented Aug 6, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

folkertdev Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu commented Aug 6, 2024

bors commented Aug 6, 2024

bors commented Aug 6, 2024

folkertdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

folkertdev Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Aug 7, 2024

bors commented Aug 7, 2024

bors commented Aug 7, 2024

jieyouxu commented Aug 7, 2024

bors commented Aug 7, 2024

migrate `thumb-none-cortex-m` to rmake #128636

migrate `thumb-none-cortex-m` to rmake #128636

folkertdev commented Aug 4, 2024 •

edited by jieyouxu

Loading

jieyouxu Aug 6, 2024 •

edited

Loading

jieyouxu commented Aug 6, 2024 •

edited

Loading

folkertdev Aug 6, 2024 •

edited

Loading

folkertdev Aug 6, 2024 •

edited

Loading