Page MenuHomePhabricator

[Nix] Prompt user to install direnv
ClosedPublic

Authored by jon on Jan 19 2023, 12:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 17, 6:49 AM
Unknown Object (File)
Tue, Dec 17, 6:49 AM
Unknown Object (File)
Tue, Dec 17, 6:49 AM
Unknown Object (File)
Tue, Dec 17, 6:49 AM
Unknown Object (File)
Tue, Dec 17, 6:49 AM
Unknown Object (File)
Tue, Dec 17, 6:49 AM
Unknown Object (File)
Tue, Dec 17, 6:48 AM
Unknown Object (File)
Tue, Dec 17, 6:47 AM
Subscribers

Details

Reviewers
atul
varun
bartek
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMM80b320989e4d: [Nix] Prompt user to install direnv
Summary

Prompt the user to install direnv through homebrew.
Sets up zsh hook if missing, provides link for other shells.

Depends on D6313

https://linear.app/comm/issue/ENG-2753

Test Plan
# if direnv is already installed, uninstall
brew uninstall direnv

nix develop
# should get prompted

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jan 19 2023, 12:57 PM
scripts/prompt_direnv_macos.sh
45–59 ↗(On Diff #21124)

Is there anything blocking from installing the hook for bash by default, in addition to zsh?

scripts/prompt_direnv_macos.sh
58 ↗(On Diff #21124)

other than bash or zsh?

Accepting, two notes inline,

scripts/prompt_direnv_macos.sh
21–35 ↗(On Diff #21124)

Do we want to make this optional? I think we should just enable it by default or highly recommend it here.

41 ↗(On Diff #21124)

Would be helpful to specify where 3.2 is coming from. Guessing it's because it's the last version of bash Apple bundles in macOS (or Xcode tools?) because of licensing issues?

It's a bit confusing without that context.

Install shell hook for bash and zsh

scripts/prompt_direnv_macos.sh
21–35 ↗(On Diff #21124)

My main concern, is that installation of the shell hooks and changes to the shells can be jarring.

"Why am i being prompted to do direnv allow . in certain directories?".

I would say that direnv requires a non-0 amount of user learning for someone to find it useful; so I'm hesitant to force people to use it.

41 ↗(On Diff #21124)

Comes from /bin/bash --version on macOS. I'll make a note of this in the comment

45–59 ↗(On Diff #21124)

Couldn't think of a good way to not make it super repetitive, but I'll think of something

58 ↗(On Diff #21124)

going to just going to support both zsh and bash

This revision is now accepted and ready to land.Jan 24 2023, 12:59 PM

Avoid prompting when in a non-interactive shell

Fixup some debugging inclusions

scripts/prompt_direnv_macos.sh
29–30 ↗(On Diff #21332)

Indentation feels off for these two lines

jon added inline comments.
scripts/prompt_direnv_macos.sh
29–30 ↗(On Diff #21332)

good eye

This revision was automatically updated to reflect the committed changes.
jon marked an inline comment as done.