nixos/direnv: init #192667

infinisil · 2022-09-23T20:42:57Z

Description of changes

nix-direnv is surprisingly annoying to configure correctly, due to it needing both system-wide adjustments (nix.conf changes), but also user-wide adjustments (telling direnv to load nix-direnv using ~/.config/direnv/direnvrc).

This module puts an end to that, allowing a direnv + nix-direnv installation with just

{
  programs.direnv.enable = true;
}

Currently this module comes with nix-direnv enabled by default and there's no way to turn it off. If there ever is a need to only enable direnv without nix-direnv we can do that in another PR. I don't need that myself.

Ping @zimbatm @Mic92 @bbenne10

Things done

Tested the module

This allows setting DIRENV_CONFIG to (nix-profile)/share/direnv and have it find the nix-direnv script, while also allowing other direnv integrations to coexist. Old path kept for backwards compat.

nix-direnv is surprisingly annoying to configure correctly, due to it needing both system-wide adjustments (`nix.conf` changes), but also user-wide adjustments (telling direnv to load nix-direnv using ~/.config/direnv/direnvrc). This module puts an end to that, allowing a direnv + nix-direnv installation with just programs.direnv.enable = true;

nixos/modules/programs/direnv.nix

zimbatm · 2022-09-26T09:59:51Z

pkgs/tools/misc/nix-direnv/default.nix

