-
-
Notifications
You must be signed in to change notification settings - Fork 454
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
fix: Issue 947 set NIX_PROFILES for use-xdg-base-directories #962
Conversation
modules/environment/default.nix
Outdated
@@ -159,6 +159,7 @@ in | |||
# Use user, default and system profiles. | |||
environment.profiles = mkMerge [ | |||
(mkOrder 800 [ "$HOME/.nix-profile" ]) | |||
(mkOrder 800 [ "\${XDG_STATE_HOME:-$HOME/.local/state}/nix/profile" ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking at this today and noticed that in NixOS/nixpkgs#260764 this was updated to remove the shell expansion:
environment.profiles = [
"$HOME/.nix-profile"
"\${XDG_STATE_HOME}/nix/profile"
"$HOME/.local/state/nix/profile"
"/etc/profiles/per-user/$USER"
];
Not sure if a similar approach should be used here, at least for consistency? Might also interact with shells like fish… but not sure!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When trying to add these manually to environment.profiles
, I also noticed that the ${XDG_STATE_HOME}
escape interacts poorly with clo4’s workaround for fish paths #122 (comment). Instead I just used this, hard-coding "$XDG_STATE_HOME/nix/profile"
without the braces:
# FIXME: Needed to address bug where $PATH is not properly set for fish.
# See: https://github.com/LnL7/nix-darwin/issues/122#issuecomment-1659465635
programs.fish.loginShellInit =
let
# This naive quoting is good enough in this case. There shouldn't be any
# double quotes in the input string, and it needs to be double quoted in case
# it contains a space (which is unlikely!)
dquote = str: "\"${str}\"";
# FIXME: Workaround for incorrect XDG paths https://github.com/LnL7/nix-darwin/issues/947
#
# Nix-darwin should set these here: https://github.com/LnL7/nix-darwin/blob/91010a5613ffd7ee23ee9263213157a1c422b705/modules/environment/default.nix#L159-L163
# as is set for nixOS: https://github.com/Gerg-L/nixpkgs/blob/6e8bbb279bd525b516e49021e5cb976f0b1f79e4/nixos/modules/config/users-groups.nix#L764-L769
# In the future we could use `config.environment.profiles` instead of hard-coding these.
profiles = [
"$HOME/.nix-profile"
"$XDG_STATE_HOME/nix/profile"
"$HOME/.local/state/nix/profile"
"/etc/profiles/per-user/$USER"
];
in ''
fish_add_path --move --prepend --path ${lib.concatMapStringsSep " " (path: dquote "${path}/bin") profiles}
set fish_user_paths $fish_user_paths
'';
Not sure if a better solution to #122 would solve this… I don't know if NixOS’ environment.paths
breaks fish users - I would have imagined that would be fixed by now if that was the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No that won't work as what happens if $XDG_STATE_HOME is not defined. Covered in NixOS/nixpkgs#260764 (comment) NixOS/nixpkgs#260764 (comment) I suppose.
#122 issue is macOS specific and has to do with Apple's path_helper which is just broken.(It forces things to the end of the path so you cannot override macOS system binaries)
Also I think it is more complex as I use home-manager as well - for fish I just manually set nix paths after all bits of nix and fish have finished playing around.
This fix is for bash and zsh setup, how it works on fish I have not really tested as as you note it gets confusing.
The issue is that the XDG path if it exists is not on $PATH in current nix-darwin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My issues with not finishing is more not understanding how PRs work with the testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nixes one looks incorrect to me
environment.profiles = [
"$HOME/.nix-profile"
"\${XDG_STATE_HOME}/nix/profile"
"$HOME/.local/state/nix/profile"
"/etc/profiles/per-user/$USER"
];
Why do we have "$HOME/.local/state/nix/profile" as if using xdc then the second line is sufficient isn't it.
I will remove the conditional XDG_STATE_HOME though as we probably do not want to set it.
@brendanzab What is the difference between "$XDG_STATE_HOME/nix/profile" and "${XDG_STATE_HOME}/nix/profile" I think we need a non xdc user to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yeah, that sounds like it could be a problem, for people not using XDG.
The issue for me was that fish didn't like the bash-style expansion (i.e. ${XDG_STATE_HOME}
or${XDG_STATE_HOME:-$HOME/.local/state}
) in combination with clo4's workaround. Without that workaround maybe the path expansion would be handled before fish gets it? I'm not sure how environment.profiles
gets provided to fish. Sorry if that is confusing… it's becoming sort of a complicated set of workarounds alas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to ignore me at any rate, I think your original solution might be better… (I don't maintain this project, I was just trying to move my nix files out of my home directory), but yeah this might end up breaking people’s workarounds unexpectedly, hah.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#122 issue is macOS specific and has to do with Apple's path_helper which is just broken.(It forces things to the end of the path so you cannot override macOS system binaries)
Ahh, ended up finding https://gist.github.com/Linerre/f11ad4a6a934dcf01ee8415c9457e7b2, which described the problem really well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for fish my config does not seem to have a problem with the syntax here.
My nix-darwin config includes
programs = {
bash.enable = true;
zsh.enable = true;
fish = {
enable = true;
useBabelfish = true;
# babelfishPackage = pkgs.babelfish;
vendor = {
completions.enable = true;
config.enable = true;
functions.enable = true;
};
# xonsh.enable = true;
};
};
environment = {
shells = with pkgs; [
bash
zsh
fish
];
# Install completions for system packages.
pathsToLink = [
"/share/bash-completion"
"/share/fish"
"/share/zsh"
];
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From #962 (comment)
Why do we have "$HOME/.local/state/nix/profile" as if using xdc then the second line is sufficient isn't it.
Unfortunately not. XDG specifies that if XDG_STATE_HOME
isn’t set, then $HOME/.local/state
should be used. On the NixOS side, it looks like ${XDG_STATE_HOME:-$HOME/.local/state}
interacted poorly with PAM, and so replacing the parameter expansion with two separate paths (the first hopefully not existing if XDG_STATE_HOME
isn’t set) seems like a reasonable workaround.
I have
environment.profiles = lib.mkOrder 801 [
"$XDG_STATE_HOME/nix/profile"
"$HOME/.local/state/nix/profile"
];
in my darwin-configuration.nix, and it is working fine (but I do have everything using XDG, so I can’t test the non-XDG situation).
don't set XDG_STATE_HOME if not set, rely on directory just not existing as nixos does. (I copied the original nixos version which has now changed) The other fix is toi fix the order to see if tests work.
I hate POSIX shell. I have now read all the nixes change and the shell manuals. Do we have PAM for use under Darwin? If not then the shell check on XDG_STATE_HOME is the correct way. If not the nixes fix is a mess. I think I can get my old way to run and see where an error is as the GitHub script does print out the PATH But I do not see anime error message saying text X failed and what it compares, |
On reviewing more, that means I tried to work it out from bash/zsh documentation rather than copying nixes. NB I use fish and manually set the path so did not notice the latest change as per nixes does not work. /etc/profile is the first file bash reads on an interactive login shell. Apple's provided version runs path_helper and then sources /etc/bashrc This means that XDG_STATE_HOME is not set before the changes here are read. In zsh easier to follow /etc/zshenv loads the variables here and again is before ~/.zshenv so XDG* are not set. So I think the change in nixos breaks things. But I have no Linux setup to test. Even the fix I proposed and was the previous one in nixos only works by convention. That being that XDG_STATE_HOME is in ~/.local/state - If for example I wanted it in ~/Library/Application Support/xdgstate then it would fail. |
Hrmm, looking at the code matches what you’re saying, but I’m not seeing
Yeah, this would be bad. I have been thinking about setting my XDG_*_HOME to more Mac-specific places, so maybe this is the time to try it … |
I think what is the issue is mixing what home-manager sets up and what nixos/nix-darwin sets. If running home-manager the hm_session_vars.sh file sets the XDG_ variables after the code here is run. Another way out is never to run home-manager on its own and always via nix-darwin and with useUserPackages = true then everything is in "/etc/profiles/per-user/$USER" and so the PATHS in ~ have nothing in them. Or the latter might depend on frameworks - I now use Snowfallorg/lib and the binaries end up in ${XDG_STATE_HOME}/nix/profile I had a previous incomplete one that copied things around so that HM just used the /etc paths. |
As for no /nix/profile you won't get it unless you are using some version of this PR. nix-darwin justi does not enter any line in PATH for the XDG path - which was the simple thing I tried to fox by copying nixes But I could not get the test to pass then nixes changed and then I actually looked at the issue and gone down too many rabbit holes. |
Yeah, see my earlier #962 (comment) – I am using the NixOS approach to this problem. But I understand this is in some pretty fragile generated shell stuff (and repurposed for other shells, and PAM, etc.) so fingers crossed that there’s something that works in the general case. |
As a note macOS does have pam - wether that messes up the PATH I doN'T know. For me I now have a biometric login (Apple Watch) which uses PAM for sudo so I am now happy to use the /etc/profiles/... PATH and so I don't have to deal with this mess-up of local and global PATHS. I thus am not working on this. |
#947 -
Puts the directory that nHome Manager uses when xdg is enabled and use-xdg-base-directories is set (~/.local/state/nix/profile/bin) into NIX_PATHS and thus the paths in the shells.
HM uses this directory instead of ~/.nix-profiles in this case