-
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
Rust: Update dependencies #19495
Rust: Update dependencies #19495
Conversation
This pulls in several fixes from these repositories: * riot-sys: * Add `random` module to `riot-headers` (RIOT-OS/rust-riot-sys#26) * README: fix 2 typos (RIOT-OS/rust-riot-sys#31) * bindgen: Use 0.64 (RIOT-OS/rust-riot-sys#30) * wolfSSL support added (RIOT-OS/rust-riot-sys#28) * extern-types: Generate replacement types dynamically (RIOT-OS/rust-riot-sys#27) * Extern types: Add netq_t, make them large (RIOT-OS/rust-riot-sys#25) * doc: Suppress warnings about things C2Rust does not uphold (RIOT-OS/rust-riot-sys#23) * export macro_DAC_LINE (RIOT-OS/rust-riot-sys#22) * Add BINDGEN_OUTPUT_FILE export (RIOT-OS/rust-riot-sys#21) * riot-wrappers: * DAC: Add wrapper around RIOTs DAC-interface (RIOT-OS/rust-riot-wrappers#36) * saul: Compatibly rename G* variants (RIOT-OS/rust-riot-wrappers#50) * tests: Add test for auto-init when auto-init debug is active (RIOT-OS/rust-riot-wrappers#48) * gcoap: Provide link encoder (RIOT-OS/rust-riot-wrappers#47) * Drop SUIT support in riot-wrappers (RIOT-OS/rust-riot-wrappers#44) * Add an auto init module (RIOT-OS/rust-riot-wrappers#45) * gcoap: Allow registration without scope for 'static listeners (RIOT-OS/rust-riot-wrappers#43) Through direct dependency changes (the bindgen update), the number of transitive dependencies could be reduced.
This helps remove duplicate code after previous updates.
I think technically this isn't a bugfix. But since it fixes a regression / incompatibility with tools that already hit a lot of Linux distros and likely will hit more over time, IMO we should backport this. |
Oh, tests/rust_minimal for
It seems that Maybe the |
And more issues:
Maybe c2rust could just add |
(Or maybe the issue is me not being up to date in the first place, let's see if there is a new version of c2rust.) Update: Yes, indeed. I'm at 0.16, upstream is at 0.17. Let's see what happens after updating. |
The launchpad build for tests/rust_minimal WFM, let's hope it's all just the c2rust version. (And by the way, can we please get rid of the inclusion of POSIX headers in the VFS API? Our VFS has no POSIX semantics, and reusing their types causes more harm than good. But that's a different issue...) |
It doesn't look updating c2rust is that easy. Just for context: We only need c2rust for Lots of breakage
|
Still fails with more recent c2rust:
|
See also #19498 that I've opened to properly answer the get-rid-of-it point. |
As I can't reproduce that I'd like to run some other checks:
|
It builds fine with Alpine 3.17 |
... and master doesn't build on edge? |
OK, then I think it's justified to bors merge this, and keep looking for the edge cases (sorry) elsewhere. The combined instructions in #19495 (comment) should help get that started. |
When invoking
That indicates that the C++ Code is not compatible with LLVM 16 even with c2rust master. Maybe I should try building c2rust against llvm-15 instead. |
Testbed for edge:
Then execute the patch from #19495 (comment) and continue with
... and that |
(How) did you get around the statvfs issue? You mentioned it, but then moved on -- and reproducing things on edge, that's where I am now. |
I just moved the struct to |
bors merge |
19495: Rust: Update dependencies r=maribu a=chrysn ### Contribution description This updates both the RIOT-specific and generic dependencies of Rust examples and modules. ### Testing procedure * Green CI should do ### Issues/PRs references Copying from one of the commits with some sed: * riot-sys: * RIOT-OS/rust-riot-sys#26 * RIOT-OS/rust-riot-sys#31 * RIOT-OS/rust-riot-sys#30 * RIOT-OS/rust-riot-sys#28 * RIOT-OS/rust-riot-sys#27 * RIOT-OS/rust-riot-sys#25 * RIOT-OS/rust-riot-sys#23 * RIOT-OS/rust-riot-sys#22 * RIOT-OS/rust-riot-sys#21 * riot-wrappers: * RIOT-OS/rust-riot-wrappers#36 * RIOT-OS/rust-riot-wrappers#50 * RIOT-OS/rust-riot-wrappers#48 * RIOT-OS/rust-riot-wrappers#47 * RIOT-OS/rust-riot-wrappers#44 * RIOT-OS/rust-riot-wrappers#45 * RIOT-OS/rust-riot-wrappers#43 ### How to do similar PRs Updating the RIOT-related dependencies (which here also updated bindgen because the dependency of riot-sys changed): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml --package riot-sys --package riot-wrappers; done ``` Updating everything (should never really be needed, b/c if something has a concrete dependency change it'd say so, but dependencies generally get better over time): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml; done ``` Creating the commit message: ``` $ git log --first-parent --format='%s (%b)' 9c29faf55d4c14d2d7f55f8df5059c52af4e5317..e4973a6ee88427f702dac41b3dce4fd6b6b9689e | sed 's/Merges: //' | sed 's/^/ * /' ``` git shortlog unfortunately doesn't show the merges the way I prefer linking them. The versions in the command line can be taken from `git diff --text` and looking for the riot-sys or riot-wrappers line, respectively. Co-authored-by: chrysn <[email protected]>
Build failed: |
Whoops ... something was not as compatible as I expected it to be, and the one driver we're having caught it in this build failure. Checking, but I expect the fix to be a fast PR to the riot-wrappers repo, and adding another round of |
The segfault is btw. not Alpine specific, see immunant/c2rust#897 for the fix. (Update: And with the segfault out of the way, building with Alpine Edge works fine again.) |
This fixes a regression by merging RIOT-OS/rust-riot-wrappers#55 (the "G" unit has been deprecated, but that deprecation was done wrong in RIOT-OS/rust-riot-wrappers#50).
The G unit mistakes have been fixed, therefore bors merge Thanks @maribu for finding and fixing the segfault; with this and some statvfs workarounds (eventually to be #19501) the test described in #19495 (comment) for alpine now passes. |
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
bors was lying. bors merge |
Build succeeded: |
Interesting, the issue with posix header being included by |
Backport provided in #19505 |
19505: Rust: Update dependencies [backport 2023.04] r=maribu a=maribu # Backport of #19495 ### Contribution description This updates both the RIOT-specific and generic dependencies of Rust examples and modules. It also follows a deprecation from the G unit renaming originally done in #19292. ### Testing procedure * Green CI should do ### Issues/PRs references Copying from one of the commits with some sed: * riot-sys: * RIOT-OS/rust-riot-sys#26 * RIOT-OS/rust-riot-sys#31 * RIOT-OS/rust-riot-sys#30 * RIOT-OS/rust-riot-sys#28 * RIOT-OS/rust-riot-sys#27 * RIOT-OS/rust-riot-sys#25 * RIOT-OS/rust-riot-sys#23 * RIOT-OS/rust-riot-sys#22 * RIOT-OS/rust-riot-sys#21 * riot-wrappers: * RIOT-OS/rust-riot-wrappers#36 * RIOT-OS/rust-riot-wrappers#50 * RIOT-OS/rust-riot-wrappers#48 * RIOT-OS/rust-riot-wrappers#47 * RIOT-OS/rust-riot-wrappers#44 * RIOT-OS/rust-riot-wrappers#45 * RIOT-OS/rust-riot-wrappers#43 * (later, when the mistake became apparent) RIOT-OS/rust-riot-wrappers#55 ### How to do similar PRs Updating the RIOT-related dependencies (which here also updated bindgen because the dependency of riot-sys changed): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml --package riot-sys --package riot-wrappers; done ``` Updating everything (should never really be needed, b/c if something has a concrete dependency change it'd say so, but dependencies generally get better over time): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml; done ``` Creating the commit message: ``` $ git log --first-parent --format='%s (%b)' 9c29faf55d4c14d2d7f55f8df5059c52af4e5317..e4973a6ee88427f702dac41b3dce4fd6b6b9689e | sed 's/Merges: //' | sed 's/^/ * /' ``` git shortlog unfortunately doesn't show the merges the way I prefer linking them. The versions in the command line can be taken from `git diff --text` and looking for the riot-sys or riot-wrappers line, respectively. Co-authored-by: chrysn <[email protected]>
19505: Rust: Update dependencies [backport 2023.04] r=maribu a=maribu # Backport of #19495 ### Contribution description This updates both the RIOT-specific and generic dependencies of Rust examples and modules. It also follows a deprecation from the G unit renaming originally done in #19292. ### Testing procedure * Green CI should do ### Issues/PRs references Copying from one of the commits with some sed: * riot-sys: * RIOT-OS/rust-riot-sys#26 * RIOT-OS/rust-riot-sys#31 * RIOT-OS/rust-riot-sys#30 * RIOT-OS/rust-riot-sys#28 * RIOT-OS/rust-riot-sys#27 * RIOT-OS/rust-riot-sys#25 * RIOT-OS/rust-riot-sys#23 * RIOT-OS/rust-riot-sys#22 * RIOT-OS/rust-riot-sys#21 * riot-wrappers: * RIOT-OS/rust-riot-wrappers#36 * RIOT-OS/rust-riot-wrappers#50 * RIOT-OS/rust-riot-wrappers#48 * RIOT-OS/rust-riot-wrappers#47 * RIOT-OS/rust-riot-wrappers#44 * RIOT-OS/rust-riot-wrappers#45 * RIOT-OS/rust-riot-wrappers#43 * (later, when the mistake became apparent) RIOT-OS/rust-riot-wrappers#55 ### How to do similar PRs Updating the RIOT-related dependencies (which here also updated bindgen because the dependency of riot-sys changed): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml --package riot-sys --package riot-wrappers; done ``` Updating everything (should never really be needed, b/c if something has a concrete dependency change it'd say so, but dependencies generally get better over time): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml; done ``` Creating the commit message: ``` $ git log --first-parent --format='%s (%b)' 9c29faf55d4c14d2d7f55f8df5059c52af4e5317..e4973a6ee88427f702dac41b3dce4fd6b6b9689e | sed 's/Merges: //' | sed 's/^/ * /' ``` git shortlog unfortunately doesn't show the merges the way I prefer linking them. The versions in the command line can be taken from `git diff --text` and looking for the riot-sys or riot-wrappers line, respectively. 19509: cpu/cc26xx_cc13xx: Fix bogus array-bound warning [backport 2023.04] r=maribu a=maribu # Backport of #19504 ### Contribution description GCC 12 create a bogus array out of bounds warning as it assumes that because there is special handling for `uart == 0` and `uart == 1`, `uart` can indeed be `1`. There is an `assert(uart < UART_NUMOF)` above that would blow up prior to any out of bounds access. In any case, optimizing out the special handling of `uart == 1` for when `UART_NUMOF == 1` likely improves the generated code and fixes the warning. /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:88:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds] 88 | ctx[uart].rx_cb = rx_cb; | ~~~^~~~~~ /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx' 52 | static uart_isr_ctx_t ctx[UART_NUMOF]; | ^~~ /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:89:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds] 89 | ctx[uart].arg = arg; | ~~~^~~~~~ /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx' 52 | static uart_isr_ctx_t ctx[UART_NUMOF]; | ^~~ ### Testing procedure The actual change is a pretty obvious one-liner, so that code review and a green CI should be sufficient. If not, running any UART example app without regression should do. ### Issues/PRs references None 19510: tools/openocd: Fix handling of OPENOCD_CMD_RESET_HALT [backport 2023.04] r=maribu a=maribu # Backport of #19506 ### Contribution description The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue. ### Testing procedure Flashing of e.g. the `cc2650-launchpad` with upstream OpenOCD should work again. ### Issues/PRs references The change was added to #19050 after testing the PR and before merging. I'm not sure if the fix never worked because of this, or if behavior of `target-export-variables` or GNU Make changed. Co-authored-by: chrysn <[email protected]> Co-authored-by: Marian Buschsieweke <[email protected]>
Contribution description
This updates both the RIOT-specific and generic dependencies of Rust examples and modules.
It also follows a deprecation from the G unit renaming originally done in #19292.
Testing procedure
Issues/PRs references
Copying from one of the commits with some sed:
random
module toriot-headers
rust-riot-sys#26How to do similar PRs
Updating the RIOT-related dependencies (which here also updated bindgen because the dependency of riot-sys changed):
Updating everything (should never really be needed, b/c if something has a concrete dependency change it'd say so, but dependencies generally get better over time):
Creating the commit message:
git shortlog unfortunately doesn't show the merges the way I prefer linking them. The versions in the command line can be taken from
git diff --text
and looking for the riot-sys or riot-wrappers line, respectively.