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

fish: always set up the basic Nix env during shell initialization #30284

Closed
wants to merge 1 commit into from

Conversation

@therealpxc
Copy link
Contributor

therealpxc commented Oct 10, 2017

Motivation for this change

Setting up Nix and Fish together can be a stumbling block for new users and an annoyance for experienced ones. It involves setting up a shell framework, installing a plugin, pointing that plugin to a POSIX shell script, deciding whether and how to suppress certain expected error messages, and figuring out how early in shell initialization this stuff needs to be sourced.

Moreover, Fish integrations (vendor-provided in some of our packages) currently only work for NixOS users. This makes them available to all Nix users, with no configuration at all.

It also locates all the logic for using fenv to set up the basic Nix environment in a single place, where it is usable by non-NixOS Nix users as well as NixOS users and nix-darwin users. It even places that logic in a separate file so that it can be used in combination with Fish from other sources, like Homebrew or APT, so at the very least the configuration of the essential NIX_* variables and placing the /bin directories of the NIX_PROFILES on the PATH happens, even for non-Nixpkgs versions of Fish.

Note that this PR also avoids reproducing #18946 on non-NixOS by preventing PATH clobbering in the same way as it is currently done on NixOS.

Things done

This is only partially tested! It's up for comments at the moment, not merging. Please do play with it and test it , though.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 10, 2017

Looks cool. I'll try to test tonight on nix-darwin.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 10, 2017

@yurrriq with current nix-darwin, this won't quite work. It will work with NixOS only because the Nixpkgs and NixOS modules are in the same repo, so I can directly include concomitant changes. I can let you know what changes from @cbarrett's PR are needed if you're interested (it's a one-liner). Otherwise, if you could test this with vanilla Nix that would also be helpful.

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 10, 2017

@therealpxc that'd be great, thanks. The only machines I have available atm are macOS with nix-darwin. Could try in nix-docker though.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 10, 2017

Steps:

  1. Rebase Colin's PR on the latest nix-darwin, which changes some of the setEnvironment stuff (thanks, @LnL7 !)
  2. Add this line in the nix-darwin module for Fish, inside the config = mkIf cfg.enable ... section, after the other environment.etc."*ShellInit"=... entries:
 environment.etc."nix/support/set-environment.sh".source = "${config.system.build.setEnvironment}";
pkgs/shells/fish/default.nix Outdated
else
set -l common_prefix @NIX_STORE/../var/nix/profiles/default/etc/profile.d
set -l script_selection nix.sh
a

This comment has been minimized.

Copy link
@yurrriq

yurrriq Oct 10, 2017

Member

Oops!

/nix/store/6f429zrnlkpc8iymsci18w7dgh5km1vl-fish-2.6.0/share/fish/__nix_preinit_env.fish (line 15):
a
^
from sourcing file /nix/store/6f429zrnlkpc8iymsci18w7dgh5km1vl-fish-2.6.0/share/fish/__nix_preinit_env.fish
	called on line 9 of file /nix/store/6f429zrnlkpc8iymsci18w7dgh5km1vl-fish-2.6.0/share/fish/__fish_build_paths.fish

from sourcing file /nix/store/6f429zrnlkpc8iymsci18w7dgh5km1vl-fish-2.6.0/share/fish/__fish_build_paths.fish
	called on line 87 of file /nix/store/6f429zrnlkpc8iymsci18w7dgh5km1vl-fish-2.6.0/share/fish/config.fish

from sourcing file /nix/store/6f429zrnlkpc8iymsci18w7dgh5km1vl-fish-2.6.0/share/fish/config.fish
	called during startup

/nix/store/ab2iwb8ks5day43sw0580hdf54nfa4pv-bash-4.4-p12/bin/bash: @NIX_STORE/../var/nix/profiles/default/etc/profile.d/nix-daemon.sh: No such file or directory
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish

This comment has been minimized.

Copy link
@yurrriq

yurrriq Oct 10, 2017

Member

With the following patch applied:

diff --git a/pkgs/shells/fish/default.nix b/pkgs/shells/fish/default.nix
index 39235f96d6..55b6d67af7 100644
--- a/pkgs/shells/fish/default.nix
+++ b/pkgs/shells/fish/default.nix
@@ -59,7 +59,7 @@ let
       else
         set -l common_prefix @NIX_STORE/../var/nix/profiles/default/etc/profile.d
         set -l script_selection nix.sh
-a
+
         string match -q (${coreutils}/bin/uname) Darwin
         # Use `set -U nix_on_darwin_no_daemon` to tell Fish to use the single-user mode script instead
         and not set -q nix_on_darwin_no_daemon

... I get:

/nix/store/ab2iwb8ks5day43sw0580hdf54nfa4pv-bash-4.4-p12/bin/bash: @NIX_STORE/../var/nix/profiles/default/etc/profile.d/nix-daemon.sh: No such file or directory

This comment has been minimized.

Copy link
@yurrriq

yurrriq Oct 10, 2017

Member

This patch gets things running smoothly for me:

diff --git a/pkgs/shells/fish/default.nix b/pkgs/shells/fish/default.nix
index 39235f96d6..30362d7e08 100644
--- a/pkgs/shells/fish/default.nix
+++ b/pkgs/shells/fish/default.nix
@@ -57,9 +57,9 @@ let
       if test -f /etc/nix/support/set-environment.sh
         set nix_env_setup /etc/nix/support/set-environment.sh
       else
-        set -l common_prefix @NIX_STORE/../var/nix/profiles/default/etc/profile.d
+        set -l common_prefix @NIX_STORE@/../var/nix/profiles/default/etc/profile.d
         set -l script_selection nix.sh
-a
+
         string match -q (${coreutils}/bin/uname) Darwin
         # Use `set -U nix_on_darwin_no_daemon` to tell Fish to use the single-user mode script instead
         and not set -q nix_on_darwin_no_daemon

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 10, 2017

Author Contributor

urgh yeah that's a typo, sorry.

If it's reaching that part of the code, it's not using your nix-darwin config, though. The changes I described to nix-darwin will make that /etc/nix/support/set-environment.sh file appear, which is how the system I'm typing from is currently configured.

The bright side is that you just inadvertently tested the non-nix-darwin behavior! <3

This comment has been minimized.

Copy link
@yurrriq

yurrriq Oct 10, 2017

Member

Actually it looks like my $PATH is not getting set correctly.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 10, 2017

Author Contributor

I apologize for my sloppiness; I wrote this when I couldn't sleep at 2am last night and I kinda cleaned it up hastily this morning before work. Thanks for helping me test. :-D

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 10, 2017

Author Contributor

@yurrriq is that with the set-environment.sh file present? I just rebuilt Fish after finding the typos you identified and manually mangled the /etc/nix/support/set-environment.sh link so that it would revert to the non-nix-darwin behavior, and everything worked fine.

Everything continues to work when it's using nix-darwin.

Did you remember to rebase the nix-darwin from Colin's PR against current? Those setEnvironment changes need to be in place; the existing state of the PR isn't enough.

This comment has been minimized.

Copy link
@yurrriq

yurrriq Oct 10, 2017

Member

That part is fixed now.

I'm still having $PATH issues, but with these checkouts together, my system works well:

@therealpxc therealpxc force-pushed the therealpxc:fish branch Oct 10, 2017
@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 10, 2017

