watchman: use default state directory (TMPDIR).

[MikeMcQuaid]

Doing this:

• avoids the need for post_install
• avoids permissions errors when running watchman as a user that didn't install it via Homebrew:

2024-06-03T13:36:47,061: [] the owner of /opt/homebrew/var/run/watchman/mike-state is uid 502 and doesn't match your euid 501

[carlocab]

Mild "request changes". (Feel free to dismiss if you feel strongly enough that my objections aren't significant enough.)

• I'd rather have multiple other users complain about this before we decide we're no longer erring on the side of caution re security hardening.
• IIRC the state directory also contains (a) logs, and (b) the configuration for watched directories. I'd rather not have these disappear on restart (especially (b)).

Withdrawing my objection, but would be good to know for certain about how this handles logs and configuration on restart.

We can also remove the pour_bottle? check with this (since we no longer hardcode HOMEBREW_PREFIX).

[MikeMcQuaid]

• I'd rather have multiple other users complain about this before we decide we're no longer erring on the side of caution re security hardening.

This will fail in any multi-user configuration and I know of several users already experiencing this.

• IIRC the state directory also contains (a) logs, and (b) the configuration for watched directories. I'd rather not have these disappear on restart (especially (b)).

As @Bo98 said above: we're now deferring to the upstream defaults here which seems sufficiently secure/sensible. Looks from d4b49b9/#1836 which originally added the custom statedir that, at the time, it defaulted to a location inside the prefix rather than TMPDIR.

I've used watchman for a few years and have never personally needed to retain either of these items. My understanding/usage of the tool is that it's used in a "watch changes to this directory" and, once it exits, it is no longer necessary to view this information.

That said: any daemons that are running when you brew reinstall watchman or brew upgrade watchman will be unaffected without a close and restart.

[MikeMcQuaid]

Error: undefined method `fetch' for nil
https://github.com/Homebrew/homebrew-core/actions/runs/9484664339/job/26145086398?pr=173850#step:3:26

@carlocab is this bottle caching related? if so: could you take a look?

[carlocab]

Error: undefined method `fetch' for nil
Homebrew/homebrew-core/actions/runs/9484664339/job/26145086398?pr=173850#step:3:26

@carlocab is this bottle caching related? if so: could you take a look?

Yep, it is. It's been on my to-do list.

[carlocab]

Homebrew/homebrew-test-bot#1108

[carlocab]

As @Bo98 said above: we're now deferring to the upstream defaults here which seems sufficiently secure/sensible. Looks from d4b49b9/#1836 which originally added the custom statedir that, at the time, it defaulted to a location inside the prefix rather than TMPDIR.

This is not the upstream default. The upstream default is here:

https://github.com/facebook/watchman/blob/b736bb585ce58ad059d2048fa531ee5cedf93ea5/CMakeLists.txt#L156-L157

The listed description even indicates that the directory must be persistent.

I've used watchman for a few years and have never personally needed to retain either of these items.

I have also been using watchman for a few years, and I need the state directory to persist across restarts. watchman installs a plist into ~/Library/LaunchAgents precisely so that it can continue watching directories (and behave as configured in response to certain events) after restart.

Blowing away the Watchman configuration upon restart makes the launch agent useless.

That said: any daemons that are running when you brew reinstall watchman or brew upgrade watchman will be unaffected without a close and restart.

Not sure what you mean by this exactly. Could you elaborate?

[MikeMcQuaid]

This is not the upstream default. The upstream default is here:

facebook/watchman@b736bb5/CMakeLists.txt#L156-L157

I guess it depends on how you define "upstream default"; I was intending it as "upstream default if no value is set".

Blowing away the Watchman configuration upon restart makes the launch agent useless.

I would be fine with not installing it if that's the case.

That said: any daemons that are running when you brew reinstall watchman or brew upgrade watchman will be unaffected without a close and restart.

Not sure what you mean by this exactly. Could you elaborate?

Just that a change like this PR will not break watchman in progress for people without a system restart.

[MikeMcQuaid]

Sadly #174554 was not sufficient to resolve the issues here. I've seen it personally and with several other users in person.