aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJustin M. Keyes <justinkz@gmail.com>2024-09-08 12:48:32 -0700
committerGitHub <noreply@github.com>2024-09-08 12:48:32 -0700
commit8a2aec99748229ad9d1e12c1cbc0768d063e8eed (patch)
treedeed001296b252ea886130fea4844a8a7a5d6a7f
parent3a881132460430d23f2fdc87822c87d47f721cfc (diff)
downloadrneovim-8a2aec99748229ad9d1e12c1cbc0768d063e8eed.tar.gz
rneovim-8a2aec99748229ad9d1e12c1cbc0768d063e8eed.tar.bz2
rneovim-8a2aec99748229ad9d1e12c1cbc0768d063e8eed.zip
fix(startup): server fails if $NVIM_APPNAME is relative dir #30310
Problem: If $NVIM_APPNAME is a relative dir path, Nvim fails to start its primary/default server, and `v:servername` is empty. Root cause is d34c64e342dfba9248d1055e702d02620a1b31a8, but this wasn't noticed until 96128a5076b7 started reporting the error more loudly. Solution: - `server_address_new`: replace slashes "/" in the appname before using it as a servername. - `vim_mktempdir`: always prefer the system-wide top-level "nvim.user/" directory. That isn't intended to be specific to NVIM_APPNAME; rather, each *subdirectory* ("nvim.user/xxx") is owned by each Nvim instance. Nvim "apps" can be identified by the server socket(s) stored in those per-Nvim subdirs. fix #30256
-rw-r--r--src/nvim/eval/funcs.c2
-rw-r--r--src/nvim/fileio.c13
-rw-r--r--src/nvim/msgpack_rpc/server.c7
-rw-r--r--src/nvim/os/stdpaths.c18
-rw-r--r--src/nvim/runtime.c6
-rw-r--r--test/functional/core/fileio_spec.lua15
-rw-r--r--test/functional/core/server_spec.lua (renamed from test/functional/vimscript/server_spec.lua)4
-rw-r--r--test/functional/options/defaults_spec.lua21
-rw-r--r--test/functional/testnvim.lua3
-rw-r--r--test/testutil.lua2
10 files changed, 52 insertions, 39 deletions
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
index de1d784577..aed6fdae14 100644
--- a/src/nvim/eval/funcs.c
+++ b/src/nvim/eval/funcs.c
@@ -7641,7 +7641,7 @@ static void get_xdg_var_list(const XDGVarType xdg, typval_T *rettv)
return;
}
const void *iter = NULL;
- const char *appname = get_appname();
+ const char *appname = get_appname(false);
do {
size_t dir_len;
const char *dir;
diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c
index f6a25a27a6..31a1c0e293 100644
--- a/src/nvim/fileio.c
+++ b/src/nvim/fileio.c
@@ -3264,18 +3264,12 @@ static void vim_mktempdir(void)
char tmp[TEMP_FILE_PATH_MAXLEN];
char path[TEMP_FILE_PATH_MAXLEN];
char user[40] = { 0 };
- char appname[40] = { 0 };
os_get_username(user, sizeof(user));
// Usernames may contain slashes! #19240
memchrsub(user, '/', '_', sizeof(user));
memchrsub(user, '\\', '_', sizeof(user));
- // Appname may be a relative path, replace slashes to make it name-like.
- xstrlcpy(appname, get_appname(), sizeof(appname));
- memchrsub(appname, '/', '%', sizeof(appname));
- memchrsub(appname, '\\', '%', sizeof(appname));
-
// Make sure the umask doesn't remove the executable bit.
// "repl" has been reported to use "0177".
mode_t umask_save = umask(0077);
@@ -3283,14 +3277,15 @@ static void vim_mktempdir(void)
// Expand environment variables, leave room for "/tmp/nvim.<user>/XXXXXX/999999999".
expand_env((char *)temp_dirs[i], tmp, TEMP_FILE_PATH_MAXLEN - 64);
if (!os_isdir(tmp)) {
+ if (strequal("$TMPDIR", temp_dirs[i])) {
+ WLOG("$TMPDIR tempdir not a directory (or does not exist): %s", tmp);
+ }
continue;
}
// "/tmp/" exists, now try to create "/tmp/nvim.<user>/".
add_pathsep(tmp);
-
- xstrlcat(tmp, appname, sizeof(tmp));
- xstrlcat(tmp, ".", sizeof(tmp));
+ xstrlcat(tmp, "nvim.", sizeof(tmp));
xstrlcat(tmp, user, sizeof(tmp));
os_mkdir(tmp, 0700); // Always create, to avoid a race.
bool owned = os_file_owned(tmp);
diff --git a/src/nvim/msgpack_rpc/server.c b/src/nvim/msgpack_rpc/server.c
index 9cfe46454d..dc2b85154f 100644
--- a/src/nvim/msgpack_rpc/server.c
+++ b/src/nvim/msgpack_rpc/server.c
@@ -121,14 +121,15 @@ char *server_address_new(const char *name)
{
static uint32_t count = 0;
char fmt[ADDRESS_MAX_SIZE];
- const char *appname = get_appname();
#ifdef MSWIN
+ (void)get_appname(true);
int r = snprintf(fmt, sizeof(fmt), "\\\\.\\pipe\\%s.%" PRIu64 ".%" PRIu32,
- name ? name : appname, os_get_pid(), count++);
+ name ? name : NameBuff, os_get_pid(), count++);
#else
char *dir = stdpaths_get_xdg_var(kXDGRuntimeDir);
+ (void)get_appname(true);
int r = snprintf(fmt, sizeof(fmt), "%s/%s.%" PRIu64 ".%" PRIu32,
- dir, name ? name : appname, os_get_pid(), count++);
+ dir, name ? name : NameBuff, os_get_pid(), count++);
xfree(dir);
#endif
if ((size_t)r >= sizeof(fmt)) {
diff --git a/src/nvim/os/stdpaths.c b/src/nvim/os/stdpaths.c
index e4435bcaa8..e9a74d197f 100644
--- a/src/nvim/os/stdpaths.c
+++ b/src/nvim/os/stdpaths.c
@@ -63,22 +63,32 @@ static const char *const xdg_defaults[] = {
#endif
};
-/// Get the value of $NVIM_APPNAME or "nvim" if not set.
+/// Gets the value of $NVIM_APPNAME, or "nvim" if not set.
+///
+/// @param namelike Write "name-like" value (no path separators) in `NameBuff`.
///
/// @return $NVIM_APPNAME value
-const char *get_appname(void)
+const char *get_appname(bool namelike)
{
const char *env_val = os_getenv("NVIM_APPNAME");
if (env_val == NULL || *env_val == NUL) {
env_val = "nvim";
}
+
+ if (namelike) {
+ // Appname may be a relative path, replace slashes to make it name-like.
+ xstrlcpy(NameBuff, env_val, sizeof(NameBuff));
+ memchrsub(NameBuff, '/', '-', sizeof(NameBuff));
+ memchrsub(NameBuff, '\\', '-', sizeof(NameBuff));
+ }
+
return env_val;
}
/// Ensure that APPNAME is valid. Must be a name or relative path.
bool appname_is_valid(void)
{
- const char *appname = get_appname();
+ const char *appname = get_appname(false);
if (path_is_absolute(appname)
// TODO(justinmk): on Windows, path_is_absolute says "/" is NOT absolute. Should it?
|| strequal(appname, "/")
@@ -193,7 +203,7 @@ char *get_xdg_home(const XDGVarType idx)
FUNC_ATTR_WARN_UNUSED_RESULT
{
char *dir = stdpaths_get_xdg_var(idx);
- const char *appname = get_appname();
+ const char *appname = get_appname(false);
size_t appname_len = strlen(appname);
assert(appname_len < (IOSIZE - sizeof("-data")));
diff --git a/src/nvim/runtime.c b/src/nvim/runtime.c
index 3aa558f305..030bda4fa5 100644
--- a/src/nvim/runtime.c
+++ b/src/nvim/runtime.c
@@ -1559,7 +1559,7 @@ static inline char *add_env_sep_dirs(char *dest, const char *const val, const ch
return dest;
}
const void *iter = NULL;
- const char *appname = get_appname();
+ const char *appname = get_appname(false);
const size_t appname_len = strlen(appname);
do {
size_t dir_len;
@@ -1626,7 +1626,7 @@ static inline char *add_dir(char *dest, const char *const dir, const size_t dir_
if (!after_pathsep(dest - 1, dest)) {
*dest++ = PATHSEP;
}
- const char *appname = get_appname();
+ const char *appname = get_appname(false);
size_t appname_len = strlen(appname);
assert(appname_len < (IOSIZE - sizeof("-data")));
xmemcpyz(IObuff, appname, appname_len);
@@ -1697,7 +1697,7 @@ char *runtimepath_default(bool clean_arg)
size_t config_len = 0;
size_t vimruntime_len = 0;
size_t libdir_len = 0;
- const char *appname = get_appname();
+ const char *appname = get_appname(false);
size_t appname_len = strlen(appname);
if (data_home != NULL) {
data_len = strlen(data_home);
diff --git a/test/functional/core/fileio_spec.lua b/test/functional/core/fileio_spec.lua
index 073041fced..d33710a63d 100644
--- a/test/functional/core/fileio_spec.lua
+++ b/test/functional/core/fileio_spec.lua
@@ -321,11 +321,11 @@ end)
describe('tmpdir', function()
local tmproot_pat = [=[.*[/\\]nvim%.[^/\\]+]=]
local testlog = 'Xtest_tmpdir_log'
- local os_tmpdir
+ local os_tmpdir ---@type string
before_each(function()
-- Fake /tmp dir so that we can mess it up.
- os_tmpdir = vim.uv.fs_mkdtemp(vim.fs.dirname(t.tmpname(false)) .. '/nvim_XXXXXXXXXX')
+ os_tmpdir = assert(vim.uv.fs_mkdtemp(vim.fs.dirname(t.tmpname(false)) .. '/nvim_XXXXXXXXXX'))
end)
after_each(function()
@@ -414,15 +414,4 @@ describe('tmpdir', function()
rm_tmpdir()
eq('E5431: tempdir disappeared (3 times)', api.nvim_get_vvar('errmsg'))
end)
-
- it('$NVIM_APPNAME relative path', function()
- clear({
- env = {
- NVIM_APPNAME = 'a/b',
- NVIM_LOG_FILE = testlog,
- TMPDIR = os_tmpdir,
- },
- })
- matches([=[.*[/\\]a%%b%.[^/\\]+]=], fn.tempname())
- end)
end)
diff --git a/test/functional/vimscript/server_spec.lua b/test/functional/core/server_spec.lua
index 85a179b3d5..0ec11169e9 100644
--- a/test/functional/vimscript/server_spec.lua
+++ b/test/functional/core/server_spec.lua
@@ -154,7 +154,7 @@ describe('server', function()
clear_serverlist()
-- Address without slashes is a "name" which is appended to a generated path. #8519
- matches([[.*[/\\]xtest1%.2%.3%.4[^/\\]*]], fn.serverstart('xtest1.2.3.4'))
+ matches([[[/\\]xtest1%.2%.3%.4[^/\\]*]], fn.serverstart('xtest1.2.3.4'))
clear_serverlist()
eq('Vim:Failed to start server: invalid argument', pcall_err(fn.serverstart, '127.0.0.1:65536')) -- invalid port
@@ -273,6 +273,6 @@ describe('startup --listen', function()
-- Address without slashes is a "name" which is appended to a generated path. #8519
clear({ args = { '--listen', 'test-name' } })
- matches([[.*[/\\]test%-name[^/\\]*]], api.nvim_get_vvar('servername'))
+ matches([[[/\\]test%-name[^/\\]*]], api.nvim_get_vvar('servername'))
end)
end)
diff --git a/test/functional/options/defaults_spec.lua b/test/functional/options/defaults_spec.lua
index ca4a6eaca7..ad94ef1206 100644
--- a/test/functional/options/defaults_spec.lua
+++ b/test/functional/options/defaults_spec.lua
@@ -915,7 +915,7 @@ describe('stdpath()', function()
assert_alive() -- Check for crash. #8393
end)
- it('supports $NVIM_APPNAME', function()
+ it('$NVIM_APPNAME', function()
local appname = 'NVIM_APPNAME_TEST' .. ('_'):rep(106)
clear({ env = { NVIM_APPNAME = appname, NVIM_LOG_FILE = testlog } })
eq(appname, fn.fnamemodify(fn.stdpath('config'), ':t'))
@@ -957,6 +957,25 @@ describe('stdpath()', function()
test_appname('a/b\\c', 0)
end)
+ it('$NVIM_APPNAME relative path', function()
+ local tmpdir = t.tmpname(false)
+ t.mkdir(tmpdir)
+
+ clear({
+ args_rm = { '--listen' },
+ env = {
+ NVIM_APPNAME = 'relative/appname',
+ NVIM_LOG_FILE = testlog,
+ TMPDIR = tmpdir,
+ },
+ })
+
+ t.matches(vim.pesc(tmpdir), fn.tempname():gsub('\\', '/'))
+ t.assert_nolog('tempdir', testlog, 100)
+ t.assert_nolog('TMPDIR', testlog, 100)
+ t.matches([=[[/\\]relative%-appname.[^/\\]+]=], api.nvim_get_vvar('servername'))
+ end)
+
describe('returns a String', function()
describe('with "config"', function()
it('knows XDG_CONFIG_HOME', function()
diff --git a/test/functional/testnvim.lua b/test/functional/testnvim.lua
index 66ce6daacb..d74e8055ef 100644
--- a/test/functional/testnvim.lua
+++ b/test/functional/testnvim.lua
@@ -14,8 +14,7 @@ local is_os = t.is_os
local ok = t.ok
local sleep = uv.sleep
---- This module uses functions from the context of the test session, i.e. in the context of the
---- nvim being tests.
+--- Functions executing in the current nvim session/process being tested.
local M = {}
local runtime_set = 'set runtimepath^=./build/lib/nvim/'
diff --git a/test/testutil.lua b/test/testutil.lua
index abfc10e806..19e297c503 100644
--- a/test/testutil.lua
+++ b/test/testutil.lua
@@ -16,7 +16,7 @@ local function shell_quote(str)
return str
end
---- This module uses functions from the context of the test runner.
+--- Functions executing in the context of the test runner (not the current nvim test session).
--- @class test.testutil
local M = {
paths = Paths,