@Profpatsch Not sure if this is something you still care about (you're probably using direnv for your nix-shell needs by now), but this does fix using Fish inside a nix-shell environment on non-NixOS.

Here is how it looks without using any module system (no nix-darwin):

screen shot 2017-10-10 at 3 55 16 pm

And here's how it looks when nix-darwin is in use (when that set-environment.sh script is exposed)

screen shot 2017-10-10 at 3 56 26 pm

And again, this requires no configuration. Just install Fish from Nixpkgs after this is merged and you get this behavior for free, whether you're on macOS, GNU+Linux, NixOS, or using nix-darwin.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 10, 2017

@yurrriq can you describe the PATH issues you're having?

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

@therealpxc

# without
set -x PATH /run/current-system/sw/bin $PATH
# in programs.fish.shellInit in my darwin-configuration.nix

PATH=/usr/local/bin /usr/bin /bin /usr/sbin /sbin /opt/X11/bin /Library/TeX/texbin /Applications/Wireshark.app/Contents/MacOS
# and I get
# fish: Unknown command 'direnv'
# on every new shell
@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

@yurrriq Please try this version of nix-darwin and lmk if you have the same problems.

Can I see the contents of /etc/nix/support/set-environment.sh?

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

@therealpxc I also don't have nix-* on my $PATH..

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

Also,

warning: Nix search path entry ‘/Users/mohacker/.nixpkgs/darwin-configuration.nix’ does not exist, ignoring
error: file ‘darwin-config’ was not found in the Nix search path (add it using $NIX_PATH or -I)

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

The Nix commands are probably missing from your PATH because at the same time as your PATH isn't being set, you're manually adding back only /run/current-system/sw/bin, and Nix-Darwin does not install Nix to the system profile.

Do you want to meet me on IRC or via videochat real quick? I think I need some more info from you but hashing it out here will be slow.

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

Unfortunately, I need to revert back now, so I can have a fully functioning system...

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

Okay. I'm starting with a fresh install on another macOS box to test.

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

Well, I'm not sure what to do now.. nix-build seems broken.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

Is it because NIX_PATH is not set correctly? We can set that manually to get you back to a working system.

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

It's something more sinister.. $NIX_PATH looks correct.

Edit: Got it. Checking out your fork (obviously) got rid of my darwin-configuration.nix 😥

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

I'm dying for lack of details about what's going on here. Do you have a working smartphone or another device? If you've got a browser you can do screen-sharing on that same system, that would be ideal.

@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Oct 11, 2017

I'm yurrriq on freenode. Let's figure something out.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

@yurriq and I are working through the issues he faced when testing, and it looks like there were a few misconfigurations (of Nix itself, making sure checkouts were on the right paths and that his darwin-configuration.nix was in the right place, and so on). We're still making sure that this will work for him, though. :-)

In the meantime, I've had a chance to do a test on a macOS machine with a brand new Nix install (multi-user) and no nix-darwin. After installing Nix and using it to install this version of Fish (just a checkout and then nix-env -f . -iA fish), I disabled the Nix configuration for bash (just in case) and started a new session, then launched Fish. Everything is looking peachy! Check it out:

screen shot 2017-10-10 at 7 19 20 pm

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

And the single-user mode functionality works just as well on macOS as the multi-user option!

screen shot 2017-10-10 at 7 59 14 pm



# This happens before $__fish_datadir/config.fish sets fish_function_path, so it is currently
# unset. We set it and then completely erase it, leaving its configuration to $__fish_datadir/config.fish

This comment has been minimized.

Copy link
@cbarrett

cbarrett Oct 11, 2017

Suggestion: Change "completely erase" to "reset," since we aren't running set -e below. (I think this comment might have been copied over from the nix-darwin module?)

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 11, 2017

Author Contributor

So replying to any of these via email from my phone was a mistake. But yeah, erasing it in case it is empty may be unnecessary and at any rate the language of complete erasure is no longer accurate.

pkgs/shells/fish/default.nix Outdated
set fish_function_path $original_fish_function_path
test -z "$fish_function_path"; and set -e fish_function_path

set -gx __fish_nix_env_preinit_sourced 1

This comment has been minimized.

Copy link
@cbarrett

cbarrett Oct 11, 2017

Question: Is it correct to use -x here? I still feel uncertain about when we want to avoid duplicates vs. start a new clean environment.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 11, 2017

Author Contributor

Yes. Without this, new shells can't inherit any environment changes, for example, to PATH-- ever. This is because all the Nix initialization scripts set it outright instead of basing it on the existing PATH. Lacking this check breaks impure nix-shell environments, among other things, and set -gx is equivalent to the export as it is used in similar checks our scripts (in NixOS) for other shells.

Additionally, if you go a few lines up, you'll see that the __fish_nix_env_preinit_sourced variable is not even read for login shells. So to ensure that you get a clean environment, you can use fish -l and you're guaranteed to get one. :-)

pkgs/shells/fish/default.nix Outdated
set -l common_prefix @NIX_STORE@/../var/nix/profiles/default/etc/profile.d
set -l script_selection nix.sh

string match -q (${coreutils}/bin/uname) Darwin

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 11, 2017

Author Contributor

I wrote this this way because at the time it felt like the easiest way to test the behavior that I want, but this is dumb because we already know at build time whether this Fish will run on Darwin or not. This should just be handled with some conditional in the Nix expression itself so that the correct behavior gets in-lined for Darwin and for other platforms.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 11, 2017

@yurrriq for Nix-Darwin we'll need something like this if we want it to plug in here. I'm not sure that's the exact way we want to do it (I think I remember LnL saying it used to work this way but no longer does, and we should find out why). We should consult with LnL when it's time, after this gets merged for Nixpkgs and NixOS.

@therealpxc therealpxc force-pushed the therealpxc:fish branch 3 times, most recently Nov 6, 2017
@therealpxc therealpxc changed the title [WIP] fish: always set up the basic Nix env during shell initialization fish: always set up the basic Nix env during shell initialization Nov 6, 2017
@therealpxc therealpxc force-pushed the therealpxc:fish branch Nov 6, 2017
@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Nov 6, 2017

This PR has been updated so that it's in a state I like well enough, and tested on macOS, NixOS, and non-NixOS GNU+Linux. It is ready for review.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Nov 6, 2017

@rnhmjoj @Profpatsch @ocharles @cbarrett , may I solicit your opinions on this behavior/functionality? I think it's really helpful for making Nix and Fish usable together, especially for users who are new (perhaps to both).

@yurrriq Thanks again for your help in early testing. This version will do what it's supposed to do, although for now it will ignore the nix-darwin preinit hooks because I've moved them to a more universal location in this PR (I hope elvish might be able to take advantage of it in the future). I'll prepare an update to nix-darwin so if this gets merged nix-darwin will be plugged in again ASAP. Feel free (but not obligated!) to try it and let me know what you think.

nixEnvironmentSetupScriptPrefix = "@NIX_STORE@/../var/nix/profiles/default/etc/profile.d";
selectPosixPreInitScript = if hostPlatform.isDarwin then ''
# anchor for indentation
# Use `set -U nix_on_darwin_no_daemon` to tell Fish to use the single-user mode script instead

This comment has been minimized.

Copy link
@cbarrett

cbarrett Nov 6, 2017

Is an environment variable the right place to configure this?

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 6, 2017

Author Contributor

How can we discern whether Nix should be using a daemon at this point in the configuration? Ordinarily, the Nix user chooses whether to use a daemon or not by modifying their shell config to source one script or the other. They can't modify the config that gets rolled into fish inside the Nix store.

This seems like a pretty natural solution to me. Do you think we should read from a file instead?

This comment has been minimized.

Copy link
@cbarrett

cbarrett Nov 6, 2017

If that's how it happens normally that's fine—I just wanted to make sure it was the normal place to do it. The only other option I was thinking was reading from /etc/nix/nix.conf but I don't think that actually has info about whether or not a daemon is used.

This comment has been minimized.

Copy link
@rnhmjoj

rnhmjoj Nov 6, 2017

Contributor

@therealpxc Sorry, I'm not really qualified to review your changes: I updated the fish package a few time but I only have a vague idea of how the environment initializing works.

I'm a bit confused because I have never had the issue described in #18946.
For what is worth I'm ok for improving the situation for non-NixOs users.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 7, 2017

Author Contributor

@rnhmjoj The issue is something which can occur if sourcing the Nix setup script happens redundantly or too late in shell initialization. It can be tricky to get right, but this PR uses the same logic as NixOS, which works by setting up the Nix environment before Fish uses any config.

@cbarrett there actually is some config that goes in /etc/nix/nix.conf that we might be able to use. I know nix-build-users is normally unset for single-user mode, for example.

pkgs/shells/fish/default.nix Outdated
test -n "$NIX_PROFILES"
and begin
if builtin status --is-login
or test -z "$__fish_nix_env_preinit_sourced" -a -z "$ETC_PROFILE_SOURCED" -a -z "$ETC_ZSHENV_SOURCED"

This comment has been minimized.

Copy link
@cbarrett

cbarrett Nov 6, 2017

This duplication is maybe worth fixing now, maybe something to keep an eye on for later. 🤷‍♂️

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 6, 2017

Author Contributor

Maybe the various shells should have a variable in common they use for this. But that's an issue that involves other shells and other parts of Nixpkgs. I think we can set it aside.

When these lines were essentially part of the NixOS module, I checked these variables for the same reason that the NixOS module does: we don't want to clobber configuration made elsewhere in NixOS or redundantly source things that have already been sourced from other shells.

But here, it's only used for the basic set-environment.sh script, which doesn't really include any user configuration. And on non-NixOS, those variables might never be set anyway. Here, maybe it makes more sense to just check NIX_PROFILES.

This comment has been minimized.

Copy link
@cbarrett

cbarrett Nov 6, 2017

Yup yup. I've logged a separate issue about cleaning this up. #31322

@therealpxc therealpxc force-pushed the therealpxc:fish branch to 5572e4b Nov 6, 2017
Copy link
Member

Profpatsch left a comment

This change should change none of the current functionality of the NixOS setup? If so, I could verify that.

# #
############### ↑ Nix hook for sourcing /etc/fish/config.fish ↑ ###############
'';


nixEnvironmentSetupScriptPrefix = "@NIX_STORE@/../var/nix/profiles/default/etc/profile.d";

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 7, 2017

Member

I’m not 100% sure ../var/nix does always hold here.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 7, 2017

Author Contributor

Any ideas on when it wouldn't? (I would love a more robust way of doing this if anyone can think of one.)

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 8, 2017

Member

Even more interesting: there is no /default/etc/profile.d on my system. Where/When should it exist? Can we factor that logic out to be more semantic instead of doing brittle bash if/then/else checks?

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 8, 2017

Author Contributor

Well, NixOS isn't a distinct platform from Linux in Nixpkgs, right? There's the additional problem that on macOS, either can appear. With a plain Nix install on macOS, we'll have the profile.d/nix.sh script, but if one is using nix-darwin, they'll also have a module system like on NixOS, and we should prefer its config. If on that platform we always read the nix.sh file, things controlled in setEnvironment (like the environment variables relating to whether and where to find the Nix daemon) will be ignored.

The idea of this PR is to pull a small part of the NixOS Fish initialization process, namely the part that makes Nix-installed binaries usable by Fish config snippets in ~/.config/fish/conf.d and $__fish_sysconfdir/conf.d, out into Nixpkgs so that it will be usable outside of NixOS. The other bit, discussed in the thread above, is in part just a consequence of moving that code into Nixpkgs and in part an effort to make sure that with these changes Fish does not become less functional/compatible on NixOS OOTB than it is on non-NixOS.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 13, 2017

Author Contributor

@Profpatsch I was just looking at the Nix package in Nixpkgs and it seems like maybe we could add stateDir to nix.passthru and then use that in the Fish package. Does that make sense? Alternatively, could we maybe expose it in the stdenv in its own right along side $NIX_STORE as something like $NIX_STATE?

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 14, 2017

Member

maybe we could add stateDir to nix.passthru

I’m not sure we even need to find the state dir. Can you point me to documentation by what these shell files are generated on each system?
It feels to me like those are (undocumented) nix interna that shouldn’t be depended upon. So before we use them, the interface should be documented and standardized.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Jan 31, 2018

Author Contributor

I’m not sure we even need to find the state dir. Can you point me to documentation by what these shell files are generated on each system?

The reason we need to find the state directory is because we need to find a Nix profile (the default profile, which gets the nix.sh and nix-daemon.sh files) before NIX_PROFILES is set. The state directory we only really need to find that default profile (instead of assuming it lives under /nix/var/nix/..., as that can change at compile time.

I don't think there's any documentation about it, but the files get put in the appropriate directories here and the templates for them are here (for no Nix daemon) and here (to a Nix daemon).
All I really need is a reliable place in which to find these shell scripts that Nix uses to set up the environment, both for NixOS and non-NixOS. I'm not invested in how it's exposed-- what I did here was find something that works for a typical Nix installation (i.e., any Nix which hasn't been compiled to use a non-default state directory).

It feels to me like those are (undocumented) nix interna that shouldn’t be depended upon. So before we use them, the interface should be documented and standardized.

This is a fair point, and I should have followed through with it when I first read it. Who should I contact about this and how?

# by Nixpkgs' fish package, but it can by used by other shells in and
# outside of Nixpkgs. This is meant to be a path which can be used in common
# among different Nix module systems, for example NixOS and nix-darwin.
environment.etc."nix/support/set-environment.sh".source = "${system.build.setEnvironment}";

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 7, 2017

Member

Would rename to nix/support/fish-set-environment.sh.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 7, 2017

Author Contributor

I have this set in this location because it's identical to the usual setEnvironment script and exposing it this way makes it usable by Nix packages which may not know whether or what module system they have. I'd like for it to potentially be usable by other non-POSIX shells which may have to resort to similar tricks if we're not to maintain several initialization scripts for Nix. (Elvish is the shell I have in mind.)

Maybe I should say I'm not gonna need it, call it fish-set-environment.sh and rename it only when it is actually used by another shell. What do you think?

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 8, 2017

Member

I’m not quite sure I understand the reason for exposing this file in the first place. Since it’s exposed via the module system, you have to use NixOS anyway to use it, so why not add it to fish and other shells directly when building their config?

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 8, 2017

Author Contributor

Well, set-environment is roughly the equivalent of the profile.d/nix.sh script on NixOS and in nix-darwin. The idea is that Fish installed via Nix should at a minimum ‘just work’ with the very basics of Nix (setting up the NIX_… environment variables and adding the Nix profile directories to the PATH.

This PR adds functionality that makes Fish-from-Nixpkgs usable as a login shell OOTB on non-NixOS. Exposing the setEnvironment file allows NixOS to have the same level of functionality prior to enabling the NixOS module. But the NixOS module still adds properly NixOS-specific functionality, like reading all the global shell configurations.

For the feature to work the same way across NixOS and non-NixOS, something equivalent to the profile.d/nix.sh file for NixOS has to be exposed prior to user configuration, and as far as I can tell setEnvironment is it.

This comment has been minimized.

Copy link
@therealpxc

therealpxc Oct 3, 2018

Author Contributor

Looks like this went in directly under /etc here

If pushing for documentation/standardization is still warranted (it probably is) that's now the thing to document

@@ -172,6 +172,13 @@ in
export PATH="$HOME/bin:$PATH"
'';

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 7, 2017

Member

nit: why here (between system.build.setEnvironment and system.activationScripts.binsh), not in the environment. section above?

This comment has been minimized.

Copy link
@therealpxc

therealpxc Nov 7, 2017

Author Contributor

No good reason. I'll go ahead and move it.


nixEnvironmentSetupScriptPrefix = "@NIX_STORE@/../var/nix/profiles/default/etc/profile.d";
selectPosixPreInitScript = if hostPlatform.isDarwin then ''
# anchor for indentation

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 14, 2017

Member

That’s brittle. Why not writeText and source?

This comment has been minimized.

Copy link
@therealpxc

therealpxc Jan 31, 2018

Author Contributor

That’s brittle. Why not writeText and source?

You mean specifically the indentation hack? Because it works, the stakes are low (the whitespace here won't break anything), and it seems good to avoid sourcing additional files during shell startup if possible. It's also more readable.

I can let that go, though.

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Jan 31, 2018

Member

If you want to avoid opening files, I’d hate to tell you how many unsuccessful stats most of the software we use do, searching for files in locations that are just crazy by nix standards …

# #
############### ↑ Nix hook for sourcing /etc/fish/config.fish ↑ ###############
'';


nixEnvironmentSetupScriptPrefix = "@NIX_STORE@/../var/nix/profiles/default/etc/profile.d";

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch Nov 14, 2017

Member

maybe we could add stateDir to nix.passthru

I’m not sure we even need to find the state dir. Can you point me to documentation by what these shell files are generated on each system?
It feels to me like those are (undocumented) nix interna that shouldn’t be depended upon. So before we use them, the interface should be documented and standardized.

@qolii

This comment has been minimized.

Copy link
Contributor

qolii commented Aug 15, 2018

@therealpxc, dare I ask how this is looking these days? It would be great to have solid fish integration everywhere!

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Aug 16, 2018

I haven't touched it, but I could come back to it if there's interest. At the very least, I could try to get this kind of Fish package added to a user repository until I come up with something cleaner.

@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Aug 16, 2018

@qolii I'll schedule some time next week to try to figure out a better way to do this and also to get the existing work into a form that's easy for users to try.

@qolii

This comment has been minimized.

Copy link
Contributor

qolii commented Aug 22, 2018

@therealpxc, that sounds great! No rush of course, but I'm looking forward to it :)

@therealpxc therealpxc force-pushed the therealpxc:fish branch from 5572e4b Oct 3, 2018
@therealpxc

This comment has been minimized.

Copy link
Contributor Author

therealpxc commented Oct 3, 2018

@qolii Sorry I let this go again for weeks. 😳

Since some other relevant changes recently went into Nixpkgs, I've rebased. I'll get some more convenient way to install up soon, and look into following up on the other documentation/standardization issues.

@therealpxc therealpxc force-pushed the therealpxc:fish branch to 43ac420 Oct 3, 2018
@qolii

This comment has been minimized.

Copy link
Contributor

qolii commented Nov 30, 2018

Hey, @therealpxc, not to be that guy, but, any luck?

Alternatively, is there any way I can help? The pain of having to open bash every time I want to use Nix on my Mac is small but real :)

@Infinisil

This comment has been minimized.

Copy link
Member

Infinisil commented Jan 15, 2020

There doesn't seem to be much interest in this for over a year now, so I'll close this for now

@Infinisil Infinisil closed this Jan 15, 2020
@yurrriq

This comment has been minimized.

Copy link
Member

yurrriq commented Jan 16, 2020

👍 I don't use Darwin anymore, so I'm no use there.

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

Successfully merging this pull request may close these issues.

None yet

10 participants
You can’t perform that action at this time.