Page MenuHomePhabricator

[nix] Introduce `.envrc` to invoke Nix via `direnv`
ClosedPublic

Authored by atul on Jan 3 2023, 12:28 PM.
Tags
None
Referenced Files
F3242525: D6152.id20530.diff
Wed, Nov 13, 10:59 PM
Unknown Object (File)
Tue, Nov 12, 10:52 PM
Unknown Object (File)
Wed, Oct 23, 3:00 AM
Unknown Object (File)
Wed, Oct 23, 2:49 AM
Unknown Object (File)
Tue, Oct 22, 8:47 PM
Unknown Object (File)
Tue, Oct 22, 5:15 PM
Unknown Object (File)
Tue, Oct 22, 12:22 PM
Unknown Object (File)
Tue, Oct 22, 12:22 PM
Subscribers

Details

Summary

So Nix stuff happens automatically when we cd into repo instead of having to run nix develop a million times.

(I installed .direnv via brew install direnv, but ideally it would be installed via Nix?)

Test Plan

cd comm

4c88eb.png (900×1 px, 277 KB)

cd comm/native

755159.png (310×1 px, 112 KB)

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Jan 3 2023, 12:35 PM
atul edited the test plan for this revision. (Show Details)
atul added inline comments.
.envrc
2

More than 80 lines, open to suggestions

Adding @ashoat as first-pass reviewer because we're introducing nix-direnv dependency here: https://github.com/nix-community/nix-direnv

ashoat requested changes to this revision.Jan 3 2023, 1:04 PM

There is an implicit requirement to brew install bash here, isn't there? I think we should hold off on this for now until @jon can take a look at ENG-2621, but open to landing it sooner if we think it won't cause any problems for anybody

This revision now requires changes to proceed.Jan 3 2023, 1:04 PM

There is an implicit requirement to brew install bash here, isn't there? I think we should hold off on this for now until @jon can take a look at ENG-2621, but open to landing it sooner if we think it won't cause any problems for anybody

Was just typing out response on Linear:

Yeah, pretty much brew install direnv and https://phab.comm.dev/D6152

I'm guessing direnv will be installed by Nix in the future so didn't add the brew install step to the dev environment docs or whatever, but put up .envrc and .gitignore changes out of convenience so I don't have to keep the changes around in my local environment.

Figure it's like the .vscode files which are convenient for those using VS Code but don't hurt those who don't.


edit: There's the brew install direnv step that I didn't include because I assume it'll be handled by Nix.. but there's also the brew install bash step that's a workaround (https://linear.app/comm/issue/ENG-2619#comment-4721ddde).

Still think it's fine to land as-is for those who want to get something working for now.

atul requested review of this revision.Jan 3 2023, 1:06 PM
ashoat added 1 blocking reviewer(s): jon.

Okay sure – looks like this won't work without brew install bash and brew install direnv. Defer to @jon on the final accept. One potential concern would be if we have devs using direnv already (for whatever reason) – I could imagine this breaking their workflow until they run brew install bash and brew install direnv (which we don't want to happen). But arguably it's unlikely anybody is already using direnv?

One potential concern would be if we have devs using direnv already (for whatever reason) – I could imagine this breaking their workflow until they run brew install bash and brew install direnv (which we don't want to happen). But arguably it's unlikely anybody is already using direnv?

Sent a message in Comm Dev Team channel. I'll land after (if) @jon accepts, but happy for anyone to revert the commit if it breaks anything on their machine in any way.

Since people have to opt-in to this and they have to have direnv pre-installed, I think this is fine for now.

I'll still pursue https://linear.app/comm/issue/ENG-2621 to make it "supported"

This revision is now accepted and ready to land.Jan 3 2023, 1:54 PM
This revision was landed with ongoing or failed builds.Jan 3 2023, 2:15 PM
This revision was automatically updated to reflect the committed changes.