Watchman socket path too long

vdumitraskovic commented 1 month ago

`brew gist-logs <formula>` link OR `brew config` AND `brew doctor` output

> brew gist-logs watchman
<empty>

> brew config

HOMEBREW_VERSION: 4.3.15
ORIGIN: https://github.com/Homebrew/brew
HEAD: fa53e7b1e51a2deb7ec5a1e12452a1182dc342f7
Last commit: 2 days ago
Core tap JSON: 14 Aug 08:52 UTC
Core cask tap JSON: 14 Aug 08:52 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_BAT_THEME: GitHub
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: nvim
HOMEBREW_MAKE_JOBS: 12
Homebrew Ruby: 3.3.4 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.3.4_1/bin/ruby
CPU: dodeca-core 64-bit arm_blizzard_avalanche
Clang: 15.0.0 build 1500
Git: 2.39.3 => /Library/Developer/CommandLineTools/usr/bin/git
Curl: 8.6.0 => /usr/bin/curl
macOS: 14.5-arm64
CLT: 15.3.0.0.1.1708646388
Xcode: 15.4
Rosetta 2: false

> brew doctor
<empty>

Verification

[X] My brew doctor output says Your system is ready to brew. and am still able to reproduce my issue.
[X] I ran brew update and am still able to reproduce my issue.
[X] I have resolved all warnings from brew doctor and that did not fix my problem.
[X] I searched for recent similar issues at https://github.com/Homebrew/homebrew-core/issues?q=is%3Aissue and found no duplicates.

What were you trying to do (and why)?

Running neovim with plugin coc.nvim cannot start/connect with the watchman instance.

What happened (include all command output)?

Following error is printed in console:

[coc.nvim]: 2024-08-14T11:06:16,226: [cli] w_stm_connect_unix(/var/folders/wh/_3nndd2n7z7gyrpxry_b_xpsk18fpn
/T/nvim.vladimir.dumitraskov/h8Wjjf/vladimir.dumitraskov-state/sock) path is too long                       
2024-08-14T11:06:16,226: [cli] unable to talk to your watchman on /var/folders/wh/_3nndd2n7z7gyrpxry_b_xpsk1
8fpn/T/nvim.vladimir.dumitraskov/h8Wjjf/vladimir.dumitraskov-state/sock! (Argument list too long)

In previous watchman versions, this was working without a problem.

I suspect the change from the commit is the reason why this started happening.

https://github.com/Homebrew/homebrew-core/commit/98431a53a13974bde9642fca6a3b9aea54d0241d#diff-c78c3d87f62f7c863480106785162f6a8daa6bf54168c63be347dc99cf8380bdL58

What did you expect to happen?

I expect that the plugin can connect to the UNIX socket without a problem.

Step-by-step reproduction instructions (by running `brew` commands)

1. `brew install neovim`
2. Configure `coc.nvim`
3. `brew install watchman`
4. Run `neovim`
5. If user name is sufficiently long enough, error is printed in console.

SMillerDev commented 1 month ago

Paging @MikeMcQuaid and @carlocab who touched this formula recently.

carlocab commented 1 month ago

Yea, this is due to 98431a53a13974bde9642fca6a3b9aea54d0241d, which I disagreed with. See #173850.

For the record, I also don't think this is the upstream default -- it's just the fallback when WATCHMAN_STATE_DIR is an empty string (which we set it to). But I guess this depends on what you mean by "default".

As a workaround, you can do

export TMPDIR=/tmp

but note that this is insecure on multi-user systems.

Bo98 commented 1 month ago

[coc.nvim]: 2024-08-14T11:06:16,226: [cli] w_stm_connect_unix(/var/folders/wh/_3nndd2n7z7gyrpxry_b_xpsk18fpn
/T/nvim.vladimir.dumitraskov/h8Wjjf/vladimir.dumitraskov-state/sock) path is too long

The macOS path limit is 1023 characters. This is only hitting 10% of that.

I wonder why the watchman & coc.nvim combo is needing such a short path.

carlocab commented 1 month ago

I wonder why the watchman & coc.nvim combo is needing such a short path.

Doesn't seem to be related to coc.nvim; it seems to happen even with watchman only:

https://github.com/neoclide/coc.nvim/issues/5107#issuecomment-2285841620

Bo98 commented 1 month ago

Ah right I misread: that'll be because it's a socket where the limit is 104 or something.

Ideally that would use var/run instead but I guess there isn't a configuration for that.

Bo98 commented 1 month ago

Doesn't seem to be related to coc.nvim; it seems to happen even with watchman only:

Well not entirely. The watchman default is $TMPDIR/$USER-state/sock but the nvim makes it $TMPDIR/nvim.$USER/randomid/$USER-state/sock.

macOS technically supports up to 253 in sockets but requires adjusting the code slightly.

MikeMcQuaid commented 1 month ago

Paging @MikeMcQuaid and @carlocab who touched this formula recently.

@SMillerDev thanks for the heads up. Nothing to add beyond: I've seen this and thanks @Bo98 and @carlocab for the debugging above.

cho-m commented 1 month ago

This probably needs further discussion to understand various Watchman use cases and scenarios to find what is best direction to go.

Some comments, questions, etc. I have on use cases follows:

multi-user use case

Going back to PR^1 for multi-user use case, I'm trying to understand what was exact issue. Watchman should have created a user-specific $HOMEBREW_PREFIX/var/run/watchman/$USER-state directory at runtime. I can confirm that creating a separate user account on my machine successfully does this.

For a mismatch, it seems like the same user account would have to have started Watchman while $USER/getuid() was showing their name and EUID was a different user. Maybe su/sudo-ing from lower permission to admin account?

In this case, running watchman may result in "incorrect" mkdir due to utilization of EUID and that directory may need to be manually removed.

non-default `HOMEBREW_PREFIX`

One situation that was indirectly improved by TMPDIR change was the non-default HOMEBREW_PREFIX^2. In this case, we now can provide Watchman bottles to more users as the binary no longer hardcodes the path. Given that Watchman is quite popular among Homebrew formulae, this seems like a major advantage to consider.

coc.nvim

Now back to this issue, there really isn't a great option in current code when using TMPDIR and user has selected a username with 16+(?) characters.

If keeping TMPDIR, I can only think of providing a caveat that user may need to override it for some use cases. Perhaps to /tmp (w/ potential risks) or some user-specific path like $HOME/tmp.

Not sure how much we can improve on from Homebrew side. Most likely would need to be handled upstream, perhaps in Watchman.

I saw that @carlocab opened a PR^3 for $HOME-based path that could help if approved upstream. I think that may increase limit to 31 character names (at least for the sock path, may have some other limits elsewhere).

MikeMcQuaid commented 1 month ago

Most likely would need to be handled upstream, perhaps in Watchman.

I agree here 👍🏻

I saw that @carlocab opened a PR3

Nice one @carlocab 🎉

Homebrew / homebrew-core