feat(nixos): add Orca desktop AppImage for Linux - #2243
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Indent Zero is shutting down on August 7th. Please migrate over to Indent 2.0 to continue getting PR reviews.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesorca-desktop package
moshi-hook package
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopPackages as Linux desktop package list
participant OrcaDesktop as orca-desktop derivation
participant AppImageSource as AppImage source
participant InstallTree as Installation tree
DesktopPackages->>OrcaDesktop: Include orca-desktop
OrcaDesktop->>AppImageSource: Select architecture-specific download and hash
AppImageSource-->>OrcaDesktop: Provide AppImage
OrcaDesktop->>InstallTree: Extract AppImage and install desktop entry
OrcaDesktop->>InstallTree: Copy bundled icons when available
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| in | ||
| '' | ||
| install -Dm644 ${contents}/orca-ide.desktop $out/share/applications/orca-ide.desktop | ||
| substituteInPlace $out/share/applications/orca-ide.desktop \ |
There was a problem hiding this comment.
Silent Exec breakage on upstream change: --replace-warn only warns when Exec=AppRun isn't found. If a future Orca release rewords the Exec line (e.g., ships Exec=./AppRun or Exec=orca-ide), this substitution will no-op, and orca-ide.desktop will be installed with the original Exec=AppRun --no-sandbox %U — which is not on user PATH, so the desktop launcher stops working with no build-time signal. Prefer --replace-fail 'Exec=AppRun' so the build fails loudly and forces the fix.
| substituteInPlace $out/share/applications/orca-ide.desktop \ | |
| substituteInPlace $out/share/applications/orca-ide.desktop \ | |
| --replace-fail 'Exec=AppRun' "Exec=$out/bin/${pname}" |
| install -Dm644 ${contents}/orca-ide.desktop $out/share/applications/orca-ide.desktop | ||
| substituteInPlace $out/share/applications/orca-ide.desktop \ | ||
| --replace-warn 'Exec=AppRun' "Exec=$out/bin/${pname}" | ||
| cp -r ${contents}/usr/share/icons $out/share/icons 2>/dev/null || true |
There was a problem hiding this comment.
Silent icon loss on upstream restructure: 2>/dev/null || true masks every failure mode of this copy — missing source dir, permission errors, etc. Today the AppImage does ship usr/share/icons/hicolor/*/apps/orca-ide.png (verified via --appimage-extract), so the guard is unnecessary. If upstream later moves icons, the build will still succeed and users get a .desktop with Icon=orca-ide that resolves to nothing. Recommend dropping the guard (or replacing it with an explicit if [ -d ... ] + comment explaining why it's optional).
| cp -r ${contents}/usr/share/icons $out/share/icons 2>/dev/null || true | |
| cp -r ${contents}/usr/share/icons $out/share/icons |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
overlays/default.nix (1)
104-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a
metablock to the derivation.The derivation lacks
meta.platforms,meta.description, andmeta.mainProgram. Add these for consistency with other package derivations and to improve discoverability withnix searchand platform filtering.♻️ Proposed addition
extraInstallCommands = let contents = prev.appimageTools.extractType2 { inherit pname version src; }; in '' install -Dm644 ${contents}/orca-ide.desktop $out/share/applications/orca-ide.desktop substituteInPlace $out/share/applications/orca-ide.desktop \ --replace-warn 'Exec=AppRun' "Exec=$out/bin/${pname}" cp -r ${contents}/usr/share/icons $out/share/icons 2>/dev/null || true ''; + meta = with prev.lib; { + description = "Orca desktop application"; + homepage = "https://github.com/stablyai/orca"; + platforms = [ "x86_64-linux" "aarch64-linux" ]; + mainProgram = pname; + }; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@overlays/default.nix` around lines 104 - 127, Add a meta block to the orca-desktop derivation defining platforms, a concise description, and mainProgram set to the package executable. Use the existing pname and supported architecture condition to align metadata with the derivation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@overlays/default.nix`:
- Around line 104-127: Add a meta block to the orca-desktop derivation defining
platforms, a concise description, and mainProgram set to the package executable.
Use the existing pname and supported architecture condition to align metadata
with the derivation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6734b8a-f06e-4a8d-9327-4b7ebd14ad1e
📒 Files selected for processing (2)
home-manager/packages/default.nixoverlays/default.nix
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="overlays/default.nix">
<violation number="1" location="overlays/default.nix:125">
P3: The `cp -r ... 2>/dev/null || true` swallows every failure mode of this icon copy (missing source dir, permission errors, etc.), so if a future Orca release restructures or removes the icons directory, the build will still succeed silently while the installed `.desktop` entry's `Icon=orca-ide` resolves to nothing. Consider dropping the error-suppressing guard, or replace it with an explicit `if [ -d ... ]` check plus a comment explaining why missing icons are acceptable.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| install -Dm644 ${contents}/orca-ide.desktop $out/share/applications/orca-ide.desktop | ||
| substituteInPlace $out/share/applications/orca-ide.desktop \ | ||
| --replace-warn 'Exec=AppRun' "Exec=$out/bin/${pname}" | ||
| cp -r ${contents}/usr/share/icons $out/share/icons 2>/dev/null || true |
There was a problem hiding this comment.
P3: The cp -r ... 2>/dev/null || true swallows every failure mode of this icon copy (missing source dir, permission errors, etc.), so if a future Orca release restructures or removes the icons directory, the build will still succeed silently while the installed .desktop entry's Icon=orca-ide resolves to nothing. Consider dropping the error-suppressing guard, or replace it with an explicit if [ -d ... ] check plus a comment explaining why missing icons are acceptable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At overlays/default.nix, line 125:
<comment>The `cp -r ... 2>/dev/null || true` swallows every failure mode of this icon copy (missing source dir, permission errors, etc.), so if a future Orca release restructures or removes the icons directory, the build will still succeed silently while the installed `.desktop` entry's `Icon=orca-ide` resolves to nothing. Consider dropping the error-suppressing guard, or replace it with an explicit `if [ -d ... ]` check plus a comment explaining why missing icons are acceptable.</comment>
<file context>
@@ -100,6 +100,32 @@
+ install -Dm644 ${contents}/orca-ide.desktop $out/share/applications/orca-ide.desktop
+ substituteInPlace $out/share/applications/orca-ide.desktop \
+ --replace-warn 'Exec=AppRun' "Exec=$out/bin/${pname}"
+ cp -r ${contents}/usr/share/icons $out/share/icons 2>/dev/null || true
+ '';
+ };
</file context>
| cp -r ${contents}/usr/share/icons $out/share/icons 2>/dev/null || true | |
| cp -r ${contents}/usr/share/icons $out/share/icons |
|
Closing per maintainer decision — not moving forward with Orca desktop AppImage.\n\nIf this should stay for future reference, reopen and we can rebase. |
Summary
orca-desktopoverlay wrapping the Orca AppImage (v1.4.168) withappimageTools.wrapType2isLinux && isDesktop).desktopfile and icons from the AppImageTest plan
nixos-rebuild switchon matic installs orca-desktopGenerated with Claude Code
https://claude.ai/code/session_01CvLckVfp8KatB5BBJw5GGt
Summary by cubic
Adds
orca-desktopAppImage (v1.4.168) as a Nix overlay and includes it in the Linux desktop package set. Also bumpsmoshi-hookto 0.2.70.New Features
orca-desktopoverlay usingappimageTools.wrapType2for x86_64 and aarch64 Linux..desktopfile and icons;Execupdated to call the wrapped binary.orca-desktopinhome-managerpackages for Linux desktops.Dependencies
moshi-hookto0.2.70with refreshed hashes.Written for commit 24d7227. Summary will update on new commits.