Skip to content

Add Linux port support and packaging#233

Open
jvannoyx4 wants to merge 6 commits into
AprilNEA:masterfrom
jvannoyx4:master
Open

Add Linux port support and packaging#233
jvannoyx4 wants to merge 6 commits into
AprilNEA:masterfrom
jvannoyx4:master

Conversation

@jvannoyx4

Copy link
Copy Markdown

Summary

This PR adds Linux support for OpenLogi, including device access setup, Linux-compatible button actions, GUI desktop integration, and Linux release packaging.

What changed

  • Added Linux mappings for desktop/workspace actions that were previously macOS-specific
  • Added Linux action injection through uinput for keyboard and pointer actions
  • Added GNOME workspace shortcut support for previous/next workspace actions
  • Added Linux GUI window chrome support, including working minimize/maximize/close controls and draggable titlebar behavior
  • Added app ID/icon integration so OpenLogi appears correctly in desktop launchers and task bars
  • Updated Linux permissions handling and udev rules for hidraw, input, and uinput access
  • Added/updated Linux install documentation
  • Added Linux release packaging:
    • .deb
    • .rpm
    • portable .tar.gz
  • Updated release workflow so Linux tarballs are included in future release assets

Validation

Tested locally on Linux:

  • cargo fmt --all -- --check
  • cargo check -p openlogi-gui -p xtask
  • cargo test -p openlogi-core linux_accelerators
  • sh -n on Linux install/uninstall/package scripts
  • Built release binaries
  • Generated Linux package artifacts:
    • openlogi-0.6.8-linux-amd64.tar.gz
    • openlogi_0.6.8_amd64.deb
    • openlogi-0.6.8-1.x86_64.rpm

@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds comprehensive Linux support for OpenLogi: a split uinput keyboard/pointer device, GNOME gsettings-aware desktop-navigation actions, app-rendered window chrome for client-side decorations, systemd agent integration, and .deb/.rpm/.tar.gz release packaging.

  • Binding layer: the single uinput virtual device is split into a keyboard and a pointer device; desktop-navigation actions now read GNOME keybindings via gsettings and fall back to common GNOME chords, with an accelerator parser backed by unit tests.
  • Permission probing: a new probe_logitech_mouse_event() check and a matching udev rule grant uaccess to Logitech mouse event nodes; however, is_logitech_mouse_event checks ID_INPUT_MOUSE=1 from the sysfs uevent file — a udev-set property that lives in udev's runtime database, not sysfs — causing the probe to always return None and the permission indicator to remain at "Unknown" even after a successful install.
  • GUI / packaging: Linux window chrome, app_id/icon propagation, systemd agent auto-start, tarball packaging in xtask, and updated release workflow are all well-implemented.

Confidence Score: 4/5

The core Linux port work is solid, but the permission probe introduced for mouse event nodes will silently never return Granted due to a wrong sysfs property lookup.

The is_logitech_mouse_event helper reads ID_INPUT_MOUSE=1 from the kernel's sysfs uevent file, but that property is written by udev to its own runtime database and is not present in sysfs. The function always returns false, so probe_logitech_mouse_event() always returns None, and the three-way classify logic keeps the permission indicator at Unknown instead of Granted even after the udev rules are installed and the mouse is connected. All other changes — binding split, gsettings lookup, window chrome, packaging — are well-tested and low-risk.

crates/openlogi-gui/src/platform/permissions.rs — the is_logitech_mouse_event sysfs property check

Important Files Changed

Filename Overview
crates/openlogi-gui/src/platform/permissions.rs Adds probe_logitech_mouse_event() and a three-way classify(). The is_logitech_mouse_event helper reads ID_INPUT_MOUSE=1 from the sysfs uevent file, but this is a udev-set property not present there; the check always fails, keeping the permission indicator at Unknown after install.
crates/openlogi-core/src/binding.rs Splits the single uinput device into separate keyboard and pointer devices, adds GNOME gsettings accelerator lookup for desktop-navigation actions with fallbacks, and adds unit tests for the accelerator parser. Logic is sound.
crates/openlogi-gui/src/window_chrome.rs New file that wraps window content with an app-rendered titlebar and close/minimize/maximize buttons when Linux client-side decorations are active. Implementation is clean.
xtask/src/linux.rs Adds build_tarball to produce a portable .tar.gz before running nfpm, creates the output directory upfront, and copies binaries and packaging files into a staged temp tree. Implementation is correct.
.github/workflows/release.yml Extends the Linux release step to collect the tarball into dist, include .tar.gz in SHA256SUMS, sign it with minisign, and attach it as a release asset. Uses nullglob correctly.
packaging/linux/udev/70-openlogi.rules Adds SUBSYSTEM=input + ENV{ID_INPUT_MOUSE}==1 + ATTRS{id/vendor}==046d rule to grant uaccess to Logitech mouse event nodes. Syntax is correct.
crates/openlogi-gui/src/ipc_client.rs Adds a start_systemd_agent() path on Linux that tries systemctl --user start before falling back to a direct binary spawn. Failure modes are handled gracefully.
packaging/linux/install.sh Adds a ./bin/ fallback binary source for tarball installs and improves error messages. Logic is correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Button press] --> B{Action type}
    B -->|Click or Scroll| D[VIRTUAL_POINTER]
    B -->|Key combo| E[VIRTUAL_KEYBOARD]
    B -->|Desktop-nav| F[press_gsettings_shortcut_any]
    F --> G[configured_shortcuts via gsettings]
    G -->|found| E
    G -->|not found| H[fallback chord]
    H --> E
    D --> I[kernel pointer event node]
    E --> J[kernel keyboard event node]
    K[GUI permission check] --> L[probe_uinput]
    K --> M[probe_logitech_hidraw]
    K --> N[probe_logitech_mouse_event]
    N --> O{ID_INPUT_MOUSE=1 in sysfs?}
    O -->|Not present - udev DB only| P[returns None]
    P --> Q[classify gives Unknown not Granted]
    L --> R[classify result shown in UI]
    M --> R
    N --> R
Loading

Reviews (2): Last reviewed commit: "Merge pull request #2 from jvannoyx4/lin..." | Re-trigger Greptile

Comment on lines +133 to +135
WindowButton::Minimize => (IconName::Minimize, tr!("Minimize")),
WindowButton::Maximize if is_maximized => (IconName::Maximize, tr!("Restore")),
WindowButton::Maximize => (IconName::Maximize, tr!("Maximize")),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The Maximize icon is used for both the maximized and normal states. The tooltip label correctly changes to "Restore", but the icon stays as Maximize, which is inconsistent with desktop conventions (usually the icon also switches to indicate the restore action).

Suggested change
WindowButton::Minimize => (IconName::Minimize, tr!("Minimize")),
WindowButton::Maximize if is_maximized => (IconName::Maximize, tr!("Restore")),
WindowButton::Maximize => (IconName::Maximize, tr!("Maximize")),
WindowButton::Minimize => (IconName::Minimize, tr!("Minimize")),
WindowButton::Maximize if is_maximized => (IconName::Restore, tr!("Restore")),
WindowButton::Maximize => (IconName::Maximize, tr!("Maximize")),

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code

Comment on lines +2006 to +2024
fn configured_shortcuts(schema: &str, key: &str) -> Vec<(Vec<KeyCode>, KeyCode)> {
let output = std::process::Command::new("gsettings")
.args(["get", schema, key])
.output()
.ok();
let Some(output) = output else {
return Vec::new();
};
if !output.status.success() {
return Vec::new();
}
let Ok(stdout) = String::from_utf8(output.stdout) else {
return Vec::new();
};
accelerator_values(&stdout)
.into_iter()
.filter_map(|shortcut| parse_accelerator(shortcut))
.collect()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 gsettings subprocess spawned on every keypress

configured_shortcuts() spawns a gsettings get child process each time a desktop-navigation action is triggered (e.g. PreviousDesktop/NextDesktop each spawn two processes per press via press_gsettings_shortcut_any). Process startup overhead (~50–150 ms) will produce a noticeable delay on every button press for those actions. These shortcuts don't change at runtime, so the result could be cached at first use (e.g. via OnceLock or LazyLock per schema/key pair).

Fix in Codex Fix in Claude Code

@recchia

recchia commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Real-world confirmation that the new SUBSYSTEM=="input" rule is needed: on Ubuntu 26.04 (GNOME Shell 50.1, native Wayland) with the currently packaged 70-openlogi.rules installed, everything is ACL'd correctly except the event nodes:

  • /dev/uinput — ✅ uaccess ACL present (user:<seat-user>:rw-)
  • Logitech hidraw — ✅ covered by the VID 046D rules
  • /dev/input/event* — ❌ root:input 0660, no ACL → the evdev hook gets EACCES opening the mouse nodes (an MX Master 3S, plus the pointer subdevice of an MX Keys keyboard — kernel node "MX Keys Mouse"), so button capture fails even though frontmost detection (GNOME extension) and HID++ device control work

So the only workaround today is the keylogger-capable input group; the scoped per-vendor uaccess rule in this PR is the right fix. I'll test the updated rule from this branch on the same machine and follow up.

@recchia

recchia commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Follow-up as promised: tested the updated rule from this branch on the same machine (Ubuntu 26.04, GNOME Shell 50.1, native Wayland, systemd-logind), installed via cp + udevadm control --reload-rules + udevadm trigger — no re-login needed.

Results:

  • ✅ Both Logitech mouse event nodes (MX Master 3S via Bolt receiver, plus the pointer subdevice the MX Keys keyboard exposes — kernel node "MX Keys Mouse") picked up the uaccess ACL (user:<seat-user>:rw-)
  • ✅ Functional check: open(O_RDWR) + exclusive EVIOCGRAB + release succeeds on both nodes as the unprivileged seat user — the exact sequence the evdev hook performs
  • /dev/uinput still opens for writing
  • ✅ Correctly scoped: a non-Logitech pointer device (Elan touchpad, ID_INPUT_MOUSE but different vendor) did not get an ACL, and keyboards remain inaccessible — so this avoids handing the user blanket input-group access

LGTM from the Wayland/GNOME side.

@recchia

recchia commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Heads-up on a Wayland app_id / StartupWMClass overlap with #191

This PR and #191 (Wayland frontmost backends) both set the GUI's Wayland app_id + the desktop file's StartupWMClass, but with different values, so they'll conflict — and it's semantic, not just textual:

In #191 the gnome_shell frontmost backend identifies OpenLogi's own window by this exact string, and StartupWMClass has to match it for GNOME to group the window under the launcher icon — so the two values have to agree on one canonical id.

I'd propose standardizing on org.openlogi.openlogi, since that's already the established identifier across the repo:

  • crates/openlogi-gui/Cargo.tomlidentifier = "org.openlogi.openlogi"
  • xtask/src/manifest.rsconst APP_ID: &str = "org.openlogi.openlogi"
  • consistent with the macOS bundle-id family (org.openlogi.agent, org.openlogi.openlogi.dev)

Two small follow-ons if we go with that:

  1. This PR's new APP_ID = "openlogi" const in main.rs would duplicate/diverge from the existing APP_ID in xtask/manifest.rs — worth reusing one source of truth.
  2. The desktop file is openlogi.desktop while the app_id is reverse-DNS; StartupWMClass=org.openlogi.openlogi bridges that fine, so no rename is required (though org.openlogi.openlogi.desktop would be the strict freedesktop convention if we ever want it).

Happy to rebase #191 onto whatever lands here — just want us aligned on the one id string before either merges so the frontmost self-identification doesn't silently break.

@recchia

recchia commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Quick update: I've aligned #191 on org.openlogi.openlogi so there's nothing blocking on my side.

Rather than leave a bare literal, I hoisted the value into openlogi_core::brand::APP_ID (the platform-free brand module that already holds the shared URLs / deep-link vocab) and the GUI now references that const; the .desktop StartupWMClass keeps a documented literal copy since it can't import Rust.

So when this PR lands, the conflict in main_window_options should resolve to just app_id: Some(brand::APP_ID.into()), and the local APP_ID = "openlogi" const here can drop in favor of the shared one. Happy to rebase #191 on top whenever it's convenient — just say the word.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants