Skip to content

Conversation

leggewie
Copy link
Collaborator

let debootstrap (which is the real dependency we have) pull in wget instead of having a hard and direct depency on it

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1118384#10

let debootstrap (which is the real dependency we have) pull in
wget instead of having a hard and direct depency on it

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1118384#10
@leggewie leggewie requested a review from a team as a code owner October 19, 2025 04:22
@github-actions github-actions bot added size/small PR with less then 50 lines 11 Milestone: Fourth quarter release Needs review Seeking for review Framework Framework components labels Oct 19, 2025
Copy link
Contributor

coderabbitai bot commented Oct 19, 2025

Walkthrough

This pull request modifies the host preparation dependencies in lib/functions/host/prepare-host.sh. The changes include adding debootstrap to the adaptive host dependencies list and removing wget from the downloaders group, while retaining aria2 and axel as alternative downloaders alongside curl.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The changes are localized to a single configuration file with straightforward dependency list modifications. Review complexity is minimal as it involves verifying that the added debootstrap package is appropriate and confirming that the wget removal is intentional and compatible with the retained downloader alternatives.

Possibly related PRs

Suggested labels

Ready to merge, 08

Suggested reviewers

  • igorpecovnik
  • rpardini
  • EvilOlaf

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "prepare-host.sh: correct host dependency chain (Closes: #8779)" is directly related to the main change in the pull request. The changeset reorganizes host dependencies by adding debootstrap and removing the direct wget dependency, allowing debootstrap to handle wget as a transitive dependency. The title accurately captures this reorganization as a "correction" of the dependency chain, which is clear, concise, and specific enough for a teammate to understand the primary change when scanning history.
Description Check ✅ Passed The description is directly related to the changeset. It explains that debootstrap (identified as the real dependency) should pull in wget instead of having a hard direct dependency on it, which matches the changes shown in the summary (adding debootstrap and removing direct wget dependency). The description provides clear context for why the changes are being made, even though it is brief. The reference to the Debian bug report further supports the relevance and intent of the modification.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8f6e17e and 0d67ae8.

📒 Files selected for processing (1)
  • lib/functions/host/prepare-host.sh (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/functions/host/prepare-host.sh (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (1)
  • reversion_armbian-bsp-cli_deb_contents (255-292)
🔇 Additional comments (2)
lib/functions/host/prepare-host.sh (2)

186-186: Explicit debootstrap dependency improves clarity.

Adding debootstrap as an explicit dependency is correct since it's the actual tool being used by Armbian. This allows wget to become a transitive dependency pulled in by debootstrap, rather than a direct dependency, following proper dependency chain semantics.

Please verify that debootstrap is actively used in the Armbian build process (e.g., grep for debootstrap invocations in the codebase to confirm this is a true dependency, not a false positive).


214-214: Wget removal is correct and safe.

The debootstrap package declares a dependency on wget, so removing it from explicit host dependencies is appropriate. Code verification confirms:

  • No direct wget calls occur in lib/functions/host/prepare-host.sh before install_host_dependencies executes (line 92)
  • debootstrap remains in the host_dependencies array (line 186) and is installed immediately, making wget available to all downstream code
  • Extension scripts using wget run after the build process begins and have access to the installed debootstrap package

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@igorpecovnik
Copy link
Member

igorpecovnik commented Oct 19, 2025

let debootstrap

This would be installing a package we don't actually use in the process. The one that comes with a distro might - in theory - have different dependencies as the one we actually use - from git. I would say we rather continue managing dependencies on our own. There are not many of them and it worked well for years.

We could only add a note to the code why / where wget is needed.

@leggewie
Copy link
Collaborator Author

That confuses me. Are you saying that somewhere you download a wget from git(hub, I assume)?

@igorpecovnik
Copy link
Member

you download a wget from git(hub, I assume)?

Yes, this is absolutely necessary as our build framework must be able to generate latest distributions, which are usually absent or broken with deboostrap that comes with stable / LTS distro, which is used as a stable base to assemble everything.

https://github.com/armbian/build/blob/main/lib/functions/rootfs/rootfs-create.sh#L60-L78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

11 Milestone: Fourth quarter release Framework Framework components Needs review Seeking for review size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

2 participants