Page MenuHomePhabricator

[nix] Replaces brew install of watchman with nix
ClosedPublic

Authored by will on Jul 16 2024, 7:25 AM.
Tags
None
Referenced Files
F3349876: D12777.id42373.diff
Fri, Nov 22, 7:34 PM
F3349797: D12777.id42383.diff
Fri, Nov 22, 7:14 PM
F3348595: D12777.diff
Fri, Nov 22, 3:21 PM
Unknown Object (File)
Wed, Nov 20, 10:02 PM
Unknown Object (File)
Wed, Nov 20, 10:02 PM
Unknown Object (File)
Fri, Nov 8, 11:05 PM
Unknown Object (File)
Fri, Nov 8, 12:26 PM
Unknown Object (File)
Mon, Oct 28, 2:16 PM
Subscribers

Details

Reviewers
ashoat
bartek
varun
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMM30b605adaa15: [nix] Replaces brew install of watchman with nix
Summary

Because we updated nix packages, we can use nix-based watchman instead of the brew install

Context: https://linear.app/comm/issue/ENG-2784/add-updated-nix-version-of-watchman#comment-79fd27f8

Test Plan
 ➜ whereis watchman
watchman: /nix/store/pmvk9zd5z98b5vvc83w4dqjq7d4w8ri1-watchman-2023.08.14.00/bin/watchman

To verify the new Nix watchman is working. I uninstalled brew-based watchman and ran nix develop. I then ran the iOS simulator and made changes to text on the login screen, verifying that changes were immediate

Diff Detail

Repository
rCOMM Comm
Branch
nix/watchman
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jul 16 2024, 7:25 AM

We can have devs uninstall brew-based watchman but the nix shell watchman takes precedence

Can you modify the Test Plan to actually test that Watchman works with React Native?

will requested review of this revision.Jul 16 2024, 7:48 AM
ashoat added inline comments.
nix/dev-shell.nix
143

Do we need this step anymore? I noticed prompt_direnv_macos.sh calls Brew, so guessing yes

145

This is the only place that script is used. Should we delete the script?

This revision is now accepted and ready to land.Jul 16 2024, 11:59 AM
nix/dev-shell.nix
143

On looking at the diff where this is introduced (https://phab.comm.dev/D6313), Jon also mentions that we might need it for direnv.

Seems to check out with the prompt_direnv_macos.sh script so I think it's best we keep it

145

Was debating on this. On second thought, if we have no plans to reintroduce any brew-based installations, we can delete for now. We always have the prior diffs if we want to recreate the script.

I'll delete before landing

feedback. remove unnecessary homebrew install deps script