alebastr / sway-systemd

Systemd integration for Sway session
MIT License
131 stars 11 forks source link

Running foot server as a service results in incomplete terminal environment #9

Closed ibrokemypie closed 3 years ago

ibrokemypie commented 3 years ago

When starting foot --server as a systemd user service, the resulting foot clients are missing many environment variables which are present in terminals not started as a user service (starting alacritty directly). I believe this is because the session.sh only exports a small number of variables to systemd and dbus, but am not sure the best course of action to resolves this.

6 related

alebastr commented 3 years ago

I believe this is because the session.sh only exports a small number of variables to systemd and dbus

That is the right assumption. I'm being very careful with the list of copied variables, so it is includes only those that I deemed absolutely necessary. Still less conservative than Arch defaults though.

If you run the shell in footclient, it'll read your shell profile and populate most of the expected environment. The missing vars are something like XDG_SESSION_ID/XDG_VNTR/etc... that supposed to be inherited from the compositor process and aren't used by regular applications. What are the variables you need for the foot --server itself that are not set by the shell?

On the possible actions:

ibrokemypie commented 3 years ago

The issue that led me to this was actually the path variable not being set correctly (in foot the path was completely different from the default path set by arch, and I dont think environment.d could solve this (unless it can somehow get the original path variable and then set the new path variable to the old one. I wanted to avoid starting daemons from the sway config because of the note that they wouldnt be picked up by the assign cgroups script, of course its not a huge deal but its less than ideal. Im not sure import-environment x would be ideal either, it would work for the path, but theres various other variables that get set by arch by different packages, such as the CLOUDSDK_* variables. I'm not actually very clear on the reasoning for only importing specific variables with session.sh so cant really say whether that would be the right place to do it, although I'm fairly certain having the default system set path is probably important for a lot of cases at the very least. Unless you have a reason not to, I feel like at least PATH should be added to the session.sh imported variables, and then perhaps theres some way to import all variables on a service by service basis? To include all of the original environment for things that need it, like terminal servers.

Obviously I am not very clear on the ins and outs of all of this, so am curious what your thoughts are about the above

alebastr commented 3 years ago

I wanted to avoid starting daemons from the sway config because of the note that they wouldnt be picked up by the assign cgroups script

Good point. It can be worked around with systemd-run --user --scope --unit app-foot\\x2deserver.scope -- foot --server though.

I dont think environment.d could solve this (unless it can somehow get the original path variable and then set the new path variable to the old one.)

There's no original path. systemd user session lives independently from your user session and it never sees your shell environment. It's getting a bit complicated, but if we somewhat simplify the picture, there will be 3 different sets of environment:

There is a way to import a PATH value used by the shell with user environment generators though, something like this:

Code ```sh #!/usr/bin/sh # /etc/systemd/user-environment-generators/99-profile-d-path # Get PATH from /etc/profile and all /etc/profile.d/* snippets and pass it to the systemd if [ -f /etc/profile ]; then source /etc/profile echo PATH=$PATH fi ```

And I believe ArchWiki/systemd/User can offer much more on that topic.

I'm not actually very clear on the reasoning for only importing specific variables

  1. to be sure that nothing unexpected (LD_LIBRARY_PATH, DBUS_SESSION_BUS_ADDRESS and other vars that can change the behavior of systemd services) slips in.
  2. to be able to revert all changes in the environment after logout, so that another DE would not get a stale SWAYIPC or WAYLAND_DISPLAY within systemd user session. You can't do that if you import everything, because you won't know which variable was set by systemd internally and which was imported.

perhaps theres some way to import all variables on a service by service basis

No, I wish there was but alas... There's only Environment= and EnvironmentFile= for static assignments. Although if you can populate EnvironmentFile before starting the service, you can do it that way.

But I'm thinking I need to add --all-environment switch to session.sh that will do exactly that - import all available environment variables. I just can't predict what could be broken by that :-)

Sorry if I skipped any important point, but it's past midnight here so I'll check once again in the morning.

ibrokemypie commented 3 years ago

Thankyou for all of this, it all makes sense. I've started using.config/environment.d which is loaded into zsh shells through .zshenv, which is working well, and also applying to anything started by sway (eg firefox and MOZ_ENABLE_WAYLAND). I think I'll use your systemd-run snippet specifically for the foot server, just so I dont have to set up generators for each possible environment variable that the shell needs (all the cloudsdk ones for example) The all-environment argument sounds like a good idea, and with how many people run systemctl --user import-environment in theyre sway configs I guess it wont cause too many problems, but with your suggestions I think this can all work without it. Thankyou! Closing this issue now as my questions have been answered!

alebastr commented 2 years ago

I added the --all-environment option, but decided to leave it private. The ability to import full environment block is already deprecated in systemd v248+ (https://github.com/systemd/systemd/commit/32854f70447db074635d33abc9d94756072d63b4) and the reasoning behind that seems sound to me.

I'd actually prefer if another new option for adding an explicitly specified set of variables would be used instead. I.e. session.sh --with-cleanup -E VAR1 -E VAR2 -E VAR3.

markstos commented 2 years ago

Note that soon foot is likely to add systemd socket activation.

https://codeberg.org/dnkl/foot/pulls/890

So, after starting the systemd socket unit for foot, foot --server will be started on demand when the first client tries to connect.

alebastr commented 2 years ago

Yep, I saw #13. Just been busy between work and Fedora things to draft a proper reply.

In regards to the topic of the current issue, I have following (incomplete) solution:

#!/bin/sh
# vim: ts=4 sw=4 noet
# /etc/systemd/user-generators/foot-server-env
#
# Generate environment for the specific user service
# `systemd-generator(7)`

# Generator arguments: normal-dir, early-dir, late-dir
# We need to use the last one for the drop-in files
LATE_DIR="$3"

# Read environment variable from the user's shell
get_user_var () {
        /usr/bin/env -- "$SHELL" -l -c "echo \$$1"
}

generate_path_dropin () {
        DESTDIR="$LATE_DIR/$1.d"
        mkdir -p "$DESTDIR"
        cat >"$DESTDIR/environment.conf" <<-EOF
        # Automatically generated by $0 at $(date --utc)
        [Service]
        Environment=PATH=$(get_user_var PATH)
        EOF
}

generate_path_dropin foot-server.service

I believe my debugging has stopped on an attempt to figure out why this script completely ignores ~/.zprofile :smile: