[nix] Replaces brew install of watchman with nix
ClosedPublic
Actions

Authored by will on Jul 16 2024, 7:25 AM.

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

will created this revision.Jul 16 2024, 7:25 AM

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

Herald added a subscriber: tomek. · View Herald TranscriptJul 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?

Harbormaster completed remote builds in B30388: Diff 42351.Jul 16 2024, 7:48 AM

will requested review of this revision.Jul 16 2024, 7:48 AM

will edited the test plan for this revision. (Show Details)Jul 16 2024, 7:54 AM

ashoat accepted this revision.Jul 16 2024, 11:59 AM

ashoat added inline comments.

nix/dev-shell.nix
143 ↗	(On Diff #42351)	Do we need this step anymore? I noticed `prompt_direnv_macos.sh` calls Brew, so guessing yes
145 ↗	(On Diff #42351)	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

will added inline comments.Jul 16 2024, 12:12 PM

nix/dev-shell.nix
143 ↗	(On Diff #42351)	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 ↗	(On Diff #42351)	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