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

x/tools/gopls: remove the "expandWorkspaceToModule" setting #63536

Open
findleyr opened this issue Oct 13, 2023 · 7 comments
Open

x/tools/gopls: remove the "expandWorkspaceToModule" setting #63536

findleyr opened this issue Oct 13, 2023 · 7 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Oct 13, 2023

Per the rationale provided in #57514: remove this unnecessary setting, in favor of always expanding/contracting the workspace as necessary (the current behavior with "expandWorkspaceToModule": true). If you need this setting, please leave a comment explaining why.

@findleyr findleyr added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Oct 13, 2023
@findleyr findleyr added this to the gopls/v0.15.0 milestone Oct 13, 2023
@hjkatz
Copy link

hjkatz commented Oct 25, 2023

I think I have a use, but I'm not sure where to comment, so here it goes.

What I'm noticing is that using neovim's lsp implementation I'm running into this issue: neovim/neovim#23291
Which causes significant startup lag ~20s when opening go files.

I most recently noticed this issue after adding a go.work file into a monorepo with an example structure:

monorepo/
  - go.work
  - go/
      - go.mod
      - go files...
  - cache/
  - js/
  - lots/of/other/dirs

Content of go.work:

go 1.21.3

use ./go

Combine this with the gopls issue with globbing seen here: #41504

We get the following behaviour:

  • go.work instructs gopls to use monorepo/ as the root rather than monorepo/go/
  • gopls requests to watch files monorepo/**/*.{go,mod,sum} (lots and lots of files)
  • nvim's lsp now tries to watch way too many directories and files

I think this behaviour is related to this issue and the expandWorkspaceToModule setting because I think some behaviour of this setting could (should?) use the contents of go.work to limit the directories to glob.

In my case I would expect gopls to read go.work and see use ./go and then set the root to ./go, not the cwd of go.work.

Alternative behaviour could be only loading the glob for any root (sub-path) that matches a use statement in go.work for the current file being opened (or attached for the lsp).

I'm all ears to hear about workarounds or solutions to my problem.

@findleyr
Copy link
Contributor Author

Thanks for raising this issue, and for the detailed explanation of the performance bottleneck you're experiencing. I was unaware of that neovim issue, and will keep it in mind.

I think I may not have explained clearly that the current default behavior will remain unchanged (which is "expandWorkspaceToModule": true). Updated the description to clarify.

I looked into your use case, and don't think that it is affected by "expandWorkspaceToModule". If neovim is computing monorepo/ as the workspace folder, I think we will always ask to watch everything in it (e.g. it is considered a workspace dir). To work around your problem, you could put the go.work file in the go/ subdir, or in an entirely different dir altogether and set the GOWORK environment variable. Longer term, if GOWORK is set, we should just watch the go.work file itself and the modules it activates.

@findleyr
Copy link
Contributor Author

I filed #63742 to follow up on the overly broad watch patterns.

@hjkatz
Copy link

hjkatz commented Oct 25, 2023

Thank you for the clarification. I think the behaviour of expaindWorkspaceToModule is good to have (i.e. gopls should try to use a heuristic function to find the correct root_path for lsp config).

Since this is kind of a 2-part issue (nvim lsp watch files implementation + gopls overbroad watchfiles glob) I decided to try to change the root_path myself and ended up with just a specific config for gopls:

lspconfig.gopls.setup({
  on_attach = [...],
  capabilities = [...],
  settings = [...],
  
  -- override root_path for issue: https://github.com/golang/go/issues/63536
  root_path = function(fname)
    local root_files = {
      'go/go.mod', -- monorepo override so root_path is ./monorepo/go/** not ./monorepo/**
      'go.work',
      'go.mod',
      '.git',
    }

    -- return first parent dir that homes a found root_file
    return lspconfig.util.root_pattern(unpack(root_files))(fname) or lspconfig.util.path.dirname(fname)
  end,
})

And now the number of watchfiles is back down in the ~3k instead of in the ~15k.

Note: The neovim lsp watchfiles function being slow still exists, but the delay is ~20ms and not like ~20s.

Alternatively users can just disable the watchfiles function entirely following steps here: neovim/neovim#23291 (comment)

@evanj
Copy link
Contributor

evanj commented Oct 26, 2023

I use this setting with vscode. My work uses some fairly large mono repos with Bazel. Most of the Go parts can use the standard Go tools, but not all. I like to open just the directory I am working on (e.g. a single library, or a single main program), since then all of VSCode's search and navigation features work well, without stuff I don't care about. To be clear, the directory looks approximately like:

monorepo
├── go.mod
├── stuff_i_dont_care_about
│   └── build_error.go
└── stuff_i_work_on
    └── stuff.go

And I open the sub-directory within the Go module called stuff_i_work_on by running code (my dir) on the command line. The problem is I can't get gopls to ignore the hundreds of build errors in other directories I don't care about. I've tried:

    "gopls": {
        "build.directoryFilters": [
            "-"
        ]
    }

which as far as I understand it, should remove everything from gopls's build, but it does not seem to work. I have also tried variants that include "+stuff_i_work_on/**". If I open the Go module root, then I can use build.directoryFilters to filter gopls correctly. So maybe this is a "bug report" for this configuration?

If I set "expandWorkspaceToModule": false then gopls/vscode works as I expect and want when I open a sub-directory.

@geitir
Copy link

geitir commented Oct 27, 2023

I have the same use case as @evanj. I use vs-code and my work uses a very large monorepo and bazel and I like to load only the service directories I care about (eg cd go-monorepo/src/.../.../service/ && code .).

I haven't touched my config in a long time, but I distinctly remember having to add expandWorkspaceToModule: false otherwise gopls would get bogged down, have a ton of errors, and sometimes crash/never resolve.

I will experiment with turning it off, since maybe the heuristics used etc have improved and report back, but I imagine it is still an issue.

@findleyr
Copy link
Contributor Author

Thanks very much for the detailed feedback!

I think it's probably incorrect to expand the workspace if GOPACKAGESDRIVER is set, so perhaps we can just fix the default behavior in that case. I'll investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants