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

Support $NIRI_CONFIG environment variable. #400

Merged
merged 2 commits into from
Jun 28, 2024
Merged

Conversation

sodiboo
Copy link
Contributor

@sodiboo sodiboo commented May 26, 2024

On NixOS, it is a common pattern for config files to be in the read-only /nix/store/. With my current niri-flake, that is where it ends up, and it is symlinked to ~/.config/niri/config.kdl.

There are various reasons to not symlink that, mainly if you need multiple configs for a single user. When supplying config files from the nix store, it is standard practice to pass them with an environment variable using the standard makeWrapper helper function (example in openrefine). This is because the CLI options on most programs are not flexible enough.

it is easy to use a custom config by passing the --config flag, to either niri or niri validate, but it becomes a problem to prefix it to all commands. This is because (1) it will be rejected when invoking subcommands, so you can no longer call niri validate or niri msg, and (2) it cannot be passed multiple times, so prefixing it like this is essentially disabling the --config flag for all users of this wrapper. Essentially, it will cause all niri commands to be invalid except niri and niri --session.

Instead of bodging something strange onto the cli arguments, i've instead added an environment variable. Then, it can be easily substituted with the standard wrapper approach.

This allows me to easily create a pkgs.niri-unstable.with-config function that can add this in a wrapper. I've been thinking of doing this for a long time, but didn't implement anything because i was unsure of any real use cases for it. Since then, i've started using niri on my login screen, something which currently relies heavily on implementation details of niri-flake. Additionally, i believe such a function (for which this environment variable is necessary) would also resolve sodiboo/niri-flake#278.

@minego
Copy link

minego commented May 26, 2024

I love this change and am very excited for it. This will greatly simplify my nixOS setup. Thank you.

Copy link
Owner

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

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

I see what the problem is. You mentioned that some programs have a config environment variable for this reason. However, I'd like to know, how do other compositors handle this? Since compositors have unique questions in this regard, namely:

Should NIRI_CONFIG be propagated to children?

  • If yes, other programs developed on NixOS may start relying on the presence of this. We might need to start force setting the variable then.
  • If not, running nested development niri (i.e. ./target/debug/niri) won't pick it up.

wiki/Configuration:-Overview.md Show resolved Hide resolved
@sodiboo
Copy link
Contributor Author

sodiboo commented May 27, 2024

I'd like to know, how do other compositors handle this?

With Sway, the sway command is the only one that accepts a config argument. The problems with prefixing an argument don't apply because (1) all other, non-"start the compositor" commands (e.g. swaymsg), are not affected and (2) there are no subcommands that "conflict" with the config argument. Additionally, passing --config mulitple times isn't an issue, it will just accept the last one. It has a couple "subcommands" but they also don't conflict with --config, so e.g. sway --config a --config b --validate will validate the config at b ignore a, and sway --config a --config b --version will print the version. (remember: --config a is injected by a wrapper, so the user really typed sway --config b --validate, which ought to work, and it does for Sway but not in the current state of niri)

Hyprland works similarly: the Hyprland executable has only three possible arguments, --help, --config and --i-am-really-stupid (allows to run as root). It's kind of strange, in that it accepts the first config and also doesn't ignore configs if passed --help. Nonetheless, this is easily "wrapped" by adding arguments to the end of the invocation. hyprctl is separate executable, and a wrapper wouldn't touch.

Wayfire supports "last passed --config is active" like Sway and Hyprland, but also reads from $WAYFIRE_CONFIG_FILE if no argument is passed to it. This environment variable seems unnecessary for use with makeWrapper, because a prefix argument works just as well with wayfire, but they might have other use cases in mind. Wayfire propagates the environment variable to processes spawned in its autostart config section.

Reworking the CLI arguments such that a prefix of --config is always valid for all invocations, and having it at the start also affects niri validate would be fine. All of the above supports this, actually. But they don't have IPC on the same executable.

Should NIRI_CONFIG be propagated to children?

Intuitively, I'd say no. makeWrapper should typically be completely opaque and really quite indistinguishable from a search/replace for the default path in a source code. Generally, propagating a config to child processes isn't a consideration for most programs, so this isn't something i had thought of before.

Ideally, if NIRI_CONFIG is set globally, it should be propagated, but not if invoked from makeWrapper. This is basically impossible to detect, so it's probably easier to just accept that we should just accept it as a global.

