generalize rcar_s4 bundle#534
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe changes update the Buildroot branch and configuration for the Renesas S4 flasher build, switch from the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant build_flasher.sh
participant Buildroot
participant replace_kernel.sh
participant udev
User->>build_flasher.sh: Run build script
build_flasher.sh->>Buildroot: Clone repo (branch 2025.05), apply rootfs_only_defconfig
build_flasher.sh->>replace_kernel.sh: Replace kernel (version 5.14.0-594.543.el9iv.aarch64)
Buildroot->>udev: Install udev rule (rename tsn0 to eth0)
udev->>System: On tsn0 detected, rename to eth0
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/replace_kernel.sh (1)
3-3: Kernel version hard-coded again – make future bumps cheaperGood to see the bump to
5.14.0-594.543. Re-touching the script for every minor bump is easy to forget; consider accepting aKVERenv-var override or parsing the latest tag from Koji instead, e.g.:-KVER="5.14.0-594.543.el9iv.aarch64" +KVER="${KVER:-5.14.0-594.5 6880 43.el9iv.aarch64}"That keeps the default while allowing
KVER=… ./replace_kernel.shon demand.packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/overlay/etc/udev/rules.d/10-rename-tsn0.rules (1)
1-2: Rule works, but guard against name collision
NAME="eth0"will fail silently if aneth0already exists (e.g. USB NIC).
To keep the rule harmless on boards that already exposeeth0, prepend, NAME!="eth0":-SUBSYSTEM=="net", ACTION=="add", KERNEL=="tsn0", NAME="eth0" +SUBSYSTEM=="net", ACTION=="add", KERNEL=="tsn0", NAME!="eth0", NAME="eth0"Also double-check that
udevis built withnet_ifnames=0or predictable-names disabled; otherwise the kernel might emitenP…and this rule won’t match.packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh (1)
5-10: Minor robustness & speed enhancements to the build scriptConsider the following quick wins:
-#!/bin/bash +#!/usr/bin/env bash +set -euo pipefail # abort on any failure -git clone --depth 1 --branch 2025.05 https://github.com/buildroot/buildroot /buildroot +git clone --depth 1 --branch 2025.05 https://github.com/buildroot/buildroot /buildroot -( cd /buildroot; make rootfs_only_defconfig && make ) +( cd /buildroot || exit 1; \ + make rootfs_only_defconfig && \ + make -j"$(nproc)" )•
set -euo pipefailcatches silent errors (shellcheck SC2164 flagged thecd).
•-j$(nproc)reduces build time dramatically.
• Using/usr/bin/env bashimproves portability across distros.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/overlay/etc/udev/rules.d/10-rename-tsn0.rules(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/replace_kernel.sh(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/rootfs_only_defconfig(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh
[warning] 10-10: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/rootfs_only_defconfig (1)
7-7: DHCP interface rename may race with udev rule – verify boot timing
BR2_SYSTEM_DHCP="eth0"assumes the rename fromtsn0 → eth0has already completed when udhcpc starts. If udhcpc is launched before udev processes10-rename-tsn0.rules, DHCP will fail and networking never comes up.Please test a cold-boot scenario on the S4 board (and any other boards that keep the original
eth0) to confirm:
ifconfig -ashowseth0only (no lingeringtsn0).- DHCP lease is obtained automatically.
If the race is observable, consider:
- Adding
udevsettle/udevadm settlein your init scripts before networking.- Leaving
BR2_SYSTEM_DHCPempty and starting udhcpc from an init-script that waits for the renamed interface.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh (1)
5-5: Parameterise the Buildroot branch instead of hard-coding itHard-coding
2025.05makes forward/back-porting harder. Export the branch name as an env variable (or accept it as an argument) so CI can bump it centrally.-BRANCH=2025.05 -git clone --depth 1 --branch $BRANCH https://github.com/buildroot/buildroot /buildroot +BRANCH="${BUILDROOT_BRANCH:-2025.05}" +git clone --depth 1 --branch "${BRANCH}" https://github.com/buildroot/buildroot /buildroot
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/overlay/etc/udev/rules.d/10-rename-tsn0.rules(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/replace_kernel.sh(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/rootfs_only_defconfig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/overlay/etc/udev/rules.d/10-rename-tsn0.rules
- packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/replace_kernel.sh
- packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/rootfs_only_defconfig
🧰 Additional context used
🪛 Shellcheck (0.10.0)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh
[warning] 10-10: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: e2e
- GitHub Check: build
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
| cp renesas_s4_defconfig /buildroot/configs/ | ||
| ( cd /buildroot; make renesas_s4_defconfig && make ) | ||
| cp rootfs_only_defconfig /buildroot/configs/ | ||
| ( cd /buildroot; make rootfs_only_defconfig && make ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address ShellCheck SC2164: check cd success
If /buildroot does not exist the subshell keeps going and hides the error. Guard the cd, or use pushd/popd, and split the two make invocations for better debuggability.
-( cd /buildroot; make rootfs_only_defconfig && make )
+(
+ cd /buildroot || { echo "ERROR: /buildroot not found"; exit 1; }
+ make rootfs_only_defconfig
+ make # consider: make -j"$(nproc)" for speed
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ( cd /buildroot; make rootfs_only_defconfig && make ) | |
| ( | |
| cd /buildroot || { echo "ERROR: /buildroot not found"; exit 1; } | |
| make rootfs_only_defconfig | |
| make # consider: make -j"$(nproc)" for speed | |
| ) |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 10-10: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh at
line 10, the subshell changes directory to /buildroot without checking if the cd
command succeeds, which can hide errors if /buildroot does not exist. Modify the
script to verify the cd command's success before running the make commands, or
replace cd with pushd/popd for safer directory changes. Also, split the combined
make commands into separate lines to improve error visibility and debugging.
| git clone --depth 1 --branch 2025.05 https://github.com/buildroot/buildroot /buildroot | ||
|
|
||
| ./add_kernel.sh | ||
| ./replace_kernel.sh |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the script and fail fast around replace_kernel.sh
./replace_kernel.sh is executed with no safety net—if it exits non-zero the build continues and produces a broken image.
Add set -euo pipefail to the top of the script (just after the she-bang) and ensure replace_kernel.sh is executable before calling it.
#!/bin/bash
+set -euo pipefail
+
+if [[ ! -x ./replace_kernel.sh ]]; then
+ echo "replace_kernel.sh is missing or not executable"; exit 1
+fi
./replace_kernel.sh📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ./replace_kernel.sh | |
| #!/bin/bash | |
| set -euo pipefail | |
| if [[ ! -x ./replace_kernel.sh ]]; then | |
| echo "replace_kernel.sh is missing or not executable" | |
| exit 1 | |
| fi | |
| ./replace_kernel.sh |
🤖 Prompt for AI Agents
In packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh at
line 7, the script runs ./replace_kernel.sh without error handling, allowing
failures to pass silently. Add "set -euo pipefail" immediately after the shebang
line to enable strict error checking and fail fast on errors. Also, before
executing replace_kernel.sh, add a check to ensure it is executable, and if not,
set the executable permission to prevent permission errors.
it still supports Renesas S4 only, but it now renames the default interface to eth0 so that the same bundle can be reused with other kernel-automotive supported boards. Also updated buildroot and kernel-automotive to latest.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh (3)
1-2: Addset -euo pipefailfor safer scripting (still missing).
The script still runs without strict-mode; silent failures will propagate. See the previous review – please add the guard immediately after the she-bang.
7-7:replace_kernel.shstill executed without checks.
Previous feedback about ensur 10BC0 ing executability and checking the exit status hasn’t been addressed. Add explicit checks or enable strict-mode.
10-10: Still vulnerable to SC2164 & hiddenmakefailures.
The subshell ignores a failedcd, and combining the twomakecalls masks which one fails. Please split and guard, as previously suggested.
🧹 Nitpick comments (1)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh (1)
3-3: Package list keeps growing — move it to a variable or install file.
Long, in-linednf install …lists are error-prone and hard to diff (e.g., today onlyawkwas added). Consider:DEPS=( git make gcc gcc-c++ which file diffutils wget cpio rsync bc lzop zip patch perl tar qemu-system-aarch64 qemu-img unzboot uboot-tools kmod awk ) dnf install --setopt=install_weak_deps=false -y "${DEPS[@]}"This improves readability and future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/overlay/etc/udev/rules.d/10-rename-tsn0.rules(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/replace_kernel.sh(1 hunks)packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/rootfs_only_defconfig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/rootfs_only_defconfig
- packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/replace_kernel.sh
- packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/overlay/etc/udev/rules.d/10-rename-tsn0.rules
🧰 Additional context used
🪛 Shellcheck (0.10.0)
packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh
[warning] 10-10: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: Pages changed - jumpstarter-docs
| dnf install --setopt=install_weak_deps=false -y git make gcc gcc-c++ which file diffutils wget cpio rsync bc lzop zip patch perl tar qemu-system-aarch64 qemu-img unzboot uboot-tools kmod awk | ||
|
|
||
| git clone --depth 1 --branch 2025.05-rc2 https://github.com/buildroot/buildroot /buildroot | ||
| git clone --depth 1 --branch 2025.05 https://github.com/buildroot/buildroot /buildroot |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin Buildroot to a tag/commit for reproducible builds.
Cloning the moving 2025.05 branch with --depth 1 risks non-deterministic results when the branch head moves. Prefer a fixed tag or commit SHA and document it.
🤖 Prompt for AI Agents
In packages/jumpstarter-driver-flashers/oci_bundles/rcar_s4/build_flasher.sh at
line 5, the git clone command uses a moving branch reference '2025.05' which can
cause non-reproducible builds. Replace the branch name with a specific tag or
commit SHA to pin the Buildroot version. Update the clone command to use this
fixed reference and add a comment explaining the choice for reproducibility.
|
it works! |
it still supports Renesas S4 only, but it now renames the default interface to eth0 so that the same bundle cn be reused with other kernel-automotive supported boards
Also updated buildroot and kernel-automotive to latest.
Summary by CodeRabbit
New Features
Updates