linux: add dbus and systemd activation services#7433
linux: add dbus and systemd activation services#7433mitchellh merged 5 commits intoghostty-org:mainfrom
Conversation
0f2a2b9 to
b0b8dd6
Compare
|
As a general note, on Linux an user may change the application id by setting the |
TBF, the user can do a lot of things to break stuff. Probably the only way to deal with this is to add some documentation to the |
|
I don't have time to review in depth but I'm also starting to wonder if we should just have a |
The main problem I think would be macOS - AFAIK you can't specify a CLI flag if you're launching Ghostty from the dock or even just double-clicking the app. There are several macOS behaviors that depend on detecting if Ghostty was launched from the desktop vs some other way. |
I think this can be solved by adding a wrapper shell script that adds the CLI flag and have the |
That does not strike me as a very robust solution. I think what might help is breaking some of this logic out into a separate function rather than a giant boolean expression in a if statement. I don't think the performance of the detection matters a whole lot since it's done once at startup, and if that information is needed outside of startup it should be done once at startup and saved in a global variable for future use. |
|
I personally am not concerned about performance here, but code complexity and the use of heuristics. It would be more robust to have the execution site tell us directly what kind of behavior do they want than trying to guess what it is. As a plus, if there's a switch, then it's trivial to perform manual testing of these behaviors. |
b0b8dd6 to
13d5cd8
Compare
|
Adding a manual flag would be nice I think, although I don't think the heuristics should entirely go away - for instance, someone can write a systemd service on their own without ever specifying via CLI that Ghostty should consider itself launched via systemd. This obviously wouldn't be an issue for the official systemd service of course, but it is something that could happen (especially when one uses a window manager that expects more user intervention and customization than normal desktop environments). Instead, we should probably respect a I do think the heuristics need a LOT more links pointing to official D-Bus/systemd documentation reaffirming the veracity and reliability of these environment variables, though... ;) |
I think this is simply not a valid case. If they decided to write a service like that, then are they really expecting Ghostty to behave differently from when they run This might also spell trouble for people using XDG autostart, since systemd would generate services for those using |
mitchellh
left a comment
There was a problem hiding this comment.
The main problem I think would be macOS - AFAIK you can't specify a CLI flag if you're launching Ghostty from the dock or even just double-clicking the app.
The main entry point is different on macOS and we already use it to specify where it was launched from: https://github.com/ghostty-org/ghostty/blob/1ff91625981ef92add92624c01b6b9095c365ed1/macos/Sources/App/macOS/main.swift This could be modified easily.
I think what @pluiedev said is probably best: introduce the config option, and change the config defaults to auto-detect. This already only happens once (per config reload which is rare) and we get caching this way.
That approach would get rid of the performance issue, make testing easier because we can simulate different launch environments, centralize detection in one place, we can probably more easily unit test as well instead of littering it throughout the codebase, and it lets users have an escape hatch to manually override our behavior if we detect incorrectly.
I looked at where we call these functions and in most cases we easily have a config available. The one place we don't is the locale loading for macOS but we can add a special case there I think because we have to load the locale before configuration is loaded necessarily for error messages at least.
I can try to extract that myself in a separate PR so we don't confuse it with the dbus/systemd stuff in here.
Related to #7433 This extracts our "launched from desktop" logic into a config option. The default value is detection using the same logic as before, but now this can be overridden by the user. This also adds the systemd and dbus activation sources from #7433. There are a number of reasons why we decided to do this: 1. It automatically gets us caching since the configuration is only loaded once (per reload, a rare occurrence). 2. It allows us to override the logic when testing. Previously, we had to do more complex environment faking to get the same behavior. 3. It forces exhaustive switches in any desktop handling code, which will make it easier to ensure valid behaviors if we introduce new launch sources (as we are in #7433). 4. It lowers code complexity since callsites don't need to have N `launchedFromX()` checks and can use a single value.
Related to #7433 This extracts our "launched from desktop" logic into a config option. The default value is detection using the same logic as before, but now this can be overridden by the user. This also adds the systemd and dbus activation sources from #7433. There are a number of reasons why we decided to do this: 1. It automatically gets us caching since the configuration is only loaded once (per reload, a rare occurrence). 2. It allows us to override the logic when testing. Previously, we had to do more complex environment faking to get the same behavior. 3. It forces exhaustive switches in any desktop handling code, which will make it easier to ensure valid behaviors if we introduce new launch sources (as we are in #7433). 4. It lowers code complexity since callsites don't need to have N `launchedFromX()` checks and can use a single value.
13d5cd8 to
4a049d7
Compare
|
Patch has been rebased on top of #7503 |
|
Do the title of the PR and commit need to change? Also, will there be any accompanying documentation about this? I'm a pretty experienced Linux person, but I don't have any idea what I could do with this. |
mitchellh
left a comment
There was a problem hiding this comment.
One simple change requested. Looking good!
alaviss
left a comment
There was a problem hiding this comment.
Some notes about the desktop and service files
| Type=Application | ||
| Comment=A terminal emulator | ||
| Exec=ghostty | ||
| TryExec=ghostty |
There was a problem hiding this comment.
According to the Desktop Entry Specification, this key is used to detect whether the application is installed so as to either show or autostart it.
For Ghostty the only way this file made it to the user system is if it's installed, so this key doesn't do much.
There was a problem hiding this comment.
The only way the .desktop file makes sense is if it's installed somewhere so presumably the user has installed the Ghostty binary somewhere too.
There was a problem hiding this comment.
That's what I'm saying, this key does nothing useful and can be removed.
It should "just work" if all the files get installed in the right place, as I would hope that they would when Ghostty is installed via a package. Not sure what we should document as I don't think that we need to document how As far as the title of the PR is concerned I think removing the word |
|
As noted in #7433 (comment), this might break when I think |
|
Added some documentation to the |
This is done to match against the default application id when Ghostty is built using debug configuration, preparing the Flatpak version for D-Bus activation support (#7433).
51e40ec to
a96dc8a
Compare
a96dc8a to
c1d04a6
Compare
|
@pluiedev's changes were resolved so I'm going to merge this. Thanks 😄 |
Flatpak currently does not export systemd user units. As such, remove references to it from D-Bus services to prevent D-Bus daemon from trying to start a non-existent service. Additionally, make sure that the D-Bus service name is correct for debug builds. Follow up to #7433
Reverts two commits: 977cd53 820b7e4 These break build from source on Linux for two reasons: 1.) The systemd user service needs to be installed in the `share` prefix, not the `lib` prefix. This lets it get picked up in `~/.local` but is also correct for just standard FHS paths. 2.) The `ghostty` path in the systemd user service needs to be absolute. We should interpolate in the build install prefix to form an absolute path.
Reverts two commits: 977cd53 820b7e4 These break build from source on Linux for two reasons: 1.) The systemd user service needs to be installed in the `share` prefix, not the `lib` prefix. This lets it get picked up in `~/.local` but is also correct for just standard FHS paths. 2.) The `ghostty` path in the systemd user service needs to be absolute. We should interpolate in the build install prefix to form an absolute path.
This replaces ghostty-org#7433. The improvements are: 1) Install the systemd user service in the proper directory depending on if it's a 'user' install or a 'system' install. This is controlled either by using the `--system` build flag (as most packages will) or by the `-Dsystem-package` flag. 2) Add the absolute path to the `ghostty` binary in the application file, the DBus service, and the systemd user service. This is done so that they do not depend on `ghostty` being in the `PATH` of whatever is launching Ghostty. That `PATH` is not necessarily the same as the `PATH` in a user shell (especially for DBus activation and systemd user services). 3) Adjust the DBus bus name that is expected by the system depending on the optimization level that Ghostty is compiled with.
This replaces ghostty-org#7433. The improvements are: 1) Install the systemd user service in the proper directory depending on if it's a 'user' install or a 'system' install. This is controlled either by using the `--system` build flag (as most packages will) or by the `-Dsystem-package` flag. 2) Add the absolute path to the `ghostty` binary in the application file, the DBus service, and the systemd user service. This is done so that they do not depend on `ghostty` being in the `PATH` of whatever is launching Ghostty. That `PATH` is not necessarily the same as the `PATH` in a user shell (especially for DBus activation and systemd user services). 3) Adjust the DBus bus name that is expected by the system depending on the optimization level that Ghostty is compiled with.
This replaces ghostty-org#7433. The improvements are: 1) Install the systemd user service in the proper directory depending on if it's a 'user' install or a 'system' install. This is controlled either by using the `--system` build flag (as most packages will) or by the `-Dsystem-package` flag. 2) Add the absolute path to the `ghostty` binary in the application file, the DBus service, and the systemd user service. This is done so that they do not depend on `ghostty` being in the `PATH` of whatever is launching Ghostty. That `PATH` is not necessarily the same as the `PATH` in a user shell (especially for DBus activation and systemd user services). 3) Adjust the DBus bus name that is expected by the system depending on the optimization level that Ghostty is compiled with.
This replaces #7433. The improvements are: 1) Install the systemd user service in the proper directory depending on if it's a 'user' install or a 'system' install. This is controlled either by using the `--system` build flag (as most packages will) or by the `-Dsystem-package` flag. 2) Add the absolute path to the `ghostty` binary in the application file, the DBus service, and the systemd user service. This is done so that they do not depend on `ghostty` being in the `PATH` of whatever is launching Ghostty. That `PATH` is not necessarily the same as the `PATH` in a user shell (especially for DBus activation and systemd user services). 3) Adjust the DBus bus name that is expected by the system depending on the optimization level that Ghostty is compiled with.
The XDG Freedesktop Portal has a _major_ undocumented requirement for programs that are launched/controlled by `systemd` to interact with the Portal. The unit _must_ be named `app-<appid>.service`. The Portal uses the systemd unit name figure out what the program's application ID is and it will only look at unit names that begin with `app-`. I can find no place that this is documented other than by inspecting the code or the issue and PR that introduced this feature. See the following code: https://github.com/flatpak/xdg-desktop-portal/blob/7d4d48cf079147c8887da17ec6c3954acd5a285c/src/xdp-utils.c#L152-L220 This may fix many people's issues with getting global shortcuts to work. Note that this is a breaking change if you have been using Ghostty compiled from source since ghostty-org#7433 was merged. You will need to ensure that any Ghosty systemd unit files _not_ prefixed with `app-` are deleted. Originally discussed on Discord: https://discord.com/channels/1005603569187160125/1394845362186879026 Co-authored-by: ambareeshbalaji@gmail.com
The XDG Freedesktop Portal has a _major_ undocumented requirement for programs that are launched/controlled by `systemd` to interact with the Portal. The unit _must_ be named `app-<appid>.service`. The Portal uses the systemd unit name figure out what the program's application ID is and it will only look at unit names that begin with `app-`. I can find no place that this is documented other than by inspecting the code or the issue and PR that introduced this feature. See the following code: https://github.com/flatpak/xdg-desktop-portal/blob/7d4d48cf079147c8887da17ec6c3954acd5a285c/src/xdp-utils.c#L152-L220 This may fix many people's issues with getting global shortcuts to work. Note that this is a breaking change if you have been using Ghostty compiled from source since ghostty-org#7433 was merged. You will need to ensure that any Ghosty systemd unit files _not_ prefixed with `app-` are deleted. Originally discussed on Discord: https://discord.com/channels/1005603569187160125/1394845362186879026 Co-authored-by: ambareeshbalaji@gmail.com
The XDG Freedesktop Portal has a _major_ undocumented requirement for programs that are launched/controlled by `systemd` to interact with the Portal. The unit _must_ be named `app-<appid>.service`. The Portal uses the systemd unit name figure out what the program's application ID is and it will only look at unit names that begin with `app-`. I can find no place that this is documented other than by inspecting the code or the issue and PR that introduced this feature. See the following code: https://github.com/flatpak/xdg-desktop-portal/blob/7d4d48cf079147c8887da17ec6c3954acd5a285c/src/xdp-utils.c#L152-L220 This may fix many people's issues with getting global shortcuts to work. Note that this is a breaking change if you have been using Ghostty compiled from source since ghostty-org#7433 was merged. You will need to ensure that any Ghosty systemd unit files _not_ prefixed with `app-` are deleted. Original discussion/PR in the XDG Desktop Portal repository: flatpak/xdg-desktop-portal#579 flatpak/xdg-desktop-portal#719 Originally discussed on Discord: https://discord.com/channels/1005603569187160125/1394845362186879026 Co-authored-by: ambareeshbalaji@gmail.com
The XDG Freedesktop Portal has a _major_ undocumented requirement for programs that are launched/controlled by `systemd` to interact with the Portal. The unit _must_ be named `app-<appid>.service`. The Portal uses the systemd unit name figure out what the program's application ID is and it will only look at unit names that begin with `app-`. I can find no place that this is documented other than by inspecting the code or the issue and PR that introduced this feature. See the following code: https://github.com/flatpak/xdg-desktop-portal/blob/7d4d48cf079147c8887da17ec6c3954acd5a285c/src/xdp-utils.c#L152-L220 This may fix many people's issues with getting global shortcuts to work. Note that this is a breaking change if you have been using Ghostty compiled from source since #7433 was merged. You will need to ensure that any Ghosty systemd unit files _not_ prefixed with `app-` are deleted. Original discussion/PR in the XDG Desktop Portal repository: flatpak/xdg-desktop-portal#579 flatpak/xdg-desktop-portal#719 Originally discussed on Discord: https://discord.com/channels/1005603569187160125/1394845362186879026 Co-authored-by: ambareeshbalaji@gmail.com
No description provided.