Skip to content
This repository was archived by the owner on Mar 29, 2026. It is now read-only.

control armbian-common: move sudo to Depends; armbian-firstlogin assumes sudo#34

Merged
leggewie merged 1 commit into
armbian:mainfrom
tabrisnet:tabrisnet_sudo
Dec 12, 2025
Merged

control armbian-common: move sudo to Depends; armbian-firstlogin assumes sudo#34
leggewie merged 1 commit into
armbian:mainfrom
tabrisnet:tabrisnet_sudo

Conversation

@tabrisnet

Copy link
Copy Markdown
Contributor

another boring change that does exactly what the commit claims.
armbian-firstlogin wants to add the first user to the sudo group.

@coderabbitai

coderabbitai Bot commented Dec 11, 2025

Copy link
Copy Markdown

Walkthrough

This change modifies the debian/control file to reclassify the sudo package for the armbian-common package. The sudo dependency is moved from the Recommends field to the Depends field, making it a mandatory installation requirement rather than an optional recommendation during package installation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with a straightforward dependency reclassification
  • No logic changes or structural alterations involved
  • Homogeneous, focused edit

Suggested labels

02, Needs review

Suggested reviewers

  • leggewie

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: moving sudo from Recommends to Depends in the armbian-common control file, and explains the motivation (armbian-firstlogin assumes sudo).
Description check ✅ Passed The description is related to the changeset, explaining that armbian-firstlogin needs sudo to add the first user to the sudo group, which justifies the dependency change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 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 d1e4a5d and 582bf5d.

📒 Files selected for processing (1)
  • debian/control (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tabrisnet
Repo: armbian/apa PR: 25
File: debian/control:23-23
Timestamp: 2025-12-06T21:01:15.488Z
Learning: In the armbian/apa repository's debian/control, iputils-ping is temporarily added to armbian-common's Depends as a workaround for a capability issue where /bin/ping cannot create raw sockets. The proper solution involves: (1) PR #27, (2) removing the chmod u+s workaround code from armbian/build (armbian/build#9033), and (3) adding libcap2-bin to armbian/build's debootstrap package list so that iputils-ping's postinst script can use setcap to properly configure /bin/ping. Once these pieces are complete and merged, iputils-ping should be removed from armbian-common's Depends as it will no longer be needed.
📚 Learning: 2025-12-06T21:01:15.488Z
Learnt from: tabrisnet
Repo: armbian/apa PR: 25
File: debian/control:23-23
Timestamp: 2025-12-06T21:01:15.488Z
Learning: In the armbian/apa repository's debian/control, iputils-ping is temporarily added to armbian-common's Depends as a workaround for a capability issue where /bin/ping cannot create raw sockets. The proper solution involves: (1) PR #27, (2) removing the chmod u+s workaround code from armbian/build (armbian/build#9033), and (3) adding libcap2-bin to armbian/build's debootstrap package list so that iputils-ping's postinst script can use setcap to properly configure /bin/ping. Once these pieces are complete and merged, iputils-ping should be removed from armbian-common's Depends as it will no longer be needed.

Applied to files:

  • debian/control
🔇 Additional comments (1)
debian/control (1)

19-36: Making sudo a hard dependency of an Essential package changes base-system semantics

Adding sudo to armbian-common’s Depends (with armbian-common marked Essential: yes) will effectively make sudo part of the base system on all Armbian installs and not easily removable via normal APT operations. This is consistent with the goal of guaranteeing sudo for armbian-firstlogin, but it is a real behavior change for users who previously chose to omit/remove sudo or use alternatives.

If that trade-off is intentional for Armbian (i.e., “Armbian systems always ship with sudo”), then this change looks correct and aligns with how armbian-common is already being used as the “must-have” base dependency bundle. Otherwise, the alternative would be to make armbian-firstlogin gracefully handle the absence of sudo instead of tightening the dependency here.

Please confirm that making sudo effectively mandatory on all images is desired policy.

Based on learnings, armbian-common Depends is already used for base-level must-haves (e.g., the temporary iputils-ping workaround), so this is consistent with that approach.


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.

@github-actions github-actions Bot added 02 Milestone: First quarter release size/small PR with less then 50 lines Needs review Seeking for review and removed size/small PR with less then 50 lines labels Dec 11, 2025
@coderabbitai coderabbitai Bot requested a review from leggewie December 11, 2025 19:57
@tabrisnet tabrisnet added the size/small PR with less then 50 lines label Dec 11, 2025
@tabrisnet tabrisnet marked this pull request as draft December 12, 2025 00:14
@tabrisnet

Copy link
Copy Markdown
Contributor Author

this may not be necessary, armbian-config depends on sudo

@leggewie

Copy link
Copy Markdown
Contributor

this may not be necessary, armbian-config depends on sudo

that's correct, but the correct thing to do would be for whatever package ships armbian-firstlogin (currently unpackaged) to depend on sudo. and indeed, I would say, sudo is a very important program to have installed certainly on Ubuntu. FWIW, ubuntu-minial explicitly depends on it.

@igorpecovnik

igorpecovnik commented Dec 12, 2025

Copy link
Copy Markdown
Member

whatever package ships armbian-firstlogin (currently unpackaged)

all those scripts are inside bsp package.
https://github.com/armbian/build/blob/main/lib/functions/bsp/armbian-bsp-cli-deb.sh#L127

https://github.com/armbian/build/blob/main/lib/functions/bsp/armbian-bsp-cli-deb.sh#L80-L88

@tabrisnet

tabrisnet commented Dec 12, 2025 via email

Copy link
Copy Markdown
Contributor Author

@igorpecovnik

igorpecovnik commented Dec 12, 2025

Copy link
Copy Markdown
Member

packages/bsp has a lot of stuff that isn't board-specific.

Indeed. We never had "armbian-generic", so those common things were simply packed to bsp. Yes, it can be split up.

should we be signing up for such a process [right now] merely in the name of clean/DRY dependencies?

Good question. Is "now" right time?

Should this stay here:
https://github.com/armbian/build/tree/main/packages/bsp/common
or we find a new place?

@leggewie leggewie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that sudo is important enough in day-to-day use that we as armbian do not support installations that do not have it.

You remove sudo from your system at runtime, it's no longer an armbian-system. That's what Ubuntu essentially says in their ubuntu-minimal.

@leggewie leggewie marked this pull request as ready for review December 12, 2025 20:31
@leggewie leggewie merged commit 43f446f into armbian:main Dec 12, 2025
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

02 Milestone: First quarter release Needs review Seeking for review size/small PR with less then 50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants