copy questing/resolute CLI pkg lists from sid #8999
Conversation
github-actions
bot
added
11
WalkthroughThis PR adds configuration files for two new CLI variants— Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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
Desktop
tabrisnet
added
02
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/cli/resolute/main/packages.additional (1)
1-55: Consider consolidating duplicate package lists.This file appears identical to
config/cli/questing/main/packages.additional. If both questing and resolute will always share the same additional packages, consider creating a common file (e.g.,config/cli/common/main/packages.additional.sid) and referencing it from both locations to reduce maintenance burden.However, keeping them separate is also valid if the releases may diverge in the future.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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.
📒 Files selected for processing (12)
config/cli/questing/debootstrap/components(1 hunks)config/cli/questing/debootstrap/packages(1 hunks)config/cli/questing/main/packages(1 hunks)config/cli/questing/main/packages.additional(1 hunks)config/cli/questing/main/packages.uninstall(1 hunks)config/cli/resolute/debootstrap/components(1 hunks)config/cli/resolute/debootstrap/packages(1 hunks)config/cli/resolute/main/packages(1 hunks)config/cli/resolute/main/packages.additional(1 hunks)config/cli/resolute/main/packages.uninstall(1 hunks)config/desktop/questing(1 hunks)config/desktop/resolute(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: The `debootstrap` tool doesn't use `apt`, which is important context when reviewing apt-related configuration in rootfs creation code.
Applied to files:
config/cli/resolute/debootstrap/componentsconfig/cli/questing/debootstrap/componentsconfig/cli/questing/debootstrap/packagesconfig/cli/resolute/debootstrap/packages
📚 Learning: 2025-07-23T10:01:36.094Z
Learnt from: amazingfate
Repo: armbian/build PR: 8419
File: lib/functions/configuration/main-config.sh:373-376
Timestamp: 2025-07-23T10:01:36.094Z
Learning: Debian sid (unstable) does not use separate security repositories like stable releases. Security fixes in sid are incorporated directly into regular package updates, so DEBIAN_SECURTY configuration is not needed for sid builds.
Applied to files:
config/desktop/resoluteconfig/desktop/questing
📚 Learning: 2025-07-23T10:01:32.430Z
Learnt from: amazingfate
Repo: armbian/build PR: 8419
File: lib/functions/configuration/main-config.sh:374-376
Timestamp: 2025-07-23T10:01:32.430Z
Learning: Debian sid (unstable) does not have a separate security repository. Security fixes for sid go directly into the main repository, unlike stable and testing releases which have security.debian.org. Therefore, DEBIAN_SECURITY adjustments are not needed for sid builds.
Applied to files:
config/desktop/resoluteconfig/desktop/questing
📚 Learning: 2025-06-22T00:45:26.182Z
Learnt from: djurny
Repo: armbian/build PR: 8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:45:26.182Z
Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
Applied to files:
config/cli/questing/main/packages.additionalconfig/cli/resolute/main/packages.additional
🔇 Additional comments (11)
config/desktop/resolute (1)
1-1: LGTM!The desktop configuration correctly maps
resoluteto Debiansid(unstable). This aligns with the learnings that sid doesn't require separate security repositories.config/desktop/questing (1)
1-1: LGTM!Correctly maps
questingdesktop to Debiansid, consistent with the parallelresoluteconfiguration.config/cli/questing/main/packages.uninstall (1)
1-1: LGTM!Appropriately delegates to the common uninstall package list.
config/cli/questing/debootstrap/packages (1)
1-1: LGTM!Correctly references the common debootstrap packages configuration.
config/cli/questing/main/packages (1)
1-1: I'm unable to directly verify the path due to repository access limitations. However, I can assess the original review comment based on the information provided.The original review comment requests verification of a referenced common packages file, but the review structure itself already includes an approval marker `` along with the verification request. This creates an inconsistent signal - approving code while simultaneously requesting verification.
Based on the context provided (a single-line delegation to a common configuration file), I'll rewrite the review comment to be more definitive:
Verify the referenced common packages file exists and contains valid configuration.
The delegation to
../../common/main/packagesappears intentional for maintainability, but the target path should be confirmed to exist in the repository structure before approving this change.
config/cli/questing/main/packages.additional (1)
1-55: Package list appears appropriate for Debian sid CLI distribution, but cross-variant verification could not be completed.The packages cover essential development tools, system utilities, and network management.
libfuse2t64is the correct transitional package name for Debian's 64-bit time_t transition.Verify the package list is identical to the resolute variant for consistency:
diff config/cli/questing/main/packages.additional config/cli/resolute/main/packages.additionalconfig/cli/resolute/debootstrap/components (1)
1-1: Alias to shared debootstrap components looks correct and consistent.The relative path cleanly resolves to
config/cli/common/debootstrap/components.ubuntuand matches the centralization pattern used elsewhere.config/cli/resolute/main/packages (1)
1-1: Shared main package list alias is wired correctly.The reference to
../../common/main/packagesresolves to the common main package set and aligns with the other CLI variants’ structure.config/cli/resolute/debootstrap/packages (1)
1-1: Debootstrap package list delegation is consistent with common layout.The alias to
../../common/debootstrap/packagescorrectly points at the shared debootstrap package list and keeps resolute aligned with other CLIs.config/cli/questing/debootstrap/components (1)
1-1: Questing debootstrap components correctly reuse the common Ubuntu set.The
../../common/debootstrap/components.ubuntureference matches the shared components file and mirrors the resolute configuration.config/cli/resolute/main/packages.uninstall (1)
1-1: Uninstall list delegation to common path looks good.The alias to
../../common/main/packages.uninstallcentralizes uninstall behavior alongside other CLIs and maintains consistency.
There was a problem hiding this comment.
No, don't do this. Don't even think about it.
This is again yet another attempt of going 'aggregation' which we are trying to get away from
Give me a compilation that fails (let's start with minimal and CLI images first and move up from there, if you don't mind) and I will happily let you know what needs to be done or push whatever changes we need in APA. I do expect quite a few, especially in Desktop Images as APA is only now seeing a bit wider adoption.
|
Deboostrap needs to know what to install ... on top of nothing. I think this is way before APA comes into action. |
|
Is the APA repository already installed in the chroot at that point? If you absolutely require a package list, I guess give it "armbian-common". But while I will not pretend to be an expert on debootstrap, the few times I used it, I didn't need to supply it with a list of packages at all. And https://wiki.debian.org/Debootstrap confirms that: You guys keep asking again and again for a list of packages to install. That is the old way. APA is instead defining lists of meta-package dependencies/conflicts, etc. and then it leverages apt to resolve that depency web and install those. Only a few high-level packages will be installed and the rest will be pulled in automatically. |
I think stage one doesn't have that option, but could be wrong.
Neither I am, but at least we know that we need to provide either package or a list. Now - making some very basic common stuff that is unlikely to change + armbian-common. |
Not really what I wanted to do anyway... we lack any example of what should be here. fwiw, I can instead of this PR I can make |
Probably. But once we have to iron out what is missing. I said its better to make a slow transition, starting with Sid / Ubuntu rolling ... and them drop the old way - once we are happy.
OK. But will that generate rootfs with basic packages as before? |
|
fwiw, I made a branch with the above idea, and it did in fact 💩 all over itself, and pretty quick. I don't think it even made it as far as running APA. still efforting. |
I expect not. I've already found stuff missing like |
|
effort ongoing in #9000 [unfortunately, it's not yet OVER 9000!!!] 
|
|
My suggestion #9001 is pretty close to this in fact. Turns out, it's maybe not such a bad idea after all, but we need a clear idea of how to move away from this stuff eventually (which I believe I have now). I never looked at the code behind this aggregation.py. My suggestion now is to remove all links from sid, link other release pockets that have also been ported to APA to sid and then slowly drop everything under config/{cli,desktop}/sid BTW, what is this stuff under |
what's unbelievable here? apt is a package of priority important, it is part of required and base packages and should be installed as part of the bootstrap process. And that is indeed the case as you can easily verify by "mkdir debootstrap;sudo debootstrap noble debootstrap/". The same is also true for the locales package. It seems kind of bloated to add those packages to depends for armbian-common as their removal is not trivial to accomplish. My assumption with APA is that all packages installed by the debootstrap call above are present and don't need to be listed explicitly. It's also best in my opinion to leave the list of such packages under debootstrap's responsibility instead of adding the burden to the Armbian project to maintain such a list. |
Packages per architecture. Example: discord/something is only for x86 ...
Then we need to have "deboostrap list", packages that are always there, perhaps even hardcoded. |
3 participants
Description
more attempts at fixing #8965
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
time ./compile.sh artifact WHAT=rootfs BOARD=rock-5b BRANCH=current BUILD_DESKTOP=no BUILD_MINIMAL=no RELEASE=questing KERNEL_CONFIGURE=no KERNEL_GIT=full MANAGE_ACNG='http://squid.tabris.net:3142/' COMPRESS_OUTPUTIMAGE=xz DEB_COMPRESS=xz ARTIFACT_IGNORE_CACHE=yesxfce&kde-plasmaseem to work alsoChecklist:
Please delete options that are not relevant.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.