desktops: optional APT pin preferences in repo block#838
Merged
Conversation
Some vendor archives (e.g. SpacemiT for bianbu on the K1 RISC-V
platform) must outrank the distro archive via /etc/apt/preferences.d
pins, otherwise apt resolves the wrong versions and the DE install
pulls broken ports of platform packages. Bianbu's upstream
instructions pin three (origin, suite) pairs at priority 1100-1200.
Add an optional structured `preferences:` list under `repo:` in the
desktop YAML schema, so the pin file is derived the same way the
sources.list.d entry is — one place in the YAML, one file on disk,
removed on uninstall.
- parse_desktop_yaml.py: emit DESKTOP_REPO_PREFS_COUNT and indexed
DESKTOP_REPO_PREFS_<n>_{ORIGIN,SUITE,PRIORITY} for each entry;
skip malformed entries with a stderr warning (no silent drops)
- module_desktop_repo.sh: when COUNT > 0, write one stanza per
entry to /etc/apt/preferences.d/<de>. Only the parsed, validated
fields land in the file — raw YAML strings are never shell-eval'd
- module_desktops.sh: remove path deletes the preferences file
(de-name regex-guarded). Leaving it behind would keep a vendor
archive outranking the distro globally after the DE is gone
- bianbu.yaml: add the three SpacemiT pins with a WHY comment
github-actions
Bot
added
size/medium
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughThe desktop module now writes APT source entries to Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
5 tasks
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/modules/desktops/module_desktop_repo.sh`:
- Around line 59-80: The pin-file creation must be executed only when the
corresponding repo was successfully set up and must fail fast on any write
error: wrap the block that reads DESKTOP_REPO_PREFS_COUNT so it only runs when
the repo guard variable (e.g., the same flag set by add_desktop_repo or
DESKTOP_REPO_ADDED) indicates the repo was added, write to a temp file (e.g.,
"${pref_file}.tmp") inside the loop and check each write's exit status (or use a
single here-doc redirected to the temp file and check its result), then
atomically mv the temp to "${pref_file}" and if any write/mv fails call exit 1;
reference DESKTOP_REPO_PREFS_COUNT, pref_file and the loop variables
(origin_var, suite_var, prio_var) when locating the code to modify.
In `@tools/modules/desktops/scripts/parse_desktop_yaml.py`:
- Around line 330-331: The current validation in the block that checks origin,
suite and priority (the if that currently uses isinstance(priority, int))
wrongly accepts booleans and non-positive numbers; update that conditional to
explicitly reject booleans (i.e., ensure not isinstance(priority, bool)) and
enforce positive values (priority must be > 0), so the combined check verifies
origin and suite are present, priority is an int and not a bool, and priority >
0 before proceeding (adjust the same conditional used to print the error and
prevent generating Pin-Priority output).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 879906d3-db49-4238-a960-8ef8d978d708
📒 Files selected for processing (4)
tools/modules/desktops/module_desktop_repo.shtools/modules/desktops/module_desktops.shtools/modules/desktops/scripts/parse_desktop_yaml.pytools/modules/desktops/yaml/bianbu.yaml
Review feedback on PR #838: 1. The pin-file block ran even when the repo setup was skipped (empty DESKTOP_REPO_URL), so a DE with `preferences:` but no matching `url`/`key_url`/`keyring` would create an orphan file under /etc/apt/preferences.d/. Nest the block inside the existing `[[ -n \$DESKTOP_REPO_URL ... ]]` guard so pins only land when the matching archive was actually configured. 2. The previous `: > \$pref_file` + `>>` loop left a truncated file on mid-write failure, which apt would misparse rather than ignore. Write to \${pref_file}.tmp, check each printf's exit status, and atomically `mv` on success. Any failure returns 1 (not exit 1 — that would kill armbian-config). No change to the emitted file content; smoke-tested against bianbu, still byte-identical to SpacemiT's documented pin file.
Review feedback on PR #838: - isinstance(priority, int) also accepts bools because bool subclasses int in Python. `priority: true` would slip through and render `Pin-Priority: True`, which apt refuses — warn and drop the entry instead of letting garbage reach the pin file. - The inline comment already advertises "priority is a positive integer", but the check didn't enforce it. Add `priority > 0` so `priority: 0` / `priority: -5` no longer produce silent `Pin-Priority: 0` stanzas. Smoke-tested: a fixture with bool, zero, negative, string, and empty-origin variants now emits five stderr warnings and keeps only the valid entry. Bianbu's 1200/1100/1100 unaffected.
The sources.list.d line was hard-coded to `<url> ${DISTROID} main`,
which doesn't fit vendor archives whose layout is not `<codename>
main`. SpacemiT's K1 RISC-V archive pins a frozen snapshot per
Ubuntu release and mirrors all four Ubuntu components — the target
line is:
deb [signed-by=...] https://archive.spacemit.com/bianbu/ \
noble/snapshots/v2.2 main universe restricted multiverse
Add two optional fields on the `repo:` block, plus per-release
overrides so the suite can vary across releases (bianbu on noble
uses v2.2; resolute uses v3.0).
- parse_desktop_yaml.py: emit DESKTOP_REPO_SUITE and
DESKTOP_REPO_COMPONENTS. Resolution order per-release override ->
repo default -> release codename (for suite) or ["main"] (for
components). Both regex-validated (`^[A-Za-z0-9._/-]+$` and
`^[A-Za-z0-9._-]+$`) so nothing shell-parseable reaches the
source file; invalid values warn and fall back to defaults.
- module_desktop_repo.sh: substitute ${DESKTOP_REPO_SUITE} /
${DESKTOP_REPO_COMPONENTS} into the source line, with
${DISTROID} / "main" as bash-side fallbacks for older YAMLs
that don't emit the new vars.
- bianbu.yaml: switch to the bianbu/ base URL, add the four-
component list, add per-release repo_suite for noble (v2.2)
and resolute (v3.0). Drop plucky — SpacemiT does not publish
a plucky snapshot under this archive layout.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/modules/desktops/module_desktop_repo.sh`:
- Around line 68-98: The prefs-writing block only runs when
DESKTOP_REPO_PREFS_COUNT > 0, leaving a stale /etc/apt/preferences.d/${de}
behind when the count becomes 0; update the logic around the
DESKTOP_REPO_PREFS_COUNT check (where pref_file and pref_tmp are computed and
used) to ensure that when DESKTOP_REPO_PREFS_COUNT is empty or equal to 0 you
explicitly remove any existing pref_file (use rm -f on the same pref_file
variable) before returning, so stale pin files are cleaned up when no prefs are
defined.
- Around line 58-61: The here-doc writing to /etc/apt/sources.list.d/${de}.list
using cat needs explicit failure handling: instead of directly redirecting into
the target, write to a temp file (e.g. /etc/apt/sources.list.d/${de}.list.tmp)
using the same content that references DESKTOP_REPO_KEYRING, DESKTOP_REPO_URL,
repo_suite and repo_components, check the write/move exit status, then
atomically mv the temp file into /etc/apt/sources.list.d/${de}.list (or log an
error and exit 1) so any write failure is detected and the script fails fast
rather than leaving a partial state.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2ed686b-d43d-4d52-96df-5287aae7290e
📒 Files selected for processing (3)
tools/modules/desktops/module_desktop_repo.shtools/modules/desktops/scripts/parse_desktop_yaml.pytools/modules/desktops/yaml/bianbu.yaml
✅ Files skipped from review due to trivial changes (1)
- tools/modules/desktops/scripts/parse_desktop_yaml.py
Armbian provides its own kernel and bootloader for the K1 board. Letting SpacemiT's u-boot-spacemit or a pinned linux-image-6.1.15 install would collide with or downgrade Armbian's own packages via the priority-1200 SpacemiT pin.
SpacemiT's bianbu archive fans each snapshot out across five flavors (base, -security, -updates, -porting, -customization) plus a floating bianbu-v<ver>-updates channel. Previously the repo block emitted a single `deb` line, so only one of those was reachable. Accept `repo.suite` / `releases.<r>.repo_suite` as either a string (one source line, existing behavior) or a list of strings (one source line per entry, all sharing url/keyring/ components). - parse_desktop_yaml.py: replace single-value DESKTOP_REPO_SUITE with DESKTOP_REPO_SUITES_COUNT + indexed DESKTOP_REPO_SUITE_<n>. Regex-validate each entry; invalid entries drop with a warning, empty list falls back to [release]. - module_desktop_repo.sh: loop over suites, writing one `deb` line per entry. Temp + atomic mv matches the preferences-file pattern, so a mid-write failure never leaves apt with a partial source list. - bianbu.yaml: list noble's 6 snapshot suites (v2.2) and resolute's 6 (v3.0, symmetric pattern).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tools/modules/desktops/module_desktop_repo.sh (1)
93-123:⚠️ Potential issue | 🟠 MajorRemove the old pin file when the preferences list becomes empty.
If
DESKTOP_REPO_PREFS_COUNTdrops to0—for example after removingrepo.preferencesor when the parser skips every malformed entry—this branch is skipped and the previous/etc/apt/preferences.d/${de}stays active. That leaves vendor pinning enabled on the next run even though the current desktop config no longer declares any pins.🛠️ Proposed fix
- if [[ -n "${DESKTOP_REPO_PREFS_COUNT}" && "${DESKTOP_REPO_PREFS_COUNT}" -gt 0 ]]; then - local pref_file="/etc/apt/preferences.d/${de}" + local pref_file="/etc/apt/preferences.d/${de}" + if [[ -n "${DESKTOP_REPO_PREFS_COUNT}" && "${DESKTOP_REPO_PREFS_COUNT}" -gt 0 ]]; then local pref_tmp="${pref_file}.tmp" local i origin_var suite_var prio_var origin suite prio if ! : > "$pref_tmp"; then echo "Error: cannot create ${pref_tmp}" >&2 return 1 fi for (( i=0; i < DESKTOP_REPO_PREFS_COUNT; i++ )); do origin_var="DESKTOP_REPO_PREFS_${i}_ORIGIN" suite_var="DESKTOP_REPO_PREFS_${i}_SUITE" prio_var="DESKTOP_REPO_PREFS_${i}_PRIORITY" origin="${!origin_var}" suite="${!suite_var}" prio="${!prio_var}" if ! printf 'Package: *\nPin: release o=%s, n=%s\nPin-Priority: %s\n\n' \ "$origin" "$suite" "$prio" >> "$pref_tmp"; then echo "Error: failed to write preferences for ${de}" >&2 rm -f "$pref_tmp" return 1 fi done if ! mv "$pref_tmp" "$pref_file"; then echo "Error: failed to install ${pref_file}" >&2 rm -f "$pref_tmp" return 1 fi + else + rm -f "$pref_file" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/modules/desktops/module_desktop_repo.sh` around lines 93 - 123, Old pin file under /etc/apt/preferences.d/${de} is left behind when DESKTOP_REPO_PREFS_COUNT is zero; detect the empty case and remove the stale pref_file so vendor pinning is not preserved. After computing pref_file (or before the existing if [[ -n "${DESKTOP_REPO_PREFS_COUNT}" && "${DESKTOP_REPO_PREFS_COUNT}" -gt 0 ]]; then ... fi block) add a branch that checks if DESKTOP_REPO_PREFS_COUNT is empty or 0 and if the pref_file exists, then remove it (handle rm failures by logging an error and returning non-zero), ensuring you reference the same pref_file variable and preserve existing error handling for pref_tmp and mv operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/modules/desktops/yaml/bianbu.yaml`:
- Around line 15-27: The APT pin "preferences" block uses wrong suite names
("mantic-spacemit", "mantic-porting", "mantic-customization") so the pins never
match the archive Release Codename; either remove the entire preferences block
or update its suite entries to the actual Release codenames (e.g. replace
"mantic-spacemit" → "noble", "mantic-porting" → "noble-porting",
"mantic-customization" → "noble-customization" — and add any missing codenames
like "noble-updates"/"noble-security" if required) so the pins in the
preferences section will correctly match the archive.
---
Duplicate comments:
In `@tools/modules/desktops/module_desktop_repo.sh`:
- Around line 93-123: Old pin file under /etc/apt/preferences.d/${de} is left
behind when DESKTOP_REPO_PREFS_COUNT is zero; detect the empty case and remove
the stale pref_file so vendor pinning is not preserved. After computing
pref_file (or before the existing if [[ -n "${DESKTOP_REPO_PREFS_COUNT}" &&
"${DESKTOP_REPO_PREFS_COUNT}" -gt 0 ]]; then ... fi block) add a branch that
checks if DESKTOP_REPO_PREFS_COUNT is empty or 0 and if the pref_file exists,
then remove it (handle rm failures by logging an error and returning non-zero),
ensuring you reference the same pref_file variable and preserve existing error
handling for pref_tmp and mv operations.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3433ab4-2a2c-44db-87c8-4fa1968794fa
📒 Files selected for processing (3)
tools/modules/desktops/module_desktop_repo.shtools/modules/desktops/scripts/parse_desktop_yaml.pytools/modules/desktops/yaml/bianbu.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/modules/desktops/scripts/parse_desktop_yaml.py
Matches the tier convention other DEs use:
minimal = bare-bones DE + hardware enablement (firmware, GPU,
kernel modules, opensbi) — needed on every tier so apt
doesn't refuse to bring up the board
mid = fuller desktop (bianbu-desktop + bianbu-desktop-en +
bianbu-standard) and optional camera stack (k1x-cam)
full = extras — Chinese locale (bianbu-desktop-zh) and
development tooling (bianbu-development)
Previously module_desktop_repo only wrote /etc/apt/preferences.d/<de> when DESKTOP_REPO_PREFS_COUNT > 0 and did nothing when the count was 0 or unset. Uninstall already cleans the file, but install is supposed to be declarative — after install, the system must match the YAML. If someone reinstalls a DE after its YAML dropped pins, the old pref file would linger and keep pinning packages the YAML no longer declares. Lift pref_file out of the count branch and add an else that rm -f's the file so install's post-state tracks the YAML either way.
github-actions
Bot
added
size/large
igorpecovnik
added a commit
that referenced
this pull request
Apr 12, 2026
Review feedback on PR #838: 1. The pin-file block ran even when the repo setup was skipped (empty DESKTOP_REPO_URL), so a DE with `preferences:` but no matching `url`/`key_url`/`keyring` would create an orphan file under /etc/apt/preferences.d/. Nest the block inside the existing `[[ -n \$DESKTOP_REPO_URL ... ]]` guard so pins only land when the matching archive was actually configured. 2. The previous `: > \$pref_file` + `>>` loop left a truncated file on mid-write failure, which apt would misparse rather than ignore. Write to \${pref_file}.tmp, check each printf's exit status, and atomically `mv` on success. Any failure returns 1 (not exit 1 — that would kill armbian-config). No change to the emitted file content; smoke-tested against bianbu, still byte-identical to SpacemiT's documented pin file.
igorpecovnik
added a commit
that referenced
this pull request
Apr 12, 2026
Review feedback on PR #838: - isinstance(priority, int) also accepts bools because bool subclasses int in Python. `priority: true` would slip through and render `Pin-Priority: True`, which apt refuses — warn and drop the entry instead of letting garbage reach the pin file. - The inline comment already advertises "priority is a positive integer", but the check didn't enforce it. Add `priority > 0` so `priority: 0` / `priority: -5` no longer produce silent `Pin-Priority: 0` stanzas. Smoke-tested: a fixture with bool, zero, negative, string, and empty-origin variants now emits five stderr warnings and keeps only the valid entry. Bianbu's 1200/1100/1100 unaffected.
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.
Summary
Some vendor archives — concretely, SpacemiT's for bianbu on the K1 RISC-V platform — must outrank the distro archive via
/etc/apt/preferences.d/pins, otherwise apt resolves the wrong versions and the DE install pulls broken ports of platform packages. Bianbu's upstream instructions pin three(origin, suite)pairs at priority 1100–1200.Rather than shipping per-DE pin-writing logic, add an optional structured
preferences:list under the existingrepo:block in the desktop YAML schema. The pin file is then derived the same way thesources.list.dentry is — one place in the YAML, one file on disk, removed on uninstall.Generates (byte-for-byte matching SpacemiT's documented pin file):
Changes
DESKTOP_REPO_PREFS_COUNTand indexedDESKTOP_REPO_PREFS_<n>_{ORIGIN,SUITE,PRIORITY}for each entry. Skip malformed entries with a stderr warning (no silent drops).COUNT > 0, write one stanza per entry to/etc/apt/preferences.d/<de>. Only parsed, validated fields land in the file — raw YAML strings are never shell-eval'd.Schema is a superset of the previous one: desktops with no
preferenceskey emitDESKTOP_REPO_PREFS_COUNT="0"and the module takes the zero-branch. Smoke-tested againstxfce(norepo:),kde-neon(repo:withoutpreferences), andbianbu(full 3-entry output).Docs PR: armbian/documentation#912 carries the schema-table update.
Test plan