Skip to content

Conversation

@Grippy98
Copy link
Contributor

Remove Symlink for Bookwork -> Trixie and remove references to apt-xapian-index since it's no longer packaged in newer Debian releases.

Description

This PR removes Trixie as a config symlink from Bookworm, copies the same files and removes references to apt-xapian-index since that is no longer packaged in newer Debian builds. This might also need to be done for Ubuntu and derivatives but outside of scope here.

How Has This Been Tested?

  • Builds and runs Trixie CSC image with Gnome, all tools on BeaglePlay

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings

@Grippy98 Grippy98 requested a review from igorpecovnik as a code owner May 16, 2025 15:03
@github-actions github-actions bot added the size/large PR with 250 lines or more label May 16, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 16, 2025

Walkthrough

This change introduces a comprehensive set of configuration files and shell scripts for setting up and customizing multiple desktop environments (Budgie, Cinnamon, GNOME, KDE Plasma, Mate, XFCE, i3-wm, and xmonad) under the "trixie" desktop configuration. It includes architecture declarations, package lists, support status files, and post-installation scripts for each environment. The scripts automate the installation of configuration files, wallpapers, icons, and desktop settings, as well as system adjustments such as PulseAudio configuration and LightDM greeter setup. Additionally, several appgroup package lists and references are added, while an obsolete configuration file is deleted.

Suggested labels

ready to merge

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.


Note

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.
Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c37788a and 9da8cf4.

