symlink'd clients no longer attaching/connecting

seanenck commented 3 days ago

Description

After updating to latest on master (commit: 90c1c6cc822b1836209514c096069b9bbeab63d9), gopls will not attach. If I revert 90c1c6cc822b1836209514c096069b9bbeab63d9 back to the previous commit, gopls attaches properly.

Looks like 90c1c6cc822b1836209514c096069b9bbeab63d9 introduced a regression.

justinmk commented 2 days ago

@dundargoc

dundargoc commented 2 days ago

@seanenck I need more info. I can't recreate this problem. What project/file are you using?

seanenck commented 2 days ago

It appears to be because on my system gopls in $PATH is actually a symlink to gopls itself (outside of path). This will happen for any go project related where gopls was working properly previously.

I was able to also reproduce this was efm-langserver too - which is not a symlink (by default) in my $PATH. If I create a symlink to efm-langserver and make sure it will resolve first via $PATH it will also not attach

Reproduction (I can do this on my system easily):

Open a nvim instance against a configured filetype and verify a working/attached client via checkhealth (e.g. efm-langserver)
Create a symlink to the LSP executable (e.g. ln -sf /usr/bin/efm-langserver efm-langserver)
Update path (e.g. export PATH="$PWD:$PATH")
Open a nvim instance against a configured filetype and verify there is no working/attached client via checkhealth.

seanenck commented 2 days ago

Additional data points, simple nvim config snippet:

print((vim.loop.fs_stat("/home/enck/.local/bin/gopls") or {}).type == 'file')
print((vim.loop.fs_stat("/home/enck/.local/bin/gopls") or {}).type == 'file')
print(vim.fn.getftype("/home/enck/.local/bin/gopls") == 'file')

will provide me:

true
true
false

(/home/enck/.local/bin/gopls is a symlink)

glepnir commented 2 days ago

I guess it return link not file could you try use vim.fn.getftype(vim.fn.resolve("/home/enck/.local/bin/gopls")))

  if (os_fileinfo_link(fname, &file_info)) {
    uint64_t mode = file_info.stat.st_mode;
    if (S_ISREG(mode)) {
      t = "file";
    } else if (S_ISDIR(mode)) {
      t = "dir";
    } else if (S_ISLNK(mode)) {

dundargoc commented 2 days ago

Thanks for the follow up. Interesting, so fs_stat doesn't make a distinction between symlinked files and regular files while getftype does. I think the most straightforward fix would be to just revert the commit?

seanenck commented 2 days ago

It does seem like vim.fn.getftype provides a more specific response than fs_stat. It's likely either revert the changes or make sure to check all the possible types of interest returned from vim.fn.getftype (e.g. file and link, I'm not sure if any of the other possible responses are valid in some places or not)

seanenck commented 2 days ago

for reference vim.fn.getftype(vim.fn.resolve("/home/enck/.local/bin/gopls"))) does report "file"

seanenck commented 1 day ago

I have root caused this.

The actual commit that caused this problem for my setup is 86dcd4d4ec014c5cb47033c52a54508acd503b97

I am using util.path.is_file to verify an lsp client executable exists before it is allowed to be used. That is fine (even though I understand it is now deprecated and I need to change my config). The problem, which probably should be fixed, is that when util.path.is_file was deprecated, it was also altered and therefore causes my symlink to not resolve.

Current state:

I will update my own config to fix my use of a deprecated function
It seems like either part of 86dcd4d4ec014c5cb47033c52a54508acd503b97 (and/or part of 90c1c6cc822b1836209514c096069b9bbeab63d9) should be reverted for util.path.is_file ... or that the new (but still deprecated) util.path.is_file function would need be fixed.

justinmk commented 1 day ago

when util.path.is_file was deprecated, it was also altered and therefore causes my symlink to not resolve.

how is that possible? the old version was:

  local function is_file(filename)
    local stat = uv.fs_stat(filename)
    return stat and stat.type == 'file' or false
  end

the "altered" version is:

function M.path.is_file(path)
  return (vim.loop.fs_stat(path) or {}).type == 'file'
end

does changing the new version to this fix your issue?

function M.path.is_file(path)
  local stat = uv.fs_stat(path)
  return stat and stat.type == 'file' or false
end

seanenck commented 1 day ago

@justinmk

I was continuing to poke at this and was editing my comment above. I see now exactly what happened to this situation...

prior to 86dcd4d4ec014c5cb47033c52a54508acd503b97, util.path.is_file is fine:

# true
local stat = vim.uv.fs_stat('/home/enck/.local/bin/gopls')
print(stat and stat.type == 'file' or false)

as-of 86dcd4d4ec014c5cb47033c52a54508acd503b97, util.path.is_file is still fine:

# true
print((vim.loop.fs_stat('/home/enck/.local/bin/gopls') or {}).type == 'file')

and then 90c1c6cc822b1836209514c096069b9bbeab63d9, util.path.is_file is no longer fine

# false
print(vim.fn.getftype('/home/enck/.local/bin/gopls') == 'file')

I did not realize until after posting my last comment that 90c1c6cc822b1836209514c096069b9bbeab63d9 was actually also altering the deprecated util.path.is_file call (so it has been changed 2 times in the last few days) and falsely thought that when 86dcd4d4ec014c5cb47033c52a54508acd503b97 was implemented it resulted in the format it is in today.

So 90c1c6cc822b1836209514c096069b9bbeab63d9 should likely be reverted specifically for util.path.is_file

neovim / nvim-lspconfig

symlink'd clients no longer attaching/connecting #3485

Description