From 4fb48c5654d9ffbffbdcdd80d0498b1ea3c8e68b Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Tue, 3 May 2022 15:08:35 +0200 Subject: feat(server): set $NVIM, unset $NVIM_LISTEN_ADDRESS #11009 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PROBLEM ------------------------------------------------------------------------ $NVIM_LISTEN_ADDRESS has conflicting purposes as both a parameter ("the current process should listen on this address") and a descriptor ("the current process is a child of this address"). This contradiction means the presence of NVIM_LISTEN_ADDRESS is ambiguous, so child Nvim always tries to listen on its _parent's_ socket. This is the cause of lots of "Failed to start server" spam in our test/CI logs: WARN 2022-04-30… server_start:154: Failed to start server: address already in use: \\.\pipe\nvim-4480-0 WARN 2022-04-30… server_start:154: Failed to start server: address already in use: \\.\pipe\nvim-2168-0 SOLUTION ------------------------------------------------------------------------ 1. Set $NVIM to the parent v:servername, *only* in child processes. - Now the correct way to detect a "parent" Nvim is to check for $NVIM. 2. Do NOT set $NVIM_LISTEN_ADDRESS in child processes. 3. On startup if $NVIM_LISTEN_ADDRESS exists, unset it immediately after server init. 4. Open a channel to parent automatically, expose it as v:parent. Fixes #3118 Fixes #6764 Fixes #9336 Ref https://github.com/neovim/neovim/pull/8247#issuecomment-380275696 Ref #8696 --- test/functional/vimscript/server_spec.lua | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'test/functional/vimscript/server_spec.lua') diff --git a/test/functional/vimscript/server_spec.lua b/test/functional/vimscript/server_spec.lua index 238d1aeb0f..de64a77b4d 100644 --- a/test/functional/vimscript/server_spec.lua +++ b/test/functional/vimscript/server_spec.lua @@ -1,6 +1,5 @@ local helpers = require('test.functional.helpers')(after_each) local eq, neq, eval = helpers.eq, helpers.neq, helpers.eval -local command = helpers.command local clear, funcs, meths = helpers.clear, helpers.funcs, helpers.meths local iswin = helpers.iswin local ok = helpers.ok @@ -16,27 +15,25 @@ end describe('server', function() before_each(clear) - it('serverstart() sets $NVIM_LISTEN_ADDRESS on first invocation', function() - -- Unset $NVIM_LISTEN_ADDRESS - command('let $NVIM_LISTEN_ADDRESS = ""') - + it('serverstart(), serverstop() does not set $NVIM', function() local s = eval('serverstart()') assert(s ~= nil and s:len() > 0, "serverstart() returned empty") - eq(s, eval('$NVIM_LISTEN_ADDRESS')) + eq('', eval('$NVIM')) + eq('', eval('$NVIM_LISTEN_ADDRESS')) eq(1, eval("serverstop('"..s.."')")) eq('', eval('$NVIM_LISTEN_ADDRESS')) end) it('sets new v:servername if $NVIM_LISTEN_ADDRESS is invalid', function() clear({env={NVIM_LISTEN_ADDRESS='.'}}) - eq('.', eval('$NVIM_LISTEN_ADDRESS')) + -- Cleared on startup. + eq('', eval('$NVIM_LISTEN_ADDRESS')) local servers = funcs.serverlist() eq(1, #servers) ok(string.len(servers[1]) > 4) -- Like /tmp/nvim…/… or \\.\pipe\… end) - it('sets v:servername at startup or if all servers were stopped', - function() + it('sets v:servername at startup or if all servers were stopped', function() local initial_server = meths.get_vvar('servername') assert(initial_server ~= nil and initial_server:len() > 0, 'v:servername was not initialized') @@ -55,11 +52,13 @@ describe('server', function() eq(1, funcs.serverstop(funcs.serverlist()[1])) eq('', meths.get_vvar('servername')) - -- v:servername will take the next available server. + -- v:servername and $NVIM take the next available server. local servername = (iswin() and [[\\.\pipe\Xtest-functional-server-pipe]] or 'Xtest-functional-server-socket') funcs.serverstart(servername) eq(servername, meths.get_vvar('servername')) + -- Not set in the current process, only in children. + eq('', eval('$NVIM')) end) it('serverstop() returns false for invalid input', function() @@ -136,7 +135,6 @@ end) describe('startup --listen', function() it('validates', function() clear() - local cmd = { unpack(helpers.nvim_argv) } table.insert(cmd, '--listen') matches('nvim.*: Argument missing after: "%-%-listen"', funcs.system(cmd)) -- cgit From 1f2c2a35ad14cfac002d87073471bd84a52860bf Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Wed, 1 Jun 2022 11:28:14 -0700 Subject: feat(server): instance "name", store pipes in stdpath(state) Problem: - Unix sockets are created in random /tmp dirs. - /tmp is messy, unclear when OSes actually clear it. - The generated paths are very ugly. This adds friction to reasoning about which paths belong to which Nvim instances. - No way to provide a human-friendly way to identify Nvim instances in logs or server addresses. Solution: - Store unix sockets in stdpath('state') - Allow --listen "name" and serverstart("name") to given a name (which is appended to a generated path). TODO: - is stdpath(state) the right place? --- test/functional/vimscript/server_spec.lua | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'test/functional/vimscript/server_spec.lua') diff --git a/test/functional/vimscript/server_spec.lua b/test/functional/vimscript/server_spec.lua index de64a77b4d..ea07be231d 100644 --- a/test/functional/vimscript/server_spec.lua +++ b/test/functional/vimscript/server_spec.lua @@ -30,7 +30,7 @@ describe('server', function() eq('', eval('$NVIM_LISTEN_ADDRESS')) local servers = funcs.serverlist() eq(1, #servers) - ok(string.len(servers[1]) > 4) -- Like /tmp/nvim…/… or \\.\pipe\… + ok(string.len(servers[1]) > 4) -- "~/.local/state/nvim…/…" or "\\.\pipe\…" end) it('sets v:servername at startup or if all servers were stopped', function() @@ -54,7 +54,7 @@ describe('server', function() -- v:servername and $NVIM take the next available server. local servername = (iswin() and [[\\.\pipe\Xtest-functional-server-pipe]] - or 'Xtest-functional-server-socket') + or './Xtest-functional-server-socket') funcs.serverstart(servername) eq(servername, meths.get_vvar('servername')) -- Not set in the current process, only in children. @@ -66,7 +66,7 @@ describe('server', function() eq(0, eval("serverstop('bogus-socket-name')")) end) - it('parses endpoints correctly', function() + it('parses endpoints', function() clear_serverlist() eq({}, funcs.serverlist()) @@ -101,6 +101,10 @@ describe('server', function() eq(expected, funcs.serverlist()) clear_serverlist() + -- Address without slashes is a "name" which is appended to a generated path. #8519 + matches([[.*[/\\]xtest1%.2%.3%.4[^/\\]*]], funcs.serverstart('xtest1.2.3.4')) + clear_serverlist() + eq('Vim:Failed to start server: invalid argument', pcall_err(funcs.serverstart, '127.0.0.1:65536')) -- invalid port eq({}, funcs.serverlist()) @@ -113,7 +117,7 @@ describe('server', function() -- Add some servers. local servs = (iswin() and { [[\\.\pipe\Xtest-pipe0934]], [[\\.\pipe\Xtest-pipe4324]] } - or { [[Xtest-pipe0934]], [[Xtest-pipe4324]] }) + or { [[./Xtest-pipe0934]], [[./Xtest-pipe4324]] }) for _, s in ipairs(servs) do eq(s, eval("serverstart('"..s.."')")) end @@ -146,9 +150,13 @@ describe('startup --listen', function() it('sets v:servername, overrides $NVIM_LISTEN_ADDRESS', function() local addr = (iswin() and [[\\.\pipe\Xtest-listen-pipe]] - or 'Xtest-listen-pipe') - clear({ env={ NVIM_LISTEN_ADDRESS='Xtest-env-pipe' }, + or './Xtest-listen-pipe') + clear({ env={ NVIM_LISTEN_ADDRESS='./Xtest-env-pipe' }, args={ '--listen', addr } }) eq(addr, meths.get_vvar('servername')) + + -- Address without slashes is a "name" which is appended to a generated path. #8519 + clear({ args={ '--listen', 'test-name' } }) + matches([[.*[/\\]test%-name[^/\\]*]], meths.get_vvar('servername')) end) end) -- cgit From f50135a32e11c535e1dc3a8e9460c5b4e640ee86 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Thu, 30 Jun 2022 13:16:46 +0200 Subject: feat: stdpath('run'), /tmp/nvim.user/ #18993 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: - Since c57f6b28d71d #8519, sockets are created in ~/.local/… but XDG spec says: "XDG_RUNTIME_DIR: Must be on the local filesystem", which implies that XDG_STATE_DIR is potentially non-local. - Not easy to inspect Nvim-created temp files (for debugging etc). Solution: - Store sockets in stdpath('run') ($XDG_RUNTIME_DIR). - Establish "/tmp/nvim.user/" as the tempdir root shared by all Nvims. - Make ok() actually useful. - Introduce assert_nolog(). closes #3517 closes #17093 --- test/functional/vimscript/server_spec.lua | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'test/functional/vimscript/server_spec.lua') diff --git a/test/functional/vimscript/server_spec.lua b/test/functional/vimscript/server_spec.lua index ea07be231d..6e95459630 100644 --- a/test/functional/vimscript/server_spec.lua +++ b/test/functional/vimscript/server_spec.lua @@ -5,6 +5,7 @@ local iswin = helpers.iswin local ok = helpers.ok local matches = helpers.matches local pcall_err = helpers.pcall_err +local mkdir = helpers.mkdir local function clear_serverlist() for _, server in pairs(funcs.serverlist()) do @@ -13,9 +14,19 @@ local function clear_serverlist() end describe('server', function() - before_each(clear) + it('serverstart() stores sockets in $XDG_RUNTIME_DIR', function() + local dir = 'Xtest_xdg_run' + mkdir(dir) + clear({ env={ XDG_RUNTIME_DIR=dir } }) + matches(dir, funcs.stdpath('run')) + if not iswin() then + matches(dir, funcs.serverstart()) + end + end) + it('serverstart(), serverstop() does not set $NVIM', function() + clear() local s = eval('serverstart()') assert(s ~= nil and s:len() > 0, "serverstart() returned empty") eq('', eval('$NVIM')) @@ -34,6 +45,7 @@ describe('server', function() end) it('sets v:servername at startup or if all servers were stopped', function() + clear() local initial_server = meths.get_vvar('servername') assert(initial_server ~= nil and initial_server:len() > 0, 'v:servername was not initialized') @@ -62,11 +74,13 @@ describe('server', function() end) it('serverstop() returns false for invalid input', function() + clear() eq(0, eval("serverstop('')")) eq(0, eval("serverstop('bogus-socket-name')")) end) it('parses endpoints', function() + clear() clear_serverlist() eq({}, funcs.serverlist()) @@ -111,6 +125,7 @@ describe('server', function() end) it('serverlist() returns the list of servers', function() + clear() -- There should already be at least one server. local n = eval('len(serverlist())') -- cgit