-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add some Rust building infrastructure and example #16274
Add some Rust building infrastructure and example #16274
Conversation
Oh, one addendum to testing: GCC is not really well supported as a toolchain; if things break, try with TOOLCHAIN=llvm. With #16129 merged, GCC support should increase because @maribu did a much better job in finding the subtle differences than I did in the workarounds currently in place in the riot-sys crate. |
Note that using this branch currently breaks most of the riot-examples as they use the older "linking in a .a archive" approach and not the newer "unar everything and let find find the .o files" one. The shell example already uses the new style and works; I expect to port over the others while this PR matures. [edit: They examples build infrastructure now contains the necessary overrides to work with this branch, and the makefiles there can be easily dropped.] |
There's a follow-up branch at https://github.com/chrysn-pull-requests/RIOT/tree/rust-lib in which a module is introduced that builds any number of (predefined but pseudomodule-enabled) crates in a single build and pulls them in. By the power of XFA, thus pure-Rust modules for RIOT can be implemented without any C code as long as they hook into XFA extension points (or, probably, interrupts). |
For this to be runnable and testable in CI, RIOT-OS/riotdocker#141 will need to be resolved. |
This can now be tested also using a branch of the riotdocker image; testing procedure has been amended. |
8b00fb3
to
3b1172f
Compare
94cd007
to
281f109
Compare
I have heard the CI is updated now - this needs a rebase. |
no it is not yet! |
CI is running current containers now, and the building is fixed |
Squashed and rebased after having become a bit stale. Just running the #16109 Makefile.ci generation locally (twice now, as before I didn't have all Rust targets installed), and then I'll push the fancy "CI: Ready to build" button. |
5294fe2
to
8b90a3c
Compare
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.
looks good to me :-)
cpu/native/Makefile.include
Outdated
@@ -10,3 +10,6 @@ ifneq (,$(filter periph_can,$(USEMODULE))) | |||
endif | |||
|
|||
TOOLCHAINS_SUPPORTED = gnu llvm afl | |||
|
|||
# Platform triple as used by Rust | |||
RUST_TARGET = i686-unknown-linux-gnu |
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.
This will fall on your feet when running make BOARD=native
e.g. on a Raspberry Pi.
For me, gcc -print-multiarch -m32
prints i386-linux-gnu
in case of riot/riotbuild
. On 64-bit only (no multilib support) distros it prints an empty string, which is pretty sensible as well. So it could actually be used to provide a decent warning when running make BOARD=native
on systems 64-bit only systems as side effect.
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 added a check in a3b7cbc to Kconfig, the .include and the .features that Rust support is only advertised for x86_64 Linux. Like with the ARM board targets, I'd keep it to what I can test in the first run, and adding more platforms like 32 bit native ARM or Darwin support (is that or is that not supported any more these days?) can be done when there are actual users who'd test it.
For RISC-V and Cortex-M-not-3, triples are known and have worked in some configuration, but do not work at the moment and stay disabled until the reference platforms (native, M3) have been established well.
Probably yes. But CI doesn't test it, I can't test it, and it's easy to
add if needed and and when it can be tested.
(Currently still fixing a few things during rebasing, just in case you'd
be starting another review round).
|
... and to dissect the static libraries into invidial .o files to link them the same way we link C.
Cargo.lock files pin the versions of all transitive dependencies, and are left that way by Cargo unless requested manually (`cargo update`) or necessitated by a change in dependencies (if the manually maintained Cargo.toml says `>0.5.2` and .lock says `0.5.1`, the latter is reset). They are fixed (as opposed to .gitignoring them and letting each user autogenerate them) to ensure that third party changes do not break RIOT CI builds; for updates, the changes from a `cargo update` can still be committed and then run through CI checks. (This is analogous to how pkg versions are pinned). They are set to binary because their diffs are usually not practical to read.
Some changes during rebasing:
Please review, it's good to go from my side (especially, the REMOVEME commit that only makes it build the Rust examples has been removed, so it's good to ACK-and-merge if reviewers agree). |
9ac1e84
to
54342cd
Compare
The Murdock run showed two errors -- one appears to be just a random timeout (tests/pkg_edhoc_c/esp32-wroom-32), the other is highly weird because it's using an old version of a Rust library that was already updated and pinned (but maybe too softly) to the newer version. This might have been a caching issue; I'll look at it but unless this persists I don't think it's a reason for delay. Holding off the murdock rerun that I recommend to do due to the first error until there is either an ACK or any comment that requires further changes. |
Correction: Nothing weird about the version thing, I just only fixed the examples and forgot about the test. Still disabled CI rebuilding (as the fix is very likely correct, and can just as well be tested after being smushed). I'd smush and turn on Murdock again once anyone says "ACK, smush". |
Feel free to squash, still looks good from my side. |
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.
Let me throw in an ACK. The changes look good to me and the author did rigorous testing.
Unlike the hello-world example (that is largely identical), this gets run during CI.
fa6818d
to
1838cdf
Compare
Contribution description
While writing Rust applications has been possible for some time, it has required a few Makefile rules which I'm by now happy enough with to start a first PR. Having this all in-tree is not essential for operation, but makes it a bit easier to store, for example, the target definitions with the architectures rather than in a big if chain that sometimes peeks into fields and sometimes into boards.
This adds
$(APPLICATION_RUST_MODULE).module
and added toBASELIBS
, but is built by running Cargo and then exploding the built static lib into the folder where LINK expects it to be.Testing procedure
BUILD_IN_DOCKER=1
make all flash term
, optionally with a cortex M0-4 board.It is tested so far with native and some cortex-m0, 3 and 4 boards (mostly EFM32, EFR32 and nRF52840). riscv support is disabled right now, I don't want to do that in the first rush until it falls into place trivially.
Questions:
${MODULE}.module
that is${CRATE}.crate
that gets its own place in the big find of LINK? I think it'd make sense because it wouldn't involve overriding a target any more, and moreover make it easier to later extend this to having potentially multiple built crates (although I think it's preferable to use one), but would make a larger diff.Issues/PRs references
Closes: #9799
[edit: Updated examples list and test description now that the riotdocker images just do the right thing]