From 531d66a5d0232e5695e90b3d592cc473be8c2d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Fri, 21 Aug 2026 18:59:00 -0300 Subject: [PATCH 01/12] install: boards: Write boot configuration to the mounted boot partition Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot/config.txt and /boot/cmdline.txt, so the Pi4 and BCM28XX scripts were appending the Navigator overlays to files the firmware never reads. A Pi4 installed over Bookworm came up with no I2C, SPI or UART configured, and the Navigator went undetected. The dtc output path was hardcoded to /boot/overlays as well. On Bookworm that only resolves because the boot partition migration leaves a /boot/overlays symlink behind, which no package owns, so the Navigator LED overlay was one missing compatibility link away from landing outside the boot partition. Resolve the boot partition once per script and abort when neither layout is mounted, rather than configuring a stub and reporting success. A Pi5 is newer than Bullseye, so its script only ever accepts /boot/firmware. --- install/boards/bcm_2712.sh | 15 ++++++++++++--- install/boards/bcm_27xx.sh | 17 ++++++++++++++--- install/boards/bcm_28xx.sh | 18 +++++++++++++++--- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/install/boards/bcm_2712.sh b/install/boards/bcm_2712.sh index e1d25d7619..47c792ac87 100644 --- a/install/boards/bcm_2712.sh +++ b/install/boards/bcm_2712.sh @@ -8,8 +8,17 @@ VERSION="${VERSION:-master}" GITHUB_REPOSITORY=${GITHUB_REPOSITORY:-bluerobotics/BlueOS} REMOTE="${REMOTE:-https://raw.githubusercontent.com/${GITHUB_REPOSITORY}}" ROOT="$REMOTE/$VERSION" -CMDLINE_FILE=/boot/firmware/cmdline.txt -CONFIG_FILE=/boot/firmware/config.txt +# Bookworm moved the boot partition to /boot/firmware and left a plain text stub at /boot that +# configures nothing. A Pi5 is newer than Bullseye, so it never uses the old /boot layout. +if [ -f /boot/firmware/config.txt ]; then + BOOT_PATH=/boot/firmware +else + echo "No boot partition at /boot/firmware, is it mounted?" >&2 + echo "Refusing to configure a file the board would never read." >&2 + exit 1 +fi +CMDLINE_FILE="$BOOT_PATH/cmdline.txt" +CONFIG_FILE="$BOOT_PATH/config.txt" alias curl="curl --retry 6 --max-time 15 --retry-all-errors --retry-delay 20 --connect-timeout 60" # Download, compile, and install spi0 mosi-only device tree overlay for @@ -18,7 +27,7 @@ echo "- compile spi0 device tree overlay." DTS_PATH="$ROOT/install/overlays" DTS_NAME="spi0-led" curl -fsSL -o /tmp/$DTS_NAME $DTS_PATH/$DTS_NAME.dts -dtc -@ -Hepapr -I dts -O dtb -o /boot/overlays/$DTS_NAME.dtbo /tmp/$DTS_NAME +dtc -@ -Hepapr -I dts -O dtb -o "$BOOT_PATH/overlays/$DTS_NAME.dtbo" /tmp/$DTS_NAME # Remove any configuration related to i2c and spi/spi1 and do the necessary changes for navigator echo "- Enable I2C, SPI and UART." diff --git a/install/boards/bcm_27xx.sh b/install/boards/bcm_27xx.sh index 6a20e49e69..1c4800b9ca 100755 --- a/install/boards/bcm_27xx.sh +++ b/install/boards/bcm_27xx.sh @@ -6,8 +6,19 @@ VERSION="${VERSION:-master}" GITHUB_REPOSITORY=${GITHUB_REPOSITORY:-bluerobotics/BlueOS} REMOTE="${REMOTE:-https://raw.githubusercontent.com/${GITHUB_REPOSITORY}}" ROOT="$REMOTE/$VERSION" -CMDLINE_FILE=/boot/cmdline.txt -CONFIG_FILE=/boot/config.txt +# Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot, +# so writing to /boot there configures nothing. +if [ -f /boot/firmware/config.txt ]; then + BOOT_PATH=/boot/firmware +elif [ -f /boot/config.txt ] && ! head -n 1 /boot/config.txt | grep -qx "DO NOT EDIT THIS FILE"; then + BOOT_PATH=/boot +else + echo "No boot partition at /boot/firmware, and /boot is missing or holds the Bookworm stub." >&2 + echo "Refusing to configure a file the board would never read." >&2 + exit 1 +fi +CMDLINE_FILE="$BOOT_PATH/cmdline.txt" +CONFIG_FILE="$BOOT_PATH/config.txt" alias curl="curl --retry 6 --max-time 15 --retry-all-errors --retry-delay 20 --connect-timeout 60" # Download, compile, and install spi0 mosi-only device tree overlay for @@ -16,7 +27,7 @@ echo "- compile spi0 device tree overlay." DTS_PATH="$ROOT/install/overlays" DTS_NAME="spi0-led" curl -fsSL -o /tmp/$DTS_NAME $DTS_PATH/$DTS_NAME.dts -dtc -@ -Hepapr -I dts -O dtb -o /boot/overlays/$DTS_NAME.dtbo /tmp/$DTS_NAME +dtc -@ -Hepapr -I dts -O dtb -o "$BOOT_PATH/overlays/$DTS_NAME.dtbo" /tmp/$DTS_NAME # Remove any configuration related to i2c and spi/spi1 and do the necessary changes for navigator echo "- Enable I2C, SPI and UART." diff --git a/install/boards/bcm_28xx.sh b/install/boards/bcm_28xx.sh index 1ac9578ae1..61044c94ad 100644 --- a/install/boards/bcm_28xx.sh +++ b/install/boards/bcm_28xx.sh @@ -2,15 +2,27 @@ echo "Configuring BCM28XX board (Raspberry Pi zero, 1, 2, 3).." -CMDLINE_FILE=/boot/cmdline.txt +# Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot, +# so writing to /boot there configures nothing. +if [ -f /boot/firmware/config.txt ]; then + BOOT_PATH=/boot/firmware +elif [ -f /boot/config.txt ] && ! head -n 1 /boot/config.txt | grep -qx "DO NOT EDIT THIS FILE"; then + BOOT_PATH=/boot +else + echo "No boot partition at /boot/firmware, and /boot is missing or holds the Bookworm stub." >&2 + echo "Refusing to configure a file the board would never read." >&2 + exit 1 +fi +CMDLINE_FILE="$BOOT_PATH/cmdline.txt" +CONFIG_FILE="$BOOT_PATH/config.txt" # Remove any configuration related to i2c and spi/spi1 and do the necessary changes for navigator echo "- Enable I2C, SPI and UART." for STRING in "dtparam=i2c_arm=" "dtparam=spi=" "dtoverlay=spi1" "dtoverlay=uart1"; do - sudo sed -i "/$STRING/d" /boot/config.txt + sudo sed -i "/$STRING/d" $CONFIG_FILE done for STRING in "dtparam=i2c_arm=on" "dtparam=spi=on" "dtoverlay=spi1-3cs" "dtoverlay=uart1"; do - echo "$STRING" | sudo tee -a /boot/config.txt + echo "$STRING" | sudo tee -a $CONFIG_FILE done # Check for valid modules file to load kernel modules From 6117e991ddc8fcdf0c14158542298649cf78bde4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Fri, 21 Aug 2026 18:59:10 -0300 Subject: [PATCH 02/12] core: tools: blueos_startup_update: Fix config.txt board section append A missing board section was appended as a single "\n[pi4]" entry, which the section lookup can never match again because re.match anchors at the start of the string. Every call appended yet another header, and the start index pointed one past the list, so the section bounds check in the conflict filter could never hold and each configuration line was deleted right after being inserted. A first pass over a stock config.txt left 30 empty [pi4] sections and no overlays, so a fresh install only got them two reboots later, once the saved file had been split back into lines. --- core/tools/blueos_startup_update/blueos_startup_update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 65cd9919df..4e7a379229 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -99,8 +99,8 @@ def boot_config_get_or_append_section(config_content: List[str], section_name: s (i for (i, line) in enumerate(config_content) if re.match(section_match_pattern, line, regex_flags)), None ) if section_start_line_number is None: - config_content.append(f"\n[{section_name}]") - section_start_line_number = len(config_content) + config_content.extend(["", f"[{section_name}]"]) + section_start_line_number = len(config_content) - 1 any_section_match_pattern = r"^\[.*\].*$" section_end_line_number = next( From e858a093d7cf5ade8d97c9c482a6134a7dcfd186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 00:59:02 -0300 Subject: [PATCH 03/12] core: tools: blueos_startup_update: Only configure dwc2 on Pi4 and Pi5 The section fell back to [pi5] for every board that is not a Pi4, and a board filter the firmware does not recognise is applied rather than ignored, so on anything older than a Pi5 that configuration would have reached the pins. Nothing gets here on a Pi3 today, main() only runs this patch for a Pi4 and a Pi5, but a board detection that once went wrong is exactly what its sibling revert_update_dwc2 exists to undo. Guard it the way update_navigator_overlays already does. --- .../blueos_startup_update/blueos_startup_update.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 4e7a379229..92703f8c0f 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -354,7 +354,16 @@ def update_dwc2() -> bool: # Add dwc2 overlay in pi4 section if it doesn't exist dwc2_overlay_config = "dtoverlay=dwc2,dr_mode=otg" - section_name = "pi4" if get_cpu_type() == CpuType.PI4 else "pi5" + cpu_type = get_cpu_type() + if cpu_type == CpuType.PI4: + section_name = "pi4" + elif cpu_type == CpuType.PI5: + section_name = "pi5" + else: + # A board filter the firmware does not know is applied instead of ignored, so a [pi5] + # section would reach the pins of every board older than the Pi5 + logger.error("Unsupported CPU type for dwc2 update") + return False boot_config_add_configuration_at_section(config_content, dwc2_overlay_config, section_name) # Remove any unprotected and conflicting dwc2 overlay configuration From 76ef23b0f64a585163e7892712664a16e95ff801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 02:06:43 -0300 Subject: [PATCH 04/12] install: boards: Insert configuration below the first board section --- install/boards/bcm_2712.sh | 2 +- install/boards/bcm_27xx.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/install/boards/bcm_2712.sh b/install/boards/bcm_2712.sh index 47c792ac87..071a521082 100644 --- a/install/boards/bcm_2712.sh +++ b/install/boards/bcm_2712.sh @@ -50,7 +50,7 @@ if ! grep -q "\[pi5\]" $CONFIG_FILE; then fi # find the line number of the [pi5] tag -line_number=$(grep -n "\[pi5\]" $CONFIG_FILE | awk -F ":" '{print $1}') +line_number=$(grep -n "\[pi5\]" $CONFIG_FILE | head -n 1 | awk -F ":" '{print $1}') echo "Line number of [pi5] tag: $line_number" diff --git a/install/boards/bcm_27xx.sh b/install/boards/bcm_27xx.sh index 1c4800b9ca..2fc9f766fa 100755 --- a/install/boards/bcm_27xx.sh +++ b/install/boards/bcm_27xx.sh @@ -50,7 +50,7 @@ if ! grep -q "\[pi4\]" $CONFIG_FILE; then fi # find the line number of the [pi4] tag -line_number=$(grep -n "\[pi4\]" $CONFIG_FILE | awk -F ":" '{print $1}') +line_number=$(grep -n "\[pi4\]" $CONFIG_FILE | head -n 1 | awk -F ":" '{print $1}') echo "Line number of [pi4] tag: $line_number" From 8943a19d8cbab6978ac09044f681f5675dff7f33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 02:06:43 -0300 Subject: [PATCH 05/12] install: boards: Abort on the first failed command --- install/boards/bcm_27xx.sh | 2 ++ install/boards/bcm_28xx.sh | 2 ++ 2 files changed, 4 insertions(+) diff --git a/install/boards/bcm_27xx.sh b/install/boards/bcm_27xx.sh index 2fc9f766fa..582182b058 100755 --- a/install/boards/bcm_27xx.sh +++ b/install/boards/bcm_27xx.sh @@ -1,5 +1,7 @@ #!/usr/bin/env bash +set -e + echo "Configuring BCM27XX board (Raspberry Pi 4).." VERSION="${VERSION:-master}" diff --git a/install/boards/bcm_28xx.sh b/install/boards/bcm_28xx.sh index 61044c94ad..983b43d540 100644 --- a/install/boards/bcm_28xx.sh +++ b/install/boards/bcm_28xx.sh @@ -1,5 +1,7 @@ #!/usr/bin/env bash +set -e + echo "Configuring BCM28XX board (Raspberry Pi zero, 1, 2, 3).." # Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot, From 0fdff1f37d004d2ac55f8427ba72dbf8ebd70a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 02:06:43 -0300 Subject: [PATCH 06/12] core: tools: blueos_startup_update: Merge stray board sections --- .../blueos_startup_update.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 92703f8c0f..4bfe9f2b58 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -117,6 +117,43 @@ def boot_config_get_or_append_section(config_content: List[str], section_name: s return (section_start_line_number, section_end_line_number) +def boot_config_merge_duplicated_sections(config_content: List[str], section_name: str) -> None: + regex_flags = re.IGNORECASE | re.DOTALL | re.MULTILINE + + section_match_pattern = r"^\[" + re.escape(section_name) + r"\].*$" + any_section_match_pattern = r"^\[.*\].*$" + + # A board filter is applied until the next one, so config.txt is a run of sections, each starting + # at its header, with the configuration that applies to every board ahead of the first header + sections: List[List[str]] = [[]] + for line in config_content: + if re.match(any_section_match_pattern, line, regex_flags): + sections.append([]) + sections[-1].append(line) + + duplicated = [ + index + for (index, section) in enumerate(sections[1:], start=1) + if re.match(section_match_pattern, section[0], regex_flags) + ] + if len(duplicated) < 2: + return + + # A duplicate header just reopens the same filter, so every stray's configuration is already in + # force, and the firmware applies whatever comes last. The strays are emptied into the end of the + # first section's run of non-blank lines, which keeps that order and keeps them where the helpers + # above, that read a section as ending at the first blank line, will find them + (survivor, *strays) = duplicated + body_end = next((index for index, line in enumerate(sections[survivor]) if index > 0 and line == ""), None) + if body_end is None: + body_end = len(sections[survivor]) + sections[survivor][body_end:body_end] = [line for index in strays for line in sections[index][1:] if line != ""] + for index in strays: + sections[index] = [] + + config_content[:] = [line for section in sections for line in section] + + def boot_config_add_configuration_at_section(config_content: List[str], config: str, section_name: str) -> None: regex_flags = re.IGNORECASE | re.DOTALL | re.MULTILINE @@ -452,6 +489,10 @@ def update_navigator_overlays() -> bool: logger.error("Unsupported CPU type for navigator overlays update") return False + # Devices patched by a release that appended a board section on every boot accumulated strays, + # and only the first of them would ever be configured + boot_config_merge_duplicated_sections(config_content, section_name) + navigator_configs_with_match_patterns.reverse() for (config, config_match_pattern) in navigator_configs_with_match_patterns: From cba89c8ed9e8069c53d01d9bd7da5782db3921ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 02:06:43 -0300 Subject: [PATCH 07/12] core: tools: blueos_startup_update: Fix modules-load merge at line start --- core/tools/blueos_startup_update/blueos_startup_update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 4bfe9f2b58..808a4d59a8 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -243,7 +243,7 @@ def boot_cmdline_add_modules(cmdline_content: List[str], config_key: str, desire cmdline_content.remove(cmdline_content[config_index]) # Replace the first configs line with the combined, append if none - if first_config_line: + if first_config_line is not None: cmdline_content[first_config_line] = f"{config_key}=" + ",".join(desired_config) else: config_line = f"{config_key}=" + ",".join(desired_config) From 46b27a97f60466866b8dbd7d358154e63c29b54b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 02:06:43 -0300 Subject: [PATCH 08/12] core: tools: blueos_startup_update: Escape configuration in patterns --- core/tools/blueos_startup_update/blueos_startup_update.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 808a4d59a8..1421b3b81d 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -161,7 +161,9 @@ def boot_config_add_configuration_at_section(config_content: List[str], config: section_content = config_content[section_start:section_end] config_already_exists = any( - section_content for section_content in section_content if re.match(config, section_content, regex_flags) + section_content + for section_content in section_content + if re.match(re.escape(config), section_content, regex_flags) ) if not config_already_exists: config_content.insert(section_start + 1, config) @@ -205,7 +207,7 @@ def boot_config_filter_conflicting_configuration_at_section( # ...except... and not ( # ...if it's the correct one.... - re.match(f"^{config}.*$", line, regex_flags) + re.match(f"^{re.escape(config)}.*$", line, regex_flags) # ...and lives inside the correct section. and (section_start < i < section_end) ) From 94cf49fd7bda37220233a3d385a7f6e5041fde76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Sat, 22 Aug 2026 02:06:43 -0300 Subject: [PATCH 09/12] core: tools: blueos_startup_update: Skip removing absent sections --- core/tools/blueos_startup_update/blueos_startup_update.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 1421b3b81d..1807956f84 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -170,6 +170,9 @@ def boot_config_add_configuration_at_section(config_content: List[str], config: def boot_config_remove_section(config_content: List[str], section_name: str) -> None: + if section_name not in boot_config_get_available_section(config_content): + return + (section_start, section_end) = boot_config_get_or_append_section(config_content, section_name) del config_content[section_start:section_end] From 934c59bcb6f8acf142102dadc6dc2a14391acb06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Thu, 3 Sep 2026 21:13:31 -0300 Subject: [PATCH 10/12] .github: workflows: Check the board configuration reached the image A board install script that writes to the /boot stub instead of the boot partition still exits 0, so the built image is the only place the mistake shows. Check that the image carries the board section, the Navigator overlays, the memory cgroup docker needs, and no serial console holding the autopilot port. --- .github/workflows/test-and-deploy.yml | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.github/workflows/test-and-deploy.yml b/.github/workflows/test-and-deploy.yml index 925238ce90..eb8778c0b1 100644 --- a/.github/workflows/test-and-deploy.yml +++ b/.github/workflows/test-and-deploy.yml @@ -303,6 +303,20 @@ jobs: - name: Add /boot additions run: | + # configure_board.sh dispatches on the host device tree, so the board configuration the image + # must end up with is decided by the runner's own hardware, not by the image it builds. + case "${{ matrix.runner }}" in + pi5-builder) + EXPECTED=("[pi5]" "dtoverlay=i2c3-pi5,baudrate=400000" "dtoverlay=spi1-3cs") + ;; + blueos-ci|pi4-builder2) + EXPECTED=("[pi4]" "dtoverlay=i2c4,pins_6_7,baudrate=1000000" "dtoverlay=spi1-3cs") + ;; + *) + echo "Unknown runner ${{ matrix.runner }}, add the board configuration its image must contain." + exit 1 + ;; + esac sudo apt-get update && sudo apt-get install -y parted kpartx # Create mount point if it doesn't exist sudo mkdir -p /mnt/piboot @@ -312,8 +326,27 @@ jobs: sudo mount "/dev/mapper/${LOOP_DEVICE}p1" /mnt/piboot # Create ssh and userconf files sudo cp install/boards/config.toml /mnt/piboot/custom.toml + # A board install script writing to the /boot stub instead of the boot partition still exits 0, + # so the image itself is the only place where a missing Navigator configuration can be caught. + MISSING=() + for LINE in "${EXPECTED[@]}"; do + sudo grep -qxF "$LINE" /mnt/piboot/config.txt || MISSING+=("config.txt: $LINE") + done + # cmdline.txt is reached through its own path and can be left behind on its own. Whatever + # board this is, the memory cgroup docker needs is enabled and the serial console that + # would hold the autopilot port is dropped. + sudo grep -q "cgroup_enable=memory" /mnt/piboot/cmdline.txt || MISSING+=("cmdline.txt: cgroup_enable=memory") + if sudo grep -q "console=serial" /mnt/piboot/cmdline.txt; then + MISSING+=("cmdline.txt: console=serial is still there") + fi sudo umount /mnt/piboot sudo kpartx -d deploy/pimod/blueos.img + if [ ${#MISSING[@]} -ne 0 ]; then + echo "Missing from the boot partition of the ${{ matrix.os }} ${{ matrix.platform }} image built on ${{ matrix.runner }}:" + printf ' %s\n' "${MISSING[@]}" + echo "The board install script did not configure the image's boot partition." + exit 1 + fi echo "Boot partition updated successfully." - name: Sanitize platform name From cbf251d8a84955e1ca598be226e9c8b02ffc6bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Thu, 3 Sep 2026 21:13:31 -0300 Subject: [PATCH 11/12] core: tools: blueos_startup_update: Add boot configuration patch tests The startup patches and the board install scripts have to agree on what a Navigator needs, and both had drifted into writing configuration where the firmware never reads it. Cover the two boot layouts BlueOS ships on: Bullseye, whose boot partition is at /boot and whose config.txt already carries a [pi4] section in the middle of the file followed by a second [all], and Bookworm, at /boot/firmware and with no board section to patch at all. Both baselines are the stock files of the images BlueOS builds from, so the result can be checked against hardware, and patching the Bullseye one reproduces the [pi4] section of a Pi4 that has been running BlueOS on Bullseye. A Navigator cannot be used on a Pi3, its pinout differs, and for the same reason the overlays of a Pi4 and a Pi5 are not interchangeable. Both are pinned down too: a Pi3 has to come out of the patches untouched, and neither board may end up carrying the other's overlays. The build time image check only runs on the upstream repository, where nobody reads it until a release ships, so its expectations are pinned here to what the install scripts actually write. --- .../test_blueos_startup_update.py | 759 ++++++++++++++++++ 1 file changed, 759 insertions(+) create mode 100644 core/tools/blueos_startup_update/test_blueos_startup_update.py diff --git a/core/tools/blueos_startup_update/test_blueos_startup_update.py b/core/tools/blueos_startup_update/test_blueos_startup_update.py new file mode 100644 index 0000000000..e4d6722f69 --- /dev/null +++ b/core/tools/blueos_startup_update/test_blueos_startup_update.py @@ -0,0 +1,759 @@ +import re +import subprocess +import sys +from pathlib import Path +from types import ModuleType +from typing import Callable, Dict, List, NamedTuple, Optional, Tuple +from unittest.mock import patch + +import pytest + +REPOSITORY_PATH = Path(__file__).parents[3] +assert (REPOSITORY_PATH / "install" / "boards").is_dir(), f"the repository root is not at {REPOSITORY_PATH}" + +# blueos_startup_update.py is a standalone script installed into /usr/bin, not a package, +# so it has to be loaded by path, and commonwealth has to be reachable for its imports +sys.path.append(str(REPOSITORY_PATH / "core" / "libs" / "commonwealth")) +MODULE_PATH = Path(__file__).parent / "blueos_startup_update.py" +blueos_startup_update = ModuleType("blueos_startup_update") +blueos_startup_update.__file__ = str(MODULE_PATH) +# Loading through importlib caches bytecode next to the script, keyed on its size and its mtime in +# whole seconds, so an edit that changes neither is silently ignored on the next run +exec( # pylint: disable=exec-used + compile(MODULE_PATH.read_text(encoding="utf-8"), str(MODULE_PATH), "exec"), blueos_startup_update.__dict__ +) + +from commonwealth.utils.general import CpuType + + +class Distribution(NamedTuple): + name: str + config_file: str + cmdline_file: str + stock_config: str + stock_cmdline: str + # Boards this release runs on, whether or not a Navigator can be used with them + cpu_types: Tuple[CpuType, ...] + + +# Boards a Navigator can be used with, and the install script that sets them up while building the +# image. The install scripts are the reference for what each board needs, and the startup patches +# have to reach the same result on devices installed before a given overlay was added. The hat sits +# on a different pinout on each board, so the two sets of overlays are not interchangeable. +NAVIGATOR_INSTALL_SCRIPTS = { + CpuType.PI4: "install/boards/bcm_27xx.sh", + CpuType.PI5: "install/boards/bcm_2712.sh", +} + +ALL_INSTALL_SCRIPTS = sorted( + str(script.relative_to(REPOSITORY_PATH)) for script in (REPOSITORY_PATH / "install" / "boards").glob("*.sh") +) + +# The scripts that write to the boot partition, and so have to work out where it is. Picked by what +# they name rather than by their name, so a new board script is enrolled on its own and a script +# that drops its boot partition probe stays enrolled. +BOOT_FILE_INSTALL_SCRIPTS = [ + script for script in ALL_INSTALL_SCRIPTS if "config.txt" in (REPOSITORY_PATH / script).read_text(encoding="utf-8") +] +assert set(NAVIGATOR_INSTALL_SCRIPTS.values()) <= set(BOOT_FILE_INSTALL_SCRIPTS), "an install script went missing" + +WORKFLOW_PATH = REPOSITORY_PATH / ".github" / "workflows" / "test-and-deploy.yml" + +# Stock config.txt of the Bullseye image BlueOS builds from, taken from +# RPi-Distro/pi-gen stage1/00-boot-files/files/config.txt on the bullseye branch. +# The board section already exists here, sits in the middle of the file, and is followed by a +# second [all] section, so patching it has to stay inside its own bounds. +BULLSEYE_STOCK_CONFIG_TXT = """# For more options and information see +# http://rpf.io/configtxt +# Some settings may impact device functionality. See link above for details + +# uncomment if you get no picture on HDMI for a default "safe" mode +#hdmi_safe=1 + +# uncomment to force a HDMI mode rather than DVI. This can make audio work in +# DMT (computer monitor) modes +#hdmi_drive=2 + +# uncomment for composite PAL +#sdtv_mode=2 + +#uncomment to overclock the arm. 700 MHz is the default. +#arm_freq=800 + +# Uncomment some or all of these to enable the optional hardware interfaces +#dtparam=i2c_arm=on +#dtparam=i2s=on +#dtparam=spi=on + +# Uncomment this to enable infrared communication. +#dtoverlay=gpio-ir,gpio_pin=17 +#dtoverlay=gpio-ir-tx,gpio_pin=18 + +# Additional overlays and parameters are documented /boot/overlays/README + +# Enable audio (loads snd_bcm2835) +dtparam=audio=on + +# Automatically load overlays for detected cameras +camera_auto_detect=1 + +# Automatically load overlays for detected DSI displays +display_auto_detect=1 + +# Enable DRM VC4 V3D driver +dtoverlay=vc4-kms-v3d +max_framebuffers=2 + +# Disable compensation for displays with overscan +disable_overscan=1 + +[cm4] +# Enable host mode on the 2711 built-in XHCI USB controller. +# This line should be removed if the legacy DWC2 controller is required +# (e.g. for USB device mode) or if USB support is not required. +otg_mode=1 + +[all] + +[pi4] +# Run as fast as firmware / board allows +arm_boost=1 + +[all] +""" + +# Stock config.txt of a Bookworm device, read back from a Raspberry Pi 4 before BlueOS touched it. +# There is no board section to patch here, so one has to be created. +BOOKWORM_STOCK_CONFIG_TXT = """# For more options and information see +# http://rptl.io/configtxt +# Some settings may impact device functionality. See link above for details + +# Uncomment some or all of these to enable the optional hardware interfaces +#dtparam=i2c_arm=on +#dtparam=i2s=on +#dtparam=spi=on + +# Enable audio (loads snd_bcm2835) +dtparam=audio=on + +# Additional overlays and parameters are documented +# /boot/firmware/overlays/README + +# Automatically load overlays for detected cameras +camera_auto_detect=1 + +# Automatically load overlays for detected DSI displays +display_auto_detect=1 + +# Automatically load initramfs files, if found +auto_initramfs=1 + +# Enable DRM VC4 V3D driver +dtoverlay=vc4-kms-v3d +max_framebuffers=2 + +# Don't have the firmware create an initial video= setting in cmdline.txt. +# Use the kernel's default instead. +disable_fw_kms_setup=1 + +# Disable compensation for displays with overscan +disable_overscan=1 + +# Run as fast as firmware / board allows +arm_boost=1 + +[cm4] +# Enable host mode on the 2711 built-in XHCI USB controller. +# This line should be removed if the legacy DWC2 controller is required +# (e.g. for USB device mode) or if USB support is not required. +otg_mode=1 + +[cm5] +dtoverlay=dwc2,dr_mode=host + +[all] +""" + +STOCK_CMDLINE_TXT = ( + "console=serial0,115200 console=tty1 root=PARTUUID=41964984-02 rootfstype=ext4 fsck.repair=yes rootwait\n" +) + +# Bullseye keeps the boot partition at /boot, Bookworm moved it to /boot/firmware and left plain +# text stubs behind at /boot. A Pi5 is newer than Bullseye, so it only ever runs Bookworm. +BULLSEYE = Distribution( + name="bullseye", + config_file="/boot/config.txt", + cmdline_file="/boot/cmdline.txt", + stock_config=BULLSEYE_STOCK_CONFIG_TXT, + stock_cmdline=STOCK_CMDLINE_TXT, + cpu_types=(CpuType.PI3, CpuType.PI4), +) + +BOOKWORM = Distribution( + name="bookworm", + config_file="/boot/firmware/config.txt", + cmdline_file="/boot/firmware/cmdline.txt", + stock_config=BOOKWORM_STOCK_CONFIG_TXT, + stock_cmdline=STOCK_CMDLINE_TXT, + cpu_types=(CpuType.PI3, CpuType.PI4, CpuType.PI5), +) + +# Every distribution and Navigator capable board combination that can reach a user +NAVIGATOR_BOARDS = [ + pytest.param(distribution, cpu_type, id=f"{distribution.name}-{cpu_type.name}") + for distribution in (BULLSEYE, BOOKWORM) + for cpu_type in distribution.cpu_types + if cpu_type in NAVIGATOR_INSTALL_SCRIPTS +] + +DISTRIBUTIONS = [pytest.param(distribution, id=distribution.name) for distribution in (BULLSEYE, BOOKWORM)] + +# What Bookworm leaves at /boot once it has moved the boot partition to /boot/firmware. Writing to +# it configures nothing, and the write still succeeds, which is how the original bug shipped. +BOOT_STUBS = { + "config.txt": "DO NOT EDIT THIS FILE\n\nThe file you are looking for has moved to /boot/firmware/config.txt\n", + "cmdline.txt": "DO NOT EDIT THIS FILE\n\nThe file you are looking for has moved to /boot/firmware/cmdline.txt\n", +} + +MOUNTED_BOOT_PARTITION = {"config.txt": BOOKWORM_STOCK_CONFIG_TXT, "cmdline.txt": STOCK_CMDLINE_TXT} + +# Boot partition layouts an install script has to tell apart, as directories of a fake root, and the +# one it has to settle on. None means it has to refuse to guess rather than configure a stub. +BOOT_LAYOUTS = [ + pytest.param({"boot/firmware": MOUNTED_BOOT_PARTITION, "boot": BOOT_STUBS}, "boot/firmware", id="bookworm"), + pytest.param({"boot": MOUNTED_BOOT_PARTITION}, "boot", id="bullseye"), + pytest.param({"boot": BOOT_STUBS}, None, id="bookworm-boot-partition-unmounted"), + pytest.param({}, None, id="no-boot-partition"), +] + + +def bash_words(word_list: str) -> List[str]: + """Expand a bash word list with bash, so quoting style and line continuations do not matter.""" + result = subprocess.run( + ["bash", "-c", f'for STRING in {word_list}; do printf "%s\\n" "$STRING"; done'], + capture_output=True, + text=True, + check=True, + ) + return result.stdout.splitlines() + + +def install_script_section(script_name: str) -> str: + """Board section an install script tags its configuration with.""" + content = (REPOSITORY_PATH / script_name).read_text(encoding="utf-8") + + section_match = re.search(r"""grep -q\w* ['"]\\\[(\w+)\\\]['"]""", content) + assert section_match, f"{script_name} does not tag its configuration with a board section" + + return section_match.group(1) + + +def install_script_configuration(script_name: str) -> Tuple[List[str], List[str]]: + """Configuration an install script deletes from, then inserts into, config.txt.""" + content = (REPOSITORY_PATH / script_name).read_text(encoding="utf-8") + + # Selecting the loops by what they do rather than by where they are keeps reformatting, requoting + # and reordering the scripts out of the test results + loops = [ + (bash_words(words), body) + for words, body in re.findall(r"for STRING in\b(.*?);?\s*do\s*\\?\n(.*?)\ndone", content, re.DOTALL) + if "$CONFIG_FILE" in body + ] + deleting = [words for words, body in loops if re.search(r"sed -i .?/\$STRING/d", body)] + inserting = [words for words, body in loops if not re.search(r"sed -i .?/\$STRING/d", body)] + assert len(deleting) == 1, f"{script_name}: expected one loop deleting from $CONFIG_FILE, got {len(deleting)}" + assert len(inserting) == 1, f"{script_name}: expected one loop writing to $CONFIG_FILE, got {len(inserting)}" + + return deleting[0], inserting[0] + + +def boot_path_probe(content: str) -> Optional[str]: + """The `if ... fi` block that derives BOOT_PATH, whatever shape the test it uses has.""" + return next( + ( + match.group(0) + for match in re.finditer(r"^if .*\n(?:.*\n)*?^fi$", content, re.MULTILINE) + if "BOOT_PATH=" in match.group(0) + ), + None, + ) + + +def apply_boot_config_patches( + cpu_type: CpuType, + distribution: Distribution, + files: Dict[str, str], + patches: Tuple[Callable[[], bool], ...] = (), +) -> Dict[str, bool]: + """Run startup patches against in-memory boot files, returning which of them want a restart.""" + + def fake_load_file(file_name: str) -> str: + return files[file_name] + + def fake_save_file(file_name: str, file_content: str, _backup_identifier: str, ensure_newline: bool = True) -> None: + files[file_name] = file_content if not ensure_newline or file_content.endswith("\n") else f"{file_content}\n" + + # dwc2 owns its own overlay inside the same board section, and cgroups shares cmdline.txt and + # the line merging with it, so all three are needed to get what the install scripts describe + patches = patches or ( + blueos_startup_update.update_navigator_overlays, + blueos_startup_update.update_dwc2, + blueos_startup_update.update_cgroups, + ) + + blueos_startup_update.config_file = distribution.config_file + blueos_startup_update.cmdline_file = distribution.cmdline_file + with patch.object(blueos_startup_update, "load_file", fake_load_file), patch.object( + blueos_startup_update, "save_file", fake_save_file + ), patch.object(blueos_startup_update, "get_cpu_type", lambda: cpu_type): + # Every patch has to run, the startup script does not stop at the first one that applies + return {patch_function.__name__: patch_function() for patch_function in patches} + + +def stock_files(distribution: Distribution) -> Dict[str, str]: + return { + distribution.config_file: distribution.stock_config, + distribution.cmdline_file: distribution.stock_cmdline, + } + + +def section_names(config_txt: str) -> List[str]: + """Section headers in file order, keeping repetitions.""" + return [line for line in config_txt.splitlines() if line.startswith("[")] + + +def root_configuration(config_txt: str) -> List[str]: + """Lines before the first section header, which apply to every board.""" + lines = config_txt.splitlines() + end = next((i for i, line in enumerate(lines) if line.startswith("[")), len(lines)) + return [line for line in lines[:end] if line] + + +def section_configuration(config_txt: str, section_name: str) -> List[str]: + """Lines of a section, which ends at the first blank line or the next section header.""" + lines = config_txt.splitlines() + if f"[{section_name}]" not in lines: + return [] + start = lines.index(f"[{section_name}]") + end = next( + (start + 1 + i for i, line in enumerate(lines[start + 1 :]) if not line or line.startswith("[")), + len(lines), + ) + return lines[start + 1 : end] + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +def test_startup_patches_match_install_script(distribution: Distribution, cpu_type: CpuType) -> None: + section_name = install_script_section(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + _, insertions = install_script_configuration(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + # What the distribution already put in the board section is not ours to remove + distribution_configuration = section_configuration(distribution.stock_config, section_name) + files = stock_files(distribution) + + applied = apply_boot_config_patches(cpu_type, distribution, files) + + assert any(applied.values()), "a fresh install has to ask for a restart" + patched_config = files[distribution.config_file] + headers = patched_config.count(f"[{section_name}]") + assert headers == 1, f"expected exactly one [{section_name}] in config.txt, found {headers}" + assert sorted(section_configuration(patched_config, section_name)) == sorted( + insertions + distribution_configuration + ) + + # An overlay of another board would drive the wrong pins of the Navigator + for other_cpu_type, other_script in NAVIGATOR_INSTALL_SCRIPTS.items(): + if other_cpu_type == cpu_type: + continue + _, other_insertions = install_script_configuration(other_script) + for line in set(other_insertions) - set(insertions): + assert line not in patched_config.splitlines(), f"{line!r} belongs to {other_cpu_type.name}" + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +def test_startup_patches_keep_distribution_configuration(distribution: Distribution, cpu_type: CpuType) -> None: + section_name = install_script_section(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + deletions, _ = install_script_configuration(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + files = stock_files(distribution) + + apply_boot_config_patches(cpu_type, distribution, files) + + # The install scripts delete these with `sed /STRING/d`, so anything containing one of them is + # configuration BlueOS owns. Every other line the distribution shipped has to survive, including + # the commented hardware hints and the sections of boards we are not running on. + patched_lines = files[distribution.config_file].splitlines() + for line in distribution.stock_config.splitlines(): + if not line or any(deletion in line for deletion in deletions): + continue + assert line in patched_lines, f"{distribution.name} lost {line!r}" + + assert section_names(files[distribution.config_file]) == section_names(distribution.stock_config) + ( + [] if f"[{section_name}]" in section_names(distribution.stock_config) else [f"[{section_name}]"] + ), "patching moved, dropped or duplicated a section" + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +def test_startup_patches_converge_on_first_run(distribution: Distribution, cpu_type: CpuType) -> None: + files = stock_files(distribution) + apply_boot_config_patches(cpu_type, distribution, files) + patched = dict(files) + + applied = apply_boot_config_patches(cpu_type, distribution, files) + + restarting = [name for name, wants_restart in applied.items() if wants_restart] + assert not restarting, f"{restarting} did not converge, a reboot loop would follow" + for file_name, content in patched.items(): + assert files[file_name] == content, f"{file_name} kept changing after the first run" + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +def test_startup_patches_replace_conflicting_configuration(distribution: Distribution, cpu_type: CpuType) -> None: + section_name = install_script_section(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + _, insertions = install_script_configuration(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + distribution_configuration = section_configuration(distribution.stock_config, section_name) + conflicting = "enable_uart=0\ndtparam=spi=off\ndtoverlay=spi1-1cs\ndtoverlay=dwc2,dr_mode=host\n" + # The same configuration outside the board section applies to boards it was never meant for, + # so it has to be removed even though the value itself is the one we want + misplaced = insertions[:2] + protected = "dtoverlay=i2c1 # custom\n" + files = stock_files(distribution) + files[distribution.config_file] += conflicting + "\n".join(misplaced) + "\n" + protected + + apply_boot_config_patches(cpu_type, distribution, files) + + patched_lines = files[distribution.config_file].splitlines() + assert sorted(section_configuration(files[distribution.config_file], section_name)) == sorted( + insertions + distribution_configuration + ) + for conflicting_line in conflicting.splitlines(): + assert conflicting_line not in patched_lines + for misplaced_line in misplaced: + assert patched_lines.count(misplaced_line) == 1, f"{misplaced_line!r} was left outside [{section_name}]" + assert protected.strip() in patched_lines, "a line the user protected was removed" + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +def test_every_patch_asks_for_the_restart_its_own_writes_need(distribution: Distribution, cpu_type: CpuType) -> None: + files = stock_files(distribution) + + applied = apply_boot_config_patches(cpu_type, distribution, files) + + # Reading the patches through any() hides the one that rewrote a boot file and reported no + # change, which is the field symptom: overlays written, no reboot asked for, no Navigator + silent = sorted(name for name, wants_restart in applied.items() if not wants_restart) + assert not silent, f"{silent} rewrote the boot files of a fresh install without asking for a restart" + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +@pytest.mark.parametrize("modules_first", [True, False], ids=["modules-load-first", "modules-load-mid-line"]) +def test_startup_patches_load_usb_ethernet_modules( + distribution: Distribution, cpu_type: CpuType, modules_first: bool +) -> None: + files = stock_files(distribution) + # A device that already loads modules keeps them, the dwc2 ones are merged into the same + # parameter. Index 0 of cmdline.txt is its own code path, a falsy index used to append instead. + files[distribution.cmdline_file] = ( + f"modules-load=i2c-dev {distribution.stock_cmdline}" + if modules_first + else distribution.stock_cmdline.replace("rootwait", "rootwait modules-load=i2c-dev") + ) + + apply_boot_config_patches(cpu_type, distribution, files) + + parameters = files[distribution.cmdline_file].split() + modules = [parameter for parameter in parameters if parameter.startswith("modules-load=")] + assert len(modules) == 1, f"cmdline.txt ended up with {len(modules)} modules-load parameters, only the first loads" + assert sorted(modules[0].removeprefix("modules-load=").split(",")) == ["dwc2", "g_ether", "i2c-dev"] + for parameter in distribution.stock_cmdline.split(): + assert parameter in parameters, f"cmdline.txt lost {parameter!r}" + + apply_boot_config_patches(cpu_type, distribution, files) + + assert files[distribution.cmdline_file].split() == parameters, "cmdline.txt kept changing, a reboot loop follows" + + +@pytest.mark.parametrize("distribution, cpu_type", NAVIGATOR_BOARDS) +def test_startup_patches_merge_stray_board_sections(distribution: Distribution, cpu_type: CpuType) -> None: + section_name = install_script_section(NAVIGATOR_INSTALL_SCRIPTS[cpu_type]) + # A release that appended a board section on every boot left devices with a run of empty + # headers, of which only the first is ever configured, and the user kept configuring the others + user_configuration = ["hdmi_force_hotplug=1", "force_turbo=1", "usb_max_current_enable=1"] + files = stock_files(distribution) + files[distribution.config_file] += f"\n[{section_name}]\n" * 27 + "".join( + f"\n[{section_name}]\n{line}\n" for line in user_configuration + ) + + applied = apply_boot_config_patches(cpu_type, distribution, files) + + patched_config = files[distribution.config_file] + headers = patched_config.count(f"[{section_name}]") + assert headers == 1, f"a device in the field kept {headers} [{section_name}] headers, only the first is configured" + for line in user_configuration: + found = patched_config.splitlines().count(line) + assert found == 1, f"merging the stray headers left {found} copies of the user configuration {line!r}" + assert line in section_configuration( + patched_config, section_name + ), f"{line!r} was left outside [{section_name}], the user configuration stopped applying" + assert any(applied.values()), "the boot files were repaired without asking for the restart that applies them" + + applied = apply_boot_config_patches(cpu_type, distribution, files) + + restarting = [name for name, wants_restart in applied.items() if wants_restart] + assert not restarting, f"{restarting} did not converge on a repaired device, a reboot loop would follow" + + +def test_merging_board_sections_leaves_a_single_section_alone() -> None: + config_content = ["[all]", "arm_boost=1", "", "[pi4]", "enable_uart=1", "", "[cm4]", "otg_mode=1"] + untouched = config_content.copy() + + blueos_startup_update.boot_config_merge_duplicated_sections(config_content, "pi4") + + assert config_content == untouched, "a config.txt with one board section was rewritten, forcing a needless restart" + + +def test_merging_board_sections_keeps_the_order_the_firmware_applies() -> None: + # A duplicate header reopens the same filter, so the firmware already reads the strays' lines + # after the survivor's. Hoisting them above it would flip the effective value of any key the user + # set under both headers, and two headers is what one boot on the bad release leaves behind + config_content = ["[pi4]", "arm_freq=1500", "", "[pi4]", "arm_freq=1800"] + + blueos_startup_update.boot_config_merge_duplicated_sections(config_content, "pi4") + + headers = config_content.count("[pi4]") + assert headers == 1, f"a device with two stray headers kept {headers} of them, the merge skipped the pair" + settings = [line for line in config_content if line.startswith("arm_freq=")] + assert settings == ["arm_freq=1500", "arm_freq=1800"], ( + f"the merge left the user configuration as {settings}: the firmware applies the last line of a " + "filter, so the effective value changed while repairing the file" + ) + (start, end) = blueos_startup_update.boot_config_get_or_append_section(config_content, "pi4") + assert set(settings) <= set(config_content[start:end]), ( + f"the merged configuration landed outside [pi4] at {config_content[start:end]}, so the patches " + "would not see it and would append their own copy on every boot" + ) + + +def test_appended_board_section_index_points_at_its_header() -> None: + config_content = ["arm_boost=1"] + + (start, end) = blueos_startup_update.boot_config_get_or_append_section(config_content, "pi4") + + # An index past the header left the section empty and appended another one on the next boot, + # so the overlays only ever landed one reboot late + assert start < len(config_content) and config_content[start] == "[pi4]", ( + f"the appended section starts at index {start} of {len(config_content)} lines, which is not its header, " + "so the overlays would land outside the section" + ) + assert start < end <= len(config_content), f"the section bounds ({start}, {end}) cannot hold any configuration" + assert blueos_startup_update.boot_config_get_or_append_section(config_content, "pi4") == ( + start, + end, + ), "looking the section up again did not find the one just appended" + assert config_content.count("[pi4]") == 1, f"config.txt collected {config_content.count('[pi4]')} board sections" + + +def test_removing_a_section_that_is_not_there_changes_nothing() -> None: + config_content = ["[all]", "arm_boost=1"] + untouched = config_content.copy() + + blueos_startup_update.boot_config_remove_section(config_content, "pi4") + + assert config_content == untouched, "removing an absent section rewrote config.txt and asked for a restart" + + +def test_pi5_keeps_both_spellings_of_the_navigator_i2c_overlay() -> None: + _, insertions = install_script_configuration(NAVIGATOR_INSTALL_SCRIPTS[CpuType.PI5]) + spellings = [line for line in insertions if line.startswith("dtoverlay=i2c3-pi5")] + assert len(spellings) == 2, f"bcm_2712.sh no longer writes both i2c3 spellings: {spellings}" + + # The two spellings reach different firmware revisions, and used as a pattern the dotted one + # matches the comma one, so a device already carrying either must still be given the other + for existing in spellings: + files = stock_files(BOOKWORM) + files[BOOKWORM.config_file] += f"\n[pi5]\n{existing}\n" + + apply_boot_config_patches(CpuType.PI5, BOOKWORM, files) + + section = section_configuration(files[BOOKWORM.config_file], "pi5") + for spelling in spellings: + assert section.count(spelling) == 1, ( + f"a Pi5 already carrying {existing!r} ended up with {section.count(spelling)} copies of " + f"{spelling!r}, the Navigator I2C bus would not come up" + ) + + +@pytest.mark.parametrize("distribution", DISTRIBUTIONS) +def test_navigator_configuration_is_never_applied_to_a_pi3(distribution: Distribution) -> None: + """A Navigator cannot be used on a Pi3, the hat expects a pinout the board does not have.""" + files = stock_files(distribution) + + # Reaching these at all takes a bad board detection, which has happened before, so neither may + # write anything of its own accord + patches = (blueos_startup_update.update_navigator_overlays, blueos_startup_update.update_dwc2) + applied = apply_boot_config_patches(CpuType.PI3, distribution, files, patches=patches) + + assert not any(applied.values()), "a Pi3 was told to restart for configuration it cannot use" + assert files == stock_files(distribution), "boot files were patched for a board without a Navigator" + + +@pytest.mark.parametrize("distribution", DISTRIBUTIONS) +def test_pi3_cleanup_keeps_distribution_sections(distribution: Distribution) -> None: + """A Pi3 got Navigator sections it cannot use, and only those may be cleaned up.""" + files = stock_files(distribution) + + apply_boot_config_patches(CpuType.PI3, distribution, files, patches=(blueos_startup_update.clean_config_pi3,)) + + patched_lines = files[distribution.config_file].splitlines() + # A filter the firmware does not recognise evaluates to true rather than false, so a section of + # a board newer than the running firmware is not merely useless on a Pi3, its contents are + # applied. Only the filters a Pi3 answers to may survive the cleanup. + pi3_filters = {"all", "pi3", "pi3+", "cm3", "cm3+"} + remaining = {name.strip("[]") for name in section_names(files[distribution.config_file])} + assert remaining <= pi3_filters, f"a Pi3 would apply the contents of {sorted(remaining - pi3_filters)}" + assert section_names(files[distribution.config_file]).count("[all]") == section_names( + distribution.stock_config + ).count("[all]") + # Configuration outside a board section applies to a Pi3 as much as to any other board + for line in root_configuration(distribution.stock_config): + assert line in patched_lines, f"{distribution.name} lost {line!r}" + + applied = apply_boot_config_patches( + CpuType.PI3, distribution, files, patches=(blueos_startup_update.clean_config_pi3,) + ) + + assert not any(applied.values()), "the cleanup did not converge, a reboot loop would follow" + + +def test_bcm28xx_enables_the_peripherals_a_pi3_needs() -> None: + # Pi zero/1/2/3 have no startup patch to cross-check this script against, so these four lines + # are the only description of what the onboard peripherals need + deletions, insertions = install_script_configuration("install/boards/bcm_28xx.sh") + + assert sorted(insertions) == [ + "dtoverlay=spi1-3cs", + "dtoverlay=uart1", + "dtparam=i2c_arm=on", + "dtparam=spi=on", + ], f"bcm_28xx.sh no longer enables I2C, SPI and UART on a Pi3: {sorted(insertions)}" + for insertion in insertions: + assert any( + deletion in insertion for deletion in deletions + ), f"{insertion!r} is written but never cleaned out first, so reinstalling would stack duplicates" + + +@pytest.mark.parametrize("script_name", sorted(NAVIGATOR_INSTALL_SCRIPTS.values())) +def test_install_scripts_insert_below_the_first_board_section(script_name: str, tmp_path: Path) -> None: + section_name = install_script_section(script_name) + content = (REPOSITORY_PATH / script_name).read_text(encoding="utf-8") + lookup = next((line for line in content.splitlines() if line.startswith("line_number=")), None) + assert lookup, f"{script_name} no longer looks up where to insert the Navigator configuration" + + # Devices on the release that appended a board section on every boot carry a run of stray + # headers. One line number per match makes every insertion `sed` fail, and the deletions have + # already run by then, so the board is left with no configuration at all and the script exits 0. + config_file = tmp_path / "config.txt" + config_file.write_text(f"[all]\n[{section_name}]\n\n[{section_name}]\n", encoding="utf-8") + result = subprocess.run( + ["bash", "-c", f'CONFIG_FILE="{config_file}"\n{lookup}\nprintf "%s" "$line_number"'], + capture_output=True, + text=True, + check=True, + ) + + assert result.stdout == "2", ( + f"{script_name} resolved the insertion point to {result.stdout!r} on a config.txt carrying stray " + f"[{section_name}] headers, so reinstalling would strip the Navigator configuration and report success" + ) + + +@pytest.mark.parametrize("script_name", BOOT_FILE_INSTALL_SCRIPTS) +@pytest.mark.parametrize("layout, expected", BOOT_LAYOUTS) +def test_install_scripts_probe_selects_the_real_boot_partition( + script_name: str, layout: Dict[str, Dict[str, str]], expected: Optional[str], tmp_path: Path +) -> None: + probe = boot_path_probe((REPOSITORY_PATH / script_name).read_text(encoding="utf-8")) + assert probe, f"{script_name} no longer probes for the boot partition, /boot may be an inert Bookworm stub" + + # A Pi5 is newer than Bullseye, so boot files at /boot are never its boot partition + if expected == "boot" and script_name == NAVIGATOR_INSTALL_SCRIPTS[CpuType.PI5]: + expected = None + + for directory, boot_files in layout.items(): + (tmp_path / directory).mkdir(parents=True, exist_ok=True) + for name, content in boot_files.items(): + (tmp_path / directory / name).write_text(content, encoding="utf-8") + + # The sibling test guarantees the probe is the only place naming /boot, so remapping that + # literal onto a fake root is safe, and running it under bash keeps any branch shape working + result = subprocess.run( + ["bash", "-c", f'{probe.replace("/boot", f"{tmp_path}/boot")}\nprintf "%s" "$BOOT_PATH"'], + capture_output=True, + text=True, + check=False, + ) + resolved = result.stdout.replace(str(tmp_path), "") or "nothing" + + if expected is None: + assert result.returncode != 0, ( + f"{script_name} resolved BOOT_PATH to {resolved} with no boot partition mounted, so the install " + "would configure a plain text file and still report success" + ) + return + assert result.returncode == 0, f"{script_name} refused a valid boot partition: {result.stderr.strip()}" + assert result.stdout == f"{tmp_path}/{expected}", ( + f"{script_name} resolved BOOT_PATH to {resolved} instead of /{expected}, so the Navigator " + "configuration would never reach the boot partition" + ) + + +@pytest.mark.parametrize("script_name", ALL_INSTALL_SCRIPTS) +def test_install_scripts_reach_boot_files_only_through_boot_path(script_name: str) -> None: + content = (REPOSITORY_PATH / script_name).read_text(encoding="utf-8") + code = "\n".join(re.sub(r"\s+#.*$", "", line) for line in content.splitlines() if not line.lstrip().startswith("#")) + probe = boot_path_probe(code) + + # Bookworm keeps the boot partition at /boot/firmware and leaves plain text stubs at /boot, so + # every boot file has to be reached through BOOT_PATH rather than a hardcoded prefix. Only the + # probe that decides which layout this is may name a path directly. + outside_probe = code.replace(probe, "") if probe else code + hardcoded = sorted(set(re.findall(r"/boot[\w./$-]*", outside_probe))) + assert not hardcoded, f"{script_name} reaches boot files without BOOT_PATH: {hardcoded}" + + +def test_image_build_checks_what_the_install_scripts_write() -> None: + # An install script that configures a stub still exits 0, so the built image is the only place + # the mistake shows. That check only runs on the upstream repository, where nobody reads it + # until a release ships, so it stopping matching the install scripts has to fail here instead. + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + sections = {install_script_section(script): script for script in NAVIGATOR_INSTALL_SCRIPTS.values()} + + checked = [] + for words in re.findall(r"EXPECTED=\((.*?)\)", workflow, re.DOTALL): + expected = bash_words(words) + headers = [line for line in expected if line.startswith("[")] + assert len(headers) == 1, f"{expected} does not look for exactly one board section" + section_name = headers[0].strip("[]") + assert section_name in sections, f"no install script writes a [{section_name}] section" + _, insertions = install_script_configuration(sections[section_name]) + for line in expected: + assert line in headers or line in insertions, f"{sections[section_name]} does not write {line!r}" + checked.append(section_name) + + assert sorted(checked) == sorted(sections), f"{sorted(sections)} are built as images, {sorted(checked)} are checked" + + # cmdline.txt is reached through a path of its own, so it is checked separately, and every board + # script writes the same two things to it whether or not a Navigator can be used with the board + for configuration in ("cgroup_enable=memory", "console=serial"): + assert f'"{configuration}" /mnt/piboot/cmdline.txt' in workflow, f"the image is not checked for {configuration}" + for script_name in BOOT_FILE_INSTALL_SCRIPTS: + content = (REPOSITORY_PATH / script_name).read_text(encoding="utf-8") + assert configuration in content, f"{script_name} no longer writes {configuration!r}" + + +@pytest.mark.parametrize("boot_file", ["config.txt", "cmdline.txt"]) +def test_startup_patches_prefer_the_bookworm_boot_partition(boot_file: str) -> None: + source = MODULE_PATH.read_text(encoding="utf-8") + + # On Bookworm both paths exist, but /boot holds a plain text stub, so it may only ever be the + # fallback of the two. + candidates = re.findall(rf'"(/boot[^"]*{re.escape(boot_file)})"', source) + assert candidates == [f"/boot/firmware/{boot_file}", f"/boot/{boot_file}"] From 1349efc33cd1a9947ff659820e6b61a85768dc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ant=C3=B4nio=20Cardoso?= Date: Thu, 3 Sep 2026 22:50:51 -0300 Subject: [PATCH 12/12] core: tools: blueos_startup_update: Refuse the Bookworm boot stub The install scripts now refuse to configure the plain text stub Bookworm leaves at /boot once the boot partition moved to /boot/firmware, but the runtime repair path did not. locate_file returns the first candidate that exists, so on a Bookworm host whose boot partition is not mounted it settled on /boot/config.txt, the stub, and the patches wrote the whole board section into a file the firmware never reads, rebooted, and from then on found their own work in place and converged into a permanently unconfigured state with no error. Refuse the stub by the same DO NOT EDIT THIS FILE marker the install scripts key on, so both halves of the contract agree, and let the existing config_file None guards skip the patch loudly rather than write nowhere. --- .../blueos_startup_update.py | 28 +++++++- .../test_blueos_startup_update.py | 70 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/core/tools/blueos_startup_update/blueos_startup_update.py b/core/tools/blueos_startup_update/blueos_startup_update.py index 1807956f84..7dd3c29cda 100755 --- a/core/tools/blueos_startup_update/blueos_startup_update.py +++ b/core/tools/blueos_startup_update/blueos_startup_update.py @@ -7,7 +7,7 @@ import time import subprocess from pathlib import Path -from typing import List, Tuple +from typing import List, Optional, Tuple import configparser import appdirs @@ -23,6 +23,10 @@ BOOT_LOOP_DETECTOR = "/root/.config/.boot_loop_detector" +# First line of the plain text files Bookworm leaves at /boot once the boot partition moved to +# /boot/firmware. The install scripts match the same line, see install/boards/bcm_27xx.sh +BOOT_STUB_MARKER = "DO NOT EDIT THIS FILE" + # Any change made in this DELTA_JSON dict should be also made # into /bootstrap/startup.json.default too! DELTA_JSON = { @@ -797,6 +801,24 @@ def setup_ssh() -> bool: return False +def locate_boot_file(candidates: List[str]) -> Optional[str]: + """Locate a boot file, refusing the plain text stub Bookworm leaves at /boot once the boot + partition moved to /boot/firmware. Writing to that stub configures nothing, so the install + scripts refuse it by the same marker and the startup patches have to refuse it the same way, + or they write the board section into a file the firmware never reads and reboot into it.""" + located = locate_file(candidates) + if located is None: + return None + # The marker is the stub's whole first line, and only that line, so a real config.txt that happens + # to carry the phrase in a comment keeps being patched. Read it with run_command rather than + # load_file: this runs before the run_command_is_working() check below, and load_file raises when + # the host connection is not up yet, which would take every patch down with it + if run_command(f'head -n 1 "{located}"', False).stdout.strip() == BOOT_STUB_MARKER: + logger.error(f"{located} is the Bookworm boot stub, the boot partition is not mounted") + return None + return located + + def main() -> int: start = time.time() # check if boot_loop_detector exists @@ -810,9 +832,9 @@ def main() -> int: # pylint: disable=global-statement global config_file global cmdline_file - config_file = locate_file(["/boot/firmware/config.txt", "/boot/config.txt"]) + config_file = locate_boot_file(["/boot/firmware/config.txt", "/boot/config.txt"]) logger.info(f"config.txt found at {config_file}") - cmdline_file = locate_file(["/boot/firmware/cmdline.txt", "/boot/cmdline.txt"]) + cmdline_file = locate_boot_file(["/boot/firmware/cmdline.txt", "/boot/cmdline.txt"]) logger.info(f"cmdline.txt found at {cmdline_file}") if not run_command_is_working(): diff --git a/core/tools/blueos_startup_update/test_blueos_startup_update.py b/core/tools/blueos_startup_update/test_blueos_startup_update.py index e4d6722f69..6843462f96 100644 --- a/core/tools/blueos_startup_update/test_blueos_startup_update.py +++ b/core/tools/blueos_startup_update/test_blueos_startup_update.py @@ -1,3 +1,4 @@ +import os import re import subprocess import sys @@ -757,3 +758,72 @@ def test_startup_patches_prefer_the_bookworm_boot_partition(boot_file: str) -> N # fallback of the two. candidates = re.findall(rf'"(/boot[^"]*{re.escape(boot_file)})"', source) assert candidates == [f"/boot/firmware/{boot_file}", f"/boot/{boot_file}"] + + +def fake_host_run_command(host_files: Dict[str, str]) -> Callable[..., "subprocess.CompletedProcess[str]"]: + """A run_command that answers the patcher's own probe from a fake host, so the guard is exercised + as written instead of against a stubbed return value.""" + + def run_command(command: str, _check: bool = True, _log_output: bool = True) -> "subprocess.CompletedProcess[str]": + match = re.fullmatch(r'head -n 1 "(?P[^"]+)"', command) + assert match, f"the guard ran an unexpected command on the host: {command}" + lines = host_files.get(match["path"], "").splitlines() + return subprocess.CompletedProcess(command, 0, stdout=f"{lines[0]}\n" if lines else "", stderr="") + + return run_command + + +def locate_boot_file_on_host(host_files: Dict[str, str], candidates: List[str]) -> Optional[str]: + located = next((path for path in candidates if path in host_files), None) + with patch.object(blueos_startup_update, "locate_file", lambda _candidates: located): + with patch.object(blueos_startup_update, "run_command", fake_host_run_command(host_files)): + return blueos_startup_update.locate_boot_file(candidates) + + +@pytest.mark.parametrize("boot_file", ["config.txt", "cmdline.txt"]) +def test_startup_patcher_refuses_the_bookworm_boot_stub(boot_file: str) -> None: + # When /boot/firmware is not mounted on a Bookworm host, find(1) settles on the /boot stub. The + # install scripts refuse it by its marker; the patcher has to as well, or it writes the board + # section into a file the firmware never reads, reboots, and finds its own work in place forever. + candidates = [f"/boot/firmware/{boot_file}", f"/boot/{boot_file}"] + at_boot = f"/boot/{boot_file}" + + assert locate_boot_file_on_host({at_boot: BOOT_STUBS[boot_file]}, candidates) is None + + # A real config.txt at /boot is a Bullseye boot partition, not a stub, and has to be accepted + assert locate_boot_file_on_host({at_boot: BULLSEYE_STOCK_CONFIG_TXT}, candidates) == at_boot + + # The marker in a comment does not make a mounted boot partition a stub. Refusing it would leave + # the board with no overlays, no cgroups and no OTG ethernet while main() still reported success, + # which is the very bug this guard exists to prevent + managed = f"# DO NOT EDIT THIS FILE - it is managed by provisioning\n{BULLSEYE_STOCK_CONFIG_TXT}" + assert locate_boot_file_on_host({at_boot: managed}, candidates) == at_boot + + +def test_startup_patcher_resolves_both_boot_files_through_the_stub_guard() -> None: + # The guard only protects the board if main() resolves through it. Reverting these two call sites + # to locate_file reproduces the original bug verbatim, so pin them by driving main() itself. + host_files = {f"/boot/{name}": stub for name, stub in BOOT_STUBS.items()} + + with patch.object(blueos_startup_update, "locate_file", lambda candidates: candidates[-1]), patch.object( + blueos_startup_update, "run_command", fake_host_run_command(host_files) + ), patch.object(blueos_startup_update, "run_command_is_working", lambda: False), patch.object( + blueos_startup_update, "BOOT_LOOP_DETECTOR", "/tmp/a-boot-loop-detector-that-is-not-there" + ), patch.object( + blueos_startup_update, "config_file", None + ), patch.object( + blueos_startup_update, "cmdline_file", None + ), patch.dict( + os.environ, {"GIT_DESCRIBE_TAGS": "1.4.0-1-gabcdef0"} + ): + assert blueos_startup_update.main() == 0 + + resolved_files = [ + ("config.txt", blueos_startup_update.config_file), + ("cmdline.txt", blueos_startup_update.cmdline_file), + ] + for name, resolved in resolved_files: + assert resolved is None, ( + f"main() resolved {name} to {resolved!r} on a host whose boot partition is not mounted, so the " + "board section goes into a file the firmware never reads and the Navigator never appears" + )