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.

Sign up for GitHub

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

Jump to bottom

add .envrc for direnv integration #9740

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

add .envrc for direnv integration #9740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .envrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
use flake
watch_file .envrc.local
[[ -f .envrc.local ]] && source_env .envrc.local
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,6 @@ result-*

# Mac OS
.DS_Store

# direnv
.direnv
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for? I didn't need it.

For context, I've been using

echo .envrc >>.git/info/exclude

to have a private direnv setup that's ignored by git.

It's not use flake, but a custom one though, with special support for XDG_DATA_DIRS.
Is that variable handled correctly by use flake? It should combine the host and project paths.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.direnv is probably created when nix-direnv is used, which puts gcroots for the devshell in that directory.

Yeah I've recently noticed that some people prefer to keep their direnv setup private in order to customize it.
But we can have the best of both worlds with this:

use flake
watch_file .envrc.local
[[ -f .envrc.local ]] && source_env .envrc.local

I updated the PR.

Generally there doesn't seem to be a clear line between what configuration should be private and what public. Personally I believe it's good to have as much as possible upstream, as long as it doesn't disturb.

So feel free to merge this or close it. I don't have strong feelings about it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The possibility of optimizing .envrc by adding the correct watch_file flags and other checks also seems to be a good reason to have it shared upstream

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle I'm in favor, but I actually have two more issues with the direnv approach, which is that it doesn't make the phase functions available. I've quietly tolerated that in my own use, but combining direnv and an actual dev shell may well lead to bad interactions. For instance it's duplicating $PATH. Maybe some other behaviors aren't idempotent in effect either.

The other issues is that we have multiple shells and we need them. Usually I run the native-clangStdenvPackages one, for the clangd support. How do we do that with direnv?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Maybe it's not supposed to be added then as it would limit people from using different shells via direnv

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nix-direnv handles XDG_DATA_DIRS correctly: https://github.com/nix-community/nix-direnv/blob/36230478c17bfe2c3fc8d4adbc2e0cdf8ac35aea/direnvrc#L163

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose shell selection could be implemented with, say, a .envrc.shellName.local file to load before use flake? This is quite appealing to me at least because I rarely have to switch shells anyway.

Then we only need a replacement for autoreconfPhase commands and such. Maybe a shim package with executables named like that could actually do the job. I don't think we're relying on global state in the form of bash variables; something that executables can't mutate.

Copy link
Member

@Mic92 Mic92 Jan 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just did nix develop .#nix-clangStdenv inside nix-direnv and it seems to work as expected, I also not aware of an instance where this was an issue. It's also possible to run direnv deny to unload direnv before doing that or edit the .envrc like this:

use flake .#nix-clangStdenv

Copy link
Member

@Mic92 Mic92 Jan 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to tell what shell direnv is called from, so .envrc.shellName.local doesn't work as far as I know but I also don't know why this is needed in this case. I am using direnv in the nix codebase for quite a while.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is that we won't have to manually invoke nix develop anymore, right? So it should be a sufficiently complete alternative for actual productive development. I'm sure that it can be that with a little bit of work.

There is no way to tell what shell direnv is called from, so .envrc.shellName.local doesn't work

I wasn't clear. My idea behind this file is that it would specify the devShells subattribute to load; not a specific unix shell implementation like fish or something.

I just did nix develop .#nix-clangStdenv inside nix-direnv and it seems to work as expected, I also not aware of an instance where this was an issue.

It's duplicating $PATH. This might lead to nasty surprises when you're changing the build inputs.

If we don't need to call nix develop, we don't need to nest shells, and then probably this issue is not a big deal.

Loading