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

Add some Rust building infrastructure and example #16274

Merged
merged 11 commits into from
Dec 16, 2021

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Apr 2, 2021

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

  • Rust target definitions to platforms -- basically Rust's version of a platform triple -- in the architectures
  • Build rules to create something almost, bot not quite, entirely unlike modules -- it's called $(APPLICATION_RUST_MODULE).module and added to BASELIBS, but is built by running Cargo and then exploding the built static lib into the folder where LINK expects it to be.
  • Two examples

Testing procedure

  • Ensure you have Rust installed -- given you need nightly versions, typically by installing rustup
    • Alternatively, run with BUILD_IN_DOCKER=1
  • In examples/rust-hello-world (or rust-gcoap), run 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:

  • Did I get the what-goes-where of the Makefiles right?
  • Would it make more sense to build something parallel to ${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.
    • If that's solved, I can probably come up with a nicer way of saying, in the example, that "this has a crate as its main rather than a module", and show less guts in there.

Issues/PRs references

Closes: #9799

[edit: Updated examples list and test description now that the riotdocker images just do the right thing]

@chrysn chrysn requested a review from jia200x as a code owner April 2, 2021 17:18
@chrysn chrysn added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Apr 2, 2021
@chrysn
Copy link
Member Author

chrysn commented Apr 2, 2021

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.

@chrysn chrysn mentioned this pull request Apr 2, 2021
11 tasks
@chrysn
Copy link
Member Author

chrysn commented Apr 3, 2021

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.]

@chrysn
Copy link
Member Author

chrysn commented Apr 3, 2021

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).

@chrysn
Copy link
Member Author

chrysn commented Apr 3, 2021

For this to be runnable and testable in CI, RIOT-OS/riotdocker#141 will need to be resolved.

@chrysn
Copy link
Member Author

chrysn commented Apr 7, 2021

This can now be tested also using a branch of the riotdocker image; testing procedure has been amended.

@benpicco benpicco added the State: waiting for CI update State: The PR requires an Update to CI to be performed first label Apr 7, 2021
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@github-actions github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: examples Area: Example Applications Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: native Platform: This PR/issue effects the native platform Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms labels Aug 25, 2021
@benpicco
Copy link
Contributor

I have heard the CI is updated now - this needs a rebase.

@kaspar030
Copy link
Contributor

I have heard the CI is updated now - this needs a rebase.

no it is not yet!

@kaspar030
Copy link
Contributor

no it is not yet!

CI is running current containers now, and the building is fixed

@chrysn
Copy link
Member Author

chrysn commented Nov 15, 2021

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.

Copy link
Member

@maribu maribu left a 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 :-)

@@ -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
Copy link
Member

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.

Copy link
Member Author

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.

chrysn added a commit to chrysn-pull-requests/RIOT that referenced this pull request Dec 14, 2021
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.
@chrysn
Copy link
Member Author

chrysn commented Dec 14, 2021 via email

... 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.
@chrysn
Copy link
Member Author

chrysn commented Dec 14, 2021

Some changes during rebasing:

  • Cargo.lock are not ignored any more (I had already switched to versioning them as things matured, as they're equivalent to pkg version hashes; having them in .gitignore too is allowed in git but probably not good practice)
  • Fixed the precise check for native being x86_64 Linux (which failed during the CI run of the unsquashed tree because I forgot to export the OS_ARCH used for that check)
  • Ran a cargo update as the newest riot-wrappers contain adjustments to drivers/periph_i2c: let i2c_acquire return void #17275 that was not breaking on C but on Rust bindings.
  • Some non-fixup commits got squashed into what I now consider a manageable patch set of meaningful standalone commits

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).

@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Dec 14, 2021
@chrysn
Copy link
Member Author

chrysn commented Dec 14, 2021

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.

@chrysn chrysn removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 14, 2021
@chrysn
Copy link
Member Author

chrysn commented Dec 14, 2021

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".

@benpicco
Copy link
Contributor

Feel free to squash, still looks good from my side.

Copy link
Member

@maribu maribu left a 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.

@chrysn chrysn added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 16, 2021
@chrysn chrysn merged commit afdabcf into RIOT-OS:master Dec 16, 2021
@chrysn chrysn deleted the rust-application branch December 16, 2021 15:22
@chrysn chrysn mentioned this pull request Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: native Platform: This PR/issue effects the native platform Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rust support for RIOT
5 participants