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

Execute shellHook on cached runs #109

Closed
colemickens opened this issue Sep 10, 2021 · 14 comments
Closed

Execute shellHook on cached runs #109

colemickens opened this issue Sep 10, 2021 · 14 comments

Comments

@colemickens
Copy link
Member

I set PS1 in the shellHook of my flake.nix (which I think isn't terrible).

However, after #91 is merged, I think this is ineffective because my shell is basically always cached...

Is there anyway to run shellHook in cached hit cases? Or if not, could there be another attribute that is always read out and executed?

@colemickens
Copy link
Member Author

Well, it seems like direnv blocks this anyway.

@Mic92
Copy link
Member

Mic92 commented Sep 11, 2021

Potentially you could source the cache after use flake?

eval "$(< "$profile_rc")"
eval $shellHook

or just use .envrc to run some commands.

@johnae
Copy link
Contributor

johnae commented Nov 13, 2021

It seems to me as if #91 broke https://github.com/numtide/devshell which is unfortunate. Personally I have a simpler take on devshell that works in a similar fashion and that is now broken too. I believe the relevant part of devshell is this:

https://github.com/numtide/devshell/blob/ab14b1a3cb253f58e02f5f849d621292fbf81fad/nix/mkNakedShell.nix#L44

It could be something with my setup that's wrong but it seems I started having problems after ugrading to 1.5.0. @zimbatm could perhaps comment on this as he's the author of devshell.

@johnae
Copy link
Contributor

johnae commented Nov 13, 2021

At least I can confirm that deleting this line:

sed -i "/eval \"\$shellHook\"/d" "$profile_rc"
makes https://github.com/numtide/devshell work again. What I mean by "not working" is that none of the packages that are supposed to be made available in the environment are.

@johnae
Copy link
Contributor

johnae commented Nov 16, 2021

In my case I was able to avoid the issues I experienced by not doing anything "important" in the shellHook so for me this is not a problem anymore.

@johnae
Copy link
Contributor

johnae commented Nov 18, 2021

@ryneeverett @Mic92 I believe departing from the behavior of nix develop has made nix-direnv unusable with https://github.com/numtide/devshell. I don't think nix-direnv is doing the right thing here.

@Mic92
Copy link
Member

Mic92 commented Nov 19, 2021

This PR enables shellHook for both use_nix and use_flake, even in cached runs
It requires nix2.4 though:

#125
I might hold of a release until at least the next nixos version is released and only
publish a release candidate before.

@lilyball
Copy link

I keep running into this too. I have my own equivalent of numtide/devshell (I didn't know about devshell until now, it sounds like it's a much more comprehensive solution) and the fact that $shellHook isn't evaluated on cached runs means I end up with the wrong environment.

@Mic92 I haven't tried the PR, since I don't want to try and shift my config over to pulling patches from GitHub. Given that Nix 2.4 is the stable nix in nixpkgs now, is there a reason not to go ahead and publish a release with this new change? By default a new release won't end up on any existing release channels, so there shouldn't be any concerns there.

@Mic92
Copy link
Member

Mic92 commented Nov 29, 2021

I keep running into this too. I have my own equivalent of numtide/devshell (I didn't know about devshell until now, it sounds like it's a much more comprehensive solution) and the fact that $shellHook isn't evaluated on cached runs means I end up with the wrong environment.

@Mic92 I haven't tried the PR, since I don't want to try and shift my config over to pulling patches from GitHub. Given that Nix 2.4 is the stable nix in nixpkgs now, is there a reason not to go ahead and publish a release with this new change? By default a new release won't end up on any existing release channels, so there shouldn't be any concerns there.

The PR is not ready quite yet.

@nova-nowiz
Copy link

Is there a quickfix for that in the meanwhile?
I tried using the nix-2.4 branch as an overlay to nix-direnv but that didn't work (probably my fault here 😄, or maybe it should be an overlay on nix-direnv-flakes?)

@Mic92
Copy link
Member

Mic92 commented Dec 13, 2021

Is there a quickfix for that in the meanwhile? I tried using the nix-2.4 branch as an overlay to nix-direnv but that didn't work (probably my fault here smile, or maybe it should be an overlay on nix-direnv-flakes?)

I am using it by sourcing it manually but atm the caching does not work: https://github.com/Mic92/dotfiles/blob/0e30755334eb53e4cbcfe49453b6bf18e3d581c2/home/.direnvrc#L3

@rraval
Copy link

rraval commented Dec 13, 2021

Is there a quickfix for that in the meanwhile?

I've added an extra line to my .envrc under use flake

use flake
eval "$shellHook"  # quotes are important here

It seems to do the right thing but I've only looked at it superficially and moved on (so I can't comment on whether this is the "right" workaround and what the cons might be).

@nova-nowiz
Copy link

Thanks for both solutions!
That really helps 😉

LunNova added a commit to LunNova/nixos-configs that referenced this issue Dec 16, 2021
jlesquembre added a commit to jlesquembre/dotfiles that referenced this issue Jan 11, 2022
yipengsun added a commit to yipengsun/nix-config that referenced this issue Jan 13, 2022
archseer added a commit to helix-editor/helix that referenced this issue Feb 25, 2022
adomixaszvers added a commit to adomixaszvers/dotfiles-nix that referenced this issue Mar 9, 2022
@typetetris typetetris mentioned this issue Apr 3, 2022
@bbenne10
Copy link
Contributor

bbenne10 commented Apr 7, 2022

2.0 should handle this case nicely. Please open a new issue if you find some problem with that line. Thank you!

@bbenne10 bbenne10 closed this as completed Apr 7, 2022
berquist added a commit to berquist/sams-cookiecutter-pypackage that referenced this issue Jun 14, 2024
berquist added a commit to berquist/sams-cookiecutter-pypackage that referenced this issue Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants