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

tree wide: accomodate RISC-V toolchain update & boards/common/makefiles/stdio_cdc_acm.dep.mk: fix [backport 2024.01] #20383

Conversation

MrKevinWeiss
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss commented Feb 13, 2024

Backport of #20379, #20380, #20382, #20358, #20324

20378

Contribution description

This fixes compilation issues in tests/pkg/tinyusb_netdev with newer versions of the RISC-V toolchain due to two competing USB stacks being pulled in. With the older toolchain the build system warns:

The following features may conflict: periph_usbdev tinyusb_device

But builds fine (even though surprises at runtime are likely). The newer toolchain takes an issue with the same symbol being linked in more than once (and more than one instance not being weak).

Testing procedure

make BOARD=seeedstudio-gd32 -C tests/pkg/tinyusb_netdev should no longer pull in two USB stacks.

Issues/PRs references

None

20380

Contribution description

This contains fixed needed after updating the RISC-V toolchain in the CI

Testing procedure

Build riotbuild from RIOT-OS/riotdocker#248 locally, then run

env DOCKER_IMAGE=<LOCAL_BUILD_ID> ./dist/tools/insufficient_memory/update_insufficient_memory_board.sh hifive1b

This should fail for each and every build in master, but work for each and every build with this PR

Issues/PRs references

Depends on:

20382, 20358, 20324

Disabling flaky native tests...

This fixes compilation issues in `tests/pkg/tinyusb_netdev` with
newer versions of the RISC-V toolchain due to two competing USB
stacks being pulled in. With the older toolchain the build system
warns:

    The following features may conflict: periph_usbdev tinyusb_device

But builds fine (even though surprises at runtime are likely). The
newer toolchain takes an issue with the same symbol being linked
in more than once (and more than one instance not being `weak`).

(cherry picked from commit 38ab147)
@MrKevinWeiss MrKevinWeiss added Area: boards Area: Board ports Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 13, 2024
@MrKevinWeiss MrKevinWeiss requested a review from maribu February 13, 2024 11:54
@riot-ci
Copy link

riot-ci commented Feb 13, 2024

Murdock results

✔️ PASSED

c186da6 tests/sys/timer_overhead: disable test on native in CI

Success Failures Total Runtime
8613 0 8613 09m:31s

Artifacts

@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 13, 2024
@maribu
Copy link
Member

maribu commented Feb 13, 2024

I fear we will have to bundle this backport with the backport of #20380 into one backport PR :/

The CI now uses `riscv-none-elf` over the previous (and technically
incorrect) `riscv-none-embed`. In addition, we no longer probe for
host toolchains with the incorrect target triple, as the source
providing it has long declared the toolchain with the incorrect
triple as obsolete.
This drops support for the legacy riscv-none-embed target triple. That
value has been incorrect since the beginning and the toolchain that
used that has been long declared obsolete and is fairly outdated.

With our CI updating the toolchain, we no longer need to check for
that.
The legacy `riscv-none-embed` target triple is incorrect and the
toolchain using it has long been obsolete. With our CI no longer
using the obsolete toolchain, there is no need to handle that one
anymore.
With the new RISC-V toolchain, some binaries grew larger, most smaller.
@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Area: examples Area: Example Applications labels Feb 20, 2024
@maribu maribu changed the title boards/common/makefiles/stdio_cdc_acm.dep.mk: fix [backport 2024.01] tree wide: accomodate RISC-V toolchain update & boards/common/makefiles/stdio_cdc_acm.dep.mk: fix [backport 2024.01] Feb 20, 2024
@maribu
Copy link
Member

maribu commented Feb 20, 2024

This should now contain all the fixes needed to get past the CI

@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 20, 2024
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 20, 2024
@maribu
Copy link
Member

maribu commented Feb 20, 2024

Maybe disabling all the flaky tests on native should also be backported. But for now, let's roll the dice again.

@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 20, 2024
@maribu
Copy link
Member

maribu commented Feb 21, 2024

Just rerolling the dice until all of the flaky native and native64 tests succeed for both GNU and LLVM toolchain on the first run is unbearable.

We either have to also backport all those "disabling flake test in CI" PRs of lately, or revert #20269 in the release branch to treat the native test failures as build failures and just re-run them up to three times.

@MrKevinWeiss
Copy link
Contributor Author

Do you have a list of the tests to disable?

@MrKevinWeiss
Copy link
Contributor Author

nvm I think I got it.

@maribu maribu added this pull request to the merge queue Feb 22, 2024
Merged via the queue into RIOT-OS:2024.01-branch with commit e354f87 Feb 22, 2024
25 checks passed
@maribu
Copy link
Member

maribu commented Feb 22, 2024

Yeah! Merging to the release branch works again 🎉

@MrKevinWeiss MrKevinWeiss deleted the backport/2024.01/boards/common/stdio_cdc_acm branch February 27, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: examples Area: Example Applications 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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants