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

mason setup is called twice #1297

Open
tomasgareau opened this issue Jan 2, 2025 · 3 comments
Open

mason setup is called twice #1297

tomasgareau opened this issue Jan 2, 2025 · 3 comments

Comments

@tomasgareau
Copy link

When setting up nvim-lspconfig, we tell Lazy that mason.nvim is a dependency and configure it right away (#865 & linked issues touch on why):

kickstart.nvim/init.lua

Lines 456 to 460 in a8f5395

-- Main LSP Configuration
'neovim/nvim-lspconfig',
dependencies = {
-- Automatically install LSPs and related tools to stdpath for Neovim
{ 'williamboman/mason.nvim', config = true }, -- NOTE: Must be loaded before dependants

But later in nvim-lspconfig's config function, we explicitly setup mason again:

kickstart.nvim/init.lua

Lines 648 to 654 in a8f5395

-- Ensure the servers and tools above are installed
-- To check the current status of installed tools and/or manually install
-- other tools, you can run
-- :Mason
--
-- You can press `g?` for help in this menu.
require('mason').setup()

These two setup calls can interfere with each other. For example, if you tweaked the second setup call to append Mason's bin path (e.g., to allow overriding Mason binaries with system binaries):

require("mason").setup({
  PATH = "append"
})

the first setup call will prepend the Mason bin path (the default behaviour) and your change will append the bin path. Your path then ends up being:

/path/to/mason/bin:...:/rest/of/your/path:...:/path/to/mason/bin

Oops haha.

I think we can just bin the second setup call and put all the mason configuration in the dependencies block:

  {
    -- Main LSP Configuration
    'neovim/nvim-lspconfig',
    dependencies = {
      -- Automatically install LSPs and related tools to stdpath for Neovim
-      { 'williamboman/mason.nvim', config = true }, -- NOTE: Must be loaded before dependants
+      {
+        "williamboman/mason.nvim",
+        opts = {
+	  -- Mason must be loaded before dependants so we need to configure it here.
+	  -- You can override its default settings by adding them to this table, e.g.:
+	  -- PATH = "append" -- append Mason's bin path so system packages take precedence
+	  PATH = "append",
+        },
+      },
      'williamboman/mason-lspconfig.nvim',
      'WhoIsSethDaniel/mason-tool-installer.nvim',

      -- etc
    },
    config = function()
      -- etc...
    
      -- Ensure the servers and tools above are installed
      --  To check the current status of installed tools and/or manually install
      --  other tools, you can run
      --    :Mason
      --
      --  You can press `g?` for help in this menu.
-      require('mason').setup()

(but I'm not sure if e.g., the second setup call is required for auto-installation after the other config). Happy to toss up a PR if the above makes sense!

@iton0
Copy link
Contributor

iton0 commented Jan 3, 2025

Nice catch. @tomasgareau make a PR but put mason setup outside the dependencies table ie keep the second setup call and make the mason dependency a string. Additionally, I would move the NOTE to the second setup call.

@tomasgareau
Copy link
Author

@iton0 I could but from what I understand that would that cause issue #554 to pop back up, no? Pull request #865 re-introduced the config = true bit to have Lazy run require("mason").setup() before its dependents. I think the second setup call may happen too late.

@iton0
Copy link
Contributor

iton0 commented Jan 3, 2025

I agree that using config = true works. The only issue I encountered when experimenting with my config and calling require("mason").setup() after require("mason-lspconfig").setup() was a bug. After reading through the issues, PRs, and checking the lazy.nvim docs, I’m not entirely sure what caused the bug.

However, your original approach is correct. I was hesitant because I thought it might affect readability, but as long as the setup is properly explained and the comments from the second setup call are incorporated, it should work well.

tomasgareau added a commit to tomasgareau/kickstart.nvim that referenced this issue Jan 3, 2025
Addresses nvim-lua#1297

Currently, we're calling `require('mason').setup(...)` twice:
* once when setting it as a dependency of `nvim-lspconfig` (since we set
	`config = true`)
* once in the `config` function we define for `nvim-lspconfig`

Calling setup twice can cause issues with, e.g., setting the `PATH`
option: you might append Mason's bin dir in one setup call and prepend
it in the other.

We've kept the setup of `mason` in the `nvim-lspconfig` dependencies
table since leaving it to the `config` function caused some
plugin-loading-order related issues in the past. See:
* nvim-lua#210
* nvim-lua#554
* nvim-lua#555
* nvim-lua#865
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants