docker: split host deps into per-group apt layers in generated Dockerfile#9999
Open
rpardini wants to merge 11 commits into
Open
Conversation
The add_host_dependencies hook filled a space-separated string that was word-split into host_dependencies[]. Make EXTRA_BUILD_DEPS a real array and merge it with a quoted expansion instead, dropping the SC2206 hack. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… hooks Convert all in-tree add_host_dependencies__* hooks from space-separated string appends to proper bash array appends, matching the new EXTRA_BUILD_DEPS array contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rray Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduces HOSTDEP_GROUP_DELIMITER/HOSTDEP_DEFAULT_GROUP and strips the optional "group::package" prefix before dpkg matching / apt install. Prep for Docker layer grouping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restructure the built-in dependency list into per-group blocks (native-toolchain, build-tools, imaging, fs-tools, compression, emulation, python, cross-arm, cross-other, clang, core) so the Docker image can be split into multiple layers. Unprefixed entries fall into 'core'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…file Instead of a single huge 'apt-get install' RUN (one giant image layer), group host_dependencies by their 'group::' prefix and emit one RUN per group, ordered heaviest/most-stable first. Produces multiple layers that Docker can pull and cache in parallel. apt update is its own layer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Assign each extension's host deps to a logical group (fs-tools, emulation, clang, cross-arm/-other, build-tools, native-toolchain) so they land in the matching Docker layer. Unprefixed deps (e.g. openssh-client) stay in 'core'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Real 'docker history' showed lopsided layers: cross-other 562MB, cross-arm 422MB, emulation 496MB, while compression was 5.5MB. Rebalance: - split cross compilers per family: cross-amd64 / cross-arm64 / cross-armhf / cross-other (riscv/loong/or1k) -> even ~200-280MB band - split the heavy 'qemu' (qemu-user-static) out of 'emulation'; move tiny binfmt-support/arch-test and the compression tools into 'core' - dedupe packages in the generator (qemu-utils was listed 4x, etc.) clang (libclang-dev, 633MB) and qemu (~490MB) are single packages and remain unavoidable floors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Real 'docker history' showed native-toolchain=334MB poles. - rustc/cargo pull libstd-rust-dev; move them to a new 'rust' group ordered *after* native-toolchain (so shared gcc/build-essential stay there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughIntroduces a ChangesGrouped host dependency system and EXTRA_BUILD_DEPS array migration
Sequence Diagram(s)sequenceDiagram
participant Extension
participant adaptative_prepare_host_dependencies
participant docker_create_dockerfile_apt_install_runs
participant install_host_side_packages
Extension->>adaptative_prepare_host_dependencies: EXTRA_BUILD_DEPS+=(group::package)
adaptative_prepare_host_dependencies->>adaptative_prepare_host_dependencies: append EXTRA_BUILD_DEPS array to host_dependencies
adaptative_prepare_host_dependencies->>docker_create_dockerfile_apt_install_runs: host_dependencies[] with group::package entries
docker_create_dockerfile_apt_install_runs->>docker_create_dockerfile_apt_install_runs: split on ::, deduplicate, order groups
docker_create_dockerfile_apt_install_runs-->>adaptative_prepare_host_dependencies: DOCKERFILE_APT_INSTALL_RUNS (per-group RUN blocks)
adaptative_prepare_host_dependencies->>install_host_side_packages: host_dependencies[]
install_host_side_packages->>install_host_side_packages: strip group:: prefix → bare package
install_host_side_packages-->>adaptative_prepare_host_dependencies: packages installed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
size/large
Member
Author
|
Those TODO's were there since forever. I wanted to split the build image layers for years now. Claude to the rescue. (Having |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/functions/host/prepare-host.sh (1)
216-220: 💤 Low valueMissing group prefix for
python3-setuptools.This conditional addition lacks the
python::group prefix used for other Python packages (lines 211, 239). It will default to thecoregroup instead of being grouped with other Python dependencies.For consistency with the rest of the Python packages in this file:
Suggested fix
if [[ 'tag:v2022.04' == "${BOOTBRANCH:-}" || 'tag:v2022.07' == "${BOOTBRANCH:-}" ]]; then display_alert "Adding package to 'host_dependencies'" "python3-setuptools" "info" - host_dependencies+=("python3-setuptools") + host_dependencies+=("python::python3-setuptools") fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/functions/host/prepare-host.sh` around lines 216 - 220, The python3-setuptools package being added to host_dependencies in the conditional block (when BOOTBRANCH matches tag:v2022.04 or tag:v2022.07) is missing the python:: group prefix that other Python packages use elsewhere in this file (like on lines 211 and 239). Add the python:: prefix to the python3-setuptools string in the host_dependencies array assignment so it is properly grouped with other Python dependencies instead of defaulting to the core group.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/functions/host/docker.sh`:
- Around line 260-287: The deduplication check at line 268 using pkg_seen locks
each package to the first group that encounters it, but the preferred_order
ranking applied later (lines 280-287) only affects layer order, not package
group assignment. To fix this, implement a two-pass approach: first collect all
unique packages and track all groups they appear in without immediate
deduplication, then in a second pass reassign package ownership based on
preferred_order ranking before building the final_order array. This ensures
shared heavy dependencies like llvm-dev land in their intended earlier layers
according to the notes on lines 276-279.
---
Nitpick comments:
In `@lib/functions/host/prepare-host.sh`:
- Around line 216-220: The python3-setuptools package being added to
host_dependencies in the conditional block (when BOOTBRANCH matches tag:v2022.04
or tag:v2022.07) is missing the python:: group prefix that other Python packages
use elsewhere in this file (like on lines 211 and 239). Add the python:: prefix
to the python3-setuptools string in the host_dependencies array assignment so it
is properly grouped with other Python dependencies instead of defaulting to the
core group.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7cf8f6a5-8f59-4a15-abaf-05078cb09b90
📒 Files selected for processing (24)
extensions/arm64-compat-vdso.shextensions/c-plus-plus-compiler.shextensions/cleanup-space-final-image.shextensions/fs-btrfs-support.shextensions/fs-cryptroot-support.shextensions/fs-f2fs-support.shextensions/fs-nilfs2-support.shextensions/fs-xfs-support.shextensions/image-output-abl.shextensions/image-output-ovf.shextensions/image-output-qcow2.shextensions/image-output-vhd-azure.shextensions/image-output-vhdx.shextensions/kernel-rust.shextensions/lvm.shextensions/mtkflash.shextensions/rkdevflash.shextensions/sunxi-tools.shlib/functions/cli/cli-docker.shlib/functions/cli/cli-patch.shlib/functions/general/python-tools.shlib/functions/host/docker.shlib/functions/host/host-utils.shlib/functions/host/prepare-host.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docker: split host deps into per-group apt layers in generated Dockerfile
Summary by CodeRabbit