@@ -26,6 +26,9 @@ stdenv.mkDerivation rec {
  installPhase = ''
    runHook preInstall
    install -m500 -D direnvrc $out/share/nix-direnv/direnvrc
+
+    # Allows NixOS to set DIRENV_CONFIG to /run/current-system/sw/share/direnv and have direnv load this


In that case, the module will also have to handle user settings since DIRENV_CONFIG is also used to locate the direnv.toml.

Alternatively, it could make sense to change direnv so it also loads all the libraries in
share/direnv/lib/*.sh, following the XDG spec?

I think that, given that direnv already reads ${XDG_CONFIG_HOME:-$HOME/.config}/direnv/lib/*.sh it makes perfect sense for it to read the equivalent system directory, does it not?

Yeah I think so too, I quote the XDG Base Directory Specification:

$XDG_CONFIG_DIRS defines the preference-ordered set of base directories to search for configuration files in addition to the $XDG_CONFIG_HOME base directory. The directories in $XDG_CONFIG_DIRS should be seperated with a colon ':'.

If $XDG_CONFIG_DIRS is either not set or empty, a value equal to /etc/xdg should be used.

I created a direnv PR for this: direnv/direnv#990

bbenne10 · 2022-09-26T12:08:50Z

This generally looks okay.
I would like to see the addition of something like programs.direnv.initExtra that allows the user to specify the additional direnv snippets they're using. The way we're overriding DIRENV_CONFIG now makes that impossible (and so the additional snippet that I use that changes where caches are stored is not available at all with this configuration method).

I will say though that I'm not completely sure I see the need to introduce this.
Direnv is somewhat explicitly a user-level tool.
nix-direnv changes this user-level tool a bit to enable caching.
That caching is still useful even if they're occasionally garbage collected without the user knowing.

All of this to say that the system changes are good but not wholly necessary to eek out use from nix-direnv.
That being said: I support this change, but I don't think such a simple approach as we see here is as useful in the general sense as @infinisil maybe hopes it'll be?

bbenne10 · 2022-09-26T12:10:11Z

nixos/modules/programs/direnv.nix

+      "/share/direnv"
+    ];
+
+    environment.sessionVariables.DIRENV_CONFIG = "/run/current-system/sw/share/direnv";


As mentioned: This disallows user customization entirely, I think.
Either we need a change to direnv to address this or we need a change here to give us the ability for the user to specify additional contents.

Mic92 · 2022-09-26T14:52:37Z

pkgs/tools/misc/nix-direnv/default.nix

@@ -26,6 +26,9 @@ stdenv.mkDerivation rec {
  installPhase = ''
    runHook preInstall
    install -m500 -D direnvrc $out/share/nix-direnv/direnvrc
+
+    # Allows NixOS to set DIRENV_CONFIG to /run/current-system/sw/share/direnv and have direnv load this
+    install -m500 -D direnvrc $out/share/direnv/lib/nix-direnv.sh


Could this not be a symlink?

infinisil · 2022-09-29T18:22:09Z

This generally looks okay. I would like to see the addition of something like programs.direnv.initExtra that allows the user to specify the additional direnv snippets they're using. The way we're overriding DIRENV_CONFIG now makes that impossible (and so the additional snippet that I use that changes where caches are stored is not available at all with this configuration method).

Sounds like this will be addressed by #192667 (comment)

I will say though that I'm not completely sure I see the need to introduce this. Direnv is somewhat explicitly a user-level tool. nix-direnv changes this user-level tool a bit to enable caching. That caching is still useful even if they're occasionally garbage collected without the user knowing.

All of this to say that the system changes are good but not wholly necessary to eek out use from nix-direnv. That being said: I support this change, but I don't think such a simple approach as we see here is as useful in the general sense as @infinisil maybe hopes it'll be?

I had a bit of trouble understanding your concern, but I believe you're trying to say that the nix.settings are somewhat optional in this module, since they only affect GC relating to nix-direnv. And if we don't need to set nix.settings, the module can be entirely user-level, like a home-manager module (of which there already is one here).

Here's some arguments for why I think such a module is useful:

It is a main feature for nix-direnv to prevent shells from being garbage-collected, see the first couple sentences of its readme. Users of nix-direnv expect that feature to work
Currently if one wants to use direnv + nix-direnv and prevent GC, there is no easy way to do this in a single step. The NixOS installation instructions for nix-direnv require a manual step of editing ~/.direnvrc, while the home-manager install instructions require a separate step for adding the correct nix.conf settings.
Personal anecdote: When I installed my NixOS configuration onto a new machine, I was confused to find that my direnv shells were not cached with nix-direnv anymore. Turns out I manually added ~/.direnvrc (as described in the NixOS installation guide for nix-direnv), which I of course forgot to copy to my new machine (and also every separate user). Preventing such situations is a core selling-point of NixOS.
If a program can be configured both on a system-level (meaning for all users) and on a user-level (overriding the system-level config), then it makes sense for there to be a NixOS module, because one might want to provide a default for all users. This is also often seen with user-level systemd services provided by the system.

bbenne10 · 2022-09-29T20:07:37Z

I had a bit of trouble understanding your concern, but I believe you're trying to say that the nix.settings are somewhat optional in this module, since they only affect GC relating to nix-direnv. And if we don't need to set nix.settings, the module can be entirely user-level, like a home-manager module (of which there already is one here).

This is exactly it.

There are already about 5 ways to install nix-direnv.
This adds a 6th that must additionally be supported.
We already spend quite a bit of debugging time figuring out what version of nix-direnv someone has and how it was installed (as each installation procedure has different pitfalls), and this will (maybe) make this just a bit harder.

infinisil · 2022-10-03T15:54:54Z

There are already about 5 ways to install nix-direnv. This adds a 6th that must additionally be supported. We already spend quite a bit of debugging time figuring out what version of nix-direnv someone has and how it was installed (as each installation procedure has different pitfalls), and this will (maybe) make this just a bit harder.

No this PR doesn't add another, it will allow you to replace the existing instructions for NixOS with much simpler ones, namely

{
  programs.direnv.enable = true;
}

nixos-discourse · 2022-10-12T15:18:48Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-37/22409/1

mweinelt · 2022-10-12T15:34:34Z

Can we get a release notes entry please, so we nudge people to use this instead of the clumsy things they configured before.

bbenne10 · 2022-10-12T15:40:03Z

No this PR doesn't add another, it will allow you to replace the existing instructions for NixOS with much simpler ones, namely
{
  programs.direnv.enable = true;
}

I will have to describe the old behavior and the new behavior until acceptance of the new version is high enough. Likely well beyond the 22.11 release, as old installs may not be updated immediately.

Do know that I have said I support this change once appropriate changes are made to direnv as to allow the user to provide augmentations to the direnv stdlib. I am holding off approval until direnv/direnv#990 is approved, merged, released and updated in nixpkgs.

infinisil · 2022-10-12T15:50:09Z

@bbenne10 Awesome that sounds great. You can also just keep everything as-is for now, the old way of installing it continues working the same way.

SuperSandro2000 · 2022-10-14T02:16:20Z

nixos/modules/programs/direnv.nix

+      Whether to enable direnv integration. Takes care of both installation and
+      setting up the sourcing of the shell. Additionally enables nix-direnv
+      integration. Note that you need to logout and login for this change to apply.


This should mention the nix setting changes.

drupol · 2023-06-02T17:10:53Z

What's the status of this useful PR ?

bbenne10 · 2023-06-03T18:52:51Z

direnv/direnv#990 is still open. I still haven't signed off on it. This PR would still artificially limit the ability of users to override direnv stdenv functions when using this installation mechanism.

that is to say: no - there's no change, as the referenced PR is stagnate right now too. If we can get that merged, I'm happy to assign here once the appropriate changes are made to allow extraConfig or such to propagate into the nix installed direnv / nix-direnv environments.

jian-lin · 2023-07-05T04:16:25Z

FYI, #241528

infinisil added 2 commits September 23, 2022 22:14

direnv: Also install to /share/direnv/lib/nix-direnv.sh

7b4b779

This allows setting DIRENV_CONFIG to (nix-profile)/share/direnv and have it find the nix-direnv script, while also allowing other direnv integrations to coexist. Old path kept for backwards compat.

github-actions bot added 6.topic: nixos 8.has: module (update) labels Sep 23, 2022

ofborg bot requested review from bbenne10 and Mic92 September 23, 2022 20:53

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Sep 23, 2022

ck3d reviewed Sep 24, 2022

View reviewed changes

nixos/modules/programs/direnv.nix Show resolved Hide resolved

zimbatm reviewed Sep 26, 2022

View reviewed changes

bbenne10 suggested changes Sep 26, 2022

View reviewed changes

Mic92 reviewed Sep 26, 2022

View reviewed changes

infinisil mentioned this pull request Sep 29, 2022

Additionally source libraries from XDG_CONFIG_DIRS direnv/direnv#990

Open

mweinelt added the 9.needs: documentation label Oct 12, 2022

SuperSandro2000 reviewed Oct 14, 2022

View reviewed changes

Gerg-L mentioned this pull request Jul 4, 2023

nixos/direnv: init #241528

Merged

1 task

drupol closed this in #241528 Jul 12, 2023

Artturin deleted the direnv-module branch August 28, 2023 00:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/direnv: init #192667

nixos/direnv: init #192667

infinisil commented Sep 23, 2022

zimbatm Sep 26, 2022

bbenne10 Sep 26, 2022

infinisil Sep 29, 2022

infinisil Sep 29, 2022

bbenne10 commented Sep 26, 2022

bbenne10 Sep 26, 2022

Mic92 Sep 26, 2022

infinisil commented Sep 29, 2022

bbenne10 commented Sep 29, 2022

infinisil commented Oct 3, 2022

nixos-discourse commented Oct 12, 2022

mweinelt commented Oct 12, 2022

bbenne10 commented Oct 12, 2022

infinisil commented Oct 12, 2022

SuperSandro2000 Oct 14, 2022

drupol commented Jun 2, 2023

bbenne10 commented Jun 3, 2023

jian-lin commented Jul 5, 2023

nixos/direnv: init #192667

nixos/direnv: init #192667

Conversation

infinisil commented Sep 23, 2022

Description of changes

Things done

zimbatm Sep 26, 2022

Choose a reason for hiding this comment

bbenne10 Sep 26, 2022

Choose a reason for hiding this comment

infinisil Sep 29, 2022

Choose a reason for hiding this comment

infinisil Sep 29, 2022

Choose a reason for hiding this comment

bbenne10 commented Sep 26, 2022

bbenne10 Sep 26, 2022

Choose a reason for hiding this comment

Mic92 Sep 26, 2022

Choose a reason for hiding this comment

infinisil commented Sep 29, 2022

bbenne10 commented Sep 29, 2022

infinisil commented Oct 3, 2022

nixos-discourse commented Oct 12, 2022

mweinelt commented Oct 12, 2022

bbenne10 commented Oct 12, 2022

infinisil commented Oct 12, 2022

SuperSandro2000 Oct 14, 2022

Choose a reason for hiding this comment

drupol commented Jun 2, 2023

bbenne10 commented Jun 3, 2023

jian-lin commented Jul 5, 2023