-
Notifications
You must be signed in to change notification settings - Fork 115
Port wolfSSH Zephyr test sample to Zephyr 4.4.0 #1065
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,34 +13,23 @@ jobs: | |
| strategy: | ||
| matrix: | ||
| config: | ||
| # Each leg runs in the Zephyr Project CI image whose bundled SDK | ||
| # matches the Zephyr release under test: v0.26.4 ships SDK 0.16.1 | ||
| # (for Zephyr 3.4.0), v0.29.0 ships SDK 1.0.0 (for Zephyr 4.4.0). | ||
| - zephyr-ref: v3.4.0 | ||
| zephyr-sdk: 0.16.1 | ||
| docker-image: v0.26.4 | ||
| - zephyr-ref: v4.4.0 | ||
| docker-image: v0.29.0 | ||
| runs-on: ubuntu-22.04 | ||
| container: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 [High] Unsupported job container entrypoint option breaks Zephyr CI · API Contract Violations The PR introduces a GitHub Actions job container and sets Fix: Remove the unsupported option. If the image needs a neutral entrypoint, publish/use an image with that entrypoint baked in instead of overriding it through job-container options. container: |
||
| image: ghcr.io/zephyrproject-rtos/ci:${{ matrix.config.docker-image }} | ||
| options: '--entrypoint /bin/bash' | ||
| env: | ||
| # The Zephyr SDK is preinstalled under /opt/toolchains in the CI image. | ||
| ZEPHYR_SDK_INSTALL_DIR: /opt/toolchains | ||
| # This should be a safe limit for the tests to run. | ||
| timeout-minutes: 20 | ||
| steps: | ||
| - name: Install dependencies | ||
| run: | | ||
| # Don't prompt for anything | ||
| export DEBIAN_FRONTEND=noninteractive | ||
| sudo apt-get update | ||
| # most of the ci-base zephyr docker image packages | ||
| sudo apt-get install -y zip bridge-utils uml-utilities \ | ||
| git cmake ninja-build gperf ccache dfu-util device-tree-compiler wget \ | ||
| python3-dev python3-pip python3-setuptools python3-tk python3-wheel xz-utils file \ | ||
| make gcc gcc-multilib g++-multilib libsdl2-dev libmagic1 \ | ||
| autoconf automake bison build-essential ca-certificates cargo ccache chrpath cmake \ | ||
| cpio device-tree-compiler dfu-util diffstat dos2unix doxygen file flex g++ gawk gcc \ | ||
| gcovr git git-core gnupg gperf gtk-sharp2 help2man iproute2 lcov libcairo2-dev \ | ||
| libglib2.0-dev libgtk2.0-0 liblocale-gettext-perl libncurses5-dev libpcap-dev \ | ||
| libpopt0 libsdl1.2-dev libsdl2-dev libssl-dev libtool libtool-bin locales make \ | ||
| net-tools ninja-build openssh-client parallel pkg-config python3-dev python3-pip \ | ||
| python3-ply python3-setuptools python-is-python3 qemu rsync socat srecord sudo \ | ||
| texinfo unzip wget ovmf xz-utils | ||
|
|
||
| - name: Install west | ||
| run: sudo pip install west | ||
|
|
||
| - name: Init west workspace | ||
| run: west init --mr ${{ matrix.config.zephyr-ref }} zephyr | ||
|
|
||
|
|
@@ -61,35 +50,26 @@ jobs: | |
| working-directory: zephyr | ||
| run: west zephyr-export | ||
|
|
||
| - name: Install pip dependencies | ||
| working-directory: zephyr | ||
| run: sudo pip install -r zephyr/scripts/requirements.txt | ||
|
|
||
| - name: Install zephyr SDK | ||
| run: | | ||
| wget -q https://github.com/zephyrproject-rtos/sdk-ng/releases/download/v${{ matrix.config.zephyr-sdk }}/zephyr-sdk-${{ matrix.config.zephyr-sdk }}_linux-x86_64_minimal.tar.xz | ||
| tar xf zephyr-sdk-${{ matrix.config.zephyr-sdk }}_linux-x86_64_minimal.tar.xz | ||
| cd zephyr-sdk-${{ matrix.config.zephyr-sdk }} | ||
| ./setup.sh -h -c -t x86_64-zephyr-elf | ||
|
|
||
| - name: Run wolfssh tests | ||
| id: wolfssh-test | ||
| working-directory: zephyr | ||
| run: | | ||
| ./zephyr/scripts/twister --testsuite-root modules/lib/wolfssh --test zephyr/samples/tests/sample.lib.wolfssh_tests -vvv | ||
| rm -rf zephyr/twister-out | ||
| ./zephyr/scripts/twister --testsuite-root modules/lib/wolfssh --test zephyr/samples/tests/sample.lib.wolfssh_nofs_tests -vvv | ||
| rm -rf zephyr/twister-out | ||
| # Run every scenario discovered under the wolfssh testsuite root. | ||
| # Selecting by scenario name with --test is avoided on purpose: the | ||
| # scenario id is path-prefixed on older twister (v3.4.0) but bare on | ||
| # newer twister (v4.x), so a fixed --test value works on only one of | ||
| # the matrix legs. The wolfssh tree defines a single sample.yaml, so | ||
| # scoping by --testsuite-root runs exactly our scenarios on both. | ||
| ./zephyr/scripts/twister --testsuite-root modules/lib/wolfssh -vvv | ||
|
|
||
| - name: Zip failure logs | ||
| - name: Archive failure logs | ||
| if: ${{ failure() && steps.wolfssh-test.outcome == 'failure' }} | ||
| run: | | ||
| zip -9 -r logs.zip zephyr/twister-out | ||
| run: tar caf logs.tar.xz zephyr/twister-out | ||
|
|
||
| - name: Upload failure logs | ||
| if: ${{ failure() && steps.wolfssh-test.outcome == 'failure' }} | ||
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: zephyr-client-test-logs | ||
| path: logs.zip | ||
| name: zephyr-client-test-logs-${{ matrix.config.zephyr-ref }} | ||
| path: logs.tar.xz | ||
| retention-days: 5 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,41 @@ | ||
| cmake_minimum_required(VERSION 3.20.0) | ||
|
|
||
| # Determine the Zephyr kernel version before find_package(Zephyr): the | ||
| # KERNEL_VERSION_* variables it would set are not available yet, and the | ||
| # RAM disk and POSIX config below depend on APIs that changed across releases. | ||
| # Parse $ZEPHYR_BASE/VERSION directly instead. | ||
| file(STRINGS $ENV{ZEPHYR_BASE}/VERSION zephyr_version_lines) | ||
| foreach(line ${zephyr_version_lines}) | ||
| if("${line}" MATCHES "VERSION_MAJOR[ \t]*=[ \t]*([0-9]+)") | ||
| set(ZEPHYR_VER_MAJOR ${CMAKE_MATCH_1}) | ||
| elseif("${line}" MATCHES "VERSION_MINOR[ \t]*=[ \t]*([0-9]+)") | ||
| set(ZEPHYR_VER_MINOR ${CMAKE_MATCH_1}) | ||
| endif() | ||
| endforeach() | ||
| if(NOT DEFINED ZEPHYR_VER_MAJOR OR NOT DEFINED ZEPHYR_VER_MINOR) | ||
| message(FATAL_ERROR | ||
| "Could not determine Zephyr version from $ENV{ZEPHYR_BASE}/VERSION") | ||
| endif() | ||
| set(ZEPHYR_VER "${ZEPHYR_VER_MAJOR}.${ZEPHYR_VER_MINOR}") | ||
|
|
||
| # RAM disk: configured via the devicetree "zephyr,ram-disk" node on Zephyr | ||
| # >= 3.5, via Kconfig before that. Only the filesystem-backed scenarios need | ||
| # it; the nofs build (CONF_FILE="prj_nofs.conf") has no disk. | ||
| if(NOT "${CONF_FILE}" MATCHES "nofs") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 [Low] RAM-disk selection keys off a substring match of the CONF_FILE filename · convention
Fix: Optional: drive the RAM disk decision off an explicit toggle (e.g. a |
||
| if(ZEPHYR_VER VERSION_LESS 3.5) | ||
| list(APPEND EXTRA_CONF_FILE ${CMAKE_CURRENT_SOURCE_DIR}/ramdisk_legacy.conf) | ||
| else() | ||
| list(APPEND EXTRA_DTC_OVERLAY_FILE ${CMAKE_CURRENT_SOURCE_DIR}/ramdisk.overlay) | ||
| endif() | ||
| endif() | ||
|
|
||
| # POSIX socket names / pthreads / clock: CONFIG_POSIX_API (in prj*.conf) | ||
| # covers these on Zephyr >= 4.1; older releases need the symbols the 4.1 | ||
| # POSIX reorg removed. | ||
| if(ZEPHYR_VER VERSION_LESS 4.1) | ||
| list(APPEND EXTRA_CONF_FILE ${CMAKE_CURRENT_SOURCE_DIR}/posix_legacy.conf) | ||
| endif() | ||
|
|
||
| find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) | ||
| project(wolfssl_tests) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # Pre-4.1 POSIX layer. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 [Low] New config/overlay files open with multi-line description blocks contrary to repo comment convention · style The new Fix: Optionally collapse the multi-line header blocks in the new .conf/.overlay files (and CMakeLists comments) to a single WHY line each; move the extended rationale to the PR description. Purely stylistic. |
||
| # | ||
| # The Zephyr 4.1 POSIX reorg removed CONFIG_PTHREAD_IPC / CONFIG_POSIX_CLOCK / | ||
| # CONFIG_NET_SOCKETS_POSIX_NAMES and folded their behaviour into | ||
| # CONFIG_POSIX_API (set in prj.conf). On older releases CONFIG_POSIX_API does | ||
| # not provide the global socket()/bind() names, so restore the explicit | ||
| # symbols and turn POSIX_API back off to reproduce the original pre-port | ||
| # configuration. | ||
| # | ||
| # Selected automatically by CMakeLists.txt on Zephyr < 4.1. | ||
| CONFIG_POSIX_API=n | ||
| CONFIG_PTHREAD_IPC=y | ||
| CONFIG_POSIX_CLOCK=y | ||
| CONFIG_NET_SOCKETS_POSIX_NAMES=y | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| /* | ||
| * RAM disk node for the wolfSSH SFTP tests. | ||
| * | ||
| * Zephyr 3.5 moved the RAM disk driver from Kconfig (CONFIG_DISK_RAM_VOLUME_*) | ||
| * to devicetree. The disk-name must match CONFIG_WOLFSSH_SFTP_DEFAULT_DIR | ||
| * ("/RAM:") in prj.conf. 128 sectors * 512 bytes = 64 KiB, matching the old | ||
| * CONFIG_DISK_RAM_VOLUME_SIZE=64. | ||
| */ | ||
| / { | ||
| ramdisk0 { | ||
| compatible = "zephyr,ram-disk"; | ||
| disk-name = "RAM"; | ||
| sector-size = <512>; | ||
| sector-count = <128>; | ||
| }; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Pre-3.5 RAM disk geometry. | ||
| # | ||
| # Before Zephyr 3.5 the RAM disk size came from Kconfig rather than the | ||
| # devicetree "zephyr,ram-disk" node in ramdisk.overlay. 64 KiB matches the | ||
| # overlay (128 sectors * 512 bytes); the volume name defaults to "RAM", | ||
| # matching CONFIG_WOLFSSH_SFTP_DEFAULT_DIR="/RAM:" in prj.conf. | ||
| # | ||
| # Selected automatically by CMakeLists.txt on Zephyr < 3.5. | ||
| CONFIG_DISK_RAM_VOLUME_SIZE=64 |
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.
🔵 [Low] Version matrix lacks fail-fast: false, so one Zephyr leg failing cancels the other · convention
The stated goal of the PR is to validate the sample on BOTH Zephyr 3.4.0 and 4.4.0. GitHub Actions matrix strategy defaults to
fail-fast: true, which cancels all in-progress matrix legs as soon as one fails. If the 3.4.0 leg (the harder-to-keep-working legacy port) fails to build, the 4.4.0 leg will be cancelled before it finishes, hiding whether 4.4.0 still passes (and vice-versa). For a version-compatibility matrix you almost always want both results independently. Verified: thestrategy:block contains onlymatrix:with nofail-fast:key.Fix: Add
fail-fast: falseunderstrategy:so each Zephyr version reports its own pass/fail independently.