📒 Files selected for processing (2)
  • config/desktop/trixie/environments/budgie/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/cinnamon/config_base/packages (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • config/desktop/trixie/environments/budgie/config_base/packages
  • config/desktop/trixie/environments/cinnamon/config_base/packages

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added 05 Milestone: Second quarter release Desktop Graphical user interface Needs review Seeking for review labels May 16, 2025
@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label May 16, 2025
@glneo
Copy link
Collaborator

glneo commented May 16, 2025

Could you do this in two steps, one breaking the symlink, and the next removing apt-xapian-index, that way the change being made is not hidden by all the new files.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 18

🧹 Nitpick comments (35)
config/desktop/trixie/environments/i3-wm/armbian (1)

1-1: Optional: Clarify redirection purpose.

Consider adding a brief comment above the path (e.g., # Redirect to shared i3-wm configuration) or including a trailing newline to improve readability and maintainability.

config/desktop/trixie/environments/i3-wm/config_base/packages (3)

2-2: Remove duplicate package entries.
The package apt-config-auto-update appears at both lines 2 and 112, leading to redundancy. Please remove one occurrence to avoid confusion and unnecessary processing.

Also applies to: 112-112


95-95: Clarify or remove commented-out package.
The entry #printer-driver-all is commented out. If this driver is intended for inclusion, uncomment it; otherwise, remove the line to keep the list clean and avoid stale comments.


1-136: Consider sorting and grouping for maintainability.
This is a long list of packages—consider ordering entries alphabetically and grouping related items (e.g., fonts, i3 components, X11 utilities). It will simplify future reviews, make duplicates more apparent, and streamline maintenance.

config/desktop/trixie/appgroups/browsers/packages (1)

1-2: Optional: Sort packages alphabetically
For improved readability and consistency across appgroup lists, consider alphabetizing entries.

-def firefox-esr
-chromium
+chromium
+firefox-esr
config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh (5)

31-32: Fix typo in wallpaper name.

There's a typo in the wallpaper name: "black-pyscho" should be "black-psycho" to match the filename.

-    <name>Armbian black-pyscho</name>
+    <name>Armbian black-psycho</name>

38-39: Fix typo in wallpaper name.

There's a typo in the wallpaper name: "bluie-circle" should be "blue-circle" to match the filename.

-    <name>Armbian bluie-circle</name>
+    <name>Armbian blue-circle</name>

108-109: Fix typo in wallpaper name.

There's a typo in the wallpaper name: "purple-penguine" should be "purple-penguin" to match the filename.

-    <name>Armbian purple-penguine</name>
+    <name>Armbian purple-penguin</name>

158-158: Fix typo in filename reference.

There's a typo in the wallpaper filename: "bakcground" should be "background".

-    <filename>/usr/share/backgrounds/armbian/Armbian-clear-rounded-bakcground-3840-2160.jpg</filename>
+    <filename>/usr/share/backgrounds/armbian/Armbian-clear-rounded-background-3840-2160.jpg</filename>

1-164: Consider adding error handling for critical operations.

The script doesn't check if operations like directory creation and file copying succeed. Adding error handling would make the script more robust and provide useful feedback when something goes wrong.

Here's an example of how you could add error handling:

#!/bin/bash
# Set -e to exit on error
set -e

mkdir -p "${destination}"/etc/skel || { echo "Failed to create skel directory"; exit 1; }
cp -R "${SRC}"/packages/blobs/desktop/skel/. "${destination}"/etc/skel || { echo "Failed to copy skel files"; exit 1; }
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/cinnamon/debian/postinst (2)

15-16: Remove or uncomment debugging commands.

These commented-out dconf dump commands appear to be debugging tools left in the code. If they're meant for development/debugging, they should be removed or clearly marked as such.

-# gather dconf settings
-# deconf dump org/nemo/ > nemo_backup
-# deconf dump org/cinnamon/ > cinnamon_desktop_backup
+# gather dconf settings

Or, if these lines are intended as comments for users who want to back up their settings:

-# gather dconf settings
-# deconf dump org/nemo/ > nemo_backup
-# deconf dump org/cinnamon/ > cinnamon_desktop_backup
+# To back up current dconf settings, you can use:
+# dconf dump org/nemo/ > nemo_backup
+# dconf dump org/cinnamon/ > cinnamon_desktop_backup

1-133: Consider adding error handling and an exit code.

The script doesn't check if critical operations succeed and doesn't set an explicit exit code. Adding error handling would make the script more robust.

You could add error handling by using set -e at the beginning of the script to exit on any error, or by checking the return values of critical commands:

#!/bin/bash
set -e  # Exit immediately if a command exits with a non-zero status

# Rest of script follows...
config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh (4)

38-39: Fix typo in wallpaper name.

There's a typo in the wallpaper name: "bluie-circle" should be "blue-circle" to match the filename.

-    <name>Armbian bluie-circle</name>
+    <name>Armbian blue-circle</name>

108-109: Fix typo in wallpaper name.

There's a typo in the wallpaper name: "purple-penguine" should be "purple-penguin" to match the filename.

-    <name>Armbian purple-penguine</name>
+    <name>Armbian purple-penguin</name>

158-158: Fix typo in filename reference.

There's a typo in the wallpaper filename: "bakcground" should be "background".

-    <filename>/usr/share/backgrounds/armbian/Armbian-clear-rounded-bakcground-3840-2160.jpg</filename>
+    <filename>/usr/share/backgrounds/armbian/Armbian-clear-rounded-background-3840-2160.jpg</filename>

1-164: Consider adding error handling for critical operations.

The script doesn't check if operations like directory creation and file copying succeed. Adding error handling would make the script more robust and provide useful feedback when something goes wrong.

Here's an example of how you could add error handling:

#!/bin/bash
# Set -e to exit on error
set -e

mkdir -p "${destination}"/etc/skel || { echo "Failed to create skel directory"; exit 1; }
cp -R "${SRC}"/packages/blobs/desktop/skel/. "${destination}"/etc/skel || { echo "Failed to copy skel files"; exit 1; }
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/cinnamon/config_base/packages (1)

86-86: Clarify reason for commented package.

The package printer-driver-all is commented out without explanation. Consider adding a comment explaining why this package is commented out to provide context for future maintainers.

-#printer-driver-all
+# printer-driver-all is commented out because it pulls in too many dependencies
config/desktop/trixie/environments/kde-plasma/debian/postinst (1)

1-5: Add shebang line to script

The script is missing a shebang line to specify the interpreter (e.g., #!/bin/bash).

+#!/bin/bash
 # overwrite stock lightdm greeter configuration
 if [ -d /etc/armbian/lightdm ]; then cp -R /etc/armbian/lightdm /etc/; fi
 
 # Disable Pulseaudio timer scheduling which does not work with sndhdmi driver
 if [ -f /etc/pulse/default.pa ]; then sed "s/load-module module-udev-detect$/& tsched=0/g" -i  /etc/pulse/default.pa; fi
config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (2)

24-164: Typos in wallpaper names

There are several typos in the wallpaper names:

  • Line 38: "bluie-circle" should be "blue-circle"
  • Line 59: "gray-resultado" has "resultado" which appears to be a leftover term
  • Line 108: "purple-penguine" should be "purple-penguin"
  • Line 115: "purplepunk-resultado" has "resultado" which appears to be a leftover term
  • Line 87: "4k-neglated" might be intended as "4k-neglected"
-    <name>Armbian bluie-circle</name>
+    <name>Armbian blue-circle</name>
-    <name>Armbian gray-resultado</name>
+    <name>Armbian gray</name>
-    <name>Armbian purple-penguine</name>
+    <name>Armbian purple-penguin</name>
-    <name>Armbian purplepunk-resultado</name>
+    <name>Armbian purplepunk</name>
-    <name>Armbian 4k-neglated</name>
+    <name>Armbian 4k-neglected</name>

24-164: Consider extracting wallpaper XML to a separate template file

The XML generation takes up most of the script (140+ lines). Consider extracting this to a separate template file that can be copied and modified, making the script more maintainable.

This would reduce code duplication across desktop environment setup scripts and make updates to the wallpaper list easier to manage.

config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh (1)

24-164: Consider parameterizing the wallpaper XML generation.

The script has a large section generating XML entries for wallpapers. Consider extracting this to a function or separate script that could dynamically generate the XML from the available wallpaper files, making it more maintainable.

# Alternative approach that could replace lines 24-164:
+# generate wallpaper list for background changer
+mkdir -p "${destination}"/usr/share/mate-background-properties
+
+# Start the XML file
+cat <<EOF > "${destination}"/usr/share/mate-background-properties/armbian.xml
+<?xml version="1.0"?>
+<!DOCTYPE wallpapers SYSTEM "mate-wp-list.dtd">
+<wallpapers>
+EOF
+
+# Loop through all wallpapers and generate entries
+for wallpaper in "${destination}"/usr/share/backgrounds/armbian/*.jpg; do
+  filename=$(basename "$wallpaper")
+  name=$(echo "$filename" | sed 's/\.jpg$//' | sed 's/-/ /g')
+  cat <<EOF >> "${destination}"/usr/share/mate-background-properties/armbian.xml
+  <wallpaper deleted="false">
+    <name>Armbian $name</name>
+    <filename>$wallpaper</filename>
+    <options>zoom</options>
+    <pcolor>#ffffff</pcolor>
+    <scolor>#000000</scolor>
+  </wallpaper>
+EOF
+done
+
+# Close the XML file
+cat <<EOF >> "${destination}"/usr/share/mate-background-properties/armbian.xml
+</wallpapers>
+EOF
config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (2)

8-10: Fix incorrect comment referring to Cinnamon desktop.

The comment refers to installing "cinnamon desktop bar icons" but this script is for KDE Plasma environment.

-#install cinnamon desktop bar icons
+#install kde-plasma desktop bar icons
 mkdir -p "${destination}"/usr/share/icons/armbian
 cp "${SRC}"/packages/blobs/desktop/desktop-icons/*.png "${destination}"/usr/share/icons/armbian

24-35: Verify if default wallpaper setting code should be uncommented.

There's a commented-out section for setting the default KDE Plasma wallpaper. Verify if this code should be uncommented for this environment.

Would you like me to help determine if this code should be uncommented by examining similar files in other desktop environments?

config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh (1)

8-10: Fix incorrect comment referring to Cinnamon desktop.

The comment refers to installing "cinnamon desktop bar icons" but this script is for the XFCE environment.

-#install cinnamon desktop bar icons
+#install xfce desktop bar icons
 mkdir -p "${destination}"/usr/share/icons/armbian
 cp "${SRC}"/packages/blobs/desktop/desktop-icons/*.png "${destination}"/usr/share/icons/armbian
config/desktop/trixie/environments/gnome/config_base/packages (1)

22-22: Remove duplicate package entry.

gnome-disk-utility is listed twice (also on line 17), which is redundant.

 gnome-menus
 gnome-screenshot
-gnome-disk-utility
 gnome-system-monitor
config/desktop/trixie/environments/xfce/debian/postinst (2)

1-18: Implementation looks good, consider adding error handling.

The post-installation script correctly configures XFCE-specific settings for the Trixie desktop environment. The script is well-structured with clear comments for each major section. Since this is part of a system configuration process, consider adding error handling to ensure that failures are reported and logged properly.


19-19: Remove trailing line number.

There appears to be a trailing line number "19" without any code at the end of the file. This should be removed to prevent potential issues.

config/desktop/trixie/environments/xmonad/config_base/packages (1)

126-126: Remove trailing line number.

Line 126 contains only a number without any package name. This trailing number should be removed to prevent potential parsing issues.

config/desktop/trixie/environments/budgie/config_base/packages (1)

157-157: Remove trailing line number.

Line 157 contains only a number without any package name. This trailing number should be removed to prevent potential parsing issues.

config/desktop/trixie/appgroups/browsers/debian/postinst (2)

1-12: Browser configuration script looks good and aligns with PR objectives.

The post-installation script correctly configures Chromium and Firefox browsers for the Trixie desktop environment. Each section is well-structured with clear comments and proper existence checks before applying changes. The script doesn't reference 'apt-xapian-index', which aligns with the PR objective.

Consider adding error handling to ensure that failures during browser configuration are reported and logged properly.


13-13: Remove trailing line number.

Line 13 contains only a number without any code. This trailing line number should be removed to prevent potential issues.

config/desktop/trixie/environments/xfce/config_base/packages (1)

73-73: Consider explaining why printer-driver-all is commented out

The package printer-driver-all is commented out in this file, but included in other environment configurations like MATE. It would be helpful to add a comment explaining why this specific package is disabled in the XFCE environment.

config/desktop/trixie/environments/mate/config_base/packages (1)

90-90: Note inconsistency with printer-driver-all inclusion

The package printer-driver-all is included here but commented out in the XFCE environment file. Consider maintaining consistency across environments or documenting the reason for the difference.

config/desktop/trixie/environments/budgie/debian/postinst (2)

1-3: Consider documenting the commented slick-greeter configuration

Line 3 contains a commented-out command that would modify the wallpaper in the slick-greeter configuration. Consider adding a comment explaining why this line is disabled or remove it if it's not needed.


16-18: Remove or comment these debug commands

These lines appear to be example commands for gathering dconf settings and are prefixed with "deconf" (likely a typo of "dconf"). Consider removing them or adding a proper comment explaining they're provided as examples.

-# deconf dump org/nemo/ > nemo_backup
-# deconf dump org/budgie/ > budgie_desktop_backup
+# Example commands to backup configurations:
+# dconf dump org/nemo/ > nemo_backup
+# dconf dump org/budgie/ > budgie_desktop_backup
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5216e73 and 03b7478.

📒 Files selected for processing (53)
  • config/desktop/trixie (0 hunks)
  • config/desktop/trixie/appgroups/browsers/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/appgroups/browsers/debian/postinst (1 hunks)
  • config/desktop/trixie/appgroups/browsers/packages (1 hunks)
  • config/desktop/trixie/appgroups/chat (1 hunks)
  • config/desktop/trixie/appgroups/desktop_tools (1 hunks)
  • config/desktop/trixie/appgroups/editors (1 hunks)
  • config/desktop/trixie/appgroups/internet/packages (1 hunks)
  • config/desktop/trixie/appgroups/multimedia (1 hunks)
  • config/desktop/trixie/appgroups/office (1 hunks)
  • config/desktop/trixie/appgroups/programming/packages (1 hunks)
  • config/desktop/trixie/appgroups/remote_desktop (1 hunks)
  • config/desktop/trixie/environments/budgie/architectures (1 hunks)
  • config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/budgie/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/budgie/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/budgie/support (1 hunks)
  • config/desktop/trixie/environments/cinnamon/architectures (1 hunks)
  • config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/cinnamon/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/cinnamon/config_base/packages.uninstall (1 hunks)
  • config/desktop/trixie/environments/cinnamon/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/cinnamon/support (1 hunks)
  • config/desktop/trixie/environments/gnome/architectures (1 hunks)
  • config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/gnome/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/gnome/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/gnome/support (1 hunks)
  • config/desktop/trixie/environments/i3-wm/architectures (1 hunks)
  • config/desktop/trixie/environments/i3-wm/armbian (1 hunks)
  • config/desktop/trixie/environments/i3-wm/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/i3-wm/debian (1 hunks)
  • config/desktop/trixie/environments/i3-wm/support (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/architectures (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/support (1 hunks)
  • config/desktop/trixie/environments/mate/architectures (1 hunks)
  • config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/mate/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/mate/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/mate/support (1 hunks)
  • config/desktop/trixie/environments/xfce/architectures (1 hunks)
  • config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/xfce/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/xfce/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/xfce/support (1 hunks)
  • config/desktop/trixie/environments/xmonad/architectures (1 hunks)
  • config/desktop/trixie/environments/xmonad/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/xmonad/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/xmonad/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/xmonad/support (1 hunks)
💤 Files with no reviewable changes (1)
  • config/desktop/trixie
🧰 Additional context used
🪛 Shellcheck (0.10.0)
config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/appgroups/browsers/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xmonad/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

🔇 Additional comments (44)
config/desktop/trixie/environments/kde-plasma/support (1)

1-1: Support marker is consistent.
The file correctly contains only the csc marker, matching the pattern used for other desktop environments.

config/desktop/trixie/appgroups/editors (1)

1-1: Correct relative path to common editors appgroup
This file properly references the shared editors appgroup configuration at ../../common/appgroups/editors, consistent with the other Trixie appgroup entries.

config/desktop/trixie/appgroups/chat (1)

1-1: File pattern matches other appgroups
The new chat appgroup file is consistent with the other entries in config/desktop/trixie/appgroups/, correctly referencing the shared common definition.

config/desktop/trixie/appgroups/office (1)

1-1: Link to shared office appgroup is correct
The relative path properly references the common office appgroup definition.

config/desktop/trixie/appgroups/remote_desktop (1)

1-1: Symlink target appears correct
The relative path ../../common/appgroups/remote_desktop correctly points from config/desktop/trixie/appgroups/remote_desktop to the shared config/desktop/common/appgroups/remote_desktop file, ensuring consistency with other appgroup references.

config/desktop/trixie/appgroups/multimedia (1)

1-1: DRY: Reuse common multimedia appgroup
By referencing ../../common/appgroups/multimedia, this file cleanly inherits the shared multimedia appgroup configuration, keeping Trixie’s setup aligned with other desktop environments and avoiding duplication.

config/desktop/trixie/environments/i3-wm/debian (1)

1-1: Redirect matches common configuration.

The new file correctly delegates to the shared i3-wm Debian config by referencing
../../../common/environments/i3-wm/debian, aligning with the pattern used for other desktop environments.

config/desktop/trixie/environments/i3-wm/support (1)

1-1: Consistent environment support pointer

This new support file correctly references the shared common i3-wm support configuration and matches the pattern used for other Trixie environments. The relative path (../../../common/environments/i3-wm/support) is accurate given the directory layout.

config/desktop/trixie/environments/i3-wm/armbian (1)

1-1: Correct reference to shared configuration.

The relative path ../../../common/environments/i3-wm/armbian accurately redirects to the common i3-wm environment config under config/desktop/common.

config/desktop/trixie/environments/i3-wm/config_base/packages (1)

1-136: No deprecated package references.
apt-xapian-index is correctly absent from this list, aligning with the PR objective to remove this obsolete package in Trixie configurations.

config/desktop/trixie/environments/mate/support (1)

1-2: Consistent environment support marker
The support file correctly contains csc, matching the other Trixie environment markers. A trailing newline is present, ensuring POSIX compliance.

config/desktop/trixie/environments/cinnamon/support (1)

1-2: Consistent environment support marker
The support file correctly contains supported, aligning with the other Trixie environment markers. A trailing newline is present for proper formatting.

config/desktop/trixie/appgroups/internet/packages (1)

1-2: Internet appgroup package list
The file lists filezilla and putty as intended. Entries are lowercase and hyphenated per Debian naming conventions, and the file ends with a newline.

config/desktop/trixie/appgroups/programming/packages (1)

1-4: Programming appgroup package list
The file correctly enumerates essential development tools (build-essential, clang, meld, regexxer) in alphabetical order, following Debian package naming conventions. Trailing newline is present.

config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh (1)

2-2:

❌ Incorrect review comment

Undefined variable destination is being referenced.

The destination variable is used but not defined in this script, which could lead to incorrect path references if the variable is not properly set by the calling environment.


🏁 Script executed:

#!/bin/bash
# Check if destination variable exists in parent scripts
grep -r "destination=" $(find "$(pwd)" -type f -name "*.sh" | grep -v "$(basename "$0")")

Length of output: 17265


🏁 Script executed:

#!/bin/bash
# Display the contents of the script to inspect how `destination` is handled
sed -n '1,200p' config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh

Length of output: 6246


Incorrect concern – destination is provided by the Armbian packaging framework

The create_desktop_package.sh scripts under config/desktop/... are executed from within the Armbian “artifact” functions, which declare and export both SRC and destination before sourcing these per-environment scripts. For example:

  • lib/functions/artifacts/artifact-armbian-base-files.sh and artifact-armbian-bsp-desktop-deb.sh include
    declare cleanup_id="" destination=""
    then set destination to the target staging area and invoke the config/desktop/.../create_desktop_package.sh.

No changes required here.

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh (1)

2-2:

❓ Verification inconclusive

Undefined variable destination is being referenced.

The destination variable is used but not defined in this script, which could lead to incorrect path references if the variable is not properly set by the calling environment.


🏁 Script executed:

#!/bin/bash
# Check if destination variable exists in parent scripts
grep -r "destination=" $(find "$(pwd)" -type f -name "*.sh" | grep -v "$(basename "$0")")

Length of output: 17265


🏁 Script executed:

#!/bin/bash
# Search for how create_desktop_package.sh scripts are invoked in artifact-armbian-desktop.sh
grep -R "create_desktop_package.sh" -n lib/functions/artifacts/artifact-armbian-desktop.sh

Length of output: 90


Ensure destination Is Defined Before Use
I didn’t find any definition or export of the destination variable in the surrounding invocation of this script. Without it, this line will fail or copy to an unintended path:

cp -R "${SRC}"/packages/blobs/desktop/lightdm "${destination}"/etc/armbian

Please verify and/or address one of the following:

  • Confirm that the parent build or artifact script exports/passes destination when calling create_desktop_package.sh.

  • Add an explicit check at the top of this script to fail fast if destination is unset:

    : "${destination:?Error: destination must be set}"
  • Or provide a sensible default for destination within the script.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/cinnamon/config_base/packages (1)

1-121: LGTM - The package list is comprehensive for Cinnamon.

This package list provides a comprehensive set of packages for the Cinnamon desktop environment, including essential components for desktop, connectivity, media, and system management. After fixing the duplicate entry, this should work well for the Trixie release.

config/desktop/trixie/environments/cinnamon/config_base/packages.uninstall (2)

1-2: Approve added uninstall package list.
The file correctly specifies packages to remove (gnome-software and gnome-keyring) for the Cinnamon environment.


1-2:

✅ Verification successful

Verify removal of deprecated apt-xapian-index.
Ensure no lingering references to the removed apt-xapian-index package remain across the Trixie desktop configurations.

Run:


🏁 Script executed:

#!/bin/bash
# Verify no references to apt-xapian-index in Trixie desktop configs
rg -R "apt-xapian-index" config/desktop/trixie

Length of output: 451


We need to re-run the scan without the invalid -R flag. Let’s verify again for any stray references:


🏁 Script executed:

#!/bin/bash
# Verify no references to apt-xapian-index in Trixie desktop configs
rg "apt-xapian-index" config/desktop/trixie

Length of output: 43


Removal of apt-xapian-index verified
No occurrences of apt-xapian-index were found in config/desktop/trixie. Safe to proceed with this uninstall.

config/desktop/trixie/environments/cinnamon/architectures (1)

1-1: Approve architecture declaration.
Using the comma-separated list arm64, amd64 aligns with other environment configurations in this release.

config/desktop/trixie/environments/budgie/support (1)

1-1: Approve support marker.
The eos identifier accurately marks the Budgie environment as supported and follows the project’s convention.

config/desktop/trixie/environments/xmonad/support (1)

1-1: Approve support marker.
The csc identifier correctly indicates CSC support for the xmonad environment, consistent with other environments.

config/desktop/trixie/environments/gnome/architectures (1)

1-1: Approve architecture declaration.
Listing supported architectures as arm64, amd64 matches the established pattern for desktop environments.

config/desktop/trixie/environments/xfce/support (1)

1-1: Consistent support marker for XFCE
The single-line supported flag matches the pattern used across other desktop environments and correctly indicates that XFCE is supported in the Trixie release.

config/desktop/trixie/environments/gnome/support (1)

1-1: Consistent support marker for GNOME
The supported marker follows the established convention for desktop environment support files and accurately reflects GNOME support on Trixie.

config/desktop/trixie/environments/xmonad/architectures (1)

1-1: Valid architecture declaration for Xmonad
Listing arm64, amd64 aligns with other environment-specific architecture files and correctly declares the supported CPU targets for the Xmonad environment.

config/desktop/trixie/environments/budgie/architectures (1)

1-1: Valid architecture declaration for Budgie
The comma-separated arm64, amd64 matches the pattern of other Budgie configuration files and correctly defines the supported architectures.

config/desktop/trixie/environments/mate/architectures (1)

1-1: Valid architecture declaration for Mate
This file accurately specifies arm64, amd64 as the supported architectures, consistent with the format used across other Mate configuration files.

config/desktop/trixie/environments/kde-plasma/architectures (1)

1-2: Architecture support looks good

The file correctly specifies arm64 and amd64 as the supported architectures for KDE Plasma on Trixie.

config/desktop/trixie/environments/xfce/architectures (1)

1-2: LGTM! Broader architecture support for XFCE

The file correctly specifies arm64, amd64, armhf, and riscv64 as supported architectures for XFCE. This broader range of supported architectures aligns with XFCE being a more lightweight desktop environment suitable for a wider range of hardware platforms.

config/desktop/trixie/environments/xmonad/debian/postinst (1)

7-9:

❓ Verification inconclusive

Implement wallpaper setting functionality

There's a comment about setting the wallpaper to Armbian, but no implementation is provided. The empty comment suggests this functionality should be added.

If this is a deliberate placeholder for future implementation, please document it as such. Otherwise, please implement the wallpaper setting functionality to match the comment's intent.


🏁 Script executed:

#!/bin/bash
# Check if any wallpaper setting commands are available in other desktop environment scripts
grep -r "wallpaper" --include="postinst" config/desktop/trixie/environments/

Length of output: 74


Add or document wallpaper setting in postinst
I scanned all postinst scripts under config/desktop and didn’t find any existing wallpaper-setting logic. The lone comment in
config/desktop/trixie/environments/xmonad/debian/postinst (lines 7–9):

# set wallpaper to armbian

is currently a no-op. Please either:

  • Implement the wallpaper setup (for example, using feh or nitrogen), or
  • Convert this into a documented placeholder (add a TODO and rationale).

If leaving it empty is intentional, clarify that in the comment.

config/desktop/trixie/environments/kde-plasma/config_base/packages (1)

1-122:

❓ Verification inconclusive

Verify package list is complete for KDE Plasma on Trixie

The package list appears comprehensive for KDE Plasma desktop. I noticed that the PR objective mentions removing references to the apt-xapian-index package, and it's correctly not included in this list.

As a precaution, consider checking if any of these packages have dependencies on apt-xapian-index or if any essential KDE Plasma packages might be missing:


🏁 Script executed:

#!/bin/bash
# Check if any packages have dependencies on apt-xapian-index
apt-cache rdepends apt-xapian-index 2>/dev/null || echo "apt-xapian-index package not found in repository"

# Verify that kde-plasma-desktop is available in Trixie repositories
apt-cache policy kde-plasma-desktop 2>/dev/null || echo "kde-plasma-desktop package not found in repository"

Length of output: 293


Verify Trixie package availability and dependencies

The sandbox’s apt-cache calls returned “package not found,” likely because it isn’t pointed at the Debian Trixie repositories. Please double-check in a proper Trixie environment (or via the Debian Packages web interface) that:

  • No listed package (directly or as a dependency) pulls in apt-xapian-index.
  • The KDE meta-package you intended—whether kde-plasma-desktop or its renamed equivalent in Trixie—is actually available and correctly named in the distro’s repos.
config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (1)

2-2:

❌ Incorrect review comment

Define the destination and SRC variables.

The script references destination and SRC variables that are not defined within this script. These should either be defined in this script or documented as expected environment variables.


🏁 Script executed:

#!/bin/bash
# Check if these variables are defined in parent scripts
grep -r "destination=" --include="*.sh" .
grep -r "SRC=" --include="*.sh" .

Length of output: 1642


Ignore undefined SRC and destination warnings for this script

This script is invoked within Armbian’s higher-level build environment (e.g. compile.sh and the various lib/functions/... scripts) where both SRC and destination are declared and exported. Grep confirms they’re defined upstream, so no in-script definitions or documentation are required here.

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh (1)

2-2:

✅ Verification successful

Define the destination and SRC variables.

The script references destination and SRC variables that are not defined within this script. These should either be defined in this script or documented as expected environment variables.


🏁 Script executed:

#!/bin/bash
# Check if these variables are defined in parent scripts
grep -r "destination=" --include="*.sh" .
grep -r "SRC=" --include="*.sh" .

Length of output: 1642


I don’t see any direct calls or definitions of destination or SRC in this script itself, but both are consistently set by upstream Armbian build tooling before this script runs (e.g. in compile.sh for SRC, and in various lib/functions/... scripts for destination). Since these variables come from the shared Armbian build framework, we don’t need to redefine them here—but we should document that they are expected to be provided by the build environment.

• Add a comment at the top of config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh noting that SRC and destination are injected by the Armbian build system.

Revised snippet:

#!/bin/bash
#
# This script is run by the Armbian build framework.
# It expects the following environment variables to be set by the build system:
#   • SRC          – path to the Armbian root directory (set by compile.sh)
#   • destination  – target rootfs directory (set by lib/functions/*)

cp -R "${SRC}/packages/blobs/desktop/lightdm" "${destination}/etc/armbian"
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/gnome/config_base/packages (1)

36-36: Verify if commented-out package is intentional.

The printer-driver-all package is commented out. Verify if this is intentional or if it should be included.

Are there specific reasons for excluding this package? It might be related to the PR objective of addressing packages not available in Trixie.

config/desktop/trixie/environments/gnome/debian/postinst (2)

3-3: Verify if commented-out configuration is intentional.

There's a commented-out line for changing the background in the slick-greeter configuration. Verify if this is intentional or if it should be uncommented.

Is this configuration still needed or has it been replaced by another mechanism?


35-36: Verify if commented-out package removal is intentional.

There's a commented-out line for removing gnome-shell-extension-desktop-icons. Verify if this is intentional or if it should be uncommented.

Is this package removal still needed, or was it intentionally disabled?

config/desktop/trixie/environments/xmonad/config_base/packages (2)

1-125: Package list looks comprehensive and aligns with PR objectives.

The package list for the xmonad environment is comprehensive and doesn't include the 'apt-xapian-index' package, which aligns with the PR objective of removing references to this package (which is no longer available in Trixie). The selection covers all necessary components for a functional xmonad desktop environment.


1-125:

✅ Verification successful

Consider reviewing package necessity.

While the package list is comprehensive, it might be worth reviewing whether all included packages are necessary for the minimal xmonad environment. This could help reduce the installation footprint if some packages are optional.


🏁 Script executed:

#!/bin/bash
# Check whether any of these packages have dependencies on apt-xapian-index
echo "Checking for any dependencies on apt-xapian-index..."
apt-cache rdepends apt-xapian-index 2>/dev/null || echo "apt-xapian-index package not found in repository"

# Get package status and check for any duplicate packages in the list
echo "Checking for duplicate entries in the package list..."
sort config/desktop/trixie/environments/xmonad/config_base/packages | uniq -d

Length of output: 457


Review package list for necessity

No duplicate entries were found in config/desktop/trixie/environments/xmonad/config_base/packages, and none of the listed packages depend on apt-xapian-index. Consider auditing this comprehensive package list to remove any non-essential packages for a minimal xmonad environment and reduce the overall installation footprint.

config/desktop/trixie/environments/budgie/config_base/packages (1)

1-156: Package list looks comprehensive and aligns with PR objectives.

The package list for the budgie environment is comprehensive and doesn't include the 'apt-xapian-index' package, which aligns with the PR objective of removing references to this package (which is no longer available in Trixie). The selection covers all necessary components for a functional budgie desktop environment.

config/desktop/trixie/environments/xfce/config_base/packages (1)

1-114:

❓ Verification inconclusive

Verification of dependency chain without apt-xapian-index

This new package list appropriately excludes the apt-xapian-index package that's mentioned in the PR objectives as no longer available in Trixie. Please verify that there are no packages in this list that depend on it.


🏁 Script executed:

#!/bin/bash
# Check if any packages in this list depend on apt-xapian-index
for pkg in $(cat config/desktop/trixie/environments/xfce/config_base/packages | grep -v "^#" | grep -v "^$"); do
  echo "Checking dependencies for $pkg..."
  apt-cache depends $pkg | grep -i apt-xapian-index || true
done

Length of output: 41966


Manual verification required for apt-xapian-index reverse-dependencies

The sandbox lacks apt-cache, so the automated check couldn’t run. Please confirm that none of the packages in:

  • config/desktop/trixie/environments/xfce/config_base/packages (lines 1–114)

reverse-depend on apt-xapian-index. You can:

• Run apt-rdepends -r apt-xapian-index on a Trixie system
• Consult https://packages.debian.org/trixie/apt-xapian-index (“Reverse Depends” section)

config/desktop/trixie/environments/mate/config_base/packages (1)

1-132: LGTM - Package list excludes apt-xapian-index as intended

The MATE environment package list correctly omits the apt-xapian-index package that is no longer available in Trixie, addressing the PR objective. The list provides a comprehensive set of packages for a fully functional MATE desktop environment.

config/desktop/trixie/environments/budgie/debian/postinst (1)

5-6: Consistent PulseAudio timer scheduling fix

Good implementation of the PulseAudio timer scheduling fix that's consistently applied across desktop environments to address known issues with the sndhdmi driver.

config/desktop/trixie/environments/mate/debian/postinst (1)

1-40: Well-structured MATE post-installation script

The script is well-organized and follows the same pattern as other desktop environment postinst scripts, ensuring consistency across the Trixie configuration. It properly configures the LightDM greeter, disables PulseAudio timer scheduling for compatibility with the sndhdmi driver, and sets up MATE-specific dconf settings.

Grippy98 added 2 commits May 16, 2025 10:19
Now just points to same content, but copied bookworm
No longer packaged in newer Debian
@github-actions github-actions bot removed the Ready to merge Reviewed, tested and ready for merge label May 16, 2025
@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label May 16, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🔭 Outside diff range comments (1)
config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (1)

24-166: 🛠️ Refactor suggestion

Automate wallpaper XML generation for maintainability

Hardcoding dozens of <wallpaper> entries is brittle and error-prone when wallpapers are added/removed. Consider generating the XML dynamically by scanning the directory:

-mkdir -p "${destination}/usr/share/cinnamon-background-properties"
-cat <<EOF > "${destination}/usr/share/cinnamon-background-properties/armbian.xml
-<?xml version="1.0"?>
-<!DOCTYPE wallpapers SYSTEM "cinnamon-wp-list.dtd">
-<wallpapers>
-  <!-- many hardcoded entries -->
-</wallpapers>
-EOF
+mkdir -p "${destination}/usr/share/cinnamon-background-properties"
+{
+  echo '<?xml version="1.0"?>'
+  echo '<!DOCTYPE wallpapers SYSTEM "cinnamon-wp-list.dtd">'
+  echo '<wallpapers>'
+  for img in "${destination}/usr/share/backgrounds/armbian/"*.jpg; do
+    filename=$(basename "$img")
+    title="${filename%.*}"
+    # Replace dashes with spaces for the display name
+    display_name="${title//-/ }"
+    cat <<EOP >> "${destination}/usr/share/cinnamon-background-properties/armbian.xml"
+  <wallpaper deleted="false">
+    <name>${display_name}</name>
+    <filename>/usr/share/backgrounds/armbian/${filename}</filename>
+    <options>zoom</options>
+    <pcolor>#ffffff</pcolor>
+    <scolor>#000000</scolor>
+  </wallpaper>
+EOP
+  done
+  echo '</wallpapers>'
+} > "${destination}/usr/share/cinnamon-background-properties/armbian.xml"

This ensures new wallpapers are picked up automatically without manual edits.

♻️ Duplicate comments (10)
config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (1)

1-3: ⚠️ Potential issue

Add a shebang, strict mode, and argument parsing
The script currently lacks a shebang (SC2148) and references SRC and destination without defining them (SC2154). Add at the top of the file to ensure the correct interpreter, fail-fast behavior, and clear usage:

+#!/usr/bin/env bash
+set -euo pipefail
+
+if [[ $# -ne 2 ]]; then
+  echo "Usage: $0 <source-dir> <destination-dir>"
+  exit 1
+fi
+SRC=$1
+destination=$2
+
 # install lightdm greeter
 cp -R "${SRC}"/packages/blobs/desktop/lightdm "${destination}"/etc/armbian
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (2)

8-11: Clarify icon installation comment

The comment reads “install cinnamon desktop bar icons” but these are generic Armbian desktop icons shared across environments. Update it to:

-#install cinnamon desktop bar icons
+# install Armbian desktop icons

1-2: 🛠️ Refactor suggestion

Add shebang, strict mode, and validate required variables

The script currently lacks a shebang and robust error handling. Also, SRC and destination are used without checks, leading to potential undefined‐variable errors (SC2148, SC2154). Please add at the top:

+#!/usr/bin/env bash
+set -euo pipefail
+
+# Ensure required environment variables are set
+if [ -z "${SRC:-}" ] || [ -z "${destination:-}" ]; then
+  echo "Error: SRC and destination environment variables must be set" >&2
+  exit 1
+fi
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xmonad/armbian/create_desktop_package.sh (2)

1-1: Add shebang and enable strict mode
The script currently lacks a shebang and safety flags. Prepend the following to ensure it runs under Bash with strict error handling:

+#!/usr/bin/env bash
+set -euo pipefail
+
 # install lightdm greeter
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


1-2: Validate SRC and destination variables
Both SRC and destination are referenced without checks. If they’re unset, the script will fail unpredictably. Add upfront validation to exit clearly:

+if [ -z "${SRC:-}" ]; then
+  echo "Error: SRC variable is not set" >&2
+  exit 1
+fi
+if [ -z "${destination:-}" ]; then
+  echo "Error: destination variable is not set" >&2
+  exit 1
+fi
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/appgroups/browsers/armbian/create_desktop_package.sh (1)

1-1: Add shebang and enable strict mode
The script lacks a shebang, making the interpreter ambiguous. Add a shebang at the top (e.g., #!/usr/bin/env bash) and enable set -euo pipefail for robust error handling.

+#!/usr/bin/env bash
+set -euo pipefail
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh (2)

1-1: Add a shebang to specify the interpreter
Without a shebang, the target shell is ambiguous and tools like ShellCheck (SC2148) will warn.

🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


2-2: Validate required environment variables
The script references SRC and destination without defining or checking them. Failing early with clear messages prevents obscure errors later.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh (1)

1-1: Add a shebang line to specify the shell interpreter.

The script currently has no shebang, so it's unclear which shell will execute it (SC2148). Please add at the very top:

+#!/bin/bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh (1)

1-1: Add a shebang line for shell interpreter
The script lacks a shebang, so ShellCheck SC2148 correctly warns about an unknown target shell.

Please add at the very top:

+#!/usr/bin/env bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🧹 Nitpick comments (14)
config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (3)

8-11: Nit: consistent comment formatting
Add a space after # for readability and consistency with other comments:

-#install cinnamon desktop bar icons
+# install cinnamon desktop bar icons

16-19: Differentiate duplicate comments
This block also installs wallpapers, but for LightDM. Clarify the comment to avoid confusion:

-# install wallpapers
+# install lightdm wallpapers

24-35: Remove or externalize commented-out script
The large commented dbus-send block for setting the wallpaper is inactive and adds noise. If it's needed, extract it into a dedicated helper script; otherwise, remove it to keep the file lean.

config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (1)

16-19: Differentiate LightDM wallpaper installation comment

To make the intent clearer, change the second wallpaper comment to:

-# install wallpapers
+# install LightDM greeter wallpapers
config/desktop/trixie/environments/xmonad/armbian/create_desktop_package.sh (1)

2-14: Use consistent quoting and preserve file attributes
Current cp -R invocations mix quoting styles and don’t preserve metadata. Consider switching to cp -a and quoting full paths:

- cp -R "${SRC}"/packages/blobs/desktop/lightdm "${destination}"/etc/armbian
+ cp -a "${SRC}/packages/blobs/desktop/lightdm" "${destination}/etc/armbian"

- mkdir -p "${destination}"/etc/skel
- cp -R "${SRC}"/packages/blobs/desktop/skel/. "${destination}"/etc/skel
+ mkdir -p "${destination}/etc/skel"
+ cp -a "${SRC}/packages/blobs/desktop/skel/." "${destination}/etc/skel"

- mkdir -p "${destination}"/usr/share/pixmaps/armbian
- cp "${SRC}"/packages/blobs/desktop/icons/armbian.png "${destination}"/usr/share/pixmaps/armbian
+ mkdir -p "${destination}/usr/share/pixmaps/armbian"
+ cp -a "${SRC}/packages/blobs/desktop/icons/armbian.png" "${destination}/usr/share/pixmaps/armbian"

- mkdir -p "${destination}"/usr/share/backgrounds/armbian/
- cp "${SRC}"/packages/blobs/desktop/desktop-wallpapers/armbian*.jpg "${destination}"/usr/share/backgrounds/armbian/
+ mkdir -p "${destination}/usr/share/backgrounds/armbian/"
+ cp -a "${SRC}/packages/blobs/desktop/desktop-wallpapers/armbian"*.jpg "${destination}/usr/share/backgrounds/armbian/"
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh (2)

8-8: Fix comment formatting for consistency.

There’s a missing space after the hash on line 8. For readability and style consistency, add a space:

-#install cinnamon desktop bar icons
+# install cinnamon desktop bar icons

23-23: Ensure a trailing newline at end of file.

Many tools expect a newline at EOF. Please add one to comply with POSIX conventions.

config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh (2)

2-23: Quote paths fully and preserve file attributes
To handle potential spaces in variables, quote the entire path and use cp -a (archive mode) instead of -R to preserve permissions, symlinks, and timestamps.

- cp -R "${SRC}"/packages/blobs/desktop/lightdm "${destination}"/etc/armbian
+ cp -a "${SRC}/packages/blobs/desktop/lightdm" "${destination}/etc/armbian"

- mkdir -p "${destination}"/etc/skel
- cp -R "${SRC}"/packages/blobs/desktop/skel/. "${destination}"/etc/skel
+ mkdir -p "${destination}/etc/skel"
+ cp -a "${SRC}/packages/blobs/desktop/skel/." "${destination}/etc/skel"

- mkdir -p "${destination}"/usr/share/icons/armbian
- cp "${SRC}"/packages/blobs/desktop/desktop-icons/*.png "${destination}"/usr/share/icons/armbian
+ mkdir -p "${destination}/usr/share/icons/armbian"
+ cp -a "${SRC}/packages/blobs/desktop/desktop-icons/"*.png "${destination}/usr/share/icons/armbian"

[...apply same pattern to wallpaper and icon sections...]
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)


24-164: Refactor static XML block into a dynamic generator
Hard-coding dozens of wallpaper entries is fragile and error-prone when files change. Instead, loop over the source directory to build the XML at runtime.

xml_dir="${destination}/usr/share/mate-background-properties"
mkdir -p "$xml_dir"
xml_file="$xml_dir/armbian.xml"

cat > "$xml_file" <<'EOF'
<?xml version="1.0"?>
<!DOCTYPE wallpapers SYSTEM "mate-wp-list.dtd">
<wallpapers>
EOF

for img in "${destination}/usr/share/backgrounds/armbian/"*.jpg; do
  name="$(basename "$img" .jpg)"
  cat >> "$xml_file" <<ITEM
  <wallpaper deleted="false">
    <name>Armbian $name</name>
    <filename>$img</filename>
    <options>zoom</options>
    <pcolor>#ffffff</pcolor>
    <scolor>#000000</scolor>
  </wallpaper>
ITEM
done

cat >> "$xml_file" <<'EOF'
</wallpapers>
EOF

This reduces maintenance overhead whenever wallpapers are added or removed.

config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh (3)

12-13: Differentiate duplicate comments for wallpaper installation.

You have two # install wallpapers blocks (lines 12–15 and 16–19). The second appears to install LightDM wallpapers. Consider renaming to avoid confusion:

-# install wallpapers
+# install LightDM wallpapers

26-164: Reduce duplication in generating the GNOME wallpaper XML.

Hard-coding 20+ <wallpaper> entries is brittle and hard to maintain. Consider generating them in a loop over the copied files:

-cat <<EOF > "${destination}"/usr/share/gnome-background-properties/armbian.xml
-<?xml version="1.0"?>
-<!DOCTYPE wallpapers SYSTEM "gnome-wp-list.dtd">
-<wallpapers>
-  <wallpaper deleted="false">
-    … (repeated entries) …
-  </wallpapers>
-EOF
+xml_file="${destination}/usr/share/gnome-background-properties/armbian.xml"
+mkdir -p "$(dirname "$xml_file")"
+cat <<EOF > "$xml_file"
+<?xml version="1.0"?>
+<!DOCTYPE wallpapers SYSTEM "gnome-wp-list.dtd">
+<wallpapers>
+EOF
+for img in "${destination}"/usr/share/backgrounds/armbian/*.jpg; do
+  name=$(basename "$img" .jpg)
+  cat <<EOF >> "$xml_file"
+  <wallpaper deleted="false">
+    <name>Armbian $name</name>
+    <filename>$img</filename>
+    <options>zoom</options>
+    <pcolor>#ffffff</pcolor>
+    <scolor>#000000</scolor>
+  </wallpaper>
+EOF
+done
+echo "</wallpapers>" >> "$xml_file"

This approach auto-discovers new wallpapers and keeps the XML in sync.


114-114: Fix inconsistent XML indentation.

At line 114 the <wallpaper> tag is indented by 4 spaces, while other entries use 2. Normalizing to 2 spaces improves readability:

-    <wallpaper deleted="false">
+  <wallpaper deleted="false">
config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh (2)

2-23: Quote entire file paths for safety
Currently the variable is quoted but the literal path segments are not. For robustness against spaces or special characters, update all cp and mkdir lines to:

- cp -R "${SRC}"/packages/blobs/desktop/lightdm "${destination}"/etc/armbian
+ cp -R "${SRC}/packages/blobs/desktop/lightdm" "${destination}/etc/armbian"

Apply similarly for mkdir -p and other cp invocations.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 2-2: destination is referenced but not assigned.

(SC2154)


26-164: Consider generating the wallpaper XML list programmatically
Hardcoding dozens of <wallpaper> entries is error-prone and difficult to maintain when adding or removing images. Instead, you could iterate over the JPEG files in the target directory and append XML entries dynamically. For example:

cat <<EOF > "$destination/usr/share/cinnamon-background-properties/armbian.xml"
<?xml version="1.0"?>
<!DOCTYPE wallpapers SYSTEM "cinnamon-wp-list.dtd">
<wallpapers>
EOF

for img in "$destination"/usr/share/backgrounds/armbian/*.jpg; do
  name=$(basename "$img" .jpg)
  cat <<ENTRY >> "$destination"/usr/share/cinnamon-background-properties/armbian.xml
  <wallpaper deleted="false">
    <name>Armbian ${name//-/ }</name>
    <filename>$img</filename>
    <options>zoom</options>
    <pcolor>#ffffff</pcolor>
    <scolor>#000000</scolor>
  </wallpaper>
ENTRY
done

echo "</wallpapers>" >> "$destination"/usr/share/cinnamon-background-properties/armbian.xml"

This reduces duplication and makes onboarding new wallpapers as simple as dropping an image into the folder.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03b7478 and c37788a.

📒 Files selected for processing (53)
  • config/desktop/trixie (0 hunks)
  • config/desktop/trixie/appgroups/browsers/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/appgroups/browsers/debian/postinst (1 hunks)
  • config/desktop/trixie/appgroups/browsers/packages (1 hunks)
  • config/desktop/trixie/appgroups/chat (1 hunks)
  • config/desktop/trixie/appgroups/desktop_tools (1 hunks)
  • config/desktop/trixie/appgroups/editors (1 hunks)
  • config/desktop/trixie/appgroups/internet/packages (1 hunks)
  • config/desktop/trixie/appgroups/multimedia (1 hunks)
  • config/desktop/trixie/appgroups/office (1 hunks)
  • config/desktop/trixie/appgroups/programming/packages (1 hunks)
  • config/desktop/trixie/appgroups/remote_desktop (1 hunks)
  • config/desktop/trixie/environments/budgie/architectures (1 hunks)
  • config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/budgie/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/budgie/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/budgie/support (1 hunks)
  • config/desktop/trixie/environments/cinnamon/architectures (1 hunks)
  • config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/cinnamon/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/cinnamon/config_base/packages.uninstall (1 hunks)
  • config/desktop/trixie/environments/cinnamon/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/cinnamon/support (1 hunks)
  • config/desktop/trixie/environments/gnome/architectures (1 hunks)
  • config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/gnome/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/gnome/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/gnome/support (1 hunks)
  • config/desktop/trixie/environments/i3-wm/architectures (1 hunks)
  • config/desktop/trixie/environments/i3-wm/armbian (1 hunks)
  • config/desktop/trixie/environments/i3-wm/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/i3-wm/debian (1 hunks)
  • config/desktop/trixie/environments/i3-wm/support (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/architectures (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/kde-plasma/support (1 hunks)
  • config/desktop/trixie/environments/mate/architectures (1 hunks)
  • config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/mate/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/mate/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/mate/support (1 hunks)
  • config/desktop/trixie/environments/xfce/architectures (1 hunks)
  • config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/xfce/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/xfce/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/xfce/support (1 hunks)
  • config/desktop/trixie/environments/xmonad/architectures (1 hunks)
  • config/desktop/trixie/environments/xmonad/armbian/create_desktop_package.sh (1 hunks)
  • config/desktop/trixie/environments/xmonad/config_base/packages (1 hunks)
  • config/desktop/trixie/environments/xmonad/debian/postinst (1 hunks)
  • config/desktop/trixie/environments/xmonad/support (1 hunks)
💤 Files with no reviewable changes (1)
  • config/desktop/trixie
✅ Files skipped from review due to trivial changes (5)
  • config/desktop/trixie/environments/cinnamon/architectures
  • config/desktop/trixie/appgroups/browsers/packages
  • config/desktop/trixie/environments/cinnamon/config_base/packages
  • config/desktop/trixie/appgroups/browsers/debian/postinst
  • config/desktop/trixie/environments/gnome/debian/postinst
🚧 Files skipped from review as they are similar to previous changes (39)
  • config/desktop/trixie/appgroups/office
  • config/desktop/trixie/appgroups/editors
  • config/desktop/trixie/environments/xmonad/architectures
  • config/desktop/trixie/environments/i3-wm/armbian
  • config/desktop/trixie/environments/gnome/support
  • config/desktop/trixie/environments/mate/support
  • config/desktop/trixie/environments/xmonad/support
  • config/desktop/trixie/appgroups/multimedia
  • config/desktop/trixie/environments/i3-wm/support
  • config/desktop/trixie/environments/gnome/architectures
  • config/desktop/trixie/environments/cinnamon/support
  • config/desktop/trixie/environments/xfce/support
  • config/desktop/trixie/appgroups/chat
  • config/desktop/trixie/appgroups/remote_desktop
  • config/desktop/trixie/environments/budgie/support
  • config/desktop/trixie/environments/cinnamon/config_base/packages.uninstall
  • config/desktop/trixie/appgroups/desktop_tools
  • config/desktop/trixie/environments/kde-plasma/support
  • config/desktop/trixie/appgroups/programming/packages
  • config/desktop/trixie/appgroups/internet/packages
  • config/desktop/trixie/environments/i3-wm/debian
  • config/desktop/trixie/environments/kde-plasma/architectures
  • config/desktop/trixie/environments/i3-wm/architectures
  • config/desktop/trixie/environments/mate/architectures
  • config/desktop/trixie/environments/kde-plasma/config_base/packages
  • config/desktop/trixie/environments/gnome/config_base/packages
  • config/desktop/trixie/environments/xfce/architectures
  • config/desktop/trixie/environments/budgie/config_base/packages
  • config/desktop/trixie/environments/xfce/debian/postinst
  • config/desktop/trixie/environments/budgie/architectures
  • config/desktop/trixie/environments/xmonad/config_base/packages
  • config/desktop/trixie/environments/xmonad/debian/postinst
  • config/desktop/trixie/environments/xfce/config_base/packages
  • config/desktop/trixie/environments/kde-plasma/debian/postinst
  • config/desktop/trixie/environments/mate/debian/postinst
  • config/desktop/trixie/environments/cinnamon/debian/postinst
  • config/desktop/trixie/environments/mate/config_base/packages
  • config/desktop/trixie/environments/i3-wm/config_base/packages
  • config/desktop/trixie/environments/budgie/debian/postinst
🧰 Additional context used
🪛 Shellcheck (0.10.0)
config/desktop/trixie/appgroups/browsers/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/cinnamon/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/mate/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xfce/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

config/desktop/trixie/environments/xmonad/armbian/create_desktop_package.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 2-2: destination is referenced but not assigned.

(SC2154)

🔇 Additional comments (6)
config/desktop/trixie/environments/kde-plasma/armbian/create_desktop_package.sh (3)

4-7: Default desktop settings installation is correct
The mkdir -p and recursive cp commands properly ensure that /etc/skel exists and its contents are copied. Good use of quoted expansions to handle spaces in paths.


12-15: Desktop wallpapers installation is correct
Creating the backgrounds directory and copying .jpg files is implemented properly.


20-23: Login screen logo installation is correct
The mkdir -p and cp steps for the Armbian login logo are straightforward and correctly quoted.

config/desktop/trixie/environments/budgie/armbian/create_desktop_package.sh (3)

4-7: Default desktop settings installation is correct

Using mkdir -p and cp -R to populate /etc/skel is appropriate here. No changes needed.


12-15: General desktop wallpapers installation

This block correctly creates the backgrounds directory and copies the standard wallpapers. No issues found.


20-23: Login screen logo installation

The logo is correctly placed under /usr/share/pixmaps/armbian. This step looks good.

@github-actions github-actions bot removed the Ready to merge Reviewed, tested and ready for merge label May 16, 2025
@Grippy98
Copy link
Contributor Author

Made the required changes, not going to fight Coderabbit on improvements since those are also applicable generically so outside the scope of this.

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

Labels

05 Milestone: Second quarter release Desktop Graphical user interface Needs review Seeking for review Ready to merge Reviewed, tested and ready for merge size/large PR with 250 lines or more

Development

Successfully merging this pull request may close these issues.

3 participants