Additionally source libraries from XDG_CONFIG_DIRS #990
Conversation
Allows the system administrator to configure direnv libraries and rc files using by default /etc/xdg/direnv/lib/*.sh and /etc/xdg/direnv/direnvrc
| # load the global ~/.direnvrc if present | ||
| if [[ -f $direnv_config_dir/direnvrc ]]; then | ||
| # shellcheck disable=SC1090,SC1091 | ||
| source "$direnv_config_dir/direnvrc" >&2 | ||
| elif [[ -f $HOME/.direnvrc ]]; then |
There was a problem hiding this comment.
It's probably enough just to extend the above block. I would rather not change this logic as now it's loading both the ~/.config/direnv/direnvrc and the ~/.direnvrc.
There was a problem hiding this comment.
We don't want to load ~/.direnvrc multiple times though, so a variable is needed, check out the new commit: 3f6363d
Is this alright?
There was a problem hiding this comment.
It's not quite there yet. It should only load the first direnvrc, starting with $direnv_config_dir/direnvrc, then (maybe) the rest of the XDG config dirs, and finally the legacy one. But I think this is best left untouched as it doesn't solve a specific use-case.
There was a problem hiding this comment.
I'm not sure I understand, can you show me how you'd change the code?
| @@ -19,7 +19,16 @@ direnv="$(command -v direnv)" | |||
| DIRENV_LOG_FORMAT="${DIRENV_LOG_FORMAT-direnv: %s}" | |||
|
|
|||
| # Where direnv configuration should be stored | |||
| direnv_config_dir="${DIRENV_CONFIG:-${XDG_CONFIG_HOME:-$HOME/.config}/direnv}" | |||
| IFS=: xdg_config_dirs=( ${XDG_CONFIG_DIRS:-/etc/xdg} ) | |||
There was a problem hiding this comment.
| IFS=: xdg_config_dirs=( ${XDG_CONFIG_DIRS:-/etc/xdg} ) | |
| direnv_config_dir="${DIRENV_CONFIG:-${XDG_CONFIG_HOME:-$HOME/.config}/direnv}" | |
| IFS=: xdg_data_dirs=( ${XDG_DATA_DIRS:-/usr/local/share/:/usr/share} ) |
I find the XDG spec confusing. I think in this scenario, the ~/.config/direnv/lib/*.sh can be considered "user" configuration, but the shared libraries are probably part of XDG_DATA_DIRS? Or it should have been XDG_DATA_HOME already.
There was a problem hiding this comment.
The spec says this:
$XDG_DATA_HOMEdefines the base directory relative to which user-specific data files should be stored
It doesn't explain what "user-specific data files" are, but from my experience I'd say it's "files that contain persistent user-specific data but which are created and managed by the program". I don't think such direnv libraries fit in there, since they're neither user-specific, nor created or managed by the program. And $XDG_DATA_DIRS is just the extension of $XDG_DATA_HOME to multiple additional directories.
I really think $XDG_CONFIG_HOME (and its extension to $XDG_CONFIG_DIRS) is the right place for these libraries, they really are configuration: They don't store user-specific data, are reusable between arbitrarily many installation, they're kind of like plugins.
There was a problem hiding this comment.
I agree for ~/.config/direnv/lib/*.sh, but as soon as you start scanning outside of the user's home, it becomes re-distributable code. For nix-direnv, you wouldn't expect the user to change the implementation themselves but rather to propose the changes upstream.
This also shows here where the code is in $out/share: NixOS/nixpkgs#192667 (comment)
There was a problem hiding this comment.
I don't understand, are you saying that because libraries outside ~ are redistributable (though I'd argue libraries in ~ are equally as redestributable), XDG_DATA_DIRS is the correct variable to read from? That doesn't make sense to me, XDG_DATA_DIRS like XDG_DATA_HOME is for data, not configuration or libraries or redistributable things. For GDPR compliance for example, if somebody asks me to wipe all user data, I'd wipe all XDG_DATA_DIRS and XDG_DATA_HOME, the applications should all continue working with the same configuration though
infinisil
force-pushed
the
xdg-config-dirs
branch
3 times, most recently
from
b017310
to
ce1c286
Compare
stdlib.sh
Outdated
| if [[ -n "$DIRENV_CONFIG" ]]; then | ||
| direnv_config_dirs+=( "$DIRENV_CONFIG" ) | ||
| fi |
There was a problem hiding this comment.
@zimbatm Btw this also slightly changes behavior: Before when DIRENV_CONFIG was set, $XDG_CONFIG_HOME/~/.config was ignored, but now it's loaded regardless. I can easily change the behavior to work as before, what do you think?
There was a problem hiding this comment.
Given how widely direnv is deployed, it's best to keep as much back-compat as possible. DIRENV_CONFIG is for when you want to override the default behaviour.
There was a problem hiding this comment.
I fixed this in f106329 now, it has the old behavior again
Only use ~/.config/direnv if DIRENV_CONFIG is not set
|
@infinisil @zimbatm Maybe my PR comes in handy here? See #1038 … I discovered this PR after creating my PR, so maybe I'll first take a look at your conversation (especially those regarding |
|
Is there any movement on this PR? I am waiting on this PR to approve and merge NixOS/nixpkgs#192667, which would be extremely helpful in integrating nix-direnv (and direnv itself) into nix installs natively. |
|
I'm waiting on clarifications of @zimbatm's comments, to me this PR would be good to merge. |
Allows the system administrator to configure direnv libraries and rc files using by default /etc/xdg/direnv/lib/*.sh and /etc/xdg/direnv/direnvrc
See NixOS/nixpkgs#192667 (comment) for motivation