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.

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

commands: Add export command #5829

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jbenden
Copy link

@jbenden jbenden commented Nov 19, 2020

The export command allows one to set environment variables
easily within their configuration. It additionally allows
one to get an environment variable using the $ENV:<name>
syntax.

With the configuration snippet listed below, which MUST appear
before any exec commands -- so that environment variables are
all available to these newly launched programs -- set a number
of variables used by GTK & Qt for forcing Wayland rendering.

export GDK_BACKEND wayland
export CLUTTER_BACKEND wayland
export MOZ_ENABLE_WAYLAND 1
export QT_QPA_PLATFORM wayland-egl
export QT_WAYLAND_DISABLE_WINDOWDECORATION 1
export ELM_DISPLAY wl
export _JAVA_AWT_WM_NONREPARENTING 0
export SDL_VIDEODRIVER wayland

While the above does not demonstrate reading variables, the
example below does demonstrates its use:

export PATH $ENV:HOME/bin:$ENV:PATH

Signed-off-by: Joseph Benden [email protected]

@emersion
Copy link
Member

Ref #3976

The `export` command allows one to set environment variables
easily within their configuration. It additionally allows
one to get an environment variable using the `$ENV:<name>`
syntax.

With the configuration snippet listed below, which MUST appear
before any `exec` commands -- so that environment variables are
all available to these newly launched programs -- set a number
of variables used by GTK & Qt for forcing Wayland rendering.

```
export GDK_BACKEND wayland
export CLUTTER_BACKEND wayland
export MOZ_ENABLE_WAYLAND 1
export QT_QPA_PLATFORM wayland-egl
export QT_WAYLAND_DISABLE_WINDOWDECORATION 1
export ELM_DISPLAY wl
export _JAVA_AWT_WM_NONREPARENTING 0
export SDL_VIDEODRIVER wayland
```

While the above does not demonstrate reading variables, the
example below does demonstrates its use:

```
export PATH $ENV:HOME/bin:$ENV:PATH
```

Signed-off-by: Joseph Benden <[email protected]>
@jbenden
Copy link
Author

jbenden commented Nov 20, 2020

I was hoping this would get accepted, as there is simply not a clean method of injecting environment variables in to a Wayland session. I understand the argument on #3976 was that i3 rejected the modification. However, it was possible with i3 in a non-janky way, as an .xinitrc could be used.

In my case, SDDM directly runs a Sway Wayland session via a .desktop file. So I've tried adding a wrapper script without success (never mind that the distribution will overwrite the files involved.)

Once Sway is loaded, my launcher dmenu tells Sway to exec any launched programs. Since Sway is doing the launch, there is no possibility of injecting variables (unless my shell is involved, somehow - and jankily.)

So, this PR cleanly implements a way to set/modify environment variables and simply removes any jank that would otherwise be present.

I implore you to please reconsider.

Thanks for your time,
-Joe

@nagisa
Copy link

nagisa commented Nov 23, 2020

You can use pam_env module (typically enabled everywhere I've seen) and the .pam_environment file.

@jbenden
Copy link
Author

jbenden commented Nov 23, 2020

@nagisa Hi! Thank you for your tip! But, according to man pages; the user_envfile is not enabled by default due to security problems it could introduce.

I also had some strange issues with passing environment variables through Sway to the other processes - it'd cause Sway to not run. I didn't really debug it much, though...

Thanks!
-Joe

@rpigott
Copy link
Member

rpigott commented Nov 23, 2020

I am also against this change.

I just don't see what's wrong with the status quo. You haven't described any specific issue you have with environments, so how are we supposed to judge if this is an appropriate or even effective solution? Asserting that it's "jank" is no argument at all.

Once Sway is loaded, my launcher dmenu tells Sway to exec any launched programs.

dmenu just prints your selection to stdout, you can write any exec wrapper script you want, including one that modifies the environment. ... | dmenu | xargs exec_wrapper.sh

In my case, SDDM directly runs a Sway Wayland session via a .desktop file.

Well sway has made it clear that display managers are not supported, so the canonical answer would be to start sway from your profile and modify the environment there. But even with sddm you can make it work.

So I've tried adding a wrapper script without success (never mind that the distribution will overwrite the files involved.)

Then don't modify the shipped .desktop session file.

You can configure sddm with a local configuration file /etc/sddm.conf.d/ that will not be overwritten. sddm includes a setting that specifies the script to execute for desktop sessions. sddm.conf(5):

SessionCommand=
Path of script to execute when starting the user session. This script receives the value of the "Exec" setting in the ".desktop" file of the selected session and run it. Default value is "/usr/share/sddm/scripts/wayland-session".

You can also override the session with another of the same name in /usr/local/share/wayland-sessions, or even another session dir of your choosing set in the config.

You could also write a wrapper script, like you say you tried. You don't need to modify the .desktop session at all. If the Exec target of the shipped .desktop is just sway for you, make a new sway script higher up in your path and it will execute it instead, e.g. ~/.local/bin/sway:

#!/bin/bash
source ${XDG_CONFIG_HOME:-$HOME/.config}/sway/environment # export whatever
exec /usr/bin/sway $@

Do none of these proposed solutions work for you?

@jbenden
Copy link
Author

jbenden commented Nov 26, 2020

Hello,

I have tried both options and neither work well enough for what I'm trying to achieve.

The primary problem is it seems Sway gets a new environment after launch; discarding any environment variables set directly before calling in to the Sway binary.

I think for now I'll just maintain this patch.

Thank you for all your suggestions and time!
-Joe

@jbenden jbenden closed this Nov 26, 2020
@bqv
Copy link

bqv commented Apr 5, 2023

The use case for this is/was updating the environment of sway after it has already launched, which is currently not possible. If this was even manageable via some gdb hackery i'd be happy, but otherwise it's slightly annoying that this was just closed.

@jbenden jbenden reopened this Apr 6, 2023
@kennylevinsen
Copy link
Member

The primary problem is it seems Sway gets a new environment after launch;

That does not sound right, sway has the environment it was given by its parent. If the environment does not match what you anticipated, it is most likely that it was not set at all (e.g. wrapper script was never used due to .desktop file issues, pam_env was misconfigured, env was put into shell file not applicable to the login manager, ...).

The mess of complexity here is exactly why we do not condone weird mechanisms to manage the environment - they are fragile, complicated, and less flexible than basic wrapper scripts called directly.

The use case for this is/was updating the environment of sway after it has already launched, which is currently not possible.

This is the norm for any running program. If you want an environment variable to affect the program, set it before you run it.

If you want to dynamically change what variables affect programs exec'd by sway, this can easily be done with exec ~/bin/my-env-wrapper my-program, where ~/bin/my-env-wrapper is something like:

#!/bin/sh
export BLAH=1
exec $@

Note that I am somewhat ambivalent when it comes to creating a means to setenv in isolation - but it does not make something possible that wasn't already easy to do.

@jbenden
Copy link
Author

jbenden commented Apr 7, 2023

But how many of us should honestly stomp over all distro-provided files to accomplish this feat?

So, I am disagreeing; and still stand by this patch being needed to manage current and future environment variables of the full SEAT of the desktop (especially without systemd on a system!).

@kennylevinsen
Copy link
Member

But how many of us should honestly stomp over all distro-provided files to accomplish this feat?

You do not need to stomp on even a single file.

If the environment must apply to sway, using a sway-run script (e.g., direct from tty, from greetd, through a new .desktop file in /usr/local or similar) does not touch or interfere with any files managed by your package manager. You could even just put a script named "sway" earlier in your $PATH and it would take precedence without modification of the caller.

If the environment must only apply to children of sway, the aforementioned example does not even touch anything outside sway's own config. It also allows changing environment dynamically should one wish to do so for some reason.

Launchers also often allow you to specify how commands are executed, so the environment can also be set there for the same effect.

So, I am disagreeing; and still stand by this patch being needed to manage current and future environment variables of the full SEAT of the desktop (especially without systemd on a system!).

Turning sway into an "environment manager" is strictly not a goal. What matters is whether users are reasonably able to have appropriate environment configured for sway and client applications to operate correctly. One could say that this and similar patches bring convenience, but there is no indication that such system is necessary to fulfill any reasonable usecase. That is not to say that there is anything wrong with improving convenience, but arguing that the change is required for reasonable operation is unproductive.

@bqv
Copy link

bqv commented Apr 7, 2023

@jbenden I'd appreciate it if you maintain this patch, I'll continue to apply it even if it isn't merged

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

Successfully merging this pull request may close these issues.

6 participants