[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

Branch

nix/watchman

Lint

No Lint Coverage

Unit

No Test Coverage

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