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

Authored by atul on Jan 3 2023, 12:28 PM.

Details

Reviewers

• jon
ashoat

Commits

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

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

cd comm/native

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jan 3 2023, 12:28 PM

Herald added a subscriber: tomek. · View Herald TranscriptJan 3 2023, 12:28 PM

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 ↗	(On Diff #20530)	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

Harbormaster completed remote builds in B15049: Diff 20530.Jan 3 2023, 12:42 PM

atul edited the summary of this revision. (Show Details)Jan 3 2023, 1:02 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

In D6152#184448, @ashoat wrote:

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

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?

In D6152#184464, @ashoat wrote:

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.