If yes, other programs developed on NixOS may start relying on the presence of this. We might need to start force setting the variable then.

If other programs need to read from the niri config, i reckon it'd make more sense to implement as a niri msg subcommand. Either to dump the path, or something more elaborate that can query settings in a better structured way (supporting defaults, etc). I don't think we should support programs trying to rely on this environment variable to read config values, and the documentation could be updated to reflect such a stance. We can enforce such a stance by deliberately not setting NIRI_CONFIG when it is passed as a command line argument, making it an unreliable way to find the "active config".

If not, running nested development niri (i.e. ./target/debug/niri) won't pick it up.

If we don't propagate it, but it is desired to use for a nested dev niri, it is fairly easy (on NixOS at least) to pass the config path as a variable. If we say that the config lives a /nix/store/c0ffee-config.kdl (hashes are longer than c0ffee), then it can contain a line like environment { NIRI_CONFIG "/nix/store/c0ffee-config.kdl"; }. This is because nix store hashes (for regular derivations) are based on inputs, so it is known in advance at "build-time" (strange terminology for a config file; but this essentially means "the point at which this config is written to the nix store") and can be circularly referenced like that.

Note that i did not say this is trivial; it requires some restructuring in niri-flake to achieve this easily. Such functionality could also just be written into niri itself with some propagate-config-env flag in the config file. But it's not strictly necessary for NixOS (and of course, on systems without a /nix/store/-like path, it's also easy to include the path in the config as /etc/niri/config.kdl or wherever you're putting it)

I would however argue that the nested compositor not picking it up is desirable in some cases too. I've personally been working on a branch once or twice that is based on an older version of niri, where my current configuration is invalid. It would honestly be fine for such a use case to have NIRI_CONFIG not propagate, and reserve $XDG_CONFIG_HOME/niri/config.kdl for a "testing config" that only applies to the nested instance. This is the reason why it's ignored when set to an invalid value, such as an empty string, so that it can be explicitly unset.


Ultimately, i don't think it's a significant problem either way. It doesn't affect my use case. The two main arguments in favor of either way are:

We don't want other programs to rely on this environment variable => unset it

and

We do want nested development niri to be able to rely on it => propagate it.

(note that general "nested niri" doesn't apply really, at least not on NixOS, because in that case it would still be executed in the wrapper and get the config no matter what)


As for adding a niri msg subcommand to get the config path, it could make sense to use this in the niri compositor as well. As in, either make the default config path first try to get it from niri msg, or add a flag like --inherit-config that will try to run with the same config as the outer compositor. This feels strange? Not sure we should be doing it, but worth considering to some extent. Maybe pointless, since a similar result could be achieved with NIRI_CONFIG=$(niri msg config-path) before any invocation.

@YaLTeR
Copy link
Owner

YaLTeR commented May 27, 2024

Thanks for the detailed explanation! Hmm, there's probably a relatively straightforward way to always accept --config and take the last value in case multiple are set. Maybe that's indeed better, if only because it doesn't add an extra way of passing the config?

niri msg subcommand to get the config path

Could be added if anyone needs it I guess.

add a flag like --inherit-config that will try to run with the same config as the outer compositor. This feels strange?

I agree it's kinda strange; don't think it's necessary.

@sodiboo
Copy link
Contributor Author

sodiboo commented May 29, 2024

there's probably a relatively straightforward way to always accept --config and take the last value in case multiple are set. Maybe that's indeed better, if only because it doesn't add an extra way of passing the config?

I don't wish to implement this personally, as it's kind of annoying to restructure the entire cli for what is seemingly one use case.

I also think that for niri in particular, it's not the most intuitive solution, because then you can pass --config to niri msg which doesn't really make sense. The thing in common for most other compositors is that the command that accepts --config is only for launching the compositor. All possible permutations of the command (except --help and --version) would actually use the config in a meaningful way.

That being said, if it were implemented and --config could reliably be passed to all niri invocations unconditionally, that would be entirely sufficient for my NixOS use-cases.

src/main.rs Outdated Show resolved Hide resolved
wiki/Configuration:-Overview.md Outdated Show resolved Hide resolved
wiki/Configuration:-Overview.md Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@YaLTeR YaLTeR enabled auto-merge (rebase) June 28, 2024 10:49
@YaLTeR YaLTeR merged commit d180e60 into YaLTeR:main Jun 28, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Generate config without home-manager
3 participants