Skip to content

Conversation

@yangsong-cnyn
Copy link
Contributor

@yangsong-cnyn yangsong-cnyn commented Dec 16, 2024

This PR updates the Raspbian OS from Debian 10 Buster to Debian 12 Bookworm. This change is necessary to add support for the Raspberry Pi 5, which requires the Bookworm release.

This OS update introduces several related changes:

  1. Python Virtual Environment: Debian 12 enforces PEP 668, which prevents the installation of Python packages into the system-wide environment using pip. To comply with this, the setup script now creates and uses a Python virtual environment (venv) for installing dependencies.

  2. Updated China Mirror: The APT source for the Tsinghua mirror has been updated to point to the bookworm repositories, ensuring users in China can download packages efficiently.

  3. Prefix Delegation Configuration: For Thread 1.4 builds, the configuration is updated to use the OpenThread DHCPv6 PD client (OTBR_DHCP6_PD_CLIENT=openthread), and systemd-networkd with its internal PD client disabled, for Thread PD.

@yangsong-cnyn yangsong-cnyn marked this pull request as draft December 16, 2024 02:03
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 6 times, most recently from 6e71980 to 767ecf1 Compare January 5, 2025 12:36
@yangsong-cnyn yangsong-cnyn marked this pull request as ready for review January 6, 2025 01:59
@yangsong-cnyn yangsong-cnyn marked this pull request as draft January 6, 2025 01:59
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 3 times, most recently from a449898 to b2cf35c Compare January 10, 2025 04:13
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 2 times, most recently from ec690b2 to fb1d76a Compare February 24, 2025 02:49
@EskoDijk
Copy link

Issue #92 can be linked to this PR! (I don't have rights to do this.)

@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 7 times, most recently from 6620272 to 3f3b962 Compare June 9, 2025 02:32
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Raspbian image from Debian Buster to Debian 12 Bookworm, primarily to add support for Raspberry Pi 5. The changes involve updating the image URL, adjusting package installations, and modifying APT sources for the new distribution.

Overall, the changes align with the goal of supporting Bookworm. However, there are a few critical and medium severity issues that need addressing, particularly concerning the validity of the new image URL and a typo in a script. Additionally, some dependency changes require clarification.

Summary of Findings

  • Potentially Invalid Raspberry Pi OS Image URL: The RASPIOS_URL in script/make-raspbian.bash (line 91) points to an image dated 2024-11-19, which appears to be a future date. This URL might be invalid and needs verification to ensure it points to a stable, released Bookworm image.
  • Critical Typo in Script Execution Permissions: A typo chmog instead of chmod exists in script/otbr-setup.bash (line 257). This will prevent /etc/rc.local from being made executable, which is a critical failure for its intended operation.
  • Unclear Rationale for DHCPV6_PD_REF=1 Removal: The DHCPV6_PD_REF=1 option was removed in script/otbr-setup.bash (e.g., former line 178) for REFERENCE_RELEASE_TYPE="1.4" builds. Clarification is needed on whether this is intentional due to deprecation, incompatibility with Bookworm, or other reasons.
  • Unclear Rationale for PyGObject Removal: The PyGObject Python package installation was removed from script/otbr-setup.bash (former line 235). The reason for this removal (e.g., no longer needed, transitive dependency, conflicts) should be clarified.
  • Risk from Unpinned nrfutil Version: The nrfutil Python package version was unpinned in script/otbr-setup.bash (line 247). This could lead to future build instabilities if newer nrfutil versions introduce breaking changes. Confirmation of intent and testing is advised.
  • Python Virtual Environment Adoption: The introduction of a Python virtual environment (.venv-otbr) in script/otbr-setup.bash (lines 225-226) is a good practice for dependency management.
  • Updated wiringpi Installation Method: The change in script/otbr-setup.bash (lines 250-251) to install wiringpi via direct download and dpkg is a necessary adaptation for Debian Bookworm, as wiringpi is often not available in standard APT repositories for newer OS versions.
  • Minor Stylistic Changes (Not commented due to settings): Some minor stylistic changes were made, such as adding blank lines in README.md (lines 48, 56) and adjusting spacing in a case statement in script/make-firmware.bash (line 187). These improve readability but were not commented on directly due to review settings instructing to only comment on medium, high, or critical issues.

Merge Readiness

This pull request makes important updates for Debian Bookworm support. However, due to the critical issues identified (potentially invalid image URL and a script typo), and several medium-severity items requiring clarification, I recommend that these changes be addressed before merging. I am unable to approve pull requests, but based on this review, changes are requested. Please ensure these points are resolved and the build process is thoroughly tested with the corrected scripts and a valid image URL.

@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 2 times, most recently from 987a1bb to f40f682 Compare June 13, 2025 04:36
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 4 times, most recently from e2debc4 to e99c509 Compare September 12, 2025 06:25
@EskoDijk
Copy link

The CI build error here looks like the one I've commented on here: #126 (comment) ; it now affects builds on every OS I tried except Ubuntu 20.04.
~Two weeks ago it still worked on newer OSes for me. And it's not due to recent code changes: even older commits don't build anymore on newer OSes.

@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 3 times, most recently from 28805ff to 2f36ec4 Compare September 24, 2025 03:03
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 2 times, most recently from 9203827 to 8de02ca Compare November 20, 2025 08:22
@yangsong-cnyn yangsong-cnyn changed the title Update the Rasbian image to Debian 12 Bookworm to support Raspi 5 update the Raspbian to Debian 12 Bookworm to support Raspi 5 Nov 20, 2025
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 7 times, most recently from aabc1ff to 78a7681 Compare November 27, 2025 02:40
@yangsong-cnyn yangsong-cnyn force-pushed the raspbian_bookworm branch 2 times, most recently from 9e47a82 to 0f5656f Compare December 8, 2025 03:52
@yangsong-cnyn yangsong-cnyn requested review from bukepo, librasungirl, superwhd and xuyirio and removed request for superwhd December 12, 2025 02:20
@yangsong-cnyn yangsong-cnyn marked this pull request as ready for review December 12, 2025 02:21
@yangsong-cnyn yangsong-cnyn deleted the raspbian_bookworm branch December 12, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants