feat(daemon): support user-scope systemd service installation on Linux#118
feat(daemon): support user-scope systemd service installation on Linux#118
Conversation
Changed Linux daemon installation from system-wide (root) to user-scope to match the Darwin implementation pattern and eliminate root permission requirements. Changes: - Updated all systemctl commands to use --user flag - Changed service path from /etc/systemd/system/ to ~/.config/systemd/user/ - Removed User=root and Group=root from service template - Changed WantedBy target from multi-user.target to default.target - Updated service management methods to use user-scope paths Closes #116 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Le He <AnnatarHe@users.noreply.github.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the Linux daemon installation process by transitioning from system-wide to user-scope systemd services. This modification removes the necessity for root privileges during installation and service management, streamlining the user experience and ensuring consistency with the daemon's behavior on Darwin systems. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the Linux daemon installation from a system-wide service to a user-scoped one, which is a great improvement as it removes the need for root permissions. The changes to the systemd service file and the systemctl commands are correct. I've identified a couple of areas for improvement regarding code duplication and error handling consistency that would make the implementation more robust and maintainable.
| // Also disable to clean up | ||
| _ = exec.Command("systemctl", "--user", "disable", "shelltime").Run() | ||
| // Remove old symlink if exists | ||
| _ = os.Remove(servicePath) |
There was a problem hiding this comment.
The error from os.Remove is ignored here. This is inconsistent with UnregisterService (line 150), which correctly handles the error by checking for os.IsNotExist. For better robustness and consistency, you should handle the error here as well. This will prevent silent failures if the file removal fails for unexpected reasons, such as permission issues.
if err := os.Remove(servicePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove old service symlink: %w", err)
}| currentUser, err := user.Current() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get current user: %w", err) | ||
| } | ||
|
|
||
| // Create systemd user directory if it doesn't exist | ||
| systemdUserDir := filepath.Join(currentUser.HomeDir, ".config/systemd/user") |
There was a problem hiding this comment.
There is code duplication in RegisterService, CheckAndStopExistingService, and UnregisterService. Each of these functions retrieves the current user and constructs paths for the systemd user directory. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider refactoring this logic into a shared helper method. This method could be responsible for providing the systemdUserDir and servicePath, centralizing the path construction logic.
1 participant
Changed Linux daemon installation from system-wide (root) to user-scope to match the Darwin implementation pattern and eliminate root permission requirements.
Closes #116
🤖 Generated with Claude Code