Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

shellHook does not seem to run #7

Closed
sveitser opened this issue Mar 29, 2019 · 13 comments
Closed

shellHook does not seem to run #7

sveitser opened this issue Mar 29, 2019 · 13 comments
Labels
feature request Request for new functionality P1 critical: next release user-facing Improvement that increases user experience

Comments

@sveitser
Copy link

Using shell.nix

with import <nixpkgs> {};

mkShell {
  shellHook = ''
  export FOO=BAR
  '';
}

I don't find FOO set when using lorri.

@Profpatsch
Copy link
Collaborator

Did you use it from direnv? shellHook is specifically only called from nix-shell, lorri direnv does not source it.

For your example, you can easily add FOO this way:

with import <nixpkgs> {};

mkShell {
  FOO="BAR"
}

(because everything you pass to mkShell is passed to mkDerivation (is passed to derivation), which convert each field in their argument attrset (like shellHook or buildPhase or in this case FOO) to an environment variable.

In general we might have to think about how to integrate shellHook in a way that is backwards-compatible to nix-shell. cc @grahamc

@Profpatsch Profpatsch added feature request Request for new functionality user-facing Improvement that increases user experience labels Mar 29, 2019
@danbst
Copy link

danbst commented Mar 29, 2019

yeah, sourcing shellHook would be great. Maybe even allow variable IS_IN_LORRI

@sveitser
Copy link
Author

Thanks for the quick reply. Yes, I was using it with direnv. We are prepending a local directory containing some utility scripts to PATH in the shellHook. When switching over from direnv + nix-shell to direnv + lorri those weren't available anymore.

@Profpatsch
Copy link
Collaborator

Profpatsch commented Mar 29, 2019

Note that we have the same problem in lorri itself, so a good solution would be class.

As in: we add the paths both in .envrc and in shell.nix.

@Profpatsch
Copy link
Collaborator

It looks like some packages set up implicit shell hooks as well: #23 (comment)

So this becomes more pressing to implement.

@Profpatsch Profpatsch added the P1 critical: next release label Apr 5, 2019
@Profpatsch
Copy link
Collaborator

It’s a bit more complicated than that actually, we can’t just re-use existing shellHooks that packages provide, because they rely on the exact environment provided by the nix builder, which lorri cannot guarantee.

@grahamc gave an example of a problematic shellHook:

rm -rf $(find-the-thing)/*
and find-the-thing is defined in another hook which we don’t load.

so lorri/direnv would effectively execute rm -rf /* in that case.

We need to provide a good mechanism to achieve something similar with lorri.

@danbst
Copy link

danbst commented Apr 8, 2019

maybe add an arg --shell-hook to make it opt-in?

@Profpatsch
Copy link
Collaborator

maybe add an arg --shell-hook to make it opt-in?

We could do something like lorriHook which users can explicitely alias to shellHook if they want to use it with lorri as well. Then we can define a strict environments we provide.

But we can’t really allow shellHooks from transitive packages, for the reason I’ve outlined above. It’s just impossible to handle in the general case. But most of those hooks are pretty easy to adapt for the user I think (e.g. just manually write GOPATH=… into your hook).

@Fresheyeball
Copy link

Fresheyeball commented Apr 11, 2019

I have non-trivial scripts in my shellHook that expose bash functions that do things like setup test environments. It would be nice if everything in shellHook ran, so that echo "instructions on how to use this shell" get printed along without whatever else is going on.

@Profpatsch
Copy link
Collaborator

I have no trivial scripts in my shellHook that expose bash functions that do things like setup test environments.

Does this mean you do have bash functions in your shellHook? Or the opposite?

@Profpatsch
Copy link
Collaborator

As a stop-gap solution we could display a warning to the user that a shellHook was not executed (and maybe the contents of the shellHook).

@Drainful
Copy link

I have a partial workaround for people who live in Emacs
https://gist.github.com/Drainful/a3cd702dc590480cf4897a9859fc499f

@grahamc
Copy link
Contributor

grahamc commented Jun 5, 2019

Addressed in #75

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature request Request for new functionality P1 critical: next release user-facing Improvement that increases user experience
Projects
None yet
Development

No branches or pull requests

6 participants