Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watchman socket path too long #181152

Open
4 tasks done
vdumitraskovic opened this issue Aug 14, 2024 · 9 comments
Open
4 tasks done

Watchman socket path too long #181152

vdumitraskovic opened this issue Aug 14, 2024 · 9 comments
Labels
bug Reproducible Homebrew/homebrew-core bug help wanted Task(s) needing PRs from the community or maintainers

Comments

@vdumitraskovic
Copy link

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

  • My brew doctor output says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.
  • 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.

98431a5#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.
@vdumitraskovic vdumitraskovic added the bug Reproducible Homebrew/homebrew-core bug label Aug 14, 2024
@SMillerDev
Copy link
Member

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

@carlocab
Copy link
Member

Yea, this is due to 98431a5, 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
Copy link
Member

Bo98 commented Aug 14, 2024

[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
Copy link
Member

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:

neoclide/coc.nvim#5107 (comment)

@Bo98
Copy link
Member

Bo98 commented Aug 14, 2024

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
Copy link
Member

Bo98 commented Aug 14, 2024

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 MikeMcQuaid added the help wanted Task(s) needing PRs from the community or maintainers label Aug 15, 2024
@MikeMcQuaid
Copy link
Member

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
Copy link
Member

cho-m commented Aug 17, 2024

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 PR1 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_PREFIX2. 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 PR3 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).

Footnotes

  1. https://github.com/Homebrew/homebrew-core/pull/179380

  2. https://github.com/Homebrew/homebrew-core/pull/181144

  3. https://github.com/facebook/watchman/pull/1236

@MikeMcQuaid
Copy link
Member

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

I agree here 👍🏻

I saw that @carlocab opened a PR3

Nice one @carlocab 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reproducible Homebrew/homebrew-core bug help wanted Task(s) needing PRs from the community or maintainers
Projects
None yet
Development

No branches or pull requests

6 participants