Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] make systemd-run example more reliable #2048

Open
wants to merge 5 commits into
base: next
Choose a base branch
from
+55 −11
Open

[DOC] make systemd-run example more reliable #2048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa9ebc7
[DOC] properly escape systemd-run example
Atemu Nov 10, 2024
d697450
[DRUN] provide desktop file names to -run-command
DaveDavenport Nov 10, 2024
0fecb6a
[DOC] use new app_id in systemd-run example
Atemu Nov 12, 2024
91dd739
[DOC] document new PATTERNs
Atemu Nov 12, 2024
51eaeda
[DOC] don't recommend --unit when app_id is not present
Atemu Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion doc/rofi.1.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ Example to run applications in a dedicated cgroup with systemd. Requires a
shell to escape and interpolate the unit name correctly.

```bash
"bash -c 'systemd-run --user --unit=app-rofi-\$(systemd-escape {cmd})-\$RANDOM {cmd}'"
"bash -c 'systemd-run --user [--unit=app-rofi-\"\$(systemd-escape \"{app_id}\")\"-\$RANDOM] {cmd}'"
Copy link
Author

Choose a reason for hiding this comment

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

While it's nice to mark this as optional, that doesn't actually help anyone because you can only have one or the other but both drun and run could be used.

What we actually need is some sort of identifier that is unit-name-safe, represents what is being ran decently well and works with both drun and run.

For desktop applications, I see the desktop_id fulfilling that role. Though I'd prefer the .desktop suffix be stripped.

For commands, I see the (escaped) executable name as the best candidate. Of course it'd "break" (as in: not represent what is being ran) if you ran sh -c ... or something but at that point you're explicitly doing that.

Perhaps it'd be best to do that within rofi and provide an abstract execution_id or something that works like I described.

Perhaps it'd be even better if we had run-as-system service functionality built right into rofi?

Executing systemd-run like this in -run-command really isn't very robust as I've discovered with scrcpy, so perhaps "native" handling would be better.

I'm really not sure how to proceed with this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know.. I never use systemd-run. What does 'run-as-system' service entail? do you have a link to some documentation?
(we did add the DBusActivatable to latest rofi).

Copy link
Author

Choose a reason for hiding this comment

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

It means that any app launched is started as an ad-hoc systemd unit. For example the firefox I'm typing this in runs as the app-rofi-firefox-24354.service user unit.

What this means in practice is that the initial process aswell as every process ever spawned it will be run in a cgroup that is separate from all other apps and you can apply any cgroup-based policy to it if you like.

This is most useful for stopping units (e.g. early OOM) but you could in theory do anything cgroups with this.

It also brings with it all of systemd's accounting and centralised log collection. See e.g. this example output:

$ systemctl --user status app-rofi-firefox-24354.service 
● app-rofi-firefox-24354.service - [systemd-run] /run/current-system/sw/bin/firefox --name firefox
     Loaded: loaded (/run/user/1000/systemd/transient/app-rofi-firefox-24354.service; transient)
  Transient: yes
     Active: active (running) since Wed 2025-01-29 14:04:39 CET; 1 day 8h ago
 Invocation: 06509bffe1dc46f79fafe6a952641d60
   Main PID: 437580 (.firefox-wrappe)
      Tasks: 594 (limit: 37644)
     Memory: 3.4G (peak: 3.8G, swap: 833M, swap peak: 1.8G, zswap: 231.6M)
        CPU: 1h 54min 32.764s
     CGroup: /user.slice/user-1000.slice/[email protected]/app.slice/app-rofi-firefox-24354.service
             ├─437580 /nix/store/kbin6wx8bciz7c50i4y93n35wfqnrfzs-firefox-134.0.2/bin/.firefox-wrapped --name firefox
             ├─437678 /nix/store/kbin6wx8bciz7c50i4y93n35wfqnrfzs-firefox-134.0.2/lib/firefox/firefox -contentproc ...
...

Jan 30 19:56:22 THESEUS firefox[437699]: Failed to create /Users for shader cache (Permission denied)---disabling.
Jan 30 19:56:23 THESEUS firefox[437699]: Failed to create /Users for shader cache (Permission denied)---disabling.
...

I can see that the "firefox app" I launched currently uses 3.4GiB and has 833M in swap across all of its processes. I can also see all the processes Firefox has spawned. I can also see this app launch instance's logs in isolation.

Theoretically I could set e.g. memory or swap limits here though I'm not currently doing that.

Setting a memory limit without running firefox in its own cgroup would be very hard indeed and even finding out how much memory it's using usually isn't trivial with such multi-process applications. cgroups make everything simpler here.

AFAIK the big DEs' launchers also run every app launched within an ad hoc systemd unit but I don't use those, so I can't tell you how it works there.

There isn't really documentation on this other than standard systemd (user) service/unit docs and cgroup docs. There isn't really much to it other than simply using these systems.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know what it does, but not how to do this from the rofi? Is there a dbus API I can call to launch the service?

```

`-run-shell-command` *cmd*
Expand Down Expand Up @@ -878,6 +878,10 @@ replaced at runtime:
- `{ssh-client}`: the configured ssh client (see -ssh-client)
- `{cmd}`: the command to execute
- `{window}`: the window ID of the selected window (in `window-command`)
- `{desktop_file_path}`: The absolute path to the desktop file launched
- `{app_id}`: The desktop file's file name without `.desktop` suffix
- `{desktop_id}`: The `{desktop_file_path}` but with all `/` replaced by `-`
(see https://specifications.freedesktop.org/desktop-entry-spec/latest/file-naming.html)

It processes the string as follows: `{key}`
is replaced by its value, if `{key}` is not set it is removed. If the `{key}`
Expand Down
17 changes: 17 additions & 0 deletions include/helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,23 @@ gboolean helper_execute_command(const char *wd, const char *cmd,
gboolean run_in_term,
RofiHelperExecuteContext *context);

/**
* @param wd The work directory (optional)
* @param cmd The cmd to execute
* @param run_in_term Indicate if command should be run in a terminal
* @param context The startup notification context, if any
* @param ... tuples of extra parameters the string can search/replace
*
* Execute command.
* If needed members of context are NULL, they will be filled.
* Pass {cmd} into the va-arg list.
*
* @returns FALSE On failure, TRUE on success
*/
gboolean helper_execute_command_full(const char *wd, const char *cmd,
gboolean run_in_term,
RofiHelperExecuteContext *context, ...);

/**
* @param file The file path
* @param height The wanted height
Expand Down
38 changes: 29 additions & 9 deletions source/helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,15 @@ void cmd_set_arguments(int argc, char **argv) {
stored_argv = argv;
}

int helper_parse_setup(char *string, char ***output, int *length, ...) {
static int helper_parse_setup_v(char *string, char ***output, int *length,
va_list ap) {
GError *error = NULL;
GHashTable *h;
h = g_hash_table_new(g_str_hash, g_str_equal);
// By default, we insert terminal and ssh-client
g_hash_table_insert(h, "{terminal}", config.terminal_emulator);
g_hash_table_insert(h, "{ssh-client}", config.ssh_client);
// Add list from variable arguments.
va_list ap;
va_start(ap, length);
while (1) {
char *key = va_arg(ap, char *);
if (key == (char *)0) {
Expand All @@ -93,7 +92,6 @@ int helper_parse_setup(char *string, char ***output, int *length, ...) {
}
g_hash_table_insert(h, key, value);
}
va_end(ap);

char *res = helper_string_replace_if_exists_v(string, h);
// Destroy key-value storage.
Expand All @@ -115,6 +113,13 @@ int helper_parse_setup(char *string, char ***output, int *length, ...) {
}
return FALSE;
}
int helper_parse_setup(char *string, char ***output, int *length, ...) {
va_list ap;
va_start(ap, length);
int retv = helper_parse_setup_v(string, output, length, ap);
va_end(ap);
return retv;
}

void helper_tokenize_free(rofi_int_matcher **tokens) {
for (size_t i = 0; tokens && tokens[i]; i++) {
Expand Down Expand Up @@ -1027,15 +1032,21 @@ gboolean helper_execute(const char *wd, char **args, const char *error_precmd,
gboolean helper_execute_command(const char *wd, const char *cmd,
gboolean run_in_term,
RofiHelperExecuteContext *context) {
return helper_execute_command_full(wd, cmd, run_in_term, context, "{cmd}",
cmd, (char *)0);
}

static gboolean helper_execute_command_full_v(const char *wd, const char *cmd,
gboolean run_in_term,
RofiHelperExecuteContext *context,
va_list ap) {
char **args = NULL;
int argc = 0;

if (run_in_term) {
helper_parse_setup(config.run_shell_command, &args, &argc, "{cmd}", cmd,
(char *)0);
helper_parse_setup_v(config.run_shell_command, &args, &argc, ap);
} else {
helper_parse_setup(config.run_command, &args, &argc, "{cmd}", cmd,
(char *)0);
helper_parse_setup_v(config.run_command, &args, &argc, ap);
Comment on lines -1034 to +1049
Copy link
Author

Choose a reason for hiding this comment

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

I just noticed this: aren't these flipped now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

run_in terminal calls the 'run_shell_command'.. I think this is ok?

Copy link
Author

Choose a reason for hiding this comment

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

I don't have a clue how this code actually works, I only noticed that the _v-suffixed version is now called in the other conditional.

}

if (args == NULL) {
Expand Down Expand Up @@ -1063,10 +1074,19 @@ gboolean helper_execute_command(const char *wd, const char *cmd,

return helper_execute(wd, args, "", cmd, context);
}
gboolean helper_execute_command_full(const char *wd, const char *cmd,
gboolean run_in_term,
RofiHelperExecuteContext *context, ...) {
va_list ap;
va_start(ap, context);
gboolean retv =
helper_execute_command_full_v(wd, cmd, run_in_term, context, ap);
va_end(ap);
return retv;
}

char *helper_get_theme_path(const char *file, const char **ext,
const char *parent_file) {

char *filename = rofi_expand_path(file);
g_debug("Opening theme, testing: %s\n", filename);
if (g_path_is_absolute(filename)) {
Expand Down
5 changes: 4 additions & 1 deletion source/modes/drun.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,10 @@ static void exec_cmd_entry(DRunModeEntry *e, const char *path) {
// terminal.
gboolean terminal =
g_key_file_get_boolean(e->key_file, e->action, "Terminal", NULL);
if (helper_execute_command(exec_path, fp, terminal, sn ? &context : NULL)) {
if (helper_execute_command_full(exec_path, fp, terminal, sn ? &context : NULL,
"{cmd}", fp, "{desktop_file_path}", e->path,
"{app_id}", e->app_id, "{desktop_id}",
e->desktop_id, (char *)0)) {
char *drun_cach_path = g_build_filename(cache_dir, DRUN_CACHE_FILE, NULL);
// Store it based on the unique identifiers (desktop_id).
history_set(drun_cach_path, e->desktop_id);
Expand Down